Merge pull request #6632 from chrischdi/pr-topology-mutation-external

✨ RuntimeSDK: Add caBundle injection to Extension controller

Author: k8s-ci-robot

exp/runtime/api/v1alpha1/extensionconfig_types.go

@@ -202,4 +202,9 @@ const (
 
 	// DiscoveryFailedReason documents failure of a Discovery call.
 	DiscoveryFailedReason string = "DiscoveryFailed"
+
+	// InjectCAFromSecretAnnotation is the annotation that specifies that a particular
+	// object wants injection of CAs. It takes the form of a reference to a Secret
+	// as namespace/name.
+	InjectCAFromSecretAnnotation string = "runtime.cluster.x-k8s.io/inject-ca-from-secret"
 )

exp/runtime/internal/controllers/extensionconfig_controller.go

@@ -18,13 +18,20 @@ package controllers
 
 import (
 	"context"
+	"strings"
 
 	"github.com/pkg/errors"
+	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/types"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
+	"sigs.k8s.io/controller-runtime/pkg/handler"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+	"sigs.k8s.io/controller-runtime/pkg/source"
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	runtimev1 "sigs.k8s.io/cluster-api/exp/runtime/api/v1alpha1"
@@ -35,6 +42,11 @@ import (
 	"sigs.k8s.io/cluster-api/util/predicates"
 )
 
+const (
+	// tlsCAKey is used as a data key in Secret resources to store a CA certificate.
+	tlsCAKey = "ca.crt"
+)
+
 // +kubebuilder:rbac:groups=runtime.cluster.x-k8s.io,resources=extensionconfigs;extensionconfigs/status,verbs=get;list;watch;patch;update
 // +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch
 
@@ -50,6 +62,11 @@ type Reconciler struct {
 func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
 	err := ctrl.NewControllerManagedBy(mgr).
 		For(&runtimev1.ExtensionConfig{}).
+		Watches(
+			&source.Kind{Type: &corev1.Secret{}},
+			handler.EnqueueRequestsFromMapFunc(r.secretToExtensionConfig),
+			builder.OnlyMetadata,
+		).
 		WithOptions(options).
 		WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(ctrl.LoggerFrom(ctx), r.WatchFilterValue)).
 		Complete(r)
@@ -103,14 +120,22 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		return r.reconcileDelete(ctx, extensionConfig)
 	}
 
+	// Copy to avoid modifying the original extensionConfig.
+	original := extensionConfig.DeepCopy()
+
+	// Inject CABundle from secret if annotation is set. Otherwise https calls may fail.
+	if err := reconcileCABundle(ctx, r.Client, extensionConfig); err != nil {
+		return ctrl.Result{}, err
+	}
+
 	// discoverExtensionConfig will return a discovered ExtensionConfig with the appropriate conditions.
 	discoveredExtensionConfig, err := discoverExtensionConfig(ctx, r.RuntimeClient, extensionConfig)
 	if err != nil {
 		errs = append(errs, err)
 	}
 
