voxpupuli / container-puppetdb

Container image for PuppetDB
Apache License 2.0
9 stars 10 forks source link

feat: add [read-database] config #73

Open pfrayer opened 2 months ago

pfrayer commented 2 months ago

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

bastelfreak commented 2 months ago

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

pfrayer commented 2 months ago

@bastelfreak my bad, fixed

rwaffen commented 1 month ago

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.

pfrayer commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

pfrayer commented 4 days ago

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:

Please tell me if this logic is OK for you

rwaffen commented 11 hours ago

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