Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Run fails if nginx.server.snippets is not defined in pillar #274

Closed
hkbakke opened this issue Jan 6, 2021 · 7 comments · Fixed by #275
Closed

[BUG] Run fails if nginx.server.snippets is not defined in pillar #274

hkbakke opened this issue Jan 6, 2021 · 7 comments · Fixed by #275

Comments

@hkbakke
Copy link

hkbakke commented Jan 6, 2021

If nginx.server.snippets is not defined in my pillar file, the run fails. Just adding {} as value is enough to work around the problem.

The issue seems to be occuring at the pop in nginx/snippets.sls.

Currently using 1920340

@hkbakke hkbakke added the bug label Jan 6, 2021
@anderbubble
Copy link
Contributor

anderbubble commented Jan 23, 2021

I looked at this a bit. First I expected the best way to fix it would be to add a default value to _nginx.pop('snippets') 1 (something like _nginx.pop('snippets', {})). But there are other uses of .pop() in the formula and none of them are specifying a default value.

So I started asking myself why snippets.sls is being loaded at all. Sure enough, it looks to me like it shouldn't be if snippets isn't defined in Pillar. 2

So I think the real bug is in the fact that nginx.snippets is defined is returning True even when snippets is not in Pillar.

EDIT:

At least in my case, "the pop in nginx/snippets.sls", as indicated by @hkbakke, is not where this is happening. In stead, it's happening in nginx/servers_config.sls. 3 This makes more sense, because that .pop() isn't wrapped in any test for snippets' presence.

    Data failed to compile:                                                                                                                                                                                                           
----------                                                                                                                                                                                                                            
    Rendering SLS 'base:nginx.servers' failed: Jinja error: 'snippets'                                                                                                                                                                
/var/cache/salt/minion/files/base/nginx/servers_config.sls(13):                                                                                                                                                                       
---                                                                                                                                                                                                                                   
[...]                                                                                                                                                                                                                                 
{%- from tplroot ~ '/libtofs.jinja' import files_switch with context %}                                                                                                                                                               
                                                                                                                                                                                                                                      
{% set server_states = [] %}                                                                                                                                                                                                          
{#- _nginx is a lightened copy of nginx map intended to passed in templates #}                                                                                                                                                        
{%- set _nginx = nginx.copy() %}                                                                                                                                                                                                      
{%- do _nginx.pop('snippets') %}    <======================                                                                                                                                                                           
{%- do _nginx.pop('servers') %}                                                                                                                                                                                                       

# Simple path concatenation.
# Needs work to make this function on windows.
{% macro path_join(file, root) -%}
[...]
---
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/salt/utils/templates.py", line 498, in render_jinja_tmpl
    output = template.render(**decoded_context)
  File "/usr/lib/python3/dist-packages/jinja2/asyncsupport.py", line 76, in render
    return original_render(self, *args, **kwargs)
  File "/usr/lib/python3/dist-packages/jinja2/environment.py", line 1008, in render
    return self.environment.handle_exception(exc_info, True)
  File "/usr/lib/python3/dist-packages/jinja2/environment.py", line 780, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/lib/python3/dist-packages/jinja2/_compat.py", line 37, in reraise
    raise value.with_traceback(tb)
  File "<template>", line 7, in top-level template code
  File "/usr/lib/python3/dist-packages/jinja2/environment.py", line 1073, in make_module
    return TemplateModule(self, self.new_context(vars, shared, locals))
  File "/usr/lib/python3/dist-packages/jinja2/environment.py", line 1152, in __init__
    body_stream = list(template.root_render_func(context))
  File "/var/cache/salt/minion/files/base/nginx/servers_config.sls", line 13, in top-level template code
    {%- do _nginx.pop('snippets') %}
KeyError: 'snippets'

anderbubble pushed a commit to anderbubble/nginx-formula that referenced this issue Jan 23, 2021
nginx.servers_config wants a lightened copy of the nginx map
to render as json; but, when it was trying to remove the
servers and snippets keys from the map it assumed their presence,
causing a KeyError if they were not present by its use of .pop().

While wrapping these in an "if" clause would likely be more
correct, along with replacing .pop() with del (if jinja even
supports that) the simplest change here is to just specify a
default value for .pop(), which obviates the KeyError.

Fixes saltstack-formulas#274
# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# On branch issues/274
# Changes to be committed:
#	modified:   nginx/servers_config.sls
#
anderbubble pushed a commit to anderbubble/nginx-formula that referenced this issue Jan 23, 2021
nginx.servers_config wants a lightened copy of the nginx map
to render as json; but, when it was trying to remove the
servers and snippets keys from the map it assumed their presence,
causing a KeyError if they were not present by its use of .pop().

While wrapping these in an "if" clause would likely be more
correct, along with replacing .pop() with del (if jinja even
supports that) the simplest change here is to just specify a
default value for .pop(), which obviates the KeyError.

Fixes saltstack-formulas#274
@Yoda-BZH
Copy link

Hello,

Any update ? The PR fixes the problem for me.

@myii
Copy link
Member

myii commented Jun 14, 2021

Hello,

Any update ? The PR fixes the problem for me.

@Yoda-BZH It seems that @javierbertoli is waiting for the final adjustments:

@anderbubble wonderful! I'll wait for your changes to merge this.

@Yoda-BZH
Copy link

ho, ok thanks :)

