Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16678,6 +16678,10 @@ spec:
type: object
tls:
properties:
SANs:
items:
type: string
type: array
caValidityDuration:
type: string
certManagementPolicy:
Expand Down
4 changes: 4 additions & 0 deletions config/crd/bases/pgv2.percona.com_perconapgclusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17092,6 +17092,10 @@ spec:
type: object
tls:
properties:
SANs:
items:
type: string
type: array
caValidityDuration:
type: string
certManagementPolicy:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14352,6 +14352,10 @@ spec:
type: array
tls:
properties:
SANs:
items:
type: string
type: array
caValidityDuration:
type: string
certManagementPolicy:
Expand Down
8 changes: 8 additions & 0 deletions deploy/bundle.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17293,6 +17293,10 @@ spec:
type: object
tls:
properties:
SANs:
items:
type: string
type: array
caValidityDuration:
type: string
certManagementPolicy:
Expand Down Expand Up @@ -36005,6 +36009,10 @@ spec:
type: array
tls:
properties:
SANs:
items:
type: string
type: array
caValidityDuration:
type: string
certManagementPolicy:
Expand Down
3 changes: 3 additions & 0 deletions deploy/cr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ spec:
# certValidityDuration: 2160h
# caValidityDuration: 26280h
# pgBackRestCertValidityDuration: 2160h
# SANs:
# - pg.example.com
# - "*.pg.internal.example.com"
# issuerConf:
# name: some-selfsigned-issuer
# kind: ClusterIssuer
Expand Down
8 changes: 8 additions & 0 deletions deploy/crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17293,6 +17293,10 @@ spec:
type: object
tls:
properties:
SANs:
items:
type: string
type: array
caValidityDuration:
type: string
certManagementPolicy:
Expand Down Expand Up @@ -36005,6 +36009,10 @@ spec:
type: array
tls:
properties:
SANs:
items:
type: string
type: array
caValidityDuration:
type: string
certManagementPolicy:
Expand Down
8 changes: 8 additions & 0 deletions deploy/cw-bundle.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17293,6 +17293,10 @@ spec:
type: object
tls:
properties:
SANs:
items:
type: string
type: array
caValidityDuration:
type: string
certManagementPolicy:
Expand Down Expand Up @@ -36005,6 +36009,10 @@ spec:
type: array
tls:
properties:
SANs:
items:
type: string
type: array
caValidityDuration:
type: string
certManagementPolicy:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ commands:
get_cr "$cluster" \
| yq '.spec.tls.certManagementPolicy = "operatorProvidedOnly"' \
| yq '.spec.tls.issuerConf = {"name":"cert-management-policy-ignored-issuer","kind":"ClusterIssuer"}' \
| yq '.spec.tls.SANs = ["pg.example.com","*.pg.internal.example.com"]' \
| yq '.spec.proxy.pgBouncer.replicas = 1' \
| yq '.spec.instances[].replicas = 1' \
| kubectl -n "$NAMESPACE" apply -f -
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,29 @@ commands:
fi
done

# spec.tls.SANs must reach the internal-PKI leaves too, not just cert-manager.
verify_san() {
local secret="$1"
local key="$2"
local escaped_key="${key//./\\.}"
shift 2
local sans
sans=$(kubectl -n "$NAMESPACE" get secret "$secret" \
-o jsonpath="{.data.${escaped_key}}" | base64 -d | openssl x509 -noout -ext subjectAltName)
for san in "$@"; do
if [[ "$sans" != *"$san"* ]]; then
echo "Certificate $secret/$key is missing SAN $san" >&2
echo "Got: $sans" >&2
return 1
fi
done
}

verify_san "${cluster}-cluster-cert" tls.crt \
pg.example.com '*.pg.internal.example.com' "${cluster}-primary"
verify_san "${cluster}-pgbouncer" pgbouncer-frontend.crt \
pg.example.com '*.pg.internal.example.com' "${cluster}-pgbouncer"

instance_sts=$(kubectl -n "$NAMESPACE" get sts \
-l postgres-operator.crunchydata.com/cluster="$cluster",postgres-operator.crunchydata.com/instance-set=instance1 \
-o jsonpath='{.items[*].metadata.name}')
Expand Down
14 changes: 14 additions & 0 deletions e2e-tests/tests/cert-manager-tls/29-add-sans.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
apiVersion: kuttl.dev/v1beta1
kind: TestStep
commands:
- timeout: 120
script: |-
set -o errexit
set -o xtrace

