search

saltstack-formulas / sudoers-formula

http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
41 stars 168 forks source link

purge_includedir doesn not seem to work #75

Open MathyV opened 3 years ago

MathyV commented 3 years ago

Your setup

Formula commit hash / release tag

v0.23.4

Versions reports (master & minion)

Salt Version:
          Salt: 3003

Dependency Versions:
          cffi: Not Installed
      cherrypy: Not Installed
      dateutil: 2.7.3
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 2.10.1
       libgit2: 0.28.3
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 0.6.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: Not Installed
      pycrypto: Not Installed
  pycryptodome: 3.6.1
        pygit2: 1.0.3
        Python: 3.8.5 (default, Jan 27 2021, 15:41:15)
  python-gnupg: 0.4.5
        PyYAML: 5.3.1
         PyZMQ: 18.1.1
         smmap: Not Installed
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.3.2

System Versions:
          dist: ubuntu 20.04 focal
        locale: utf-8
       machine: x86_64
       release: 5.4.0-73-generic
        system: Linux
       version: Ubuntu 20.04 focal

Pillar / config used

sudoers:
  purge_includedir: true
  groups:
    sudo:
    - 'ALL=(ALL) NOPASSWD: ALL'

Bug details

Describe the bug

The purge_includedir option doesn't do anything when set to true

Steps to reproduce the bug

  1. Set option
  2. Add a random file to /etc/sudoers.d/
  3. Execute salt
  4. Watch how the file is not deleted

Expected behaviour

Extra files to disappear

Attempts to fix the bug

I made sure the correct value is exported in Pillar:

    sudoers:
        ----------
        groups:
            ----------
            sudo:
                - ALL=(ALL) NOPASSWD: ALL
        purge_includedir:
            True

Additional context

I think this might be related to https://github.com/saltstack/salt/issues/26605 ?

daks commented 3 years ago

I just tested the same version v0.23.4 of the formula and it works for me as expected.

It could be interesting to try to add some Kitchen/Inspec tests to validate the correct behaviour of the option but this is not done actually.

MathyV commented 3 years ago

I'll see if I can create a minimal reproducible case

MathyV commented 3 years ago

I can reproduce it with these minimal files:

salt/test.sls

include:
- sudoers

pillar/top.sls

base:
  '*':
  - common.sudoers

pillar/common/sudoers.sls

sudoers:
  purge_includedir: true
  groups:
    sudo:
    - 'ALL=(ALL) NOPASSWD: ALL'

Files I create in /etc/sudoers.d aren't deleted when the state is applied. I tried both with a changed sudo config and without.

daks commented 3 years ago

I can reproduce it with these minimal files:

salt/test.sls

include:
- sudoers

Can you add sudoers.included to this state list? I think the problem comes from it missing. This state manages the "included_dir" and its purge as you can see at https://github.com/saltstack-formulas/sudoers-formula/blob/master/sudoers/included.sls#L12.

And sudoers state does not include sudoers.included: not sure if there is a good reason to keep this behaviour or if we could make it a 'meta-state' like in others formulas.

MathyV commented 3 years ago

That did the trick, perhaps it's already enough to document it somewhere, I didn't realize I needed to include the extra state but it does kinda make sense.