diff --git a/internal/controller/postgrescluster/pgbouncer.go b/internal/controller/postgrescluster/pgbouncer.go index 0bac7b0326..89554596ae 100644 --- a/internal/controller/postgrescluster/pgbouncer.go +++ b/internal/controller/postgrescluster/pgbouncer.go @@ -257,7 +257,6 @@ func (r *Reconciler) reconcilePGBouncerSecret( if client.IgnoreNotFound(err) != nil { return nil, err } - secretFound := err == nil if !cluster.Spec.Proxy.PGBouncerEnabled() { // PgBouncer is disabled; delete the Secret if it exists. @@ -269,12 +268,6 @@ func (r *Reconciler) reconcilePGBouncerSecret( err = client.IgnoreNotFound(err) - if cluster.Spec.TLS.GetCertManagementPolicy() == v1beta1.CertManagementUserProvidedOnly { - if !secretFound { - return nil, errors.Errorf("user-provided PgBouncer secret %q is missing", naming.ClusterPGBouncer(cluster).Name) - } - return existing, nil - } var userSecret *corev1.Secret if ref := cluster.Spec.Proxy.PGBouncer.UsersSecret; ref != nil && ref.Name != "" { userSecret = &corev1.Secret{} @@ -284,7 +277,8 @@ func (r *Reconciler) reconcilePGBouncerSecret( } var frontendCertManagerSecret *corev1.Secret - if cluster.Spec.Proxy.PGBouncer.CustomTLSSecret == nil { + if cluster.Spec.Proxy.PGBouncer.CustomTLSSecret == nil && + cluster.Spec.TLS.GetCertManagementPolicy() != v1beta1.CertManagementUserProvidedOnly { certManagerManaged, certErr := r.isRootCACertManagerManaged(ctx, cluster) if certErr != nil { return nil, errors.Wrap(certErr, "failed to check if cert-manager manages root CA") @@ -335,7 +329,7 @@ func (r *Reconciler) reconcilePGBouncerSecret( ) var additionalTrustedCAs [][]byte - if err == nil { + if err == nil && cluster.Spec.TLS.GetCertManagementPolicy() != v1beta1.CertManagementUserProvidedOnly { additionalTrustedCAs, err = r.getAdditionalTrustedCAs(ctx, cluster) } if err == nil { diff --git a/internal/controller/postgrescluster/pki.go b/internal/controller/postgrescluster/pki.go index ea7fdf3682..1bfb76ffcf 100644 --- a/internal/controller/postgrescluster/pki.go +++ b/internal/controller/postgrescluster/pki.go @@ -81,7 +81,7 @@ func (r *Reconciler) reconcileTLSCondition(ctx context.Context, cluster *v1beta1 } if cluster.Spec.Proxy != nil && cluster.Spec.Proxy.PGBouncer != nil { - if err := checkSecret(nil, naming.ClusterPGBouncer(cluster).Name); err != nil { + if err := checkSecret(cluster.Spec.Proxy.PGBouncer.CustomTLSSecret, naming.ClusterPGBouncer(cluster).Name); err != nil { return errors.Wrap(err, "check PgBouncer TLS secret") } } diff --git a/internal/controller/postgrescluster/pki_test.go b/internal/controller/postgrescluster/pki_test.go index e4b8109c4c..8f79fe223e 100644 --- a/internal/controller/postgrescluster/pki_test.go +++ b/internal/controller/postgrescluster/pki_test.go @@ -75,6 +75,9 @@ func TestReconcileTLSCondition(t *testing.T) { cluster.Spec.CustomReplicationClientTLSSecret = &corev1.SecretProjection{ LocalObjectReference: corev1.LocalObjectReference{Name: "custom-replication"}, } + cluster.Spec.Proxy.PGBouncer.CustomTLSSecret = &corev1.SecretProjection{ + LocalObjectReference: corev1.LocalObjectReference{Name: "custom-pgbouncer-tls"}, + } instance := &appsv1.StatefulSet{ObjectMeta: metav1.ObjectMeta{ Name: "hippo-instance1-abcd", @@ -100,7 +103,7 @@ func TestReconcileTLSCondition(t *testing.T) { naming.PostgresTLSSecret(cluster).Name, "custom-replication", naming.PGBackRestSecret(cluster).Name, - naming.ClusterPGBouncer(cluster).Name, + "custom-pgbouncer-tls", naming.InstanceCertificates(instance).Name, }, ", ")+". certManagementPolicy is userProvidedOnly") }) @@ -121,13 +124,16 @@ func TestReconcileTLSCondition(t *testing.T) { cluster.Spec.CustomReplicationClientTLSSecret = &corev1.SecretProjection{ LocalObjectReference: corev1.LocalObjectReference{Name: "custom-replication"}, } + cluster.Spec.Proxy.PGBouncer.CustomTLSSecret = &corev1.SecretProjection{ + LocalObjectReference: corev1.LocalObjectReference{Name: "custom-pgbouncer-tls"}, + } objects := []client.Object{ &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "custom-root-ca", Namespace: cluster.Namespace}}, &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "custom-postgres-tls", Namespace: cluster.Namespace}}, &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "custom-replication", Namespace: cluster.Namespace}}, &corev1.Secret{ObjectMeta: naming.PGBackRestSecret(cluster)}, - &corev1.Secret{ObjectMeta: naming.ClusterPGBouncer(cluster)}, + &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "custom-pgbouncer-tls", Namespace: cluster.Namespace}}, } r := &Reconciler{Client: fake.NewClientBuilder().WithObjects(objects...).Build()} diff --git a/internal/pgbouncer/reconcile.go b/internal/pgbouncer/reconcile.go index 632b4e5c9d..fda4bbf077 100644 --- a/internal/pgbouncer/reconcile.go +++ b/internal/pgbouncer/reconcile.go @@ -84,7 +84,17 @@ func Secret(ctx context.Context, } if inCluster.Spec.Proxy.PGBouncer.CustomTLSSecret == nil { - if frontendCertManagerSecret != nil { + if inCluster.Spec.TLS.GetCertManagementPolicy() == v1beta1.CertManagementUserProvidedOnly { + for _, key := range []string{ + certFrontendAuthoritySecretKey, + certFrontendPrivateKeySecretKey, + certFrontendSecretKey, + } { + if v, ok := inSecret.Data[key]; ok { + outSecret.Data[key] = v + } + } + } else if frontendCertManagerSecret != nil { if err == nil { outSecret.Data[certFrontendAuthoritySecretKey], err = frontendAuthorityCert(inRoot, frontendCertManagerSecret) } @@ -134,7 +144,8 @@ func Secret(ctx context.Context, // bundle so PgBouncer also trusts them when verifying client // certificates. Entries keep their given order so identical inputs // always produce identical bundle bytes. - if err == nil && len(additionalCAs) > 0 { + if err == nil && len(additionalCAs) > 0 && + inCluster.Spec.TLS.GetCertManagementPolicy() != v1beta1.CertManagementUserProvidedOnly { bundle := outSecret.Data[certFrontendAuthoritySecretKey] for _, ca := range additionalCAs { if len(bundle) > 0 && bundle[len(bundle)-1] != '\n' { @@ -181,10 +192,12 @@ func Pod( } configVolume := corev1.Volume{Name: configVolumeMount.Name} configVolume.Projected = &corev1.ProjectedVolumeSource{ - Sources: append(append(append([]corev1.VolumeProjection{}, - podConfigFiles(inCluster.Spec.Proxy.PGBouncer.Config, inConfigMap, inSecret)...), - frontendCertificate(inCluster.Spec.Proxy.PGBouncer.CustomTLSSecret, inSecret, - len(inCluster.Spec.Proxy.PGBouncer.AdditionalTrustedCAs) > 0)...), + Sources: append( + append(append([]corev1.VolumeProjection{}, + podConfigFiles(inCluster.Spec.Proxy.PGBouncer.Config, inConfigMap, inSecret)...), + frontendCertificate(inCluster.Spec.Proxy.PGBouncer.CustomTLSSecret, inSecret, + len(inCluster.Spec.Proxy.PGBouncer.AdditionalTrustedCAs) > 0 && + inCluster.Spec.TLS.GetCertManagementPolicy() != v1beta1.CertManagementUserProvidedOnly)...), backendAuthority(inPostgreSQLCertificate), ), } diff --git a/internal/pgbouncer/reconcile_test.go b/internal/pgbouncer/reconcile_test.go index ada053bafe..10c845517e 100644 --- a/internal/pgbouncer/reconcile_test.go +++ b/internal/pgbouncer/reconcile_test.go @@ -14,6 +14,7 @@ import ( "gotest.tools/v3/assert" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/percona/percona-postgresql-operator/v2/internal/feature" "github.com/percona/percona-postgresql-operator/v2/internal/naming" @@ -104,6 +105,85 @@ func TestSecret(t *testing.T) { assert.DeepEqual(t, before, intent) } +func TestSecretCertManagementPolicy(t *testing.T) { + t.Parallel() + + root, err := pki.NewRootCertificateAuthority() + assert.NilError(t, err) + + tests := []struct { + name string + policy v1beta1.CertManagementPolicy + customTLS bool + expectTLSData bool + }{ + { + name: "auto generates TLS data in the operator Secret", + policy: v1beta1.CertManagementAuto, + expectTLSData: true, + }, + { + name: "auto with custom TLS leaves TLS data out of the operator Secret", + policy: v1beta1.CertManagementAuto, + customTLS: true, + }, + { + name: "user provided only preserves TLS data", + policy: v1beta1.CertManagementUserProvidedOnly, + expectTLSData: true, + }, + { + name: "user provided only with custom TLS does not generate TLS data", + policy: v1beta1.CertManagementUserProvidedOnly, + customTLS: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := t.Context() + cluster := new(v1beta1.PostgresCluster) + cluster.Spec.Proxy = &v1beta1.PostgresProxySpec{ + PGBouncer: new(v1beta1.PGBouncerPodSpec), + } + cluster.Spec.TLS = &v1beta1.TLSSpec{CertManagementPolicy: tt.policy} + if tt.customTLS { + cluster.Spec.Proxy.PGBouncer.CustomTLSSecret = &corev1.SecretProjection{ + LocalObjectReference: corev1.LocalObjectReference{Name: "custom-pgbouncer-tls"}, + } + } + assert.NilError(t, cluster.Default(ctx, nil)) + + existing := &corev1.Secret{Data: map[string][]byte{ + "pgbouncer-frontend.ca-roots": []byte("user-ca"), + "pgbouncer-frontend.crt": []byte("user-cert"), + "pgbouncer-frontend.key": []byte("user-key"), + }} + intent := new(corev1.Secret) + service := &corev1.Service{ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns1", Name: "some-name", + }} + + err := Secret(ctx, cluster, root, existing, nil, + service, intent, nil, nil) + require.NoError(t, err) + + assert.Assert(t, len(intent.Data["pgbouncer-password"]) != 0) + assert.Assert(t, len(intent.Data["pgbouncer-verifier"]) != 0) + assert.Assert(t, len(intent.Data["pgbouncer-users.txt"]) != 0) + + for _, key := range []string{ + "pgbouncer-frontend.ca-roots", + "pgbouncer-frontend.crt", + "pgbouncer-frontend.key", + } { + _, found := intent.Data[key] + assert.Equal(t, found, tt.expectTLSData, key) + } + }) + } +} + func TestSecretAdditionalCAs(t *testing.T) { t.Parallel() @@ -196,6 +276,21 @@ func TestSecretAdditionalCAs(t *testing.T) { assert.Assert(t, !ok) }) + t.Run("UserProvidedOnlyDoesNotAppend", func(t *testing.T) { + cluster := newCluster("3.1.0") + cluster.Spec.TLS = &v1beta1.TLSSpec{ + CertManagementPolicy: v1beta1.CertManagementUserProvidedOnly, + } + existing := &corev1.Secret{Data: map[string][]byte{ + "pgbouncer-frontend.ca-roots": []byte("user-ca"), + }} + intent := new(corev1.Secret) + + require.NoError(t, Secret(ctx, cluster, nil, existing, nil, service, intent, nil, [][]byte{ca1})) + + assert.DeepEqual(t, intent.Data["pgbouncer-frontend.ca-roots"], []byte("user-ca")) + }) + t.Run("NoChangeWhenCalledAgain", func(t *testing.T) { cluster := newCluster("3.1.0") existing := new(corev1.Secret)