source ../../functions

kubectl -n "$NAMESPACE" patch pg cert-manager-tls --type=merge \
-p '{"spec":{"tls":{"SANs":["pg.example.com","*.pg.internal.example.com"]}}}'

wait_cluster_consistency cert-manager-tls
55 changes: 55 additions & 0 deletions e2e-tests/tests/cert-manager-tls/30-verify-sans.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
apiVersion: kuttl.dev/v1beta1
kind: TestStep
commands:
- timeout: 600
script: |-
set -o errexit
set -o xtrace

source ../../functions

# The Certificate CR already existed before spec.tls.SANs was set, so this
# also covers the DNSNames drift path in ApplyClusterCertificate.
wait_for_san_in_cert() {
local cert_name="$1"
local san="$2"

for i in {1..60}; do
if kubectl -n "$NAMESPACE" get certificate "$cert_name" -o jsonpath='{range .spec.dnsNames[*]}{.}{"\n"}{end}' | grep -Fxq -- "$san"; then
return 0
fi
sleep 5
done
echo "Certificate $cert_name never got SAN $san"
kubectl -n "$NAMESPACE" get certificate "$cert_name" -o jsonpath='{.spec.dnsNames}'
return 1
}

wait_for_san_in_cert cert-manager-tls-cluster-cert pg.example.com
wait_for_san_in_cert cert-manager-tls-cluster-cert '*.pg.internal.example.com'
wait_for_san_in_cert cert-manager-tls-pgbouncer-cert pg.example.com

wait_for_san_in_secret() {
local secret_name="$1"
local san="$2"

for i in {1..60}; do
local sans
sans=$(kubectl -n "$NAMESPACE" get secret "$secret_name" -o jsonpath='{.data.tls\.crt}' \
| base64 -d | openssl x509 -noout -ext subjectAltName)
if [[ "$sans" == *"$san"* ]]; then
return 0
fi
sleep 5
done
echo "Secret $secret_name certificate never got SAN $san"
echo "Got: $sans"
return 1
}

wait_for_san_in_secret cert-manager-tls-cluster-cert pg.example.com
wait_for_san_in_secret cert-manager-tls-cluster-cert '*.pg.internal.example.com'
wait_for_san_in_secret cert-manager-tls-pgbouncer-frontend-tls pg.example.com

