salt: file.directory clean does not work as expected

So I want Salt to clear out the /etc/apt/sources.list.d folder of any sources.list files it doesn’t manage.

First attempt:

core-apt-sources-dir-clean:
  file.directory:
    - name: /etc/apt/sources.list.d
    - user: root
    - group: root
    - mode: 755
    - clean: True
    - require:
      - pkgrepo: core-apt-ubuntu

core-apt-ubuntu:
  pkgrepo.managed:
    - file: /etc/apt/sources.list.d/ubuntu.list
    - name: deb [arch=amd64] https://{{ repouser }}:{{ repopass }}@repo.company.com/ubuntu/ {{ grains['lsb_distrib_codename'] }} production
    - key_url: salt://core/apt/files/aptly.pub

Nope:

----------
          ID: core-apt-ubuntu
    Function: pkgrepo.managed
        Name: deb [arch=amd64] https://user:pass@repo.company.com/ubuntu/ trusty production
      Result: True
     Comment: Configured package repo 'deb [arch=amd64] https://user:pass@config.pathintelligence.com/ubuntu/ trusty production'
     Started: 14:11:25.777941
    Duration: 4029.592 ms
     Changes:
              ----------
              repo:
                  deb [arch=amd64] https://user:pass@repo.company.com/ubuntu trusty production
----------
          ID: core-apt-sources-dir-clean
    Function: file.directory
        Name: /etc/apt/sources.list.d
      Result: True
     Comment: Files cleaned from directory /etc/apt/sources.list.d
     Started: 14:11:29.807755
    Duration: 0.886 ms
     Changes:
              ----------
              removed:
                  - /etc/apt/sources.list.d/ubuntu.list

Ok, maybe it can only exclude something if it’s a file:

core-apt-sources-dir-clean:
  file.directory:
    - name: /etc/apt/sources.list.d
    - user: root
    - group: root
    - mode: 755
    - clean: True
    - require:
      - file: core-apt-ubuntu

core-apt-ubuntu:
  file.managed:
    - name: /etc/apt/sources.list.d/ubuntu.list
    - source: salt://core/apt/files/ubuntu.list.jinja
    - template: jinja
    - user: root
    - group: root
    - mode: 644
    - makedirs: True

----------
          ID: core-apt-ubuntu
    Function: file.managed
        Name: /etc/apt/sources.list.d/ubuntu.list
      Result: True
     Comment: File /etc/apt/sources.list.d/ubuntu.list updated
     Started: 14:36:44.264193
    Duration: 4.971 ms
     Changes:
              ----------
              diff:
                  New file
              mode:
                  0644
----------
          ID: core-apt-sources-dir-clean
    Function: file.directory
        Name: /etc/apt/sources.list.d
      Result: True
     Comment: Files cleaned from directory /etc/apt/sources.list.d
     Started: 14:36:44.270779
    Duration: 0.973 ms
     Changes:
              ----------
              removed:
                     - /etc/apt/sources.list.d/ubuntu.list

Nope.

The docs are a little sparse:

clean
    Make sure that only files that are set up by salt and required by this function are kept. If this option is set then everything in this directory will be deleted unless it is required.
require
    Require other resources such as packages or files
exclude_pat
    When 'clean' is set to True, exclude this pattern from removal list and preserve in the destination.

Which to me reads that if something is required the end file it produces (if in that directory) won’t get removed, which doesn’t appear to be the case.

I also gave exclude_pat a go with no joy:

core-apt-sources-dir-clean:
  file.directory:
    - name: /etc/apt/sources.list.d
    - user: root
    - group: root
    - mode: 755
    - clean: True
    - exclude_pat: '(myrepo|ubuntu).list'

Also tried (myrepo|ubuntu) & (myrepo|ubuntu)\.list with no joy.

Tested on salt versions v2015.5.0 to v2015-5-5 and 2015.8 (all from git using the bootstrap), running on Ubuntu 14.04.

