Populate pod environment variables from secrets #481

kupson · 2019-02-04T16:17:46Z

Fixes #480.

Example Secret:

apiVersion: v1
kind: Secret
type: Opaque
metadata:
  name: minio-secret-test
data:
  access_key: dGVzdAo=
  secret_key: dGVzdAo=

Configuration:

apiVersion: acid.zalan.do/v1
configuration:
  ...
  kubernetes:
    ...
    pod_environment_secret_name: minio-secret-test
    pod_environment_secret_keys:
      access_key: AWS_ACCESS_KEY_ID
      secret_key: AWS_SECRET_ACCESS_KEY

Generated Pod config snippet:

  env:
    - name: AWS_ACCESS_KEY_ID
      valueFrom:
        secretKeyRef:
          key: access_key
          name: minio-secret-test
    - name: AWS_SECRET_ACCESS_KEY
      valueFrom:
        secretKeyRef:
          key: secret_key
          name: minio-secret-test

pkg/cluster/k8sres.go

coveralls · 2019-02-04T16:25:17Z

Coverage remained the same at 23.705% when pulling cfa128d701e105f9d3adf19c91e906e674baeea9 on kupson:podenv-via-secrets into ba23de3 on zalando-incubator:master.

docs/administrator.md

sdudoladov · 2019-02-19T13:59:48Z

docs/reference/operator_parameters.md

it worth mentioning that the operator will not sync this secret unlike some others secrets

I updated the MR to mention that SecretKeySelector is used to reference secret's key. Do you think that's good enough?

Should be fixed by kupson@2fa68d1

sdudoladov · 2019-02-19T14:05:47Z

pkg/apis/acid.zalan.do/v1/operator_configuration_type.go

This means there will be a single k8s secret with env vars shared between all pods of PG clusters. In the case of minio this is valid behavior, otherwise one would need to setup access control. This needs at least some documentation

That's consistent with behaviour of PodEnvironmentConfigMap - a single ConfigMap that's shared between all pods.

I think of PodEnvironmentSecretName/PodEnvironmentSecretKeys as of secret version of PodEnvironmentConfigMap.

sdudoladov · 2019-02-19T14:10:25Z

pkg/cluster/k8sres.go

why sorting ? to simplify displaying ?

I moved it moved from:

postgres-operator/pkg/cluster/k8sres.go

Lines 777 to 778 in 5f6abfb

sort.Slice(customPodEnvVarsList,

func(i, j int) bool { return customPodEnvVarsList[i].Name < customPodEnvVarsList[j].Name })

That's because the customPodEnvVarsList is being appended not only by PodEnvironmentConfigMap code but also PodEnvironmentSecretName.

Mentioned in: https://github.com/zalando/postgres-operator/pull/481/files#r258046362

…nvironment_secret_name

pkg/cluster/k8sres.go

Results of: hack/update-codegen.sh

Use impossible symbol \x00 as a separator. This ensures that "AB: CD" and "A: BCD" produce different hashes.

…o separate functions - better variable names - better error handling

Now this function only handles Pod environment variables and it's hashes.

… expected to go live.

Logs: spec diff between old and new statefulsets: Template.ObjectMeta.Annotations: map[string]string(nil) != map[string]string{} ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Template.Spec.InitContainers[0].TerminationMessagePath: "/dev/termination-log" != "" Template.Spec.InitContainers[0].TerminationMessagePolicy: "File" != "" Template.Spec.InitContainers[0].ImagePullPolicy: "Always" != "" Template.Spec.Containers[0].TerminationMessagePath: "/dev/termination-log" != "" Template.Spec.Containers[0].TerminationMessagePolicy: "File" != "" Template.Spec.RestartPolicy: "Always" != "" Template.Spec.DNSPolicy: "ClusterFirst" != "" Template.Spec.DeprecatedServiceAccount: "infra-postgres-operator-dev-dmz1-comm-patroni" != "" Template.Spec.SecurityContext: &v1.PodSecurityContext{} != nil Template.Spec.SchedulerName: "default-scheduler" != "" Template.Spec.Tolerations: []v1.Toleration(nil) != []v1.Toleration{} VolumeClaimTemplates[0].Status.Phase: "Pending" != "" PodManagementPolicy: "OrderedReady" != "" UpdateStrategy.Type: "OnDelete" != "" RevisionHistoryLimit: &int32(10) != nil

