do not populate CA

Signed-off-by: Wei Weng <[email protected]>

Author: Wei Weng

pkg/webhook/webhook.go

@@ -193,25 +193,20 @@ func NewWebhookConfig(mgr manager.Manager, webhookServiceName string, port int32
 		webhookCertSecretName:         webhookCertSecretName,
 	}
 
-	var caPEM []byte
-	var err error
-
 	if useCertManager {
-		// When using cert-manager, certificates are mounted as files by Kubernetes
-		// cert-manager creates tls.crt and tls.key, but we need ca.crt for the webhook config
-		caPEM, err = w.loadCertManagerCA(certDir)
-		if err != nil {
-			return nil, fmt.Errorf("failed to load cert-manager CA certificate: %w", err)
-		}
+		// When using cert-manager, the CA bundle is automatically injected by cert-manager's CA injector
+		// based on the cert-manager.io/inject-ca-from annotation. We don't need to load or set the CA here.
+		// The certificates (tls.crt and tls.key) are mounted by Kubernetes and used automatically by the webhook server.
+		klog.V(2).InfoS("Using cert-manager for certificate management", "certDir", certDir)
 	} else {
 		// Use self-signed certificate generation (original flow)
-		caPEM, err = w.genCertificate(certDir)
+		caPEM, err := w.genCertificate(certDir)
 		if err != nil {
 			return nil, fmt.Errorf("failed to generate self-signed certificate: %w", err)
 		}
+		w.caPEM = caPEM
 	}
 
-	w.caPEM = caPEM
 	return &w, nil
 }
 
@@ -675,9 +670,14 @@ func (w *Config) createClientConfig(validationPath string) admv1.WebhookClientCo
 	}
 	serviceEndpoint := w.serviceURL + validationPath
 	serviceRef.Path = ptr.To(validationPath)
