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

feat: add [read-database] config #73

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pfrayer
Copy link

@pfrayer pfrayer commented Jun 26, 2024

PuppetDB supports a [read-database] settings: https://www.puppet.com/docs/puppetdb/8/configure.html#read-database-settings
PuppetDB needs to have 2 database users: one with RW, one with RO

It's mentioned in #43

This PR adds a simple read-database.conf in /etc/puppetlabs/puppetdb/conf.d/ to use this feature.
The default values for user comes from the official documentation: https://www.puppet.com/docs/puppetdb/8/configure_postgres.html

@pfrayer pfrayer requested a review from a team as a code owner June 26, 2024 10:51
@bastelfreak bastelfreak added the enhancement New feature or request label Jun 26, 2024
@bastelfreak
Copy link
Member

@pfrayer thanks for the PR! Can you check the used email address in the commit? It isn't associated with your github account.

@pfrayer pfrayer force-pushed the feat/pfrayer/read_database branch from e1b9112 to f203497 Compare June 26, 2024 11:52
@pfrayer
Copy link
Author

pfrayer commented Jun 26, 2024

@bastelfreak my bad, fixed

@rwaffen
Copy link
Member

rwaffen commented Jul 5, 2024

I think this is also a breaking change, because now you would have to make sure that you have a read user created. If someone simply updates the container with an already running configuration, this breaks.

rwaffen
rwaffen previously approved these changes Jul 19, 2024
@pfrayer
Copy link
Author

pfrayer commented Jul 19, 2024

You are right, as-is this is a breaking change.
I will make it more conditional to set the read-database.conf file only if PUPPETDB_READ_USER and PUPPETDB_READ_PASSWORD are given.

@rwaffen
Copy link
Member

rwaffen commented Jul 19, 2024

okay, so i should wait with the merge? or will you do another PR? ... just doing a v1.6.0 to preserve the present state and then i wanted to merge this one an do a 2.0.0 release. if if we could have a condition, that would be greate

@pfrayer
Copy link
Author

pfrayer commented Jul 19, 2024

Yes wait before merging this PR, I'll update it with the condition stuff.

@pfrayer pfrayer force-pushed the feat/pfrayer/read_database branch from 978232c to 89e2f8d Compare August 23, 2024 13:35
@pfrayer
Copy link
Author

pfrayer commented Aug 23, 2024

Sorry for the delay I was in holiday (so not so sorry)

I added the conditional logic about this read-database config. The idea is:

  • always include the read-database.conf file at build time
  • conditionally remove this file at run time if needed env vars are not set (in docker-entrypoint.sh)

Please tell me if this logic is OK for you

@rwaffen
Copy link
Member

rwaffen commented Aug 27, 2024

ah dang it. ci fails because cannot access the secrets from a fork-pr. will investigate later...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

3 participants