saltstack-formulas / nginx-formula

Nginx Salt Formula
http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
163 stars 421 forks source link

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

Closed hkbakke closed 3 years ago

hkbakke commented 3 years ago

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 https://github.com/saltstack-formulas/nginx-formula/commit/19203409aeb40b042cffb0e4d39bc66303f17842

anderbubble commented 3 years ago

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'
Yoda-BZH commented 3 years ago

Hello,

Any update ? The PR fixes the problem for me.

myii commented 3 years ago

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 commented 3 years ago

ho, ok thanks :)

myii commented 3 years ago

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 commented 3 years ago

:tada: This issue has been resolved in version 2.7.2 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

saltstack-formulas-travis commented 3 years ago

:tada: This issue has been resolved in version 2.7.3 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: