saltstack-formulas / apache-formula

Set up and configure the Apache HTTP server
http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
50 stars 285 forks source link

[BUG] in vhosts/standard.sls the map.pop affects the apache variable as well because it is a reference, not a copy #377

Open doubletwist13 opened 2 years ago

doubletwist13 commented 2 years ago

Your setup

Formula commit hash / release tag

apache-formula 1.2.2

Versions reports (master & minion)

          Salt: 3004

Dependency Versions:
          cffi: Not Installed
      cherrypy: unknown
      dateutil: Not Installed
     docker-py: Not Installed
         gitdb: 0.6.4
     gitpython: 1.0.1
        Jinja2: 2.11.1
       libgit2: Not Installed
      M2Crypto: 0.35.2
          Mako: Not Installed
       msgpack: 0.6.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: Not Installed
      pycrypto: Not Installed
  pycryptodome: Not Installed
        pygit2: Not Installed
        Python: 3.6.8 (default, Nov 18 2021, 10:07:16)
  python-gnupg: Not Installed
        PyYAML: 5.4.1
         PyZMQ: 17.0.0
         smmap: 0.9.0
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.1.4

System Versions:
          dist: oracle 7.9 
        locale: UTF-8
       machine: x86_64
       release: 5.4.17-2136.302.6.1.el7uek.x86_64
        system: Linux
       version: Oracle Linux Server 7.9 

Pillar / config used

apache:
  sites:
    example.com_redirect:
      port: '80'
      ServerName: example.com
      RedirectSource: '/'
      RedirectTarget: 'https://example.com'
    example.com_ssl:
      port: '443'
      ServerName: example.com
      DocumentRoot: /path/to/webroot

Bug details

Describe the bug

In apache/config/vhosts/standard.sls lines 8-11 attempt to make a copy of the 'apache' variable to 'map' and then trim to only include the 'sites' data but instead of making a copy it just makes a reference. This causes the map.pop to trim the 'apache' variable as well as the 'map' variable.

Steps to reproduce the bug

Expected behaviour

map.pop should trim only the 'map' variable, not the 'apache' variable.

Attempts to fix the bug

Just need to tell it to copy the variable instead of creating a reference by changing from:

{%- set map = apache %}
{%- do map.pop('sites', None) %}

TO

{%- set map = apache.copy() %}
{%- do map.pop('sites', None) %}

Additional context

myii commented 2 years ago

@doubletwist13 Thanks for the report. That change was provided in https://github.com/saltstack-formulas/apache-formula/commit/1f488b6af3f63aeaa1803d913684a4871f9a9411. @ixs, would you be able to respond to this?

ixs commented 2 years ago

Yeah, this looks good. Thank you @doubletwist13 for the good description and digging. Reference vs. copy is exactly what's going on.

I'll whip up a PR to fix this issue now.

doubletwist13 commented 2 years ago

Just checking in. I didn't see a PR for this and it looks like the error still exists in the master branch.

I could maybe do a PR - as long as the change doesn't require any changes to tests - as that's a bit beyond my current ability