FxKu · 2019-06-19T13:06:53Z

@kupson we went for the secret mount approach by @Shinzu. With the mounted Volume the secret is always in sync compared to setting env variables. Would this also work for you?

kupson · 2019-07-02T09:38:37Z

It would be suboptimal as it would require some scripting at the Patroni container start. Patroni is configured by environment variables so this approach seems more natural -- why use Volumes for configuration imported for secrets but otherwise have pod_environment_configmap?

The PGPASSWORD_STANDBY and PGPASSWORD_SUPERUSER environment variables already holds values from secrets. Are there plans to change them to volumes too?

BTW: Any change in the source Secrets would cause annotation update and Pod restart so the Env variables would be in sync with the Secret values.

frittentheke · 2020-03-20T14:14:55Z

@FxKu ... we just reached the same issue @kupson was having about the configuration of the Spilo startup. See my remarks / analysis in comment: #858 (comment)

I was about to open a new issue / discussion regarding proper "multi tenancy" configuration of the operators' S3 configuration.

Would you reconsider this PR or even want to rework the cloud credential handling to allow i.e. "k8s namespaces" / "teams" / "tenants" to bring their own buckets and cloud credentials.

With the centrally configured bucket, shared AWS credentials and even with s3:prefix filters in the bucket policy no real protection apart from the random UID in the path (which one needs to know) there always remains the danger that someone fetched backups of a fully unrelated database not even belonging to team / namespace.

@kupson

… highly inspired by @kupson and his very similar PR zalando#481

@kupson

* Extend operator configuration to allow for a pod_environment_secret just like pod_environment_configmap * Add all keys from PodEnvironmentSecrets as ENV vars (using SecretKeyRef to protect the value) * Apply envVars from pod_environment_configmap and pod_environment_secrets before doing the global settings from the operator config. This allows them to be overriden by the user (via configmap / secret) * Add ability use a Secret for custom pod envVars (via pod_environment_secret) to admin documentation * Add pod_environment_secret to Helm chart values.yaml * Add unit tests for PodEnvironmentConfigMap and PodEnvironmentSecret - highly inspired by @kupson and his very similar PR #481 * Added new parameter pod_environment_secret to operatorconfig CRD and configmap examples * Add pod_environment_secret to the operationconfiguration CRD Co-authored-by: Christian Rohmann <[email protected]>

FxKu · 2020-07-30T08:52:29Z

Closing this now as the successor PR #946 got merged. Thanks again @kupson

kupson requested review from CyberDem0n, FxKu, Jan-M, alexeyklyukin, avaczi, erthalion and sdudoladov as code owners February 4, 2019 16:17

golangcibot reviewed Feb 4, 2019

View reviewed changes

pkg/cluster/k8sres.go Outdated Show resolved Hide resolved

sdudoladov reviewed Feb 19, 2019

View reviewed changes

kupson added 4 commits March 19, 2019 18:05

Populate pod environment variables from secrets

bb7e74f

Fix suggested by golangcibot

669182a

Address review comment - ConfigMap is not deprecated

199784a

Mentioned in: https://github.com/zalando/postgres-operator/pull/481/files#r258046362

Be clear that SecretKeySelector is used to reference secrets in pod_e…

aa99b59

…nvironment_secret_name

kupson force-pushed the podenv-via-secrets branch from cfa128d to aa99b59 Compare March 20, 2019 13:29

golangcibot reviewed Mar 25, 2019

View reviewed changes

pkg/cluster/k8sres.go Outdated Show resolved Hide resolved

