Merge pull request #10469 from chrischdi/pr-cert-manager-fix-x509-errors

🐛 clusterctl: ensure cert-manager objects get applied before other provider objects

Author: k8s-ci-robot

cmd/clusterctl/client/repository/components.go

@@ -18,6 +18,7 @@ package repository
 
 import (
 	"fmt"
+	"sort"
 	"strings"
 
 	"github.com/pkg/errors"
@@ -259,6 +260,22 @@ func NewComponents(input ComponentsInput) (Components, error) {
 	// Add common labels.
 	objs = addCommonLabels(objs, input.Provider)
 
+	// Deploying cert-manager objects and especially Certificates before Mutating-
+	// ValidatingWebhookConfigurations and CRDs ensures cert-manager's ca-injector
+	// receives the event for the objects at the right time to inject the new CA.
+	sort.SliceStable(objs, func(i, j int) bool {
+		// First prioritize Namespaces over everything.
+		if objs[i].GetKind() == "Namespace" {
+			return true
+		}
+		if objs[j].GetKind() == "Namespace" {
+			return false
+		}
+
+		// Second prioritize cert-manager objects.
+		return objs[i].GroupVersionKind().Group == "cert-manager.io"
+	})
+
 	return &components{
 		Provider:        input.Provider,
 		version:         input.Options.Version,

test/framework/clusterctl/ca_injection.go

@@ -0,0 +1,225 @@
+/*
+Copyright 2024 The Kubernetes 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 clusterctl
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	"github.com/pkg/errors"
+	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
+	corev1 "k8s.io/api/core/v1"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/types"
+	kerrors "k8s.io/apimachinery/pkg/util/errors"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+)
+
+const certManagerCAAnnotation = "cert-manager.io/inject-ca-from"
+
+func verifyCAInjection(ctx context.Context, c client.Client) error {
+	v := newCAInjectionVerifier(c)
+
+	errs := v.verifyCustomResourceDefinitions(ctx)
+	errs = append(errs, v.verifyMutatingWebhookConfigurations(ctx)...)
+	errs = append(errs, v.verifyValidatingWebhookConfigurations(ctx)...)
+
+	return kerrors.NewAggregate(errs)
+}
+
+// certificateInjectionVerifier waits for cert-managers ca-injector to inject the
+// referred CA certificate to all CRDs, MutatingWebhookConfigurations and
+// ValidatingWebhookConfigurations.
+// As long as the correct CA certificates are not injected the kube-apiserver will
+// reject the requests due to certificate verification errors.
+type certificateInjectionVerifier struct {
+	Client client.Client
+}
+
+// newCAInjectionVerifier creates a new CRD migrator.
+func newCAInjectionVerifier(client client.Client) *certificateInjectionVerifier {
+	return &certificateInjectionVerifier{
+		Client: client,
+	}
+}
+
+func (c *certificateInjectionVerifier) verifyCustomResourceDefinitions(ctx context.Context) []error {
+	crds := &apiextensionsv1.CustomResourceDefinitionList{}
+	if err := c.Client.List(ctx, crds, client.HasLabels{clusterv1.ProviderNameLabel}); err != nil {
+		return []error{err}
+	}
+
+	errs := []error{}
+	for i := range crds.Items {
+		crd := crds.Items[i]
+		ca, err := c.getCACertificateFor(ctx, &crd)
+		if err != nil {
+			errs = append(errs, err)
+			continue
+		}
+		if ca == "" {
+			continue
+		}
+
+		if crd.Spec.Conversion.Webhook == nil || crd.Spec.Conversion.Webhook.ClientConfig == nil {
+			continue
+		}
+
+		if string(crd.Spec.Conversion.Webhook.ClientConfig.CABundle) != ca {
+			changedCRD := crd.DeepCopy()
+			changedCRD.Spec.Conversion.Webhook.ClientConfig.CABundle = nil
+			errs = append(errs, fmt.Errorf("injected CA for CustomResourceDefinition %s does not match", crd.Name))
+			if err := c.Client.Patch(ctx, changedCRD, client.MergeFrom(&crd)); err != nil {
+				errs = append(errs, err)
+			}
+		}
+	}
+
+	return errs
+}
+
+func (c *certificateInjectionVerifier) verifyMutatingWebhookConfigurations(ctx context.Context) []error {
+	mutateHooks := &admissionregistrationv1.MutatingWebhookConfigurationList{}
+	if err := c.Client.List(ctx, mutateHooks, client.HasLabels{clusterv1.ProviderNameLabel}); err != nil {
+		return []error{err}
+	}
+
+	errs := []error{}
+	for i := range mutateHooks.Items {
+		mutateHook := mutateHooks.Items[i]
+		ca, err := c.getCACertificateFor(ctx, &mutateHook)
+		if err != nil {
+			errs = append(errs, err)
+			continue
+		}
+		if ca == "" {
+			continue
+		}
+
+		var changed bool
+		changedHook := mutateHook.DeepCopy()
+		for i := range mutateHook.Webhooks {
+			webhook := mutateHook.Webhooks[i]
+			if string(webhook.ClientConfig.CABundle) != ca {
+				changed = true
+				webhook.ClientConfig.CABundle = nil
+				changedHook.Webhooks[i] = webhook
+				errs = append(errs, fmt.Errorf("injected CA for MutatingWebhookConfiguration %s hook %s does not match", mutateHook.Name, webhook.Name))
+			}
+		}
+		if changed {
+			if err := c.Client.Patch(ctx, changedHook, client.MergeFrom(&mutateHook)); err != nil {
+				errs = append(errs, err)
+			}
+		}
+	}
+
+	return errs
+}
+
+func (c *certificateInjectionVerifier) verifyValidatingWebhookConfigurations(ctx context.Context) []error {
+	validateHooks := &admissionregistrationv1.ValidatingWebhookConfigurationList{}
+	if err := c.Client.List(ctx, validateHooks, client.HasLabels{clusterv1.ProviderNameLabel}); err != nil {
+		return []error{err}
+	}
+
+	errs := []error{}
+	for i := range validateHooks.Items {
+		validateHook := validateHooks.Items[i]
+		ca, err := c.getCACertificateFor(ctx, &validateHook)
+		if err != nil {
+			errs = append(errs, err)
+			continue
+		}
+		if ca == "" {
+			continue
+		}
+
+		var changed bool
+		changedHook := validateHook.DeepCopy()
+		for i := range validateHook.Webhooks {
+			webhook := validateHook.Webhooks[i]
+			if string(webhook.ClientConfig.CABundle) != ca {
+				changed = true
+				webhook.ClientConfig.CABundle = nil
+				changedHook.Webhooks[i] = webhook
+				errs = append(errs, fmt.Errorf("injected CA for ValidatingWebhookConfiguration %s hook %s does not match", validateHook.Name, webhook.Name))
+			}
+		}
+		if changed {
+			if err := c.Client.Patch(ctx, changedHook, client.MergeFrom(&validateHook)); err != nil {
+				errs = append(errs, err)
+			}
+		}
+	}
+
+	return errs
+}
+
+// getCACertificateFor returns the ca certificate from the secret referred by the
+// Certificate object. It reads the namespaced name of the Certificate from the
+// injection annotation of the passed object.
+func (c *certificateInjectionVerifier) getCACertificateFor(ctx context.Context, obj client.Object) (string, error) {
+	annotationValue, ok := obj.GetAnnotations()[certManagerCAAnnotation]
+	if !ok || annotationValue == "" {
+		return "", nil
+	}
+
+	certificateObjKey, err := splitObjectKey(annotationValue)
+	if err != nil {
+		return "", errors.Wrapf(err, "getting certificate object key for %s %s", obj.GetObjectKind().GroupVersionKind().Kind, obj.GetName())
+	}
+
+	certificate := &unstructured.Unstructured{}
+	certificate.SetKind("Certificate")
+	certificate.SetAPIVersion("cert-manager.io/v1")
+
+	if err := c.Client.Get(ctx, certificateObjKey, certificate); err != nil {
+		return "", errors.Wrapf(err, "getting certificate %s for %s %s", certificateObjKey, obj.GetObjectKind().GroupVersionKind().Kind, obj.GetName())
+	}
+
+	secretName, _, err := unstructured.NestedString(certificate.Object, "spec", "secretName")
+	if err != nil || secretName == "" {
+		return "", errors.Wrapf(err, "reading .spec.secretName name from certificate %s for %s %s", certificateObjKey, obj.GetObjectKind().GroupVersionKind().Kind, obj.GetName())
+	}
+
+	secretObjKey := client.ObjectKey{Namespace: certificate.GetNamespace(), Name: secretName}
+	certificateSecret := &corev1.Secret{}
+	if err := c.Client.Get(ctx, secretObjKey, certificateSecret); err != nil {
+		return "", errors.Wrapf(err, "getting secret %s for certificate %s for %s %s", secretObjKey, certificateObjKey, obj.GetObjectKind().GroupVersionKind().Kind, obj.GetName())
+	}
+
+	ca, ok := certificateSecret.Data["ca.crt"]
+	if !ok {
+		return "", errors.Errorf("data for \"ca.crt\" not found in secret %s for %s %s", secretObjKey, obj.GetObjectKind().GroupVersionKind().Kind, obj.GetName())
+	}
+
+	return string(ca), nil
+}
+
+// splitObjectKey splits the string by the name separator and returns it as client.ObjectKey.
+func splitObjectKey(nameStr string) (client.ObjectKey, error) {
+	splitPoint := strings.IndexRune(nameStr, types.Separator)
+	if splitPoint == -1 {
+		return client.ObjectKey{}, errors.Errorf("expected object key %s to contain namespace and name", nameStr)
+	}
+	return client.ObjectKey{Namespace: nameStr[:splitPoint], Name: nameStr[splitPoint+1:]}, nil
+}

test/framework/clusterctl/clusterctl_helpers.go

@@ -23,6 +23,7 @@ import (
 	"path/filepath"
 	"time"
 
+	"github.com/blang/semver/v4"
 	. "github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/klog/v2"
@@ -95,6 +96,16 @@ func InitManagementClusterAndWatchControllerLogs(ctx context.Context, input Init
 
 		if input.ClusterctlBinaryPath != "" {
 			InitWithBinary(ctx, input.ClusterctlBinaryPath, initInput)
+			// Old versions of clusterctl may deploy CRDs, Mutating- and/or ValidatingWebhookConfigurations
+			// before creating the new Certificate objects. This check ensures the CA's are up to date before
+			// continuing.
+			clusterctlVersion, err := getClusterCtlVersion(input.ClusterctlBinaryPath)
+			Expect(err).ToNot(HaveOccurred())
+			if clusterctlVersion.LT(semver.MustParse("1.7.2")) {
+				Eventually(func() error {
+					return verifyCAInjection(ctx, client)
+				}, time.Minute*5, time.Second*10).Should(Succeed(), "Failed to verify CA injection")
+			}
 		} else {
 			Init(ctx, initInput)
 		}
@@ -183,14 +194,24 @@ func UpgradeManagementClusterAndWait(ctx context.Context, input UpgradeManagemen
 		LogFolder:                 input.LogFolder,
 	}
 
+	client := input.ClusterProxy.GetClient()
+
 	if input.ClusterctlBinaryPath != "" {
 		UpgradeWithBinary(ctx, input.ClusterctlBinaryPath, upgradeInput)
+		// Old versions of clusterctl may deploy CRDs, Mutating- and/or ValidatingWebhookConfigurations
+		// before creating the new Certificate objects. This check ensures the CA's are up to date before
+		// continuing.
+		clusterctlVersion, err := getClusterCtlVersion(input.ClusterctlBinaryPath)
+		Expect(err).ToNot(HaveOccurred())
+		if clusterctlVersion.LT(semver.MustParse("1.7.2")) {
+			Eventually(func() error {
+				return verifyCAInjection(ctx, client)
+			}, time.Minute*5, time.Second*10).Should(Succeed(), "Failed to verify CA injection")
+		}
 	} else {
 		Upgrade(ctx, upgradeInput)
 	}
 
-	client := input.ClusterProxy.GetClient()
-
 	log.Logf("Waiting for provider controllers to be running")
 	controllersDeployments := framework.GetControllerDeployments(ctx, framework.GetControllerDeploymentsInput{
 		Lister: client,

test/framework/convenience.go

@@ -19,6 +19,7 @@ package framework
 import (
 	"reflect"
 
+	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 	appsv1 "k8s.io/api/apps/v1"
 	coordinationv1 "k8s.io/api/coordination/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -71,6 +72,9 @@ func TryAddDefaultSchemes(scheme *runtime.Scheme) {
 	_ = apiextensionsv1beta.AddToScheme(scheme)
 	_ = apiextensionsv1.AddToScheme(scheme)
 
+	// Add the admission registration scheme (Mutating-, ValidatingWebhookConfiguration).
+	_ = admissionregistrationv1.AddToScheme(scheme)
+
 	// Add RuntimeSDK to the scheme.
 	_ = runtimev1.AddToScheme(scheme)