-	config := admv1.WebhookClientConfig{
-		CABundle: w.caPEM,
+	config := admv1.WebhookClientConfig{}
+
+	// When using cert-manager, leave CABundle empty so cert-manager's CA injector can populate it.
+	// The cert-manager.io/inject-ca-from annotation triggers automatic CA injection.
+	if !w.useCertManager {
+		config.CABundle = w.caPEM
 	}
+
 	switch *w.clientConnectionType {
 	case options.Service:
 		config.Service = &serviceRef
@@ -703,26 +703,6 @@ func (w *Config) genCertificate(certDir string) ([]byte, error) {
 	return caPEM, nil
 }
 
-// loadCertManagerCA loads the CA certificate from the mounted cert-manager Secret.
-// When using cert-manager, Kubernetes mounts the Secret as files in the certDir.
-// cert-manager creates: ca.crt, tls.crt, and tls.key.
-// The tls.crt and tls.key are automatically used by the webhook server.
-// We only need to read ca.crt for the webhook configuration's CABundle.
-func (w *Config) loadCertManagerCA(certDir string) ([]byte, error) {
-	caPath := filepath.Join(certDir, "ca.crt")
-	caCert, err := os.ReadFile(caPath)
-	if err != nil {
-		return nil, fmt.Errorf("failed to read ca.crt from %s: %w", caPath, err)
-	}
-
-	if len(caCert) == 0 {
-		return nil, fmt.Errorf("ca.crt is empty at %s", caPath)
-	}
-
-	klog.V(2).InfoS("Successfully loaded CA certificate from cert-manager mounted Secret", "path", caPath)
-	return caCert, nil
-}
-
 // genSelfSignedCert generates the self signed Certificate/Key pair
 func (w *Config) genSelfSignedCert() (caPEMByte, certPEMByte, keyPEMByte []byte, err error) {
 	// CA config

pkg/webhook/webhook_test.go

@@ -1,8 +1,6 @@
 package webhook
 
 import (
-	"os"
-	"path/filepath"
 	"strings"
 	"testing"
 
@@ -15,26 +13,7 @@ import (
 	"github.com/kubefleet-dev/kubefleet/pkg/utils"
 )
 
-const mockCA = `-----BEGIN CERTIFICATE-----
-MIIBkTCB+wIJAKHHCgVZU7c4MA0GCSqGSIb3DQEBCwUAMBExDzANBgNVBAMMBnRl
-c3RDQTAeFw0yNDAxMDEwMDAwMDBaFw0yNTAxMDEwMDAwMDBaMBExDzANBgNVBAMM
-BnRlc3RDQTCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEAwjBCKwYJKoZIhvcN
-AQkBFhZmb3JtYXRAZXhhbXBsZS5jb20wHhcNMjQwMTAxMDAwMDAwWhcNMjUwMTAx
-MDAwMDAwWjARMQ8wDQYDVQQDDAZ0ZXN0Q0EwXDANBgkqhkiG9w0BAQEFAANLADBI
-AkEAwjBCKwYJKoZIhvcNAQEBBQADQQAwPgJBAMIwQisGCSqGSIb3DQEBAQUAA0EA
-MD4CQQDCMEIrBgkqhkiG9w0BAQEFAANBADANBgkqhkiG9w0BAQUFAANBAMA=
------END CERTIFICATE-----`
-
-// setupMockCertManagerFiles creates mock certificate files for testing cert-manager mode.
-func setupMockCertManagerFiles(t *testing.T, certDir string) {
-	t.Helper()
-	if err := os.WriteFile(filepath.Join(certDir, "ca.crt"), []byte(mockCA), 0600); err != nil {
-		t.Fatalf("failed to create mock ca.crt: %v", err)
-	}
-	t.Cleanup(func() {
-		os.RemoveAll(certDir)
-	})
-}
+const testWebhookServiceName = "test-webhook"
 
 func TestBuildFleetMutatingWebhooks(t *testing.T) {
 	url := options.WebhookClientConnectionType("url")
@@ -189,11 +168,6 @@ func TestNewWebhookConfig(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Setenv("POD_NAMESPACE", "test-namespace")
 
-			// Create mock certificate files for cert-manager mode
-			if tt.useCertManager {
-				setupMockCertManagerFiles(t, tt.certDir)
-			}
-
 			got, err := NewWebhookConfig(tt.mgr, tt.webhookServiceName, tt.port, tt.clientConnectionType, tt.certDir, tt.enableGuardRail, tt.denyModifyMemberClusterLabels, tt.enableWorkload, tt.useCertManager, "fleet-webhook-server-cert", "fleet-webhook-server-cert")
 			if (err != nil) != tt.wantErr {
 				t.Errorf("NewWebhookConfig() error = %v, wantErr %v", err, tt.wantErr)
@@ -216,67 +190,6 @@ func TestNewWebhookConfig(t *testing.T) {
 		})
 	}
 }
-func TestLoadCertManagerCA_NotFound(t *testing.T) {
-	config := &Config{}
-	_, err := config.loadCertManagerCA("/nonexistent/path")
-	if err == nil {
-		t.Error("Expected error when certificate files don't exist")
-	}
-}
-
-func TestLoadCertManagerCA_EmptyFile(t *testing.T) {
-	dir := t.TempDir()
-	// Create empty files
-	if err := os.WriteFile(filepath.Join(dir, "tls.crt"), []byte{}, 0600); err != nil {
-		t.Fatalf("failed to create empty tls.crt: %v", err)
-	}
-	if err := os.WriteFile(filepath.Join(dir, "ca.crt"), []byte{}, 0600); err != nil {
-		t.Fatalf("failed to create empty ca.crt: %v", err)
-	}
-
-	config := &Config{}
-	_, err := config.loadCertManagerCA(dir)
-	if err == nil {
-		t.Error("Expected error for empty certificate files")
-	}
-	if !strings.Contains(err.Error(), "empty") {
-		t.Errorf("Expected error message to contain 'empty', got: %v", err)
-	}
-}
-
-func TestLoadCertManagerCA_Success(t *testing.T) {
-	t.Run("loads ca.crt successfully", func(t *testing.T) {
-		dir := t.TempDir()
-		caContent := []byte("test-ca-content")
-		if err := os.WriteFile(filepath.Join(dir, "ca.crt"), caContent, 0600); err != nil {
-			t.Fatalf("failed to create ca.crt: %v", err)
-		}
-
-		config := &Config{}
-		result, err := config.loadCertManagerCA(dir)
-		if err != nil {
-			t.Errorf("Unexpected error: %v", err)
-		}
-		if string(result) != string(caContent) {
-			t.Errorf("Expected %s, got %s", caContent, result)
-		}
-	})
-}
-
-func TestNewWebhookConfig_CertManagerNotMounted(t *testing.T) {
-	t.Setenv("POD_NAMESPACE", "test-namespace")
-
-	dir := t.TempDir()
-	// Don't create any certificate files to simulate cert-manager not ready
-
-	_, err := NewWebhookConfig(nil, "test-webhook", 8080, nil, dir, true, true, false, true, "fleet-webhook-server-cert", "fleet-webhook-server-cert")
-	if err == nil {
-		t.Error("Expected error when cert-manager certificates not mounted")
-	}
-	if !strings.Contains(err.Error(), "failed to load cert-manager CA certificate") {
-		t.Errorf("Expected error about loading cert-manager CA, got: %v", err)
-	}
-}
 
 func TestNewWebhookConfig_SelfSignedCertError(t *testing.T) {
 	t.Setenv("POD_NAMESPACE", "test-namespace")
@@ -356,3 +269,74 @@ func TestBuildWebhookAnnotations(t *testing.T) {
 		})
 	}
 }
+
+func TestCreateClientConfig(t *testing.T) {
+	serviceConnectionType := options.Service
+	testCases := map[string]struct {
+		config         Config
+		validationPath string
+		expectCABundle bool
+	}{
+		"useCertManager is false - CABundle should be set": {
+			config: Config{
+				useCertManager:       false,
+				caPEM:                []byte("test-ca-bundle"),
+				serviceNamespace:     "fleet-system",
+				serviceName:          "fleet-webhook-service",
+				servicePort:          443,
+				clientConnectionType: &serviceConnectionType,
+			},
+			validationPath: "/validate",
+			expectCABundle: true,
+		},
+		"useCertManager is true - CABundle should be empty for cert-manager injection": {
+			config: Config{
+				useCertManager:       true,
+				caPEM:                []byte("test-ca-bundle"),
+				serviceNamespace:     "fleet-system",
+				serviceName:          "fleet-webhook-service",
+				servicePort:          443,
+				clientConnectionType: &serviceConnectionType,
+			},
+			validationPath: "/validate",
+			expectCABundle: false,
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			clientConfig := tc.config.createClientConfig(tc.validationPath)
+
+			if tc.expectCABundle {
+				if len(clientConfig.CABundle) == 0 {
+					t.Errorf("Expected CABundle to be set, but it was empty")
+				}
+				if diff := cmp.Diff(tc.config.caPEM, clientConfig.CABundle); diff != "" {
+					t.Errorf("CABundle mismatch (-want +got):\n%s", diff)
+				}
+			} else {
+				if len(clientConfig.CABundle) != 0 {
+					t.Errorf("Expected CABundle to be empty for cert-manager injection, but got: %v", clientConfig.CABundle)
+				}
+			}
+
+			// Verify service reference is set correctly
+			if clientConfig.Service == nil {
+				t.Errorf("Expected Service to be set")
+			} else {
+				if clientConfig.Service.Namespace != tc.config.serviceNamespace {
+					t.Errorf("Expected Service.Namespace=%s, got %s", tc.config.serviceNamespace, clientConfig.Service.Namespace)
+				}
+				if clientConfig.Service.Name != tc.config.serviceName {
+					t.Errorf("Expected Service.Name=%s, got %s", tc.config.serviceName, clientConfig.Service.Name)
+				}
+				if *clientConfig.Service.Port != tc.config.servicePort {
+					t.Errorf("Expected Service.Port=%d, got %d", tc.config.servicePort, *clientConfig.Service.Port)
+				}
+				if *clientConfig.Service.Path != tc.validationPath {
+					t.Errorf("Expected Service.Path=%s, got %s", tc.validationPath, *clientConfig.Service.Path)
+				}
+			}
+		})
+	}
+}