apache-formula icon indicating copy to clipboard operation
apache-formula copied to clipboard

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

Open doubletwist13 opened this issue 3 years ago • 3 comments

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

doubletwist13 avatar Jan 04 '22 19:01 doubletwist13

@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?

myii avatar Jan 05 '22 06:01 myii

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.

ixs avatar Jan 05 '22 09:01 ixs

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

doubletwist13 avatar Mar 13 '22 06:03 doubletwist13