Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(PDB-2571) Ensure puppetdb.ini file has correct permissions #228

Conversation

kbarber
Copy link
Contributor

@kbarber kbarber commented Apr 28, 2016

Much like read-database.ini, we need to ensure the permissions for puppetdb.ini are set explicitly
to ensure permissions are still correct after configuration. Without this users with different umask
settings may find their files are no longer accessible after the module runs.

Signed-off-by: Ken Barber ken@bob.sh

@kbarber kbarber added this to the 5.1.3 (master) milestone Apr 28, 2016
@ajroetker
Copy link
Contributor

@kbarber Don't database.ini and jetty.ini have this same problem? Any reason not to fix those too?

@kbarber
Copy link
Contributor Author

kbarber commented May 5, 2016

@ajroetker actually they don't, they get installed by the package and are set absolutely to the correct permission afaik. This file resource I've fixed is only created by the module. Of course, we can always define permissions over all files we touch in the module if we think its a good idea, it may mean changing things in two places later on if we ever change our minds.

@senior
Copy link
Contributor

senior commented May 9, 2016

@kbarber any idea why there are ruby syntax errors on the older ruby versions we're testing?

@kbarber kbarber force-pushed the ticket/master/pdb-2571-fix-perms-of-config-files branch from 2347811 to 5a81cf0 Compare May 10, 2016 13:48
@kbarber
Copy link
Contributor Author

kbarber commented May 10, 2016

@senior trailing commas in 1.8.7 aren't supported, my fault.

Much like read-database.ini, we need to ensure the permissions for puppetdb.ini and others are set explicitly
to ensure permissions are still correct after configuration. Without this users with different umask
settings may find their files are no longer accessible after the module runs.

This patch fixes the globally for all the ini files we currently manage (repl.ini is not managed fwiw).

This also fixes a bug whereby we were missing puppetdb::server::global from the main server class, it adds this
back and fixes the tests to ensure we don't lose it.

Signed-off-by: Ken Barber <ken@bob.sh>
@kbarber kbarber force-pushed the ticket/master/pdb-2571-fix-perms-of-config-files branch from 5a81cf0 to 59100fd Compare May 12, 2016 17:31
@kbarber
Copy link
Contributor Author

kbarber commented May 12, 2016

@ajroetker okay, I've been a little more ambitious now, and managed all the files. Found a bug also, and added tests where needed.

@ajroetker
Copy link
Contributor

Awesome, testing this now!

@@ -6,12 +6,6 @@
$confdir = $puppetdb::params::confdir,
) inherits puppetdb::params {


file { "${confdir}/config.ini":
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just moved to global.pp, because well, it didn't feel like it should be here? I don't know - maybe we should just collapse both these pp files into one.

There are a lot of refactoring possibilities around configuration I think, right now we just keep copying files ... perhaps we can refactor it all into one big file, seems like its scaffolding for scaffolding sake.

@ajroetker ajroetker merged commit 6b26a28 into puppetlabs:master May 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants