postgresql-container icon indicating copy to clipboard operation
postgresql-container copied to clipboard

WIP: use secrets for postgres creds

Open ibotty opened this issue 9 years ago • 22 comments

See issue #101.

This is only in the 9.5 directory for now.

What do you think about it? I don't really like the logic re "simple_db", etc, but I left it as is. When using secrets, it might make sense to not use magic account names (e.g. the admin user postgres can be in any secret, no need for "user" credentials, one valid set of credentials is enough, etc.). If you have a preference on how that should be handled, I'll gladly implement it.

ibotty avatar Sep 26 '16 20:09 ibotty

Can one of the admins verify this patch?

openshift-bot avatar Sep 26 '16 20:09 openshift-bot

Can one of the admins verify this patch?

centos-ci avatar Sep 26 '16 20:09 centos-ci

Can one of the admins verify this patch?

rhscl-bot avatar Sep 26 '16 20:09 rhscl-bot

[test]

praiskup avatar Sep 29 '16 13:09 praiskup

As long as some async inotify machinery isn't involved, I am not strictly against this. But it is basically useless, because environment variables can be used securely, too (see issue #101). So, to me, this doesn't help security and adds additional complexity for maintainers, reviewers, qa and everybody listening to this code-stream.

The thing is that this is still only going to be used by OpenShift, and we basically add yet another API for container <-> environment communication. Do we plan to remove the support for env. variables?

praiskup avatar Sep 29 '16 13:09 praiskup

Also, there is already prefered API --- kube can directly contact the server (e.g. via unix socket or IP), do such things in transaction way, and react on PostgreSQL errors properly. Except for protecting against technical debt, I'm mostly interested saving everybody's resources..

praiskup avatar Sep 29 '16 14:09 praiskup

But it is basically useless, because environment variables can be used securely, too (see issue #101).

For me the most important part of this patch is, that it makes it possible to keep in sync many database users/credentials. That's not nicely done with env vars (and pretty verbose too).

ibotty avatar Sep 29 '16 14:09 ibotty

@ibotty I still don't follow, what is meant by many database users/credentials? Do you mean many users within single container, or syncing one user/credentials among many containers?

praiskup avatar Sep 29 '16 14:09 praiskup

Sorry, we are talking past each other. So: an example (I am running a variation of that patch in production):

I have many database users connecting to a single database (running in one container). To do that, I mount many secrets (user1-pgcreds, ..., userN-pgcreds) in the container (mountpath: /run/secrets/pgusers/user1, ..., /run/secrets/pgusers/userN). On container start, all database users get set the right password.

ibotty avatar Sep 29 '16 14:09 ibotty

That makes sense to me. Can we work with subdirectories like .../userA/password, etc.?

praiskup avatar Sep 29 '16 15:09 praiskup

I mean, I see the benefit of defining new "API" which is able to handle more complicated init scenarios, I'm not sure that working with files is better than simply allowing you to bind-mount arbitrary script and sourcing/executing it.

praiskup avatar Sep 29 '16 15:09 praiskup

Subdirectories .../userA/password? If you mean, that .../userA is a directory, and password is a file, that's how it works. A secret of the form

username: my_user
password: my_pass

is mapped to the filesystem when mounted as two files in /path/to/secret, username and password.

ibotty avatar Sep 29 '16 15:09 ibotty

I do understand your argument re code complexity btw. I am totally fine with weeding out the replicated logic re simple_db, and only doing the traverse users directory part.

ibotty avatar Sep 29 '16 15:09 ibotty

Can one of the admins verify this patch?

rhscl-bot avatar Oct 10 '16 08:10 rhscl-bot

it definitely seems simpler to have the image support an init script (Which i think we already have a PR for) and allow that init script to be provided via a mounted secret. The init script can then create multiple users, or do other useful things.

This mounting of additional files in this way feels fragile.

bparees avatar Oct 10 '16 17:10 bparees

I disagree. Without the possibility to set multiple users it's not easily possible to, say, have a template generate an application, consisting of a database pod, a management pod and a frontend pod (with restricted db permissions). The implementation of that feature is really easy. Most parts are only moving around functionality (which makes sense regardless of mounting creds with secrets). If the crude simpledb logic were not included, the core part of that patch is maybe 5 lines!

ibotty avatar Oct 10 '16 17:10 ibotty

Can one of the admins verify this patch?

centos-ci avatar Nov 25 '16 12:11 centos-ci

Can one of the admins verify this patch?

centos-ci avatar Feb 07 '17 12:02 centos-ci

Can one of the admins verify this patch?

rhscl-bot avatar Nov 28 '17 15:11 rhscl-bot

Can one of the admins verify this patch?

rhscl-bot avatar Sep 14 '18 02:09 rhscl-bot

Can one of the admins verify this patch?

rhscl-bot avatar Jul 19 '19 08:07 rhscl-bot

Can one of the admins verify this patch?

rhscl-automation avatar Sep 03 '20 08:09 rhscl-automation