postgresql-container
postgresql-container copied to clipboard
WIP: use secrets for postgres creds
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.
Can one of the admins verify this patch?
Can one of the admins verify this patch?
Can one of the admins verify this patch?
[test]
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?
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..
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 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?
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.
That makes sense to me. Can we work with subdirectories like .../userA/password, etc.?
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.
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.
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.
Can one of the admins verify this patch?
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.
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!
Can one of the admins verify this patch?
Can one of the admins verify this patch?
Can one of the admins verify this patch?
Can one of the admins verify this patch?
Can one of the admins verify this patch?
Can one of the admins verify this patch?