Salt: `clean_file: true` in `pkgrepo.managed` state has no effect when 'name' is already present

[eliasp]

When the specified name of a pkgrepo.managed state is already present in the target file, clean_file: true doesn't have any effect and leaves old stale entries in the file.

This can be easily reproduced (on Ubuntu 14.04) by applying first this state, containing a wrong repository URL:

saltstack-repo:
  pkgrepo.managed:
    - humanname:  SaltStack Package Repository
    - name:       deb http://repo.saltstack.com/apt/THIS-WILL-NEVER-WORK/ubuntu/ubuntu{{ grains['osrelease_info'][0] }}/latest {{ grains['oscodename'] }} main
    - dist:       {{ grains['oscodename'] }}
    - file:       /etc/apt/sources.list.d/saltstack.list
    - gpgcheck:   1
    - key_url:    https://repo.saltstack.com/apt/ubuntu/ubuntu{{ grains['osrelease_info'][0] }}/latest/SALTSTACK-GPG-KEY.pub

…then correct the URL in name:

saltstack-repo:
  pkgrepo.managed:
    - humanname:  SaltStack Package Repository
    - name:       deb http://repo.saltstack.com/apt/ubuntu/ubuntu{{ grains['osrelease_info'][0] }}/latest {{ grains['oscodename'] }} main
    - dist:       {{ grains['oscodename'] }}
    - file:       /etc/apt/sources.list.d/saltstack.list
    - gpgcheck:   1
    - key_url:    https://repo.saltstack.com/apt/ubuntu/ubuntu{{ grains['osrelease_info'][0] }}/latest/SALTSTACK-GPG-KEY.pub

…then finally add clean_file: true:

saltstack-repo:
  pkgrepo.managed:
    - humanname:  SaltStack Package Repository
    - name:       deb http://repo.saltstack.com/apt/ubuntu/ubuntu{{ grains['osrelease_info'][0] }}/latest {{ grains['oscodename'] }} main
    - dist:       {{ grains['oscodename'] }}
    - file:       /etc/apt/sources.list.d/saltstack.list
    - gpgcheck:   1
    - key_url:    https://repo.saltstack.com/apt/ubuntu/ubuntu{{ grains['osrelease_info'][0] }}/latest/SALTSTACK-GPG-KEY.pub
    - clean_file: true

The file will still contain the broken repository line:

# salt ubuntu1404-minion state.sls_id saltstack-repo linux.base
ubuntu1404-minion:
----------
          ID: saltstack-repo
    Function: pkgrepo.managed
        Name: deb http://repo.saltstack.com/apt/ubuntu/ubuntu14/latest trusty main
      Result: True
     Comment: Package repo 'deb http://repo.saltstack.com/apt/ubuntu/ubuntu14/latest trusty main' already configured
     Started: 13:09:24.117955
    Duration: 65.011 ms
     Changes:   

Summary for ubuntu1404-minion
------------
Succeeded: 1
Failed:    0
------------
Total states run:     1
Total run time:  65.011 ms

# salt ubuntu1404-minion cp.get_file_str /etc/apt/sources.list.d/saltstack.list
ubuntu1404-minion:
    deb http://repo.saltstack.com/apt/THIS-WILL-NEVER-WORK/ubuntu/ubuntu14/latest trusty main
    deb http://repo.saltstack.com/apt/ubuntu/ubuntu14/latest trusty main

Version report:

# salt ubuntu1404-minion test.versions_report
ubuntu1404-minion:
    Salt Version:
               Salt: 2015.8.1

    Dependency Versions:
             Jinja2: 2.7.2
           M2Crypto: Not Installed
               Mako: 0.9.1
             PyYAML: 3.10
              PyZMQ: 14.4.0
             Python: 2.7.6 (default, Jun 22 2015, 17:58:13)
               RAET: Not Installed
            Tornado: 4.2.1
                ZMQ: 4.0.4
               cffi: Not Installed
           cherrypy: Not Installed
           dateutil: 1.5
              gitdb: Not Installed
          gitpython: Not Installed
              ioflo: Not Installed
            libnacl: Not Installed
       msgpack-pure: Not Installed
     msgpack-python: 0.3.0
       mysql-python: 1.2.3
          pycparser: Not Installed
           pycrypto: 2.6.1
             pygit2: Not Installed
       python-gnupg: Not Installed
              smmap: Not Installed
            timelib: Not Installed

    System Versions:
               dist: Ubuntu 14.04 trusty
            machine: x86_64
            release: 3.13.0-55-generic
             system: Ubuntu 14.04 trusty

[jfindlay]

@eliasp, thanks for the report.

[michaelkarrer81]

+1 i can confirm the bug

[multani]

I can confirm the bug as well.

I believe this come from the cleaning step which happens after the check if the repository is correctly configured or not.

[jfindlay]

@multani, you are welcome to submit a pull request if you have a fix.

[warden]

This is actually very similar to #32605.

[kivoli]

I just stumbled across this as well. IMO in case of clean_file: true the state should not bother with some in-file magic but just compare the whole file to the desired end state - like file.managed does, right? If I find the time I’ll look into this next week.

[dosercz]

It would be fine to fix it. Current behaviour is not good:(

[boltronics]

Still an issue in Salt 2016.11.4.

[kivoli]

Still an issue in Salt 2017.7.1

[absmith82]

Still and issue Salt 2017.7.4

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[dosercz]

It looks like it's not fixed yet.

[stale[bot]]

Thank you for updating this issue. It is no longer marked as stale.

[couilllard45682]

Still not fixed on 2019.2.0
````
~# salt-call --versions
Salt Version:
Salt: 2019.2.0

Dependency Versions:
cffi: Not Installed
cherrypy: Not Installed
dateutil: 2.6.1
docker-py: Not Installed
gitdb: Not Installed
gitpython: Not Installed
ioflo: Not Installed
Jinja2: 2.10
libgit2: Not Installed
libnacl: Not Installed
M2Crypto: Not Installed
Mako: Not Installed
msgpack-pure: Not Installed
msgpack-python: 0.5.6
mysql-python: Not Installed
pycparser: Not Installed
pycrypto: 2.6.1
pycryptodome: Not Installed
pygit2: Not Installed
Python: 3.6.8 (default, Jan 14 2019, 11:02:34)
python-gnupg: 0.4.1
PyYAML: 3.12
PyZMQ: 16.0.2
RAET: Not Installed
smmap: Not Installed
timelib: Not Installed
Tornado: 4.5.3
ZMQ: 4.2.5

System Versions:
dist: Ubuntu 18.04 bionic
locale: UTF-8
machine: x86_64
release: 4.15.0-1044-aws
system: Linux
version: Ubuntu 18.04 bionic
```

[CodingMinds]

Still not fixed on 2019.2.2

salt-ssh --versions         
Salt Version:
           Salt: 2019.2.2

Dependency Versions:
           cffi: 1.12.3
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.10.3
        libgit2: 0.28.4
        libnacl: Not Installed
       M2Crypto: 0.35.2
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.6.2
   mysql-python: Not Installed
      pycparser: 2.19
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: 0.28.1
         Python: 3.7.6 (default, Dec 27 2019, 09:51:21)
   python-gnupg: Not Installed
         PyYAML: 3.13
          PyZMQ: 18.1.1
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.3.2

System Versions:
           dist:   
         locale: UTF-8
        machine: x86_64
        release: 19.2.0
         system: Darwin
        version: 10.15.2 x86_64

[sagetherage]

de-scoping from Magnesium since there is still no test case written in the PR linked