-	// Always patch the ExtensionConfig as it may contain updates in conditions.
-	if err = patchExtensionConfig(ctx, r.Client, extensionConfig, discoveredExtensionConfig); err != nil {
+	// Always patch the ExtensionConfig as it may contain updates in conditions or clientConfig.caBundle.
+	if err = patchExtensionConfig(ctx, r.Client, original, discoveredExtensionConfig); err != nil {
 		errs = append(errs, err)
 	}
 
@@ -150,6 +175,29 @@ func (r *Reconciler) reconcileDelete(_ context.Context, extensionConfig *runtime
 	return ctrl.Result{}, nil
 }
 
+// secretToExtensionConfig maps a secret to ExtensionConfigs to reconcile them on updates of the secrets.
+func (r *Reconciler) secretToExtensionConfig(secret client.Object) []reconcile.Request {
+	result := []ctrl.Request{}
+
+	extensionConfigs := runtimev1.ExtensionConfigList{}
+	if err := r.Client.List(context.Background(), &extensionConfigs); err != nil {
+		return nil
+	}
+
+	for _, ext := range extensionConfigs.Items {
+		if secretNameRaw, ok := ext.GetAnnotations()[runtimev1.InjectCAFromSecretAnnotation]; ok {
+			secretName := splitNamespacedName(secretNameRaw)
+			// append all extensions to the result which refer the object as secret
+			if secretName.Namespace == secret.GetNamespace() && secretName.Name == secret.GetName() {
+				name := client.ObjectKey{Namespace: ext.GetNamespace(), Name: ext.GetName()}
+				result = append(result, ctrl.Request{NamespacedName: name})
+			}
+		}
+	}
+
+	return result
+}
+
 // discoverExtensionConfig attempts to discover the Handlers for an ExtensionConfig.
 // If discovery succeeds it returns the ExtensionConfig with Handlers updated in Status and an updated Condition.
 // If discovery fails it returns the ExtensionConfig with no update to Handlers and a Failed Condition.
@@ -164,3 +212,41 @@ func discoverExtensionConfig(ctx context.Context, runtimeClient runtimeclient.Cl
 	conditions.MarkTrue(discoveredExtension, runtimev1.RuntimeExtensionDiscoveredCondition)
 	return discoveredExtension, nil
 }
+
+// reconcileCABundle reconciles the CA bundle for the ExtensionConfig.
+// cert-manager code: pkg/controller/cainjector/sources.go certificateDataSource.
+func reconcileCABundle(ctx context.Context, client client.Client, config *runtimev1.ExtensionConfig) error {
+	secretNameRaw, ok := config.Annotations[runtimev1.InjectCAFromSecretAnnotation]
+	if !ok {
+		return nil
+	}
+	secretName := splitNamespacedName(secretNameRaw)
+
+	if secretName.Namespace == "" || secretName.Name == "" {
+		return errors.Errorf("secret name %q must be in the form namespace/name", secretNameRaw)
+	}
+
+	var secret corev1.Secret
+	// Note: this is an expensive API call because secrets are explicitly not cached.
+	if err := client.Get(ctx, secretName, &secret); err != nil {
+		return errors.Wrapf(err, "failed to get secret %s", secretNameRaw)
+	}
+
+	caData, hasCAData := secret.Data[tlsCAKey]
+	if !hasCAData {
+		return errors.Errorf("secret %s does not contain a %s", secretNameRaw, tlsCAKey)
+	}
+
+	config.Spec.ClientConfig.CABundle = caData
+	return nil
+}
+
+// splitNamespacedName turns the string form of a namespaced name
+// (<namespace>/<name>) back into a types.NamespacedName.
+func splitNamespacedName(nameStr string) types.NamespacedName {
+	splitPoint := strings.IndexRune(nameStr, types.Separator)
+	if splitPoint == -1 {
+		return types.NamespacedName{Name: nameStr}
+	}
+	return types.NamespacedName{Namespace: nameStr[:splitPoint], Name: nameStr[splitPoint+1:]}
+}

exp/runtime/internal/controllers/extensionconfig_controller_test.go

@@ -17,6 +17,8 @@ limitations under the License.
 package controllers
 
 import (
+	"context"
+	"crypto/tls"
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
@@ -27,10 +29,13 @@ import (
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apiserver/pkg/admission/plugin/webhook/testcerts"
 	utilfeature "k8s.io/component-base/featuregate/testing"
 	"k8s.io/utils/pointer"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	runtimev1 "sigs.k8s.io/cluster-api/exp/runtime/api/v1alpha1"
 	runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1"
@@ -67,9 +72,18 @@ func TestExtensionReconciler_Reconcile(t *testing.T) {
 		RuntimeClient: runtimeClient,
 	}
 
-	server := fakeExtensionServer(discoveryHandler("first", "second", "third"))
-	extensionConfig := fakeExtensionConfigForURL(ns.Name, "ext1", server.URL)
+	caCertSecret := fakeCASecret(ns.Name, "ext1-webhook", testcerts.CACert)
+	server, err := fakeSecureExtensionServer(discoveryHandler("first", "second", "third"))
+	g.Expect(err).NotTo(HaveOccurred())
 	defer server.Close()
+	extensionConfig := fakeExtensionConfigForURL(ns.Name, "ext1", server.URL)
+	extensionConfig.Annotations[runtimev1.InjectCAFromSecretAnnotation] = caCertSecret.GetNamespace() + "/" + caCertSecret.GetName()
+
+	// Create the secret which contains the ca certificate.
+	g.Expect(env.CreateAndWait(ctx, caCertSecret)).To(Succeed())
+	defer func() {
+		g.Expect(env.CleanupAndWait(ctx, caCertSecret)).To(Succeed())
+	}()
 	// Create the ExtensionConfig.
 	g.Expect(env.CreateAndWait(ctx, extensionConfig)).To(Succeed())
 	defer func() {
@@ -120,7 +134,8 @@ func TestExtensionReconciler_Reconcile(t *testing.T) {
 
 	t.Run("Successful reconcile and discovery on Extension update", func(t *testing.T) {
 		// Start a new ExtensionServer where the second handler is removed.
-		updatedServer := fakeExtensionServer(discoveryHandler("first", "third"))
+		updatedServer, err := fakeSecureExtensionServer(discoveryHandler("first", "third"))
+		g.Expect(err).ToNot(HaveOccurred())
 		defer updatedServer.Close()
 		// Close the original server  it's no longer serving.
 		server.Close()
@@ -195,7 +210,8 @@ func TestExtensionReconciler_discoverExtensionConfig(t *testing.T) {
 		registry := runtimeregistry.New()
 		g.Expect(runtimehooksv1.AddToCatalog(cat)).To(Succeed())
 		extensionName := "ext1"
-		srv1 := fakeExtensionServer(discoveryHandler("first"))
+		srv1, err := fakeSecureExtensionServer(discoveryHandler("first"))
+		g.Expect(err).ToNot(HaveOccurred())
 		defer srv1.Close()
 
 		runtimeClient := runtimeclient.New(runtimeclient.Options{
@@ -228,7 +244,7 @@ func TestExtensionReconciler_discoverExtensionConfig(t *testing.T) {
 		extensionName := "ext1"
 
 		// Don't set up a server to run the extensionDiscovery handler.
-		// srv1 := fakeExtensionServer(discoveryHandler("first"))
+		// srv1 := fakeSecureExtensionServer(discoveryHandler("first"))
 		// defer srv1.Close()
 
 		runtimeClient := runtimeclient.New(runtimeclient.Options{
@@ -254,6 +270,70 @@ func TestExtensionReconciler_discoverExtensionConfig(t *testing.T) {
 	})
 }
 
+func Test_reconcileCABundle(t *testing.T) {
+	g := NewWithT(t)
+
+	scheme := runtime.NewScheme()
+	g.Expect(corev1.AddToScheme(scheme)).To(Succeed())
+
+	tests := []struct {
+		name         string
+		client       client.Client
+		config       *runtimev1.ExtensionConfig
+		wantCABundle []byte
+		wantErr      bool
+	}{
+		{
+			name:    "No-op because no annotation is set",
+			client:  fake.NewClientBuilder().WithScheme(scheme).Build(),
+			config:  fakeCAInjectionRuntimeExtensionConfig("some-namespace", "some-extension-config", "", ""),
+			wantErr: false,
+		},
+		{
+			name: "Inject ca-bundle",
+			client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(
+				fakeCASecret("some-namespace", "some-ca-secret", []byte("some-ca-data")),
+			).Build(),
+			config:       fakeCAInjectionRuntimeExtensionConfig("some-namespace", "some-extension-config", "some-namespace/some-ca-secret", ""),
+			wantCABundle: []byte(`some-ca-data`),
+			wantErr:      false,
+		},
+		{
+			name: "Update ca-bundle",
+			client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(
+				fakeCASecret("some-namespace", "some-ca-secret", []byte("some-new-data")),
+			).Build(),
+			config:       fakeCAInjectionRuntimeExtensionConfig("some-namespace", "some-extension-config", "some-namespace/some-ca-secret", "some-old-ca-data"),
+			wantCABundle: []byte(`some-new-data`),
+			wantErr:      false,
+		},
+		{
+			name:    "Fail because secret does not exist",
+			client:  fake.NewClientBuilder().WithScheme(scheme).WithObjects().Build(),
+			config:  fakeCAInjectionRuntimeExtensionConfig("some-namespace", "some-extension-config", "some-namespace/some-ca-secret", ""),
+			wantErr: true,
+		},
+		{
+			name: "Fail because secret does not contain a ca.crt",
+			client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(
+				fakeCASecret("some-namespace", "some-ca-secret", nil),
+			).Build(),
+			config:  fakeCAInjectionRuntimeExtensionConfig("some-namespace", "some-extension-config", "some-namespace/some-ca-secret", ""),
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			err := reconcileCABundle(context.TODO(), tt.client, tt.config)
+			g.Expect(err != nil).To(Equal(tt.wantErr))
+
+			g.Expect(tt.config.Spec.ClientConfig.CABundle).To(Equal(tt.wantCABundle))
+		})
+	}
+}
+
 func discoveryHandler(handlerList ...string) func(http.ResponseWriter, *http.Request) {
 	handlers := []runtimehooksv1.ExtensionHandler{}
 	for _, name := range handlerList {
@@ -290,8 +370,9 @@ func fakeExtensionConfigForURL(namespace, name, url string) *runtimev1.Extension
 			APIVersion: runtimehooksv1.GroupVersion.String(),
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      name,
-			Namespace: namespace,
+			Name:        name,
+			Namespace:   namespace,
+			Annotations: map[string]string{},
 		},
 		Spec: runtimev1.ExtensionConfigSpec{
 			ClientConfig: runtimev1.ClientConfig{
@@ -302,9 +383,51 @@ func fakeExtensionConfigForURL(namespace, name, url string) *runtimev1.Extension
 	}
 }
 
-func fakeExtensionServer(discoveryHandler func(w http.ResponseWriter, r *http.Request)) *httptest.Server {
+func fakeSecureExtensionServer(discoveryHandler func(w http.ResponseWriter, r *http.Request)) (*httptest.Server, error) {
 	mux := http.NewServeMux()
 	mux.HandleFunc("/", discoveryHandler)
-	srv := httptest.NewServer(mux)
-	return srv
+
+	sCert, err := tls.X509KeyPair(testcerts.ServerCert, testcerts.ServerKey)
+	if err != nil {
+		return nil, err
+	}
+	testServer := httptest.NewUnstartedServer(mux)
+	testServer.TLS = &tls.Config{
+		MinVersion:   tls.VersionTLS13,
+		Certificates: []tls.Certificate{sCert},
+	}
+	testServer.StartTLS()
+
+	return testServer, nil
+}
+
+func fakeCASecret(namespace, name string, caData []byte) *corev1.Secret {
+	secret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+		},
+		Data: map[string][]byte{},
+	}
+	if caData != nil {
+		secret.Data["ca.crt"] = caData
+	}
+	return secret
+}
+
+func fakeCAInjectionRuntimeExtensionConfig(namespace, name, annotationString, caBundleData string) *runtimev1.ExtensionConfig {
+	ext := &runtimev1.ExtensionConfig{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        name,
+			Namespace:   namespace,
+			Annotations: map[string]string{},
+		},
+	}
+	if annotationString != "" {
+		ext.Annotations[runtimev1.InjectCAFromSecretAnnotation] = annotationString
+	}
+	if caBundleData != "" {
+		ext.Spec.ClientConfig.CABundle = []byte(caBundleData)
+	}
+	return ext
 }

exp/runtime/internal/controllers/warmup.go

@@ -98,6 +98,11 @@ func warmupRegistry(ctx context.Context, client client.Client, reader client.Rea
 		extensionConfig := &extensionConfigList.Items[i]
 		original := extensionConfig.DeepCopy()
 
+		// Inject CABundle from secret if annotation is set. Otherwise https calls may fail.
+		if err := reconcileCABundle(ctx, client, extensionConfig); err != nil {
+			errs = append(errs, err)
+		}
+
 		extensionConfig, err := discoverExtensionConfig(ctx, runtimeClient, extensionConfig)
 		if err != nil {
 			errs = append(errs, err)