[BUG] output different with test=true

[nchauvat]

Hi,

I am using salt-formula v1.10.2 with salt 3004 on Debian systems.

When applying the state, the generated salt.list contains the line

deb [signed-by=/usr/share/keyrings/salt-archive-keyring.gpg arch=amd64] https://repo.saltproject.io/py3/debian/10/amd64/3004 buster main

when testing the state, it tells that I change will be made

          ----------
          line:
              ----------
              new:
                  deb [arch=amd64] https://repo.saltproject.io/py3/debian/10/amd64/3004 buster main
              old:
                  deb [signed-by=/usr/share/keyrings/salt-archive-keyring.gpg arch=amd64] https://repo.saltproject.io/py3/debian/10/amd64/3004 buster main

this sounds like a false change warning, doesn't it ?

[myii]

this sounds like a false change warning, doesn't it ?

@nchauvat No, that's actually expected, after #514 was merged just over half a year ago. You can have a look at the discussion and commit(s) there. I'll close this issue for now but feel free to add any further comments/queries.

[nchauvat]

Hi @myii

I do not understand the link with the PR you point me to. It looks like it is about renaming from saltstack.list to salt.list which is not something I have a problem about. Could you be more specific ?

The problem I have with v1.10.2 is that running with test=true tells it will change something (remove the signed-by part), but applying the state does not actually change anything. Hence my question about a false change warning.

[dafyddj]

I have experienced this myself and was going to investigate. AFAICT this is unrelated to #514.

I won't reopen though, as I believe the issue is in the upstream Salt project.

[nchauvat]

Hi @dafyddj

What makes you think the problem lies in Salt ? Is there something I could do to help you investigate ?

[hatifnatt]

There is a plenty of issues with pkgrepo.managed on Debian based systems i.e.:

• [BUG] pkgrepo.managed always returns a change with "non standard declaration" saltstack/salt#61483
• [BUG] pkgrepo.managed always trigger changes if url ends with / saltstack/salt#60907

Therefore this issue is not specific to this formula.

[myii]

Apologies, @nchauvat -- there have been a number of different problems with the pkgrepo module recently; I got mixed up between them when looking at your output. Thanks to @dafyddj and @hatifnatt for clarifying things.

I've reopened this issue, not because there's a bug with the formula per se -- but one of two things needs to happen first (and ideally both, if time/effort permits):

1. The Debian-family pkgrepo state needs to be replaced by the file.managed method that @javierbertoli has been providing across other formulas so far:
   1. feat(debian): use keyrings instead of key_ids nginx-formula#289
   2. feat(debian): use repository keyring instead of key_id postgres-formula#322
   3. feat(debian): use repository keyring instead of key_id docker-formula#312
2. This specific bug encountered with test=true should be reported to the saltstack/salt repo.
   1. I've done some introspection using pudb and it really isn't a pretty situation -- the problem extends even beyond the Salt code and into the python-apt modules.
   2. Briefly, the problem starts here, where sanitizedkwargs comes back without the signed-by=/usr/share/keyrings/salt-archive-keyring.gpg bit.
   3. When test=true, this results in sanitizedkwargs being used to simulate the comparison before and after.

Realistically, number 1 is the obvious resolution here. Number 2 will take a fair bit of time writing the bug report, which I don't have right now.