Fix DuplicateSecretName issue

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>

internal/controller/certificates/certificates.go

@@ -0,0 +1,87 @@
+/*
+Copyright 2022 The cert-manager Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package certificates
+
+import (
+	"context"
+	"slices"
+	"sort"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/labels"
+
+	internalinformers "github.com/cert-manager/cert-manager/internal/informers"
+	cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
+	cmlisters "github.com/cert-manager/cert-manager/pkg/client/listers/certmanager/v1"
+)
+
+func CertificateOwnsSecret(
+	ctx context.Context,
+	certificateLister cmlisters.CertificateLister,
+	secretLister internalinformers.SecretLister,
+	crt *cmapi.Certificate,
+) (bool, []string, error) {
+	crts, err := certificateLister.Certificates(crt.Namespace).List(labels.Everything())
+	if err != nil {
+		return false, nil, err
+	}
+
+	sort.Slice(crts, func(i, j int) bool {
+		return crts[i].CreationTimestamp.Before(&crts[j].CreationTimestamp) ||
+			crts[i].Name < crts[j].Name
+	})
+
+	var duplicates []string
+	for _, namespaceCrt := range crts {
+		// Check if it has the same Secret.
+		if namespaceCrt.Spec.SecretName == crt.Spec.SecretName {
+			// If it does, mark the Certificate as having a duplicate Secret.
+			duplicates = append(duplicates, namespaceCrt.Name)
+		}
+	}
+
+	// If there are no duplicates, return early.
+	if len(duplicates) == 1 && duplicates[0] == crt.Name {
+		return true, nil, nil
+	}
+
+	ownerCertificate := duplicates[0]
+
+	// Fetch the Secret and determine if it is owned by any of the Certificates.
+	secret, err := secretLister.Secrets(crt.Namespace).Get(crt.Spec.SecretName)
+	if err != nil && !apierrors.IsNotFound(err) {
+		return false, nil, err
+	} else if err == nil {
+		if annotation, hasAnnotation := secret.GetAnnotations()[cmapi.CertificateNameKey]; hasAnnotation && slices.Contains(duplicates, annotation) {
+			ownerCertificate = annotation
+		}
+	}
+
+	// If the Secret does not exist, only the first Certificate in the list
+	// is the owner of the Secret.
+	return crt.Name == ownerCertificate, sliceWithoutValue(duplicates, crt.Name), nil
+}
+
+func sliceWithoutValue(slice []string, value string) []string {
+	result := make([]string, 0, len(slice)-1)
+	for _, v := range slice {
+		if v != value {
+			result = append(result, v)
+		}
+	}
+	return result
+}

internal/controller/certificates/policies/checks.go

@@ -172,6 +172,17 @@ func SecretIssuerAnnotationsMismatch(input Input) (string, string, bool) {
 	return "", "", false
 }
 
+// SecretCertificateNameAnnotationsMismatch - When the issuer annotations are defined,
+// it must match the issuer ref.
+func SecretCertificateNameAnnotationsMismatch(input Input) (string, string, bool) {
+	name, ok := input.Secret.Annotations[cmapi.CertificateNameKey]
+	if (ok) && // only check if an annotation is present
+		name != input.Certificate.Name {
+		return IncorrectCertificate, fmt.Sprintf("Issuing certificate as Secret was previously issued for %q", name), true
+	}
+	return "", "", false
+}
+
 // SecretPublicKeyDiffersFromCurrentCertificateRequest checks that the current CertificateRequest
 // contains a CSR that is signed by the key stored in the Secret. A failure is often caused by the
 // Secret being changed outside of the control of cert-manager, causing the current CertificateRequest

internal/controller/certificates/policies/constants.go

@@ -39,6 +39,9 @@ const (
 	// IncorrectIssuer is a policy violation reason for a scenario where
 	// Certificate has been issued by incorrect Issuer.
 	IncorrectIssuer string = "IncorrectIssuer"
+	// IncorrectCertificate is a policy violation reason for a scenario where
+	// Certificate has been issued by incorrect Certificate.
+	IncorrectCertificate string = "IncorrectCertificate"
 	// RequestChanged is a policy violation reason for a scenario where
 	// CertificateRequest not valid for Certificate's spec.
 	RequestChanged string = "RequestChanged"

internal/controller/certificates/policies/policies.go

@@ -71,7 +71,8 @@ func NewTriggerPolicyChain(c clock.Clock) Chain {
 		SecretIsMissingData,    // Make sure the Secret has the required keys set
 		SecretPublicKeysDiffer, // Make sure the PrivateKey and PublicKey match in the Secret
 
-		SecretIssuerAnnotationsMismatch, // Make sure the Secret's IssuerRef annotations match the Certificate spec
+		SecretIssuerAnnotationsMismatch,          // Make sure the Secret's IssuerRef annotations match the Certificate spec
+		SecretCertificateNameAnnotationsMismatch, // Make sure the Secret's CertificateName annotation matches the Certificate's name
 
 		SecretPrivateKeyMismatchesSpec,                      // Make sure the PrivateKey Type and Size match the Certificate spec
 		SecretPublicKeyDiffersFromCurrentCertificateRequest, // Make sure the Secret's PublicKey matches the current CertificateRequest
@@ -88,7 +89,8 @@ func NewReadinessPolicyChain(c clock.Clock) Chain {
 		SecretIsMissingData,    // Make sure the Secret has the required keys set
 		SecretPublicKeysDiffer, // Make sure the PrivateKey and PublicKey match in the Secret
 
-		SecretIssuerAnnotationsMismatch, // Make sure the Secret's IssuerRef annotations match the Certificate spec
+		SecretIssuerAnnotationsMismatch,          // Make sure the Secret's IssuerRef annotations match the Certificate spec
+		SecretCertificateNameAnnotationsMismatch, // Make sure the Secret's CertificateName annotation matches the Certificate's name
 
 		SecretPrivateKeyMismatchesSpec,                      // Make sure the PrivateKey Type and Size match the Certificate spec
 		SecretPublicKeyDiffersFromCurrentCertificateRequest, // Make sure the Secret's PublicKey matches the current CertificateRequest

pkg/controller/certificates/trigger/trigger_controller.go

@@ -160,6 +160,19 @@ func (c *controller) ProcessItem(ctx context.Context, key string) error {
 		return nil
 	}
 
+	// If we detect that the Certificate has a duplicate Secret name set, we don't trigger issuance.
+	isOwner, duplicates, err := internalcertificates.CertificateOwnsSecret(ctx, c.certificateLister, c.secretLister, crt)
+	if err != nil {
+		return err
+	}
+	if !isOwner {
+		log.V(logf.DebugLevel).Info("Certificate has duplicate Secret name with other Certificates in the same namespace, skipping trigger.", "duplicates", duplicates)
+
+		c.scheduledWorkQueue.Add(key, 3*time.Minute)
+
+		return nil
+	}
+
 	input, err := c.dataForCertificate(ctx, crt)
 	if err != nil {
 		return err