From a1894ab2b0d919a9c52981153686f6cbb1e08000 Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Wed, 9 Jul 2025 13:02:33 +0300 Subject: [PATCH 1/5] K8SPG-570 update custom secret with labels when they are missing --- .../controller/postgrescluster/postgres.go | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/internal/controller/postgrescluster/postgres.go b/internal/controller/postgrescluster/postgres.go index 22e9e5453..32697e570 100644 --- a/internal/controller/postgrescluster/postgres.go +++ b/internal/controller/postgrescluster/postgres.go @@ -19,8 +19,10 @@ import ( "github.com/pkg/errors" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/controller-runtime/pkg/client" @@ -494,6 +496,14 @@ func (r *Reconciler) reconcilePostgresUserSecrets( userSpecs[string(specUsers[i].Name)] = &specUsers[i] } + for _, user := range specUsers { + if user.SecretName != "" { + if err := r.updateCustomSecretLabels(ctx, cluster, user); err != nil { + return specUsers, nil, err + } + } + } + secrets := &corev1.SecretList{} selector, err := naming.AsSelector(naming.ClusterPostgresUsers(cluster.Name)) if err == nil { @@ -582,6 +592,51 @@ func (r *Reconciler) reconcilePostgresUserSecrets( return specUsers, userSecrets, err } +// updateCustomSecretLabels checks if a custom secret exists and updates it +// with required labels if they are missing that enabled the +// naming.AsSelector(naming.ClusterPostgresUsers(cluster.Name)) to identify them. +func (r *Reconciler) updateCustomSecretLabels( + ctx context.Context, cluster *v1beta1.PostgresCluster, user v1beta1.PostgresUserSpec, +) error { + secretName := string(user.SecretName) + userName := string(user.Name) + + secret := &corev1.Secret{} + err := r.Client.Get(ctx, types.NamespacedName{ + Name: secretName, + Namespace: cluster.Namespace, + }, secret) + if err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + return errors.Wrap(err, fmt.Sprintf("failed to get user %s secret %s", userName, secretName)) + } + + requiredLabels := map[string]string{ + naming.LabelCluster: cluster.Name, + naming.LabelPostgresUser: userName, + } + + needsUpdate := false + if secret.Labels == nil { + secret.Labels = make(map[string]string) + } + + for labelKey, labelValue := range requiredLabels { + if existing, exists := secret.Labels[labelKey]; !exists || existing != labelValue { + secret.Labels[labelKey] = labelValue + needsUpdate = true + } + } + + if needsUpdate { + return errors.WithStack(r.Client.Update(ctx, secret)) + } + + return nil +} + // reconcilePostgresUsersInPostgreSQL creates users inside of PostgreSQL and // sets their options and database access as specified. func (r *Reconciler) reconcilePostgresUsersInPostgreSQL( From 6c9d857df81413cdbd43191b56d34ef89fc3ddf6 Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Wed, 9 Jul 2025 14:00:58 +0300 Subject: [PATCH 2/5] use patch instead of update --- internal/controller/postgrescluster/postgres.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/controller/postgrescluster/postgres.go b/internal/controller/postgrescluster/postgres.go index 32697e570..de7d73389 100644 --- a/internal/controller/postgrescluster/postgres.go +++ b/internal/controller/postgrescluster/postgres.go @@ -613,6 +613,8 @@ func (r *Reconciler) updateCustomSecretLabels( return errors.Wrap(err, fmt.Sprintf("failed to get user %s secret %s", userName, secretName)) } + orig := secret.DeepCopy() + requiredLabels := map[string]string{ naming.LabelCluster: cluster.Name, naming.LabelPostgresUser: userName, @@ -631,7 +633,7 @@ func (r *Reconciler) updateCustomSecretLabels( } if needsUpdate { - return errors.WithStack(r.Client.Update(ctx, secret)) + return errors.WithStack(r.Client.Patch(ctx, secret.DeepCopy(), client.MergeFrom(orig))) } return nil From 7e7e05b1d33ba0b6cd5adc9d29f9986032714011 Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Wed, 9 Jul 2025 18:04:12 +0300 Subject: [PATCH 3/5] adjustsments on the logic --- internal/controller/postgrescluster/postgres.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/internal/controller/postgrescluster/postgres.go b/internal/controller/postgrescluster/postgres.go index de7d73389..ab4052a28 100644 --- a/internal/controller/postgrescluster/postgres.go +++ b/internal/controller/postgrescluster/postgres.go @@ -583,8 +583,6 @@ func (r *Reconciler) reconcilePostgresUserSecrets( if err == nil { userSecrets[userName], err = r.generatePostgresUserSecret(cluster, user, secret) - } - if err == nil { err = errors.WithStack(r.apply(ctx, userSecrets[userName])) } } @@ -592,8 +590,8 @@ func (r *Reconciler) reconcilePostgresUserSecrets( return specUsers, userSecrets, err } -// updateCustomSecretLabels checks if a custom secret exists and updates it -// with required labels if they are missing that enabled the +// updateCustomSecretLabels checks if a custom secret exists - can be created manually through kubectl apply +// and updates it with required labels if they are missing that enabled the // naming.AsSelector(naming.ClusterPostgresUsers(cluster.Name)) to identify them. func (r *Reconciler) updateCustomSecretLabels( ctx context.Context, cluster *v1beta1.PostgresCluster, user v1beta1.PostgresUserSpec, From 32e847dd369192a92de89053b19aab87116d83cf Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Thu, 10 Jul 2025 12:42:26 +0300 Subject: [PATCH 4/5] fix concurrency issue by checking the secret update --- .../controller/postgrescluster/postgres.go | 67 +++++++++++++++++-- 1 file changed, 61 insertions(+), 6 deletions(-) diff --git a/internal/controller/postgrescluster/postgres.go b/internal/controller/postgrescluster/postgres.go index ab4052a28..b858ed80b 100644 --- a/internal/controller/postgrescluster/postgres.go +++ b/internal/controller/postgrescluster/postgres.go @@ -25,6 +25,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/percona/percona-postgresql-operator/internal/feature" @@ -496,6 +497,9 @@ func (r *Reconciler) reconcilePostgresUserSecrets( userSpecs[string(specUsers[i].Name)] = &specUsers[i] } + // K8SPG-570 for secrets that were created manually, update them + // with the right labels so that the selector called next to track them + // and utilize their data. for _, user := range specUsers { if user.SecretName != "" { if err := r.updateCustomSecretLabels(ctx, cluster, user); err != nil { @@ -590,9 +594,10 @@ func (r *Reconciler) reconcilePostgresUserSecrets( return specUsers, userSecrets, err } -// updateCustomSecretLabels checks if a custom secret exists - can be created manually through kubectl apply -// and updates it with required labels if they are missing that enabled the -// naming.AsSelector(naming.ClusterPostgresUsers(cluster.Name)) to identify them. +// K8SPG-570 +// updateCustomSecretLabels checks if a custom secret exists - can be created manually through +// kubectl apply - and updates it with required labels if they are missing. This enables the +// naming.AsSelector(naming.ClusterPostgresUsers(cluster.Name)) to identify these secrets. func (r *Reconciler) updateCustomSecretLabels( ctx context.Context, cluster *v1beta1.PostgresCluster, user v1beta1.PostgresUserSpec, ) error { @@ -611,11 +616,10 @@ func (r *Reconciler) updateCustomSecretLabels( return errors.Wrap(err, fmt.Sprintf("failed to get user %s secret %s", userName, secretName)) } - orig := secret.DeepCopy() - requiredLabels := map[string]string{ naming.LabelCluster: cluster.Name, naming.LabelPostgresUser: userName, + naming.LabelRole: naming.RolePostgresUser, } needsUpdate := false @@ -631,7 +635,58 @@ func (r *Reconciler) updateCustomSecretLabels( } if needsUpdate { - return errors.WithStack(r.Client.Patch(ctx, secret.DeepCopy(), client.MergeFrom(orig))) + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + current := &corev1.Secret{} + if err := r.Client.Get(ctx, types.NamespacedName{ + Name: secretName, + Namespace: cluster.Namespace, + }, current); err != nil { + return err + } + + currentOrig := current.DeepCopy() + if current.Labels == nil { + current.Labels = make(map[string]string) + } + + updateNeeded := false + for labelKey, labelValue := range requiredLabels { + if existing, exists := current.Labels[labelKey]; !exists || existing != labelValue { + current.Labels[labelKey] = labelValue + updateNeeded = true + } + } + + if !updateNeeded { + return nil + } + + return r.Client.Patch(ctx, current, client.MergeFrom(currentOrig)) + }) + + if err != nil { + return errors.Wrap(err, fmt.Sprintf("failed to update secret %s", secretName)) + } + + verifyErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { + verifySecret := &corev1.Secret{} + if err := r.Client.Get(ctx, types.NamespacedName{ + Name: secretName, + Namespace: cluster.Namespace, + }, verifySecret); err != nil { + return err + } + + for labelKey, labelValue := range requiredLabels { + if existing, exists := verifySecret.Labels[labelKey]; !exists || existing != labelValue { + return errors.Errorf("secret %s label %s not yet propagated", secretName, labelKey) + } + } + + return nil + }) + + return errors.Wrap(verifyErr, "failed to update secret") } return nil From 8914e70f90cf0dfe3d1a1f973fb20c3a0e180c7e Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Fri, 11 Jul 2025 13:12:29 +0300 Subject: [PATCH 5/5] update e2e test --- e2e-tests/tests/users/16-assert.yaml | 49 +++++++++++++++++++ ...-user-with-predefined-password-secret.yaml | 15 ++++++ ...data-user-db-with-predefined-password.yaml | 23 +++++++++ e2e-tests/tests/users/18-assert.yaml | 10 ++++ ...from-user-db-with-predefined-password.yaml | 19 +++++++ 5 files changed, 116 insertions(+) create mode 100644 e2e-tests/tests/users/16-assert.yaml create mode 100644 e2e-tests/tests/users/16-create-user-with-predefined-password-secret.yaml create mode 100644 e2e-tests/tests/users/17-write-data-user-db-with-predefined-password.yaml create mode 100644 e2e-tests/tests/users/18-assert.yaml create mode 100644 e2e-tests/tests/users/18-read-from-user-db-with-predefined-password.yaml diff --git a/e2e-tests/tests/users/16-assert.yaml b/e2e-tests/tests/users/16-assert.yaml new file mode 100644 index 000000000..934432d24 --- /dev/null +++ b/e2e-tests/tests/users/16-assert.yaml @@ -0,0 +1,49 @@ +apiVersion: postgres-operator.crunchydata.com/v1beta1 +kind: PostgresCluster +metadata: + name: users + ownerReferences: + - apiVersion: pgv2.percona.com/v2 + kind: PerconaPGCluster + name: users + controller: true + blockOwnerDeletion: true + finalizers: + - postgres-operator.crunchydata.com/finalizer +status: + instances: + - name: instance1 + readyReplicas: 3 + replicas: 3 + updatedReplicas: 3 + pgbackrest: + repoHost: + apiVersion: apps/v1 + kind: StatefulSet + ready: true + repos: + - bound: true + name: repo1 + replicaCreateBackupComplete: true + stanzaCreated: true + proxy: + pgBouncer: + readyReplicas: 3 + replicas: 3 +--- +apiVersion: pgv2.percona.com/v2 +kind: PerconaPGCluster +metadata: + name: users +status: + pgbouncer: + ready: 3 + size: 3 + postgres: + instances: + - name: instance1 + ready: 3 + size: 3 + ready: 3 + size: 3 + state: ready diff --git a/e2e-tests/tests/users/16-create-user-with-predefined-password-secret.yaml b/e2e-tests/tests/users/16-create-user-with-predefined-password-secret.yaml new file mode 100644 index 000000000..e487590b4 --- /dev/null +++ b/e2e-tests/tests/users/16-create-user-with-predefined-password-secret.yaml @@ -0,0 +1,15 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +timeout: 10 +commands: + - script: |- + set -o errexit + set -o xtrace + + source ../../functions + + kubectl -n ${NAMESPACE} create secret generic eagle-credentials --from-literal=password=eagle-db-password + sleep 5 + + kubectl -n ${NAMESPACE} patch perconapgcluster/${test_name} --type=json -p '[{"op":"add", "path":"/spec/autoCreateUserSchema","value":true},{"op":"add", "path":"/spec/users","value":[{"name":"eagle","databases":["nest"],"password":{"type":"ASCII"},"secretName":"eagle-credentials"}]}]' + sleep 15 diff --git a/e2e-tests/tests/users/17-write-data-user-db-with-predefined-password.yaml b/e2e-tests/tests/users/17-write-data-user-db-with-predefined-password.yaml new file mode 100644 index 000000000..994f333c8 --- /dev/null +++ b/e2e-tests/tests/users/17-write-data-user-db-with-predefined-password.yaml @@ -0,0 +1,23 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +commands: + - script: |- + set -o errexit + set -o xtrace + + source ../../functions + + predefinedPassword=eagle-db-password + user='eagle' + db_name='nest' + schema='eagle' + hostname=$(get_pgbouncer_host eagle-credentials) + + + run_psql \ + 'CREATE TABLE IF NOT EXISTS customApp (id int PRIMARY KEY);' \ + "-h $hostname -U $user -d $db_name" "$predefinedPassword" + run_psql \ + "INSERT INTO $schema.customApp (id) VALUES (100500)" \ + "-h $hostname -U $user -d $db_name" "$predefinedPassword" + diff --git a/e2e-tests/tests/users/18-assert.yaml b/e2e-tests/tests/users/18-assert.yaml new file mode 100644 index 000000000..6824d5be2 --- /dev/null +++ b/e2e-tests/tests/users/18-assert.yaml @@ -0,0 +1,10 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +timeout: 30 +--- +kind: ConfigMap +apiVersion: v1 +metadata: + name: 18-read-from-user-db-with-predefined-password +data: + data: ' 100500' \ No newline at end of file diff --git a/e2e-tests/tests/users/18-read-from-user-db-with-predefined-password.yaml b/e2e-tests/tests/users/18-read-from-user-db-with-predefined-password.yaml new file mode 100644 index 000000000..a8151a972 --- /dev/null +++ b/e2e-tests/tests/users/18-read-from-user-db-with-predefined-password.yaml @@ -0,0 +1,19 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +timeout: 30 +commands: + - script: |- + set -o errexit + set -o xtrace + + source ../../functions + + predefinedPassword=eagle-db-password + user='eagle' + db_name='nest' + schema='eagle' + hostname=$(get_pgbouncer_host eagle-credentials) + + data=$(run_psql "SELECT * from $schema.customApp;" "-h $hostname -U $user -d $db_name" "$predefinedPassword") + + kubectl create configmap -n "${NAMESPACE}" 18-read-from-user-db-with-predefined-password --from-literal=data="${data}"