introduce hash on each secret

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

internal/controller/certificates/policies/checks.go

@@ -34,7 +34,9 @@ import (
 
 	cmmeta "github.com/cert-manager/cert-manager/internal/apis/meta"
 	internalcertificates "github.com/cert-manager/cert-manager/internal/controller/certificates"
+	apiutil "github.com/cert-manager/cert-manager/pkg/api/util"
 	cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
+	cmmetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
 	"github.com/cert-manager/cert-manager/pkg/util/pki"
 )
 
@@ -157,6 +159,51 @@ func SecretKeystoreFormatMismatch(input Input) (string, string, bool) {
 	return "", "", false
 }
 
+func SecretCertificateHashAnnotationMissingAndStable(input Input) (string, string, bool) {
+	if input.Secret.Annotations[cmapi.CertificateHashAnnotationKey] == "" {
+		readyCondition := apiutil.GetCertificateCondition(input.Certificate, cmapi.CertificateConditionReady)
+		issuingCondition := apiutil.GetCertificateCondition(input.Certificate, cmapi.CertificateConditionIssuing)
+
+		// If the certificate's ready condition is not up-to-date, we do not change the
+		// certificate hash annotation. Also, if the certificate is already being issued,
+		// we do not need to update the certificate hash annotation (as it will be updated once
+		// the certificate is issued).
+
+		if readyCondition != nil && readyCondition.ObservedGeneration != input.Certificate.Generation {
+			return "", "", false
+		}
+
+		if issuingCondition != nil && issuingCondition.Status == cmmetav1.ConditionTrue {
+			return "", "", false
+		}
+
+		return CertificateHashMissing, "Updating Secret, because it is missing a certificate hash annotation", true
+	}
+
+	return "", "", false
+}
+
+func SecretCertificateHashAnnotationMismatch(
+	fallbackChain Chain,
+) Func {
+	return func(input Input) (reason, message string, failed bool) {
+		certificateHash := input.Secret.Annotations[cmapi.CertificateHashAnnotationKey]
+		if certificateHash == "" {
+			return fallbackChain.Evaluate(input)
+		}
+
+		upToDate, err := pki.IsCertificateUpToDateWithVersionedHash(input.Certificate, certificateHash)
+		if err != nil {
+			return CertificateHashMismatch, fmt.Sprintf("Failed to check certificate is up to date: %v", err), true
+		}
+		if !upToDate {
+			return CertificateHashMismatch, "Certificate has been changed since issuance (hash has been updated)", true
+		}
+
+		return "", "", false
+	}
+}
+
 // SecretIssuerAnnotationsMismatch - When the issuer annotations are defined,
 // it must match the issuer ref.
 func SecretIssuerAnnotationsMismatch(input Input) (string, string, bool) {
@@ -444,10 +491,11 @@ func SecretManagedLabelsAndAnnotationsManagedFieldsMismatch(fieldManager string)
 
 		// Ignore the CertificateName and IssuerRef annotations as these cannot be set by the postIssuance controller.
 		managedAnnotations.Delete(
-			cmapi.CertificateNameKey,       // SecretCertificateNameAnnotationMismatch checks the value
-			cmapi.IssuerNameAnnotationKey,  // SecretIssuerAnnotationsMismatch checks the value
-			cmapi.IssuerKindAnnotationKey,  // SecretIssuerAnnotationsMismatch checks the value
-			cmapi.IssuerGroupAnnotationKey, // SecretIssuerAnnotationsMismatch checks the value
+			cmapi.CertificateNameKey,           // SecretCertificateNameAnnotationMismatch checks the value
+			cmapi.IssuerNameAnnotationKey,      // SecretIssuerAnnotationsMismatch checks the value
+			cmapi.IssuerKindAnnotationKey,      // SecretIssuerAnnotationsMismatch checks the value
+			cmapi.IssuerGroupAnnotationKey,     // SecretIssuerAnnotationsMismatch checks the value
+			cmapi.CertificateHashAnnotationKey, // SecretCertificateHashAnnotationMismatch checks the value
 		)
 
 		// Remove the non cert-manager labels from the managed labels so we can compare

internal/controller/certificates/policies/constants.go

@@ -36,6 +36,12 @@ const (
 	// SecretMismatch is a policy violation reason for a scenario where Secret's
 	// private key does not match spec.
 	SecretMismatch string = "SecretMismatch"
+	// CertificateHashMissing is a policy violation reason for a scenario where
+	// Certificate's hash does not exist on the Secret.
+	CertificateHashMissing string = "CertificateHashMissing"
+	// CertificateHashMismatch is a policy violation reason for a scenario where
+	// Certificate's hash does not match the hash of the certificate on the Secret.
+	CertificateHashMismatch string = "CertificateHashMismatch"
 	// IncorrectIssuer is a policy violation reason for a scenario where
 	// Certificate has been issued by incorrect Issuer.
 	IncorrectIssuer string = "IncorrectIssuer"

internal/controller/certificates/policies/policies.go

@@ -71,13 +71,21 @@ 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
-		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
-		CurrentCertificateRequestMismatchesSpec,             // Make sure the current CertificateRequest matches the Certificate spec
-		CurrentCertificateNearingExpiry(c),                  // Make sure the Certificate in the Secret is not nearing expiry
+		SecretCertificateHashAnnotationMismatch(
+			// BACKWARDS COMPATIBILITY: The following checks are only performed in case no
+			// CertificateRequest hash annotation is present on the Secret. This is to
+			// ensure that existing users of cert-manager do not have all their existing
+			// Certificates marked as not ready.
+			Chain{
+				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
+				CurrentCertificateRequestMismatchesSpec,             // Make sure the current CertificateRequest matches the Certificate spec
+			},
+		),
+
+		CurrentCertificateNearingExpiry(c), // Make sure the Certificate in the Secret is not nearing expiry
 	}
 }
 
