From 7d577437ac6bb66717c6280223db8bb3bb772695 Mon Sep 17 00:00:00 2001 From: Dmitry Volodin Date: Sat, 19 Feb 2022 16:52:42 +0300 Subject: [PATCH] Copy pod_environment_secret to the cluster namespaces --- ...gresql-operator-default-configuration.yaml | 2 +- .../v1/operator_configuration_type.go | 2 +- .../acid.zalan.do/v1/zz_generated.deepcopy.go | 1 + pkg/cluster/k8sres.go | 6 +- pkg/cluster/k8sres_test.go | 8 ++- pkg/cluster/resources.go | 18 +++++ pkg/cluster/sync.go | 62 +++++++++++++++- pkg/cluster/sync_test.go | 71 +++++++++++++++++-- pkg/cluster/util.go | 12 +++- pkg/util/config/config.go | 2 +- pkg/util/k8sutil/k8sutil.go | 16 +++++ 11 files changed, 181 insertions(+), 19 deletions(-) diff --git a/manifests/postgresql-operator-default-configuration.yaml b/manifests/postgresql-operator-default-configuration.yaml index 87b5436d5..946d3c736 100644 --- a/manifests/postgresql-operator-default-configuration.yaml +++ b/manifests/postgresql-operator-default-configuration.yaml @@ -79,7 +79,7 @@ configuration: pdb_name_format: "postgres-{cluster}-pdb" pod_antiaffinity_topology_key: "kubernetes.io/hostname" # pod_environment_configmap: "default/my-custom-config" - # pod_environment_secret: "my-custom-secret" + # pod_environment_secret: "default/my-custom-secret" pod_management_policy: "ordered_ready" # pod_priority_class_name: "postgres-pod-priority" pod_role_label: spilo-role diff --git a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go index 1298c6834..9586ca517 100644 --- a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go +++ b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go @@ -90,7 +90,7 @@ type KubernetesMetaConfiguration struct { // TODO: use a proper toleration structure? PodToleration map[string]string `json:"toleration,omitempty"` PodEnvironmentConfigMap spec.NamespacedName `json:"pod_environment_configmap,omitempty"` - PodEnvironmentSecret string `json:"pod_environment_secret,omitempty"` + PodEnvironmentSecret spec.NamespacedName `json:"pod_environment_secret,omitempty"` PodPriorityClassName string `json:"pod_priority_class_name,omitempty"` MasterPodMoveTimeout Duration `json:"master_pod_move_timeout,omitempty"` EnablePodAntiAffinity bool `json:"enable_pod_antiaffinity,omitempty"` diff --git a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go index d960cd102..20a91d88e 100644 --- a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go +++ b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go @@ -246,6 +246,7 @@ func (in *KubernetesMetaConfiguration) DeepCopyInto(out *KubernetesMetaConfigura } } out.PodEnvironmentConfigMap = in.PodEnvironmentConfigMap + out.PodEnvironmentSecret = in.PodEnvironmentSecret return } diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index a42aa2d06..5a39dc5eb 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -893,13 +893,13 @@ func (c *Cluster) getPodEnvironmentConfigMapVariables() ([]v1.EnvVar, error) { func (c *Cluster) getPodEnvironmentSecretVariables() ([]v1.EnvVar, error) { secretPodEnvVarsList := make([]v1.EnvVar, 0) - if c.OpConfig.PodEnvironmentSecret == "" { + if c.OpConfig.PodEnvironmentSecret.Name == "" { return secretPodEnvVarsList, nil } secret, err := c.KubeClient.Secrets(c.Namespace).Get( context.TODO(), - c.OpConfig.PodEnvironmentSecret, + c.OpConfig.PodEnvironmentSecret.Name, metav1.GetOptions{}) if err != nil { return nil, fmt.Errorf("could not read Secret PodEnvironmentSecretName: %v", err) @@ -910,7 +910,7 @@ func (c *Cluster) getPodEnvironmentSecretVariables() ([]v1.EnvVar, error) { v1.EnvVar{Name: k, ValueFrom: &v1.EnvVarSource{ SecretKeyRef: &v1.SecretKeySelector{ LocalObjectReference: v1.LocalObjectReference{ - Name: c.OpConfig.PodEnvironmentSecret, + Name: c.OpConfig.PodEnvironmentSecret.Name, }, Key: k, }, diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go index 503959f28..442828cd7 100644 --- a/pkg/cluster/k8sres_test.go +++ b/pkg/cluster/k8sres_test.go @@ -789,7 +789,9 @@ func TestPodEnvironmentSecretVariables(t *testing.T) { subTest: "Secret referenced by PodEnvironmentSecret does not exist", opConfig: config.Config{ Resources: config.Resources{ - PodEnvironmentSecret: "idonotexist", + PodEnvironmentSecret: spec.NamespacedName{ + Name: "idonotexist", + }, }, }, err: fmt.Errorf("could not read Secret PodEnvironmentSecretName: Secret PodEnvironmentSecret not found"), @@ -798,7 +800,9 @@ func TestPodEnvironmentSecretVariables(t *testing.T) { subTest: "Pod environment vars reference all keys from secret configured by PodEnvironmentSecret", opConfig: config.Config{ Resources: config.Resources{ - PodEnvironmentSecret: testPodEnvironmentSecretName, + PodEnvironmentSecret: spec.NamespacedName{ + Name: testPodEnvironmentSecretName, + }, }, }, envVars: []v1.EnvVar{ diff --git a/pkg/cluster/resources.go b/pkg/cluster/resources.go index 48a82ee04..0093d372a 100644 --- a/pkg/cluster/resources.go +++ b/pkg/cluster/resources.go @@ -526,6 +526,24 @@ func (c *Cluster) deleteSecret(uid types.UID, secret v1.Secret) error { return nil } +func (c *Cluster) getSecretWithRetry(name, namespace string) (*v1.Secret, error) { + secret := &v1.Secret{} + err := retryutil.Retry(c.OpConfig.ResourceCheckInterval, c.OpConfig.ResourceCheckTimeout, + func() (bool, error) { + var err error + secret, err = c.KubeClient.Secrets(namespace).Get( + context.TODO(), + name, + metav1.GetOptions{}) + if err != nil { + return false, nil + } + return true, nil + }, + ) + return secret, err +} + func (c *Cluster) createRoles() (err error) { // TODO: figure out what to do with duplicate names (humans and robots) among pgUsers return c.syncRoles() diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index bbf023764..5a6e3c3bf 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -19,6 +19,7 @@ import ( v1 "k8s.io/api/core/v1" policybeta1 "k8s.io/api/policy/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" ) var requireMasterRestartWhenDecreased = []string{ @@ -621,7 +622,6 @@ func (c *Cluster) checkAndSetGlobalPostgreSQLConfiguration(pod *v1.Pod, patroniC } func (c *Cluster) syncSecrets() error { - c.logger.Info("syncing secrets") c.setProcessName("syncing secrets") generatedSecrets := c.generateUserSecrets() @@ -629,6 +629,11 @@ func (c *Cluster) syncSecrets() error { retentionUsers := make([]string, 0) currentTime := time.Now() + c.logger.Debug("coping PodEnvironmentSecretName if needed") + if err := c.copyPodEnvironmentSecret(); err != nil { + return err + } + for secretUsername, generatedSecret := range generatedSecrets { secret, err := c.KubeClient.Secrets(generatedSecret.Namespace).Create(context.TODO(), generatedSecret, metav1.CreateOptions{}) if err == nil { @@ -795,6 +800,61 @@ func (c *Cluster) updateSecret( return nil } +func (c *Cluster) copyPodEnvironmentSecret() error { + if c.OpConfig.PodEnvironmentSecret.Name == "" { + return nil + } + // Searching for a Secret within a namespace defined by the configuration + originalSecret, err := c.getSecretWithRetry(c.OpConfig.PodEnvironmentSecret.Name, c.OpConfig.PodEnvironmentSecret.Namespace) + if err != nil { + return fmt.Errorf("could not read Secret PodEnvironmentSecretName: %w", err) + } + + if c.OpConfig.PodEnvironmentSecret.Namespace == c.Namespace { + return nil + } + // Attempting to find a Secret in the cluster's namespace if that namespace is not equal to the namespace defined by the configuration + secret, err := c.KubeClient.Secrets(c.Namespace).Get( + context.TODO(), + c.OpConfig.PodEnvironmentSecret.Name, + metav1.GetOptions{}) + if err != nil { + if !k8sutil.ResourceNotFound(err) { + return fmt.Errorf("could not read Secret PodEnvironmentSecretName in cluster namespace: %w", err) + } + // Secret within cluster namespace not found. Let's create it + createSecret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: originalSecret.Name, + Namespace: c.Namespace, + Labels: originalSecret.Labels, + Annotations: originalSecret.Annotations, + }, + Data: originalSecret.Data, + } + _, err = c.KubeClient.Secrets(c.Namespace).Create(context.TODO(), createSecret, metav1.CreateOptions{}) + return k8sutil.ResourceIgnoreAlreadyExists(err) + } + // The secret exists. We need to check if it needs to be updated or not + if !reflect.DeepEqual(originalSecret.Data, secret.Data) { + patchData, err := secretDataPath(originalSecret.Data) + if err != nil { + return fmt.Errorf("could not form patch for the Secret %q: %w", c.OpConfig.PodEnvironmentSecret.Name, err) + } + _, err = c.KubeClient.Secrets(c.Namespace).Patch( + context.TODO(), + c.OpConfig.PodEnvironmentSecret.Name, + types.MergePatchType, + patchData, + metav1.PatchOptions{}, + "") + if err != nil { + return fmt.Errorf("could not patch Secret %q: %w", c.OpConfig.PodEnvironmentSecret.Name, err) + } + } + return nil +} + func (c *Cluster) syncRoles() (err error) { c.setProcessName("syncing roles") diff --git a/pkg/cluster/sync_test.go b/pkg/cluster/sync_test.go index 226555a66..9aad371c8 100644 --- a/pkg/cluster/sync_test.go +++ b/pkg/cluster/sync_test.go @@ -2,17 +2,12 @@ package cluster import ( "bytes" + "context" "io/ioutil" "net/http" "testing" "time" - "context" - - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - "github.com/golang/mock/gomock" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" @@ -23,6 +18,9 @@ import ( "github.com/zalando/postgres-operator/pkg/util/config" "github.com/zalando/postgres-operator/pkg/util/k8sutil" "github.com/zalando/postgres-operator/pkg/util/patroni" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/fake" ) @@ -48,7 +46,8 @@ func newFakeK8sSyncClient() (k8sutil.KubernetesClient, *fake.Clientset) { func newFakeK8sSyncSecretsClient() (k8sutil.KubernetesClient, *fake.Clientset) { return k8sutil.KubernetesClient{ - SecretsGetter: clientSet.CoreV1(), + SecretsGetter: clientSet.CoreV1(), + NamespacesGetter: clientSet.CoreV1(), }, clientSet } @@ -365,3 +364,61 @@ func TestUpdateSecret(t *testing.T) { } } } + +func TestCopyPodEnvironmentSecret(t *testing.T) { + client, _ := newFakeK8sSyncSecretsClient() + + const ( + clusterNamespace = metav1.NamespaceDefault + clusterName = "acid-test-cluster" + secretEnvVar = "POD_ENV_VARIABLE" + secretNamespace = "secret-ns" + secretName = "pod-environment-secret" + ) + + podEnvironmentSecret := &v1.Secret{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: secretNamespace, + }, + StringData: map[string]string{ + secretEnvVar: "data", + }, + } + + var cluster = New( + Config{ + OpConfig: config.Config{ + Resources: config.Resources{ + PodEnvironmentSecret: spec.NamespacedName{ + Name: secretName, + Namespace: secretNamespace, + }, + ResourceCheckInterval: time.Duration(3), + ResourceCheckTimeout: time.Duration(10), + }, + }, + }, client, acidv1.Postgresql{}, logger, eventRecorder) + cluster.Name = clusterName + cluster.Namespace = clusterNamespace + + // Run copyPodEnvironmentSecret without podEnvironmentSecret presence in the namespace + err := cluster.copyPodEnvironmentSecret() + assert.Error(t, err) + + // Create a Secret in the separate namespace and try to run copyPodEnvironmentSecret again + _, err = client.Namespaces().Create(context.TODO(), &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: clusterNamespace}}, metav1.CreateOptions{}) + assert.NoError(t, err) + _, err = client.Secrets(secretNamespace).Create(context.TODO(), podEnvironmentSecret, metav1.CreateOptions{}) + assert.NoError(t, err) + + assert.NoError(t, cluster.copyPodEnvironmentSecret()) + // Validate, that original Secret was copied to the cluster namespace + copySecret, err := client.Secrets(clusterNamespace).Get(context.TODO(), secretName, metav1.GetOptions{}) + assert.NoError(t, err) + assert.Equal(t, podEnvironmentSecret.Data[secretEnvVar], copySecret.Data[secretEnvVar]) + + // Run copyPodEnvironmentSecret and validate that no error happens on retry (Secret already copied) + assert.NoError(t, cluster.copyPodEnvironmentSecret()) +} diff --git a/pkg/cluster/util.go b/pkg/cluster/util.go index 43c3282d4..9a2be2c8d 100644 --- a/pkg/cluster/util.go +++ b/pkg/cluster/util.go @@ -147,15 +147,21 @@ func specPatch(spec interface{}) ([]byte, error) { }{spec}) } +func secretDataPath(data interface{}) ([]byte, error) { + return json.Marshal(struct { + Data interface{} `json:"data"` + }{data}) +} + // metaAnnotationsPatch produces a JSON of the object metadata that has only the annotation // field in order to use it in a MergePatch. Note that we don't patch the complete metadata, since // it contains the current revision of the object that could be outdated at the time we patch. func metaAnnotationsPatch(annotations map[string]string) ([]byte, error) { - var meta metav1.ObjectMeta - meta.Annotations = annotations return json.Marshal(struct { ObjMeta interface{} `json:"metadata"` - }{&meta}) + }{&metav1.ObjectMeta{ + Annotations: annotations, + }}) } func (c *Cluster) logPDBChanges(old, new *policybeta1.PodDisruptionBudget, isUpdate bool, reason string) { diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index 0dc1004a7..04d03fda0 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -54,7 +54,7 @@ type Resources struct { MinCPULimit string `name:"min_cpu_limit" default:"250m"` MinMemoryLimit string `name:"min_memory_limit" default:"250Mi"` PodEnvironmentConfigMap spec.NamespacedName `name:"pod_environment_configmap"` - PodEnvironmentSecret string `name:"pod_environment_secret"` + PodEnvironmentSecret spec.NamespacedName `name:"pod_environment_secret"` NodeReadinessLabel map[string]string `name:"node_readiness_label" default:""` NodeReadinessLabelMerge string `name:"node_readiness_label_merge" default:"OR"` MaxInstances int32 `name:"max_instances" default:"-1"` diff --git a/pkg/util/k8sutil/k8sutil.go b/pkg/util/k8sutil/k8sutil.go index dd6ec1e8b..270cbdd69 100644 --- a/pkg/util/k8sutil/k8sutil.go +++ b/pkg/util/k8sutil/k8sutil.go @@ -124,6 +124,22 @@ func ResourceNotFound(err error) bool { return apierrors.IsNotFound(err) } +// ResourceIgnoreNotFound return an error ignoring IsNotFound +func ResourceIgnoreNotFound(err error) error { + if ResourceNotFound(err) { + return nil + } + return err +} + +// ResourceIgnoreAlreadyExists return an error ignoring IsAlreadyExists +func ResourceIgnoreAlreadyExists(err error) error { + if ResourceAlreadyExists(err) { + return nil + } + return err +} + // NewFromConfig create Kubernetes Interface using REST config func NewFromConfig(cfg *rest.Config) (KubernetesClient, error) { kubeClient := KubernetesClient{}