-
Notifications
You must be signed in to change notification settings - Fork 981
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
Allow pod environment variables to also be sourced from a secret #946
Conversation
@FxKu lol ... I event replied to that other PR and toally forgot about it. But if you agree that sourcing a secret for environment variables is a sensible addition to the operator (noting again, that Spilo just loves env variables - just like @kupson said: #481 (comment)) Also the ability to override the WAL_ and LOG_ parameters are an important addition to be able to let folks configure their own backup targets (we like to call it "bring your own bucket"). I gladly add some unit tests or copy some of @kupson if he does not mind and this PR might get merged. |
I would be fine with these changes. Unit test we've talked about are still missing, though ;) |
105b618
to
b52fb62
Compare
Yeah, really sorry for the delay. I'll get those done soon. |
b52fb62
to
e7dd7ec
Compare
@FxKu I did some cleaning up and added the unit tests greatly inspired by @kupson and his PR #481 I refrained from adding the hashing of the secrets contents to recognize changes which @kupson did implement. This is a more general problem (kubernetes/kubernetes#22368) in Kubernetes and applies to the ConfigMap which was a valid source for env variables just as well. Until Kubernetes "fixes" this upstream, there are tools like https://github.com/stakater/Reloader which could be added as a sidecar to achieve this functionality without it being a somewhat special and unexpected behavior among Kubernetes uses. |
Guys! please let's move on and merge this PR. |
@frittentheke thanks for the update. Can you rebase so that we get travis fixed. 3 very minor things I've overlooked so far. Can you add lines also in the configmap and default-config CRD manifests + one entry in the operatorconfigurations.yaml CRD in the helm chart. It's too many places to update, I have to admit. |
…ust like pod_environment_configmap
…ef to protect the value)
…ets before doing the global settings from the operator config. This allows them to be overriden by the user (via configmap / secret)
…secret) to admin documentation
… highly inspired by @kupson and his very similar PR zalando#481
…configmap examples
@FxKu I added the entries to the example operator config CRD and configmap. |
e7dd7ec
to
b34db30
Compare
Thanks for fixing 2/3 😃 I should have used links. |
@FxKu urgh, sorry for missing that one ... pretty much the most important one ;-) |
👍 |
1 similar comment
👍 |
Thanks @frittentheke for your work (and patience). Also special thanks to @kupson for bringing up the idea and writing tests which were obtained here. |
@frittentheke @FxKu guys, May I know in which version this feature (defining secret var in configmap) will be released. |
This requires the secret to be in the same namespace as the Postgres cluster, right? I'm using Postgres clusters in many different namespaces. It would be nice if there could be a single secret in the postgres-operator namespace. Any ideas on how to implement this securely? Maybe, for each Postgres cluster, the postgres-operator could copy the secret from the postgres-operator namespace to the Postgres cluster namespace. |
This might be a little out-of-scope for postgres-operator I think. Sharing/cloning secrets across namespaces is a wider configuration issue with it's own solutions like: |
@vijay-dcrust it's released in 1.6 now https://github.com/zalando/postgres-operator/releases/tag/v1.6.0 |
Currently the Operator allows to configure a ConfigMap which key-value pairs are then added to the pod environent variables (https://postgres-operator.readthedocs.io/en/latest/administrator/#custom-pod-environment-variables).
Depending on your clusters RBAC roles and settings, ConfigMaps are usually accessible quite freely (compared to secrets, which can be restricted independently) and having the value exposed in the pod spec adds to this issue of exposing credentials to access all your valuable data.
This PR adds the same functionality, but using a Secret as a source and references to the keys in the secret instead of copying the key-value pairs - like suggested in the two issues #480 and #597
But to allow for a bit more flexibility I changed the order in which the
WAL_
andLOG_
envVars are applied. This then allows for those variables to be overruled by those with the same key from the referenced Secret (or ConfigMap). This change applies only to this short "whilelist" of variables. There is still protection to not have someone fiddle with the variables required for the operator to configure the core functionality.With Spilo taking all its configuration and switches from environment variables, this PR intends to allow for more flexibility, such as to set custom S3 buckets. Also just setting custom (AWS) credentials is now possible (see my issues with this in #858 (comment))