# The generated service names must survive alongside the custom ones.
wait_for_san_in_secret cert-manager-tls-cluster-cert cert-manager-tls-primary
1 change: 1 addition & 0 deletions internal/controller/postgrescluster/pgbouncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ func (r *Reconciler) reconcileCertManagerPGBouncerSecret(ctx context.Context, cl
if dnsErr != nil {
return nil, errors.Wrap(dnsErr, "get pgbouncer service DNS names")
}
dnsNames = append(dnsNames, cluster.Spec.TLS.GetSANs()...)

err := c.ApplyPGBouncerCertificate(ctx, cluster, dnsNames)
if err != nil {
Expand Down
5 changes: 5 additions & 0 deletions internal/controller/postgrescluster/pki.go
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,10 @@ func (r *Reconciler) reconcileInternalClusterCertificate(

dnsNames := append(primaryServiceDNSNames, replicaServiceDNSNames...)
dnsFQDN := dnsNames[0]
// ponytail: SANs land in DNSNames verbatim, matching the MySQL operators.
// A bare IP entry is a dNSName, not an iPAddress SAN; thread IPAddresses
// through internal/pki if real IP SANs are ever needed.
Comment thread
yoav-katz marked this conversation as resolved.
Outdated
dnsNames = append(dnsNames, cluster.Spec.TLS.GetSANs()...)

if err == nil {
// Unmarshal and validate the stored leaf. These first errors can
Expand Down Expand Up @@ -523,6 +527,7 @@ func (r *Reconciler) reconcileCertManagerClusterCertificate(
return nil, errors.Wrap(err, "get replica service DNS names")
}
dnsNames := append(primaryDNSNames, replicaDNSNames...)
dnsNames = append(dnsNames, cluster.Spec.TLS.GetSANs()...)

err = c.ApplyClusterCertificate(ctx, cluster, dnsNames)
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions internal/pgbouncer/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ func Secret(ctx context.Context,
return errors.Wrap(err, "get service dns names")
}
dnsFQDN = dnsNames[0]
dnsNames = append(dnsNames, inCluster.Spec.TLS.GetSANs()...)
}

if err == nil {
Expand Down
11 changes: 11 additions & 0 deletions percona/certmanager/certmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"os"
"regexp"
"slices"
"time"

"github.com/cert-manager/cert-manager/pkg/apis/certmanager"
Expand Down Expand Up @@ -545,6 +546,11 @@ func (c *controller) ApplyClusterCertificate(ctx context.Context, cluster *v1bet
needsUpdate = true
}

if !slices.Equal(existing.Spec.DNSNames, dnsNames) {
existing.Spec.DNSNames = dnsNames
needsUpdate = true
}

if !needsUpdate {
return nil
}
Expand Down Expand Up @@ -766,6 +772,11 @@ func (c *controller) ApplyPGBouncerCertificate(ctx context.Context, cluster *v1b
needsUpdate = true
}

if !slices.Equal(existing.Spec.DNSNames, dnsNames) {
existing.Spec.DNSNames = dnsNames
needsUpdate = true
}

if !needsUpdate {
return nil
}
Expand Down
37 changes: 37 additions & 0 deletions percona/certmanager/certmanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1529,3 +1529,40 @@ func TestApplyCertificateIssuerRefDrift(t *testing.T) {
assert.Equal(t, "vault-issuer", cert.Spec.IssuerRef.Name)
})
}

func TestApplyCertificateDNSNamesDrift(t *testing.T) {
t.Run("cluster certificate picks up added SANs on update", func(t *testing.T) {
cluster := testCluster()
cluster.Name = "drift-cluster-sans"
client := setupFakeClient(t, cluster)
ctrl := NewController(client, client.Scheme(), false)

dnsNames := []string{"drift-cluster-sans-primary.test-namespace.svc"}
require.NoError(t, ctrl.ApplyClusterCertificate(t.Context(), cluster, dnsNames))

withSANs := append(dnsNames, "pg.example.com", "*.pg.internal.example.com")
require.NoError(t, ctrl.ApplyClusterCertificate(t.Context(), cluster, withSANs))

cert := &v1.Certificate{}
secretName := naming.PostgresTLSSecret(cluster)
require.NoError(t, client.Get(t.Context(), sigs.ObjectKey{Namespace: cluster.Namespace, Name: secretName.Name}, cert))
assert.Equal(t, withSANs, cert.Spec.DNSNames)
})

t.Run("pgbouncer certificate picks up added SANs on update", func(t *testing.T) {
cluster := testCluster()
cluster.Name = "drift-pgbouncer-sans"
client := setupFakeClient(t, cluster)
ctrl := NewController(client, client.Scheme(), false)

dnsNames := []string{"drift-pgbouncer-sans-pgbouncer.test-namespace.svc"}
require.NoError(t, ctrl.ApplyPGBouncerCertificate(t.Context(), cluster, dnsNames))

withSANs := append(dnsNames, "bouncer.example.com")
require.NoError(t, ctrl.ApplyPGBouncerCertificate(t.Context(), cluster, withSANs))

cert := &v1.Certificate{}
require.NoError(t, client.Get(t.Context(), sigs.ObjectKey{Namespace: cluster.Namespace, Name: cluster.Name + "-pgbouncer-cert"}, cert))
assert.Equal(t, withSANs, cert.Spec.DNSNames)
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,17 @@ type TLSSpec struct {
CertManagementPolicy CertManagementPolicy `json:"certManagementPolicy,omitempty"`
// +optional
IssuerConf *cmmeta.IssuerReference `json:"issuerConf,omitempty"`
// SANs are extra Subject Alternative Names appended to the Postgres server
// and pgBouncer certificates the operator issues.
// +optional
SANs []string `json:"SANs,omitempty"`
}

func (s *TLSSpec) GetSANs() []string {
if s == nil {
return nil
}
return s.SANs
}

func (s *TLSSpec) GetCertManagementPolicy() CertManagementPolicy {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading