voxpupuli / puppet-selinux

Puppet Module to manage SELinux on RHEL machines
https://forge.puppet.com/puppet/selinux
Apache License 2.0
48 stars 148 forks source link

add missing dep on puppetlabs/selinux_core #355

Open jhoblitt opened 2 years ago

jhoblitt commented 2 years ago

This module is using the selmodule resource type provided by puppetlabs/selinux_core.

kenyon commented 2 years ago

I think we don't list core modules in metadata.json.

jhoblitt commented 2 years ago

The dep not being declared breaks rspec tests on my control repo.

smortex commented 2 years ago

Hum, yeah the *_core modules are bundled in the AIO packages, so I guess your tests are not running in an AIO context. On FreeBSD, we do not ship the *_core modules with the puppet package, so I add the *_core modules to my control-repo Puppetfile. Would that be an option for you?

Note that the bundled *_core module are not the latest ones, the only case I think we add dependencies for them to metadata.json is when we need a version that is more recent than what is shipped in the AIO package.

ekohl commented 2 years ago

Yes, *_core modules are in a weird place. We do need to specify them in .fixtures.yml because we use the gem to install them (which does not contain them). I suppose your control repo has a similar problem.

jhoblitt commented 2 years ago

I suspect everyone on this thread knows the history of the *_core modules and we don't need to rehash it.

The specific reason why this has caused me grief is that I use r10k with a branched env workflow. As everyone knows, r10k doesn't do [any] dependency resolution. The result of this is that changes to the Puppetfile have occasionally broken rspec tests as the *_core modules are not bundled with the rubygem puppet release (librarian-pupet is also run as a github action to catch incompatible recursive dependencies). Almost all of the *_core modules were already present in my Puppetfile and puppetlabs/selinux_core will have to be added.

However, I am arguing that puppetlabs/selinux_core is a proper dependency of this module. The fact that this module works on AIO without it declared does not mean this dependency does not exist. I would also argue that metadata.json would be usable as the source of fixtures for the vast majority of modules if the true dependencies on *_core were declared. The only downside to adding *_core deps into metadata.json is the potential maintenance burden of some day having to remove them if they become unnecessary.

The real question is: Do voxpupuli modules only support AIO provided puppet?

jhoblitt commented 2 years ago

If stdlib is added to vendor_modules in a future AIO release, would we then have to remove the dep on stdlib from metadata.json in all voxpupuli modules?

jcpunk commented 1 year ago

Personally, I'd rather see the dependencies declared even if they are guaranteed to be satisfied. Completeness is nice.

jhoblitt commented 1 year ago

I would like listing all deps to be the official voxpupuli policy.