About this issue

Original URL
State: closed
Created 9 years ago
Reactions: 1
Comments: 23 (17 by maintainers)

Commits related to this issue

Remove non-Salted external apt sources Workaround https://github.com/saltstack/salt/issues/26605 by cleaning the sources.list.d folder first, and running pkgrepo states that add repositories to the f... — committed to aneeshusa/servo-saltfs by aneeshusa 8 years ago
Auto merge of #234 - aneeshusa:clean-external-apt-repos, r=larsbergstrom Remove non-Salted external apt sources Workaround https://github.com/saltstack/salt/issues/26605 by cleaning the sources.list... — committed to servo/saltfs by deleted user 8 years ago

Most upvoted comments

@waynew, sorry, but I fail to see how can exclude_pat help in the particular example that originated this bug report?

Usually, a formula managing an application and providing a repository for it, will add a file under the /etc/apt/sources.list.d directory.

So say that I have, originally, this situation:

/etc/apt/sources.list.d/manual_repo_a.list
/etc/apt/sources.list.d/manual_repo_b.list
/etc/apt/sources.list.d/manual_repo_c.list

Then, I add 2 formulas, with repos, which add these entries

/etc/apt/sources.list.d/salt_managed_repo_a.list
/etc/apt/sources.list.d/salt_managed_repo_b.list

And I add the resource

clean_dir:
  file.directory:
    - name: /etc/apt/sources.list.d/
    - clean: True

The expected behavior is that salt would remove the manual_repo_* entried and keep the salt_managed_* ones. (The names are patterned here for the sake of discussion, but in real life, there’s no convention on these files naming other than the extension .list)

The bug describes that salt will delete ALL the entries and re-create the salt_managed_* ones, each time it’s run. Or delete them at all, depending on the order of the states.

With the require/_in workaround, it will work/fail depending the case (see the example I provided above).

And unless I’m not understanding it correctly, the workaround you propose means that each time a new formula or piece of repo-managing code is added to a node, the exclude_pat pattern will need to be modified to reflect this? If this is the case, it does not seem to be a workaround at all.

Am I missing something?

If not, imho, the severity should stand the same, right?

javierbertoli on Aug 23, 2021

After further experimentation and really digging into this, it appears that the existing behavior is basically correct. That is to say, this was always (apparently) only intended to ignore files that were managed by a file state.

For the original issue, the expected spelling is:

core-apt-sources-dir-clean:
  file.directory:
    - name: /etc/apt/sources.list.d
    - user: root
    - group: root
    - mode: 755
    - clean: True
    - require:
      - file: core-apt-ubuntu

core-apt-ubuntu:
  pkgrepo.managed:
    - file: /etc/apt/sources.list.d/ubuntu.list
    - name: deb [arch=amd64] https://{{ repouser }}:{{ repopass }}@repo.company.com/ubuntu/ {{ grains['lsb_distrib_codename'] }} production
    - key_url: salt://core/apt/files/aptly.pub
  # vvvvvv --Add this-- vvvvvv
  file.managed:
    - name: /etc/apt/sources.list.d/ubuntu.list
    - create: False
    - replace: False

(create/replace: False aren’t strictly necessary, but they do help avoid warnings)

It would be neat if all states/modules could specify the files that they’re managing, but without some potentially pretty impressive changes that isn’t possible. Instead, this particular functionality simply depends on the required state being a file state - in the above example that was added under the original ID, but it’s fine if it’s a different ID, as long as the requisite is spelled correctly. And the require can be in either direction with a require from file.directory or require_in from the other state.

I’ve opened a PR to make all of this very explicit in the documentation as well as provide some examples. Since the bug that used to be there was resolved as mentioned earlier up this comment chain, and this isn’t really a bug in the code, and I’ve got the PR open, I’m going to go ahead and close this issue.

waynew on Feb 3, 2022