search

puppetlabs / puppetlabs-apt

Puppet module to help manage Apt
https://forge.puppetlabs.com/puppetlabs/apt
Apache License 2.0
216 stars 462 forks source link

Puppet produces invalid list of sources that it cannot correct anymore #1133

Closed XSpielinbox closed 9 months ago

XSpielinbox commented 9 months ago

Describe the Bug

Leaving out the repos in an apt::source definition results in a main component being added implicitly. This however can result in an invalid apt-source when the release is /: When there is apt-source with the content like deb https://pkgs.k8s.io/core:/stable:/v1.28/deb / main, this is invalid (absolute Suite Component). When now working around this by defining

release => '/',
repos    => '',

this however still results in an error like the following:

Error: Could not prefetch package provider 'apt': Execution of '/usr/bin/apt-mark showmanual' returned 100: E: Malformed entry 3 in list file /etc/apt/sources.list.d/kubernetes.list (absolute Suite Component)
E: The list of sources could not be read.
Error: Failed to apply catalog: Execution of '/usr/bin/apt-mark showmanual' returned 100: E: Malformed entry 3 in list file /etc/apt/sources.list.d/kubernetes.list (absolute Suite Component)
E: The list of sources could not be read.

So puppet was able to successfully add the main component to the source but is unable to remove it again. One has to manually edit the corresponding source entry to something different before puppet will change it again.

Expected Behavior

Puppet is in all cases able to correct the error in an apt source definition.

Steps to Reproduce

Steps to reproduce the behavior:

  1. run puppet agent -t with an apt::source, that does not specify repos. // alternatively add it manually to the source
  2. change the apt::source to a repos ''.
  3. try to rerun puppet agent -t

Environment

Additional Context

This makes it impossible to workaround the issue, that Puppet enforces to specify a repos (components), even though they are optional in an apt source. see #1132

Ramesh7 commented 9 months ago

Hi @XSpielinbox, Thanks for raising and also providing the reproducing steps, using that I was able to repro the same. I have raised the same internally and will priorities and get back here. Thanks again !!

Ramesh7 commented 9 months ago

Hi @XSpielinbox, I have raised PR to better handing the $repos and $release, can you please review and suggest if it covers the expectation or please suggest changes if any. Thanks

Ramesh7 commented 9 months ago

@XSpielinbox wanted to know more detail around this problem, AFAIK the https://github.com/puppetlabs/puppetlabs-apt/pull/1138 should put a workaround as $repos either its passed from user or not based on $release it will form source list and update. Even though the $repos assigned to default if user is not passing or user explicitly passes it.

If I use $repos = '' & $release = '/' (ending with /) then it will generate source list : 

deb <url> /

Which is a valid source list and if we pass $release = 'test' then the source list will be :

deb <url> test main

Not sure if you want $repos to set undef and let user takes control of it?

XSpielinbox commented 9 months ago

@Ramesh7 I am not sure, whether I understood your question right. The core of this issue is that Puppet produces a file (that in a certain sense matches the state, defined by the user (that this state is flawed, shall not further be relevant here. This flaw is what #1132 is about)), that puppet cannot change anymore, because it apparently does not recognize that it differs from the new desired state.

The core of this issue is that I change the desired state and puppet does not correct the file accordingly. That the state it is stuck as is invalid may be part of the cause why puppet cannot change it anymore or completely unrelated - I don't know. To me what I deem relevant about it being an invalid state in this issue is the fact that it being an invalid state makes it a more serious issue to me, that puppet cannot change the file anymore, because with this puppet in a certain sense bricked my system.

For me it is totally ok or maybe even a valid use-case that puppet can produce a state that does not work properly, does not make sense or whatever, but puppet should ALWAYS be able to bring my system in another state, if I changed the code accordingly, that the desired state is a different one. (I am deliberately ignoring modifications here that are required for boot up or so. That if I destroy my kernel, puppet is not able to correct it anymore, I cannot complain about, but for some not that critical file like a config of apt I don't see why puppet should not be able to correct an invalid state.)

Ramesh7 commented 9 months ago

Thanks for detailed explanation @XSpielinbox, based on my understanding the puppet internally doing sha256 to guarantees the state of file is into desired state which I have observed working file while working on apt source list. I have manually modified as well as via code and it was able to bring into desired state without any issues. Can you please provide with reproduce steps so that I can re-pro and take it forward?

XSpielinbox commented 9 months ago

Hm. I will take a deeper look on what is necessary to reproduce it besides the steps already stated above. Perhaps I should add that I am still forced to use Puppet 6 (even though I know that it's EOL - I have been asking for month now to upgrade with no prevail). It still uses md5 as far as I can tell. I did not find a md5 hash collision though as far as I can tell. Do you think this could be related?

I will try to reproduce it with a never Puppet version in a private test setup.

kenyon commented 9 months ago

Best thing to do is create an acceptance test which causes the problem, then write the code to fix the test failures. Then there is less room for interpretation and error.

XSpielinbox commented 9 months ago

I am unable to reproduce the error with the same version of the puppetlabs-apt module, but Puppet version 7 instead of 6, running on Debian 11 instead of Ubuntu 20.04 (though I suspect the latter OS difference to not be the relevant difference here). Unfortunately, I don't have an easy access to an Ubuntu 20.04 Puppet 6 vs. 7 comparison test setup.

I would suspect however, that this is actually a bug in Puppet 6 and not in this puppet module. I would therefore close this issue. In case someone finds something that indicates otherwise, feel free to reopen or I can reopen it in that case too.