From 1caea89a74874e8a42cc3f09524547f18fb272d3 Mon Sep 17 00:00:00 2001
From: Quan Tian <quan.tian@broadcom.com>
Date: Wed, 10 Apr 2024 00:44:04 +0800
Subject: [PATCH] Do not try to update type of Secret in selfSignedCertProvider
 (#6205)

If a cluster used user-provided certificate and created a Secret named
antrea-controller-tls of Opaque type, changing to use self-signed
certificate would fail because the type field is immutable.

To support switching the certificate provider, we don't try to update
the type of Secret if it already exists.

Signed-off-by: Quan Tian <quan.tian@broadcom.com>
---
 .../certificate/selfsignedcert_provider.go    |  5 ++++-
 .../selfsignedcert_provider_test.go           | 19 +++++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/pkg/apiserver/certificate/selfsignedcert_provider.go b/pkg/apiserver/certificate/selfsignedcert_provider.go
index 7ae466ee54c..a7c25ce85e6 100644
--- a/pkg/apiserver/certificate/selfsignedcert_provider.go
+++ b/pkg/apiserver/certificate/selfsignedcert_provider.go
@@ -315,9 +315,11 @@ func (p *selfSignedCertProvider) saveCertKeyToSecret(secret *corev1.Secret, cert
 		if bytes.Equal(cert, secret.Data[corev1.TLSCertKey]) && bytes.Equal(key, secret.Data[corev1.TLSPrivateKeyKey]) {
 			return nil
 		}
-		secret.Type = corev1.SecretTypeTLS
+		// Do not update the existing Secret's type. Otherwise, the update would fail if it's not of type
+		// "kubernetes.io/tls" as the type field is immutable.
 		secret.Data[corev1.TLSCertKey] = cert
 		secret.Data[corev1.TLSPrivateKeyKey] = key
+		klog.InfoS("Updating Secret to persist self-signed cert", "secret", klog.KObj(secret))
 		_, err := p.client.CoreV1().Secrets(p.secretNamespace).Update(context.TODO(), secret, metav1.UpdateOptions{})
 		return err
 	}
@@ -329,6 +331,7 @@ func (p *selfSignedCertProvider) saveCertKeyToSecret(secret *corev1.Secret, cert
 			corev1.TLSPrivateKeyKey: key,
 		},
 	}
+	klog.InfoS("Creating Secret to persist self-signed cert", "secret", klog.KObj(secret))
 	_, err := p.client.CoreV1().Secrets(p.secretNamespace).Create(context.TODO(), caSecret, metav1.CreateOptions{})
 	return err
 }
diff --git a/pkg/apiserver/certificate/selfsignedcert_provider_test.go b/pkg/apiserver/certificate/selfsignedcert_provider_test.go
index 2123f3ab307..4ca9b1db725 100644
--- a/pkg/apiserver/certificate/selfsignedcert_provider_test.go
+++ b/pkg/apiserver/certificate/selfsignedcert_provider_test.go
@@ -164,6 +164,12 @@ func TestSelfSignedCertProviderRotate(t *testing.T) {
 	}, 2*time.Second, 50*time.Millisecond)
 }
 
+func copyAndMutateSecret(secret *corev1.Secret, mutator func(_ *corev1.Secret)) *corev1.Secret {
+	s := secret.DeepCopy()
+	mutator(s)
+	return s
+}
+
 func TestSelfSignedCertProviderRun(t *testing.T) {
 	t.Setenv(env.PodNamespaceEnvKey, testSecretNamespace)
 	testSecret := &corev1.Secret{
@@ -220,6 +226,19 @@ func TestSelfSignedCertProviderRun(t *testing.T) {
 			expectedCert:     testOneYearCert2,
 			expectedKey:      testOneYearKey2,
 		},
+		{
+			name:          "should not update secret type when secret is opaque",
+			tlsSecretName: testSecretName,
+			existingSecret: copyAndMutateSecret(testSecret, func(s *corev1.Secret) {
+				s.Type = corev1.SecretTypeOpaque
+			}),
+			expectedSecret: copyAndMutateSecret(testSecret2, func(s *corev1.Secret) {
+				s.Type = corev1.SecretTypeOpaque
+			}),
+			minValidDuration: time.Hour * 24 * 370,
+			expectedCert:     testOneYearCert2,
+			expectedKey:      testOneYearKey2,
+		},
 		{
 			name:             "should generate TLS and update secret when secret is empty",
 			tlsSecretName:    testSecretName,