javierbertoli added a commit to netmanagers/nginx-formula that referenced this issue Jun 14, 2021
myii pushed a commit to anderbubble/nginx-formula that referenced this issue Jun 14, 2021
nginx.servers_config wants a lightened copy of the nginx map
to render as json; but, when it was trying to remove the
servers and snippets keys from the map it assumed their presence,
causing a KeyError if they were not present by its use of .pop().

While wrapping these in an "if" clause would likely be more
correct, along with replacing .pop() with del (if jinja even
supports that) the simplest change here is to just specify a
default value for .pop(), which obviates the KeyError.

Fixes saltstack-formulas#274
myii pushed a commit to anderbubble/nginx-formula that referenced this issue Jun 14, 2021
nginx.servers_config wants a lightened copy of the nginx map
to render as json; but, when it was trying to remove the
servers and snippets keys from the map it assumed their presence,
causing a KeyError if they were not present by its use of .pop().

While wrapping these in an "if" clause would likely be more
correct, along with replacing .pop() with del (if jinja even
supports that) the simplest change here is to just specify a
default value for .pop(), which obviates the KeyError.

Fixes saltstack-formulas#274
@myii
Copy link
Member

myii commented Jun 14, 2021

Hello,

Any update ? The PR fixes the problem for me.

@Yoda-BZH So #275 has been merged but with a minor adjustment. Can you confirm that it is still working for you?

saltstack-formulas-travis pushed a commit that referenced this issue Jun 14, 2021
## [2.7.2](v2.7.1...v2.7.2) (2021-06-14)

### Bug Fixes

* **certificates:** ensure `openssl` installed before `cmd.run` ([0cd7c7b](0cd7c7b)), closes [/gitlab.com/saltstack-formulas/nginx-formula/-/jobs/1345325819#L2830](https://github.com//gitlab.com/saltstack-formulas/nginx-formula/-/jobs/1345325819/issues/L2830)
* **snippets:** ignore servers or snippets when undefined ([6cb486d](6cb486d)), closes [#274](#274)
@saltstack-formulas-travis

🎉 This issue has been resolved in version 2.7.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

javierbertoli added a commit to netmanagers/nginx-formula that referenced this issue Jun 14, 2021
saltstack-formulas-travis pushed a commit that referenced this issue Jun 14, 2021
## [2.7.3](v2.7.2...v2.7.3) (2021-06-14)

### Tests

* **snippets:** add tests for snippets includes ([1c83b6d](1c83b6d)), closes [#275](#275) [#274](#274)
@saltstack-formulas-travis

🎉 This issue has been resolved in version 2.7.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants