saltstack-formulas / node-formula

Manage node.js with SaltStack
http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
26 stars 102 forks source link

Add a falsy default to each instance of pillar.get #41

Closed smitelli closed 5 years ago

smitelli commented 6 years ago

In an environment with the option pillar_raise_on_missing: True and pillar data which defines only some keys, this formula fails with messages like:

local:
    Data failed to compile:
----------
    Rendering SLS 'base:node' failed: Jinja error: u'Pillar key not found: node:install_from_source'
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/salt/utils/templates.py", line 389, in render_jinja_tmpl
    output = template.render(**decoded_context)
  File "/usr/lib/python2.7/dist-packages/jinja2/environment.py", line 1008, in render
    return self.environment.handle_exception(exc_info, True)
  File "/usr/lib/python2.7/dist-packages/jinja2/environment.py", line 780, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "<template>", line 3, in top-level template code
  File "/usr/lib/python2.7/dist-packages/salt/modules/pillar.py", line 184, in get
    raise KeyError('Pillar key not found: {0}'.format(key))
KeyError: u'Pillar key not found: node:install_from_source'

; line 3

---
{% set pillar_get = salt['pillar.get'] -%}
include:
{%- if pillar_get('node:install_from_source') %}    <======================
  - .source
{%- elif pillar_get('node:install_from_binary') %}
  - .binary
{%- else %}
  - .pkg
[...]
---

(In this case, the pillar had set install_from_ppa: True and did not define install_from_source and friends.)

Fixing it requires either explicitly defining every pillar key that the formula checks, or modifying the source of the formula itself. Having tried the former, now I'm trying the latter. This PR adds a falsy default value (the empty string '') to every instance of pillar.get, behaving like it would if pillar_raise_on_missing were not set.

aboe76 commented 5 years ago

@smitelli ow, merged in the wrong order, can you please rebase?

smitelli commented 5 years ago

@aboe76 All done. Had a minor conflict in node/init.sls; may want to give it a sanity check.

noelmcloughlin commented 5 years ago

Thanks for the work @smitelli @aboe76

BTW I created two issues based on the Travis CI.