pkg/cluster/k8sres.go Outdated Show resolved Hide resolved

pkg/cluster/k8sres.go Outdated Show resolved Hide resolved

kupson changed the title ~~Populate pod environment variables from secrets~~ WIP: Populate pod environment variables from secrets Mar 26, 2019

kupson added 10 commits March 26, 2019 18:34

Autogenerated code for PodEnvironmentSecretKeys

ee2707e

Results of: hack/update-codegen.sh

Restart StatefulSet Pods on pod environment secret's changes.

36211c6

Hash.Write() cannot return error, ignore returned values.

276818e

Use separator when updating hash

3d4cf56

Use impossible symbol \x00 as a separator. This ensures that "AB: CD" and "A: BCD" produce different hashes.

Extract PodEnvironmentConfigMap and PodEnvironmentSecret handling int…

cf15625

…o separate functions - better variable names - better error handling

Better annotation name

4833dff

Less indentation

641373c

Comment about hash content

79a63d1

Extract annotations handling from getPodEnvironmentSecretVariables()

6279942

Now this function only handles Pod environment variables and it's hashes.

Move annotation format string to constants module

01a007a

kupson force-pushed the podenv-via-secrets branch from 9b4c6f8 to 01a007a Compare March 26, 2019 18:36

kupson added 2 commits March 27, 2019 14:58

Add unit tests for PodEnvironmentConfigMap and PodEnvironmentSecretName

5305c85

Update documentation - when changes in PodEnv ConfigMap or Secret are…

723fde1

… expected to go live.

kupson mentioned this pull request Mar 27, 2019

goveralls timeouts on pull requests #523

Closed

kupson added 2 commits March 27, 2019 17:59

Don't add follow-secret annotation if there is no secret

a117e2e

erthalion self-assigned this Mar 29, 2019

kupson changed the title ~~WIP: Populate pod environment variables from secrets~~ Populate pod environment variables from secrets Apr 1, 2019

add hashUpdate helper

bd0df37

kupson force-pushed the podenv-via-secrets branch from 2028cae to bd0df37 Compare April 1, 2019 13:59

redbaron mentioned this pull request Apr 4, 2019

Add secret mount to operator #535

Merged

FxKu mentioned this pull request Apr 28, 2020

Allow pod environment variables to also be sourced from a secret #946

Merged

frittentheke added a commit to frittentheke/postgres-operator that referenced this pull request Jul 15, 2020

Add unit tests for PodEnvironmentConfigMap and PodEnvironmentSecret -…

d64c467

… highly inspired by @kupson and his very similar PR zalando#481

FxKu closed this Jul 30, 2020

	sort.Slice(customPodEnvVarsList,
	func(i, j int) bool { return customPodEnvVarsList[i].Name < customPodEnvVarsList[j].Name })

Populate pod environment variables from secrets #481

Populate pod environment variables from secrets #481

Uh oh!

Conversation

kupson commented Feb 4, 2019

Uh oh!

Uh oh!

coveralls commented Feb 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

sdudoladov Feb 19, 2019

Choose a reason for hiding this comment

Uh oh!

kupson Mar 20, 2019

Choose a reason for hiding this comment

Uh oh!

kupson Mar 25, 2019

Choose a reason for hiding this comment

Uh oh!

sdudoladov Feb 19, 2019

Choose a reason for hiding this comment

Uh oh!

kupson Mar 19, 2019

Choose a reason for hiding this comment

Uh oh!

sdudoladov Feb 19, 2019

Choose a reason for hiding this comment

Uh oh!

kupson Mar 19, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

FxKu commented Jun 19, 2019

Uh oh!

kupson commented Jul 2, 2019

Uh oh!

frittentheke commented Mar 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FxKu commented Jul 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

coveralls commented Feb 4, 2019 •

edited

Loading

frittentheke commented Mar 20, 2020 •

edited

Loading

FxKu commented Jul 30, 2020 •

edited

Loading