Cherry-pick #4396 and #4493: Fix cert reissuance on SAN mismatch [v1.40]

pkg/controller/certificatemanager/certificatemanager.go

@@ -502,7 +502,7 @@ func (cm *certificateManager) getKeyPair(cli client.Client, secretName, secretNa
 	}
 
 	var issuer certificatemanagement.KeyPairInterface
-	if x509Cert.Issuer.CommonName == rmeta.TigeraOperatorCAIssuerPrefix {
+	if strings.HasPrefix(x509Cert.Issuer.CommonName, rmeta.TigeraOperatorCAIssuerPrefix) {
 		if cm.keyPair.CertificateManagement != nil {
 			return certificateManagementKeyPair(cm, secretName, secretNamespace, dnsNames), nil, nil
 		}

pkg/controller/certificatemanager/certificatemanager_test.go

@@ -54,67 +54,73 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-var _ = Describe("Test CertificateManagement suite", func() {
-	const (
-		appSecretName       = "my-app-tls"
-		appSecretName2      = "my-app-tls-2"
-		legacySecretName    = "legacy-secret"
-		appNs               = "my-app"
-		legacyKeyFieldName  = "key"
-		legacyCertFieldName = "cert"
-	)
+const (
+	appSecretName       = "my-app-tls"
+	appSecretName2      = "my-app-tls-2"
+	legacySecretName    = "legacy-secret"
+	appNs               = "my-app"
+	legacyKeyFieldName  = "key"
+	legacyCertFieldName = "cert"
+
+	certValidity = 365 * 24 * time.Hour
+)
 
-	var (
-		cli                      client.Client
-		scheme                   *k8sruntime.Scheme
-		installation             *operatorv1.InstallationSpec
-		cm                       *operatorv1.CertificateManagement
-		clusterDomain            = "cluster.local"
-		appDNSNames              = []string{appSecretName}
-		ctx                      = context.TODO()
-		certificateManager       certificatemanager.CertificateManager
-		expiredSecret            *corev1.Secret
-		legacySecret             *corev1.Secret
-		expiredLegacySecret      *corev1.Secret
-		byoSecret                *corev1.Secret
-		expiredBYOSecret         *corev1.Secret
-		legacyBYOSecret          *corev1.Secret
-		legacyWithClientKeyUsage *corev1.Secret
-	)
+var (
 	// Configure certs to match legacy operator-generated cert extensions - i.e., only valid for use as a server certificate.
-	legacyOpts := []crypto.CertificateExtensionFunc{tls.SetServerAuth}
-	modernOpts := []crypto.CertificateExtensionFunc{tls.SetServerAuth, tls.SetClientAuth}
-
-	// Precompute expensive operations once.
-	BeforeSuite(func() {
-		var err error
-
-		certkeyusage.SetCertKeyUsage(legacySecretName, []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth})
-		// Create a legacy secret (how certs were before v1.24) with non-standardized legacy key and cert name, and no CA.
-		// Use a secret
-		legacySecret, err = secret.CreateTLSSecret(nil, legacySecretName, appNs, legacyKeyFieldName, legacyCertFieldName, time.Hour, legacyOpts, legacySecretName)
-		Expect(err).NotTo(HaveOccurred())
-
-		// This is a special case, which may or may not exist in the wild. It's a legacy-style certificate signed by tigera-operator but also with client usage.
-		legacyWithClientKeyUsage, err = secret.CreateTLSSecret(nil, appSecretName, appNs, legacyKeyFieldName, legacyCertFieldName, time.Hour, modernOpts, appSecretName)
-		Expect(err).NotTo(HaveOccurred())
+	legacyOpts = []crypto.CertificateExtensionFunc{tls.SetServerAuth}
+	modernOpts = []crypto.CertificateExtensionFunc{tls.SetServerAuth, tls.SetClientAuth}
+
+	legacySecret             *corev1.Secret
+	expiredLegacySecret      *corev1.Secret
+	byoSecret                *corev1.Secret
+	expiredBYOSecret         *corev1.Secret
+	legacyBYOSecret          *corev1.Secret
+	legacyWithClientKeyUsage *corev1.Secret
+)
 
-		// Create a byo secret with non-standardized legacy key and cert name (like our docs for felix/typha).
-		cryptoCA, err := tls.MakeCA("byo-ca")
-		Expect(err).NotTo(HaveOccurred())
-		byoSecret, err = secret.CreateTLSSecret(cryptoCA, appSecretName, appNs, "key.key", "cert.crt", time.Hour, modernOpts, appSecretName)
-		Expect(err).NotTo(HaveOccurred())
-		legacyBYOSecret, err = secret.CreateTLSSecret(cryptoCA, legacySecretName, appNs, "key.key", "cert.crt", time.Hour, legacyOpts, legacySecretName)
-		Expect(err).NotTo(HaveOccurred())
-		expiredBYOSecret, err = secret.CreateTLSSecret(cryptoCA, appSecretName, appNs, "key.key", "cert.crt", -time.Hour, modernOpts, appSecretName)
-		Expect(err).NotTo(HaveOccurred())
+// Precompute expensive operations once.
+var _ = BeforeSuite(func() {
+	var err error
+
+	certkeyusage.SetCertKeyUsage(legacySecretName, []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth})
+	// Create a legacy secret (how certs were before v1.24) with non-standardized legacy key and cert name, and no CA.
+	// Use a secret
+	legacySecret, err = secret.CreateTLSSecret(nil, legacySecretName, appNs, legacyKeyFieldName, legacyCertFieldName, certValidity, legacyOpts, legacySecretName)
+	Expect(err).NotTo(HaveOccurred())
+
+	// This is a special case, which may or may not exist in the wild. It's a legacy-style certificate signed by tigera-operator but also with client usage.
+	legacyWithClientKeyUsage, err = secret.CreateTLSSecret(nil, legacySecretName, appNs, legacyKeyFieldName, legacyCertFieldName, certValidity, modernOpts, appSecretName)
+	Expect(err).NotTo(HaveOccurred())
+
+	// Create a byo secret with non-standardized legacy key and cert name (like our docs for felix/typha).
+	cryptoCA, err := tls.MakeCA("byo-ca")
+	Expect(err).NotTo(HaveOccurred())
+	byoSecret, err = secret.CreateTLSSecret(cryptoCA, appSecretName, appNs, "key.key", "cert.crt", certValidity, modernOpts, appSecretName)
+	Expect(err).NotTo(HaveOccurred())
+	legacyBYOSecret, err = secret.CreateTLSSecret(cryptoCA, legacySecretName, appNs, "key.key", "cert.crt", certValidity, legacyOpts, legacySecretName)
+	Expect(err).NotTo(HaveOccurred())
+	expiredBYOSecret, err = secret.CreateTLSSecret(cryptoCA, appSecretName, appNs, "key.key", "cert.crt", -time.Hour, modernOpts, appSecretName)
+	Expect(err).NotTo(HaveOccurred())
+
+	// Create a CA in the manner of older operator versions.
+	legacyCryptoCA, err := tls.MakeCA(rmeta.TigeraOperatorCAIssuerPrefix + "@some-hash")
+	Expect(err).NotTo(HaveOccurred())
+	expiredLegacySecret, err = secret.CreateTLSSecret(legacyCryptoCA, appSecretName, appNs, legacyKeyFieldName, legacyCertFieldName, -time.Hour, legacyOpts, appSecretName)
+	Expect(err).NotTo(HaveOccurred())
+})
 
-		// Create a CA in the manner of older operator versions.
-		legacyCryptoCA, err := tls.MakeCA(rmeta.TigeraOperatorCAIssuerPrefix + "@some-hash")
-		Expect(err).NotTo(HaveOccurred())
-		expiredLegacySecret, err = secret.CreateTLSSecret(legacyCryptoCA, appSecretName, appNs, legacyKeyFieldName, legacyCertFieldName, -time.Hour, legacyOpts, appSecretName)
-		Expect(err).NotTo(HaveOccurred())
-	})
+var _ = Describe("Test CertificateManagement suite", func() {
+	var (
+		cli                client.Client
+		scheme             *k8sruntime.Scheme
+		installation       *operatorv1.InstallationSpec
+		cm                 *operatorv1.CertificateManagement
+		clusterDomain      = "cluster.local"
+		appDNSNames        = []string{appSecretName}
+		ctx                = context.TODO()
+		certificateManager certificatemanager.CertificateManager
+		expiredSecret      *corev1.Secret
+	)
 
 	BeforeEach(func() {
 		for _, secret := range []*corev1.Secret{legacySecret, byoSecret, legacyWithClientKeyUsage, legacyBYOSecret, expiredBYOSecret, expiredLegacySecret} {
@@ -548,13 +554,24 @@ var _ = Describe("Test CertificateManagement suite", func() {
 			By("verifying it does replace a secret when dns names are missing")
 			keyPair, err := certificateManager.GetOrCreateKeyPair(cli, appSecretName, appNs, appDNSNames)
 			Expect(err).NotTo(HaveOccurred())
+			Expect(cli.Create(ctx, keyPair.Secret(appNs))).NotTo(HaveOccurred())
+			Expect(err).NotTo(HaveOccurred())
 			test.VerifyCertSANs(keyPair.GetCertificatePEM(), appDNSNames...)
 			keyPair, err = certificateManager.GetOrCreateKeyPair(cli, appSecretName, appNs, missingDNSNames)
 			Expect(err).NotTo(HaveOccurred())
 			test.VerifyCertSANs(keyPair.GetCertificatePEM(), missingDNSNames...)
 
+			By("verifying it does replace a legacy secret when dns names are missing")
+			Expect(cli.Create(ctx, legacyWithClientKeyUsage)).NotTo(HaveOccurred())
+			keyPair, err = certificateManager.GetOrCreateKeyPair(cli, legacySecretName, appNs, appDNSNames)
+			Expect(err).NotTo(HaveOccurred())
+			test.VerifyCertSANs(keyPair.GetCertificatePEM(), appDNSNames...)
+			keyPair, err = certificateManager.GetOrCreateKeyPair(cli, legacySecretName, appNs, missingDNSNames)
+			Expect(err).NotTo(HaveOccurred())
+			test.VerifyCertSANs(keyPair.GetCertificatePEM(), missingDNSNames...)
+
 			By("verifying it does not replace a BYO secret, nor throw an error")
-			Expect(cli.Create(ctx, byoSecret)).NotTo(HaveOccurred())
+			Expect(cli.Update(ctx, byoSecret)).NotTo(HaveOccurred())
 			keyPair, err = certificateManager.GetOrCreateKeyPair(cli, appSecretName, appNs, missingDNSNames)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(keyPair.UseCertificateManagement()).To(BeFalse())

pkg/controller/compliance/compliance_controller_test.go

@@ -249,7 +249,8 @@ var _ = Describe("Compliance controller tests", func() {
 
 		// Custom cert has the compliance svc DNS names as well as other DNS names
 		dnsNames := append(expectedDNSNames, "compliance.example.com", "192.168.10.13")
-		newSecret, err := secret.CreateTLSSecret(nil,
+		testCA := test.MakeTestCA("compliance-test")
+		newSecret, err := secret.CreateTLSSecret(testCA,
 			render.ComplianceServerCertSecret, common.OperatorNamespace(), corev1.TLSPrivateKeyKey,
 			corev1.TLSCertKey, tls.DefaultCertificateDuration, nil, dnsNames...,
 		)