[BUG] pkgrepo.managed always trigger changes if url ends with /

[marek-obuchowicz]

Description
After migrating to py3, I see that salt always wants to update package repos, if their definition contains templated values. Repositories are already setup.

Setup

Master/minion running salt 3003.3, Debian 9 VMs in AWS

• VM running on a cloud service, please be explicit and add details

Steps to Reproduce the behavior

php1-repo:
  pkgrepo.managed:
    - humanname: PHP7.1 repository
    - name: deb https://packages.sury.org/php/ {{ grains.lsb_distrib_codename }} main
    - file: /etc/apt/sources.list.d/php1.list
    - key_url: https://packages.sury.org/php/apt.gpg
    - refresh: False

php2-repo:
  pkgrepo.managed:
    - humanname: PHP7.1 repository
    - name: deb https://packages.sury.org/php/ stretch mainXXX # make it unique for this example
    - file: /etc/apt/sources.list.d/php2.list
    - key_url: https://packages.sury.org/php/apt.gpg
    - refresh: False

then run state.apply twice

Expected behavior
First run should add two package repositories. Second run should not detect any changes.

Screenshots
After applying state once, second (and each next) run still wants to change php1 repo:

local:
  Name: deb https://packages.sury.org/php/ stretch main - Function: pkgrepo.managed - Result: Differs Started: - 14:06:36.967772 Duration: 69.358 ms

Versions Report

salt --versions-report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)

Salt Version:
          Salt: 3003.3

Dependency Versions:
          cffi: Not Installed
      cherrypy: Not Installed
      dateutil: 2.5.3
     docker-py: Not Installed
         gitdb: 2.0.0
     gitpython: 2.1.1
        Jinja2: 2.9.4
       libgit2: Not Installed
      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: Not Installed
        Python: 3.5.3 (default, Apr  5 2021, 09:00:41)
  python-gnupg: Not Installed
        PyYAML: 3.12
         PyZMQ: 17.1.2
         smmap: 2.0.1
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.2.1

System Versions:
          dist: debian 9 stretch
        locale: UTF-8
       machine: x86_64
       release: 4.9.0-16-amd64
        system: Linux
       version: Debian GNU/Linux 9 stretch

[marek-obuchowicz]

note: before migration we've using python2 + salt 3000.3

[Ch3LL]

I'm able to replicate this. It was also working on python3 + salt 3000.3 and now is broken. Git bisect shows this commit 6df3ca4 as the first one to break things.

[marek-obuchowicz]

I think you are correct. It only affects some of repositories I'm setting up with saltstack. After having a look on this commit, I can confirm that problematic repositories include / at the end of repo URL, while those which work correctly (aren't re-created on subsequent runs) don't end with / character.

[nchauvat]

Hi,

The commit mentionned above says it stopped stripping the trailing slash to make sure pkgrepo.absent would be able to remove the source file when needed. Reverting does not seem to be the solution. What would be the correct behavior ?

Is someone working on a fix ? I am willing to help.

[marek-obuchowicz]

@nchauvat IMO, if repository specified in salt state either has or has not trailing slash, pkg.present and pkg.absent should both work (add and/or remove repository) without re-triggering the state change on subsequent calls.

[nchauvat]

@marek-obuchowicz the thing is that I am not writing the repository url.

I came to this bug from saltstack-formulas/salt-formula#533

As stated in that issue, the Salt Formula tries to add a source list file on Debian systems, but the output is different when test=true and when test=false.

The current issue #60907 is thought to be related to the above.

[marek-obuchowicz]

@nchauvat I just reported an issue with trailing /. I can't confirm/deny of those two issues are related to each other, but when I look at commit mentioned above by @Ch3LL - it seems that those issues might be independent.