search

puppetlabs / puppetlabs-puppetdb

A puppet module for installing and managing puppetdb
https://forge.puppetlabs.com/puppetlabs/puppetdb
Apache License 2.0
55 stars 231 forks source link

Revert class parameter removal #373

Closed austb closed 1 year ago

austb commented 1 year ago

@bastelfreak @ekohl I wanted to open this PR to get some more information from the community and for discussion about possibly reverting or changing removal of the parameter in this PR. We were concerned about the backward incompatibility of this change, since a user specifying the puppetdb_user in their manifest would result in a catalog failing to compile.

Is the primary goal of this change to reduce change events after a puppetdb package upgrade (when the default settings are used)?

We had a few alternate options that we'd like to get feedback on.

  1. Revert this change and change the ownership of the config file in the package so that it matches the puppetdb:puppetdb settings in the module
  2. Modify this change to allow a user to specify root as the owner to avoid the change event on upgrade
  3. Leave this change and re-add the parameter with a deprecation note so that users catalogs would continue to compile
austb commented 1 year ago

Wait, I'm sorry for the spam. I read the changed class as puppetdb::globals not puppetdb::server::globals, which is private.

bastelfreak commented 1 year ago

I pointed this out a few times: the whole module is in a bad shape. there are zero public tests. I would highly appreciate it if the CAT team takes over maintenance of it (or when a pipeline is added based on their standards).

austb commented 1 year ago

One question that I still have: would it be preferable to change this permission on the package side of things?

ekohl commented 1 year ago

My reasoning is that the service should not have permissions to modify its own config. It's managed by Puppet and giving it the permission to modify can only break things. So we make root the owner. It does need to read the config, so we use the group puppetdb. Then nobody else should read it, since the file contains credentials. So mode 0640 is used.

I haven't checked packaging, but that should apply the same permissions.

austb commented 1 year ago

Thanks for the responses. I will get a release of this module out shortly.