@@ -89,13 +97,21 @@ 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
-		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
-		CurrentCertificateRequestMismatchesSpec,             // Make sure the current CertificateRequest matches the Certificate spec
-		CurrentCertificateHasExpired(c),                     // Make sure the Certificate in the Secret has not expired
+		SecretCertificateHashAnnotationMismatch(
+			// BACKWARDS COMPATIBILITY: The following checks are only performed in case no
+			// CertificateRequest hash annotation is present on the Secret. This is to
+			// ensure that existing users of cert-manager do not have all their existing
+			// Certificates marked as not ready.
+			Chain{
+				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
+				CurrentCertificateRequestMismatchesSpec,             // Make sure the current CertificateRequest matches the Certificate spec
+			},
+		),
+
+		CurrentCertificateHasExpired(c), // Make sure the Certificate in the Secret has not expired
 	}
 }
 
@@ -104,6 +120,13 @@ func NewReadinessPolicyChain(c clock.Clock) Chain {
 // correctness of metadata and output formats of Certificate's Secrets.
 func NewSecretPostIssuancePolicyChain(ownerRefEnabled bool, fieldManager string) Chain {
 	return Chain{
+		// Make sure the Secret has the Certificate hash annotation, if not, we will
+		// calculate a hash from the Certificate and add it to the Secret based on the
+		// Certificate's ready condition.
+		// We do this to ensure that all Secrets have the Certificate hash annotation and
+		// we can remove the fallback chain in a future release.
+		SecretCertificateHashAnnotationMissingAndStable,
+
 		SecretBaseLabelsMismatch,                                             // Make sure the managed labels have the correct values
 		SecretCertificateDetailsAnnotationsMismatch,                          // Make sure the managed certificate details annotations have the correct values
 		SecretManagedLabelsAndAnnotationsManagedFieldsMismatch(fieldManager), // Make sure the only the expected managed labels and annotations exist

pkg/apis/certmanager/v1/types.go

@@ -89,6 +89,10 @@ const (
 	// Annotation key for the 'group' of the Issuer resource.
 	IssuerGroupAnnotationKey = "cert-manager.io/issuer-group"
 
+	// Annotation key for the hash of the Certificate that created the certificate
+	// block encoded in this Secret resource.
+	CertificateHashAnnotationKey = "cert-manager.io/certificate-hash"
+
 	// Annotation key for the name of the certificate that a resource is related to.
 	CertificateNameKey = "cert-manager.io/certificate-name"
 

pkg/controller/certificates/issuing/internal/secret.go

@@ -60,6 +60,7 @@ type SecretsManager struct {
 // SecretData is a structure wrapping private key, Certificate and CA data
 type SecretData struct {
 	PrivateKey, Certificate, CA         []byte
+	CertificateHash                     string
 	CertificateName                     string
 	IssuerName, IssuerKind, IssuerGroup string
 }
@@ -200,6 +201,10 @@ func (s *SecretsManager) setValues(crt *cmapi.Certificate, secret *corev1.Secret
 		secret.Annotations[cmapi.IssuerGroupAnnotationKey] = data.IssuerGroup
 	}
 
+	if data.CertificateHash != "" {
+		secret.Annotations[cmapi.CertificateHashAnnotationKey] = data.CertificateHash
+	}
+
 	secret.Labels[cmapi.PartOfCertManagerControllerLabelKey] = "true"
 
 	return nil

pkg/controller/certificates/issuing/issuing_controller.go

@@ -385,6 +385,11 @@ func (c *controller) issueCertificate(ctx context.Context, nextRevision int, crt
 		crt.Spec.PrivateKey = &cmapi.CertificatePrivateKey{}
 	}
 
+	certificateHash, err := utilpki.CertificateVersionedHash(crt)
+	if err != nil {
+		return err
+	}
+
 	pkData, err := utilpki.EncodePrivateKey(pk, crt.Spec.PrivateKey.Encoding)
 	if err != nil {
 		return err
@@ -393,6 +398,7 @@ func (c *controller) issueCertificate(ctx context.Context, nextRevision int, crt
 		PrivateKey:      pkData,
 		Certificate:     req.Status.Certificate,
 		CA:              req.Status.CA,
+		CertificateHash: certificateHash,
 		CertificateName: crt.Name,
 		IssuerName:      req.Spec.IssuerRef.Name,
 		IssuerKind:      req.Spec.IssuerRef.Kind,

pkg/controller/certificates/issuing/secret_manager.go

@@ -19,16 +19,19 @@ package issuing
 import (
 	"context"
 	"errors"
+	"fmt"
 
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 
 	"github.com/cert-manager/cert-manager/internal/controller/certificates/policies"
+	apiutil "github.com/cert-manager/cert-manager/pkg/api/util"
 	cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 	cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
 	"github.com/cert-manager/cert-manager/pkg/controller/certificates/issuing/internal"
 	logf "github.com/cert-manager/cert-manager/pkg/logs"
+	utilpki "github.com/cert-manager/cert-manager/pkg/util/pki"
 )
 
 // ensureSecretData ensures that the Certificate's Secret is up to date with
@@ -65,23 +68,62 @@ func (c *controller) ensureSecretData(ctx context.Context, log logr.Logger, crt
 		return nil
 	}
 
+	// Check whether the Certificate's Secret has correct output format and
+	// metadata.
+	reason, message, isViolation := c.postIssuancePolicyChain.Evaluate(policies.Input{
+		Certificate: crt,
+		Secret:      secret,
+	})
+
+	certificateHash := secret.Annotations[cmapi.CertificateHashAnnotationKey]
+	if certificateHash == "" {
+		log.V(logf.DebugLevel).Info("secret doesn't contain certificate hash annotation")
+
+		// If the certificate hash is not set, we try to calculate a hash from
+		// the certificate data and set it on the secret. We only do this after
+		// we verified that the certificate is Ready, so we don't set the wrong
+		// hash on a secret that is not up to date.
+		// This is to ensure that all secrets have the certificate hash
+		// annotation and we can remove the fallback chain in a future release.
+
+		readyCondition := apiutil.GetCertificateCondition(crt, cmapi.CertificateConditionReady)
+		issuingCondition := apiutil.GetCertificateCondition(crt, cmapi.CertificateConditionIssuing)
+
+		// If the certificate's ready condition is not up-to-date, we do not change the
+		// certificate hash annotation. Also, if the certificate is already being issued,
+		// we do not need to update the certificate hash annotation (as it will be updated once
+		// the certificate is issued).
+		if readyCondition != nil && readyCondition.ObservedGeneration != crt.Generation {
+			// Leave the certificate hash annotation empty, so that the next time the
+			// certificate is reconciled, we will try to set the certificate hash again.
+		} else if issuingCondition != nil && issuingCondition.Status == cmmeta.ConditionTrue {
+			// Leave the certificate hash annotation empty, so that the next time the
+			// certificate is reconciled, we will try to set the certificate hash again.
+		} else if readyCondition == nil || readyCondition.Status != cmmeta.ConditionTrue {
+			log.V(logf.DebugLevel).Info("certificate is not ready, setting the certificate hash to a mismatching value")
+
+			certificateHash = "v999:secret-not-up-to-date"
+		} else {
+			hash, err := utilpki.CertificateVersionedHash(crt)
+			if err != nil {
+				return fmt.Errorf("failed to compute certificate hash: %w", err)
+			}
+
+			certificateHash = hash
+		}
+	}
+
 	data := internal.SecretData{
 		PrivateKey:      secret.Data[corev1.TLSPrivateKeyKey],
 		Certificate:     secret.Data[corev1.TLSCertKey],
 		CA:              secret.Data[cmmeta.TLSCAKey],
+		CertificateHash: certificateHash,
 		CertificateName: secret.Annotations[cmapi.CertificateNameKey],
 		IssuerName:      secret.Annotations[cmapi.IssuerNameAnnotationKey],
 		IssuerKind:      secret.Annotations[cmapi.IssuerKindAnnotationKey],
 		IssuerGroup:     secret.Annotations[cmapi.IssuerGroupAnnotationKey],
 	}
 
-	// Check whether the Certificate's Secret has correct output format and
-	// metadata.
-	reason, message, isViolation := c.postIssuancePolicyChain.Evaluate(policies.Input{
-		Certificate: crt,
-		Secret:      secret,
-	})
-
 	if isViolation {
 		switch reason {
 		case policies.InvalidCertificate, policies.ManagedFieldsParseError: