Merge pull request #222 from openshift-cherrypick-robot/cherry-pick-2…

…19-to-release-4.14

[release-4.14] OCPBUGS-19318: fix admission webhook CA injection

go.mod

@@ -12,6 +12,7 @@ require (
 	github.com/prometheus/client_golang v1.14.0
 	github.com/prometheus/common v0.37.0
 	github.com/spf13/cobra v1.6.1
+	github.com/stretchr/testify v1.8.1
 	k8s.io/api v0.27.4
 	k8s.io/apiextensions-apiserver v0.27.4
 	k8s.io/apimachinery v0.27.4
@@ -64,6 +65,7 @@ require (
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pkg/profile v1.3.0 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/prometheus/client_model v0.3.0 // indirect
 	github.com/prometheus/procfs v0.8.0 // indirect
 	github.com/robfig/cron v1.2.0 // indirect

pkg/controller/cabundleinjector/admissionwebhook.go

@@ -0,0 +1,161 @@
+package cabundleinjector
+
+import (
+	"bytes"
+	"context"
+
+	admissionregv1 "k8s.io/api/admissionregistration/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/klog/v2"
+
+	"github.com/openshift/library-go/pkg/controller/factory"
+	"github.com/openshift/service-ca-operator/pkg/controller/api"
+)
+
+// webhookConfigAccessor provides a common interface we can use in order to inject
+// the CAs in both the validating and mutating webhookconfig objects
+type webhookConfigAccessor[T admissionregv1.MutatingWebhookConfiguration | admissionregv1.ValidatingWebhookConfiguration] interface {
+	metav1.Object
+	GetWebhookClientCA(index int) *admissionregv1.WebhookClientConfig
+	WebhooksLen() int
+	DeepCopy() webhookConfigAccessor[T]
+	GetObject() *T
+}
+
+type mutatingWebhookConfigAccessor struct {
+	*admissionregv1.MutatingWebhookConfiguration
+}
+
+func newMutatingWebhookAccessor(webhookConfig *admissionregv1.MutatingWebhookConfiguration) webhookConfigAccessor[admissionregv1.MutatingWebhookConfiguration] {
+	return &mutatingWebhookConfigAccessor{webhookConfig}
+}
+
+func (a *mutatingWebhookConfigAccessor) GetWebhookClientCA(index int) *admissionregv1.WebhookClientConfig {
+	return &a.MutatingWebhookConfiguration.Webhooks[index].ClientConfig
+}
+
+func (a *mutatingWebhookConfigAccessor) WebhooksLen() int {
+	return len(a.MutatingWebhookConfiguration.Webhooks)
+}
+
+func (a *mutatingWebhookConfigAccessor) DeepCopy() webhookConfigAccessor[admissionregv1.MutatingWebhookConfiguration] {
+	return &mutatingWebhookConfigAccessor{
+		MutatingWebhookConfiguration: a.MutatingWebhookConfiguration.DeepCopy(),
+	}
+}
+
+func (a *mutatingWebhookConfigAccessor) GetObject() *admissionregv1.MutatingWebhookConfiguration {
+	return a.MutatingWebhookConfiguration
+}
+
+type validatingWebhookConfigAccessor struct {
+	*admissionregv1.ValidatingWebhookConfiguration
+}
+
+func newValidatingWebhookAccessor(webhookConfig *admissionregv1.ValidatingWebhookConfiguration) webhookConfigAccessor[admissionregv1.ValidatingWebhookConfiguration] {
+	return &validatingWebhookConfigAccessor{webhookConfig}
+}
+
+func (a *validatingWebhookConfigAccessor) GetWebhookClientCA(index int) *admissionregv1.WebhookClientConfig {
+	return &a.ValidatingWebhookConfiguration.Webhooks[index].ClientConfig
+}
+
+func (a *validatingWebhookConfigAccessor) WebhooksLen() int {
+	return len(a.ValidatingWebhookConfiguration.Webhooks)
+}
+
+func (a *validatingWebhookConfigAccessor) DeepCopy() webhookConfigAccessor[admissionregv1.ValidatingWebhookConfiguration] {
+	return &validatingWebhookConfigAccessor{
+		ValidatingWebhookConfiguration: a.ValidatingWebhookConfiguration.DeepCopy(),
+	}
+}
+
+func (a *validatingWebhookConfigAccessor) GetObject() *admissionregv1.ValidatingWebhookConfiguration {
+	return a.ValidatingWebhookConfiguration
+}
+
+type cachedWebhookConfigGetter[T admissionregv1.MutatingWebhookConfiguration | admissionregv1.ValidatingWebhookConfiguration] interface {
+	Get(name string) (*T, error)
+}
+
+type webhookConfigUpdater[T admissionregv1.MutatingWebhookConfiguration | admissionregv1.ValidatingWebhookConfiguration] interface {
+	Update(ctx context.Context, webhookConfig *T, updateOptions metav1.UpdateOptions) (*T, error)
+}
+
+// webhookCABundleInjector creates a controller that injects the service-ca bundle
+// to validating and mutating webhookconfigurations
+type webhookCABundleInjector[T admissionregv1.MutatingWebhookConfiguration | admissionregv1.ValidatingWebhookConfiguration] struct {
+	webhookConfigType        string
+	client                   webhookConfigUpdater[T]
+	lister                   cachedWebhookConfigGetter[T]
+	newWebhookConfigAccessor func(*T) webhookConfigAccessor[T]
+	caBundle                 []byte
+}
+
+func (bi *webhookCABundleInjector[T]) Sync(ctx context.Context, syncCtx factory.SyncContext) error {
+	webhookConfig, err := bi.lister.Get(syncCtx.QueueKey())
+	if apierrors.IsNotFound(err) {
+		return nil
+	} else if err != nil {
+		return err
+	}
+	webhookConfigAccessor := bi.newWebhookConfigAccessor(webhookConfig)
+
+	webhooksNeedingUpdate := []int{}
+	for i := 0; i < webhookConfigAccessor.WebhooksLen(); i++ {
+		webhookClientConfig := webhookConfigAccessor.GetWebhookClientCA(i)
+		if !bytes.Equal(webhookClientConfig.CABundle, bi.caBundle) {
+			webhooksNeedingUpdate = append(webhooksNeedingUpdate, i)
+		}
+	}
+	if len(webhooksNeedingUpdate) == 0 {
+		return nil
+	}
+
+	klog.Infof("updating %s %s with the service signing CA bundle", bi.webhookConfigType, webhookConfigAccessor.GetName())
+
+	// make a copy to avoid mutating cache state
+	webhookConfigCopy := webhookConfigAccessor.DeepCopy()
+	for _, i := range webhooksNeedingUpdate {
+		webhookConfigCopy.GetWebhookClientCA(i).CABundle = bi.caBundle
+	}
+	_, err = bi.client.Update(ctx, webhookConfigCopy.GetObject(), metav1.UpdateOptions{})
+	return err
+}
+
+func newMutatingWebhookInjectorConfig(config *caBundleInjectorConfig) controllerConfig {
+	informer := config.kubeInformers.Admissionregistration().V1().MutatingWebhookConfigurations()
+	syncer := &webhookCABundleInjector[admissionregv1.MutatingWebhookConfiguration]{
+		webhookConfigType:        "mutatingwebhookconfiguration",
+		client:                   config.kubeClient.AdmissionregistrationV1().MutatingWebhookConfigurations(),
+		lister:                   informer.Lister(),
+		newWebhookConfigAccessor: newMutatingWebhookAccessor,
+		caBundle:                 config.caBundle,
+	}
+	return controllerConfig{
+		name:               "MutatingWebhookCABundleInjector",
+		sync:               syncer.Sync,
+		informer:           informer.Informer(),
+		annotationsChecker: annotationsChecker(api.InjectCABundleAnnotationName),
+	}
+}
+
+func newValidatingWebhookInjectorConfig(config *caBundleInjectorConfig) controllerConfig {
+	informer := config.kubeInformers.Admissionregistration().V1().ValidatingWebhookConfigurations()
+	syncer := &webhookCABundleInjector[admissionregv1.ValidatingWebhookConfiguration]{
+		webhookConfigType:        "validatingwebhookconfiguration",
+		client:                   config.kubeClient.AdmissionregistrationV1().ValidatingWebhookConfigurations(),
+		lister:                   informer.Lister(),
+		newWebhookConfigAccessor: newValidatingWebhookAccessor,
+		caBundle:                 config.caBundle,
+	}
+	return controllerConfig{
+		name:     "ValidatingWebhookCABundleInjector",
+		sync:     syncer.Sync,
+		informer: informer.Informer(),
+		annotationsChecker: annotationsChecker(
+			api.InjectCABundleAnnotationName,
+		),
+	}
+}

pkg/controller/cabundleinjector/admissionwebhook_test.go

@@ -0,0 +1,128 @@
+package cabundleinjector
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+
+	admissionregv1 "k8s.io/api/admissionregistration/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/informers"
+	"k8s.io/client-go/kubernetes/fake"
+	"k8s.io/client-go/tools/cache"
+	"k8s.io/client-go/util/workqueue"
+
+	"github.com/openshift/library-go/pkg/operator/events"
+	"github.com/openshift/service-ca-operator/pkg/controller/api"
+)
+
+func TestWebhookCABundleInjectorSync(t *testing.T) {
+	testCABundle := []byte("something")
+
+	tests := []struct {
+		name             string
+		webhooks         []admissionregv1.ValidatingWebhook
+		expectedWebhooks []admissionregv1.ValidatingWebhook
+		wantErr          bool
+	}{
+		{
+			name: "no webhooks",
+		},
+		{
+			name: "single webhook to fill",
+			webhooks: []admissionregv1.ValidatingWebhook{
+				{
+					ClientConfig: admissionregv1.WebhookClientConfig{},
+				},
+			},
+			expectedWebhooks: []admissionregv1.ValidatingWebhook{
+				{
+					ClientConfig: admissionregv1.WebhookClientConfig{
+						CABundle: testCABundle,
+					},
+				},
+			},
+		},
+		{
+			name: "multiple webhooks to fill",
+			webhooks: []admissionregv1.ValidatingWebhook{
+				{
+					ClientConfig: admissionregv1.WebhookClientConfig{},
+				},
+				{
+					ClientConfig: admissionregv1.WebhookClientConfig{
+						CABundle: []byte("random other string"),
+					},
+				},
+			},
+			expectedWebhooks: []admissionregv1.ValidatingWebhook{
+				{
+					ClientConfig: admissionregv1.WebhookClientConfig{
+						CABundle: testCABundle,
+					},
+				},
+				{
+					ClientConfig: admissionregv1.WebhookClientConfig{
+						CABundle: testCABundle,
+					},
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			testWebhook := &admissionregv1.ValidatingWebhookConfiguration{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-webhook",
+					Annotations: map[string]string{
+						api.InjectCABundleAnnotationName: "true",
+					},
+				},
+				Webhooks: tt.webhooks,
+			}
+
+			testCtx, cancel := context.WithCancel(context.Background())
+			defer cancel()
+
+			webhookClient := fake.NewSimpleClientset(testWebhook)
+			webhookInformer := informers.NewSharedInformerFactory(webhookClient, 1*time.Hour)
+			go webhookInformer.Start(testCtx.Done())
+			waitSuccess := cache.WaitForCacheSync(testCtx.Done(), webhookInformer.Admissionregistration().V1().ValidatingWebhookConfigurations().Informer().HasSynced)
+			require.True(t, waitSuccess)
+
+			injector := webhookCABundleInjector[admissionregv1.ValidatingWebhookConfiguration]{
+				webhookConfigType:        "testwebhook",
+				newWebhookConfigAccessor: newValidatingWebhookAccessor,
+				client:                   webhookClient.AdmissionregistrationV1().ValidatingWebhookConfigurations(),
+				lister:                   webhookInformer.Admissionregistration().V1().ValidatingWebhookConfigurations().Lister(),
+				caBundle:                 testCABundle,
+			}
+
+			if gotErr := injector.Sync(testCtx, testContext{"test-webhook"}); (gotErr != nil) != tt.wantErr {
+				t.Errorf("webhookCABundleInjector.Sync() = %v, want %v", gotErr, tt.wantErr)
+			}
+
+			gotWebhook, err := webhookClient.AdmissionregistrationV1().ValidatingWebhookConfigurations().Get(testCtx, "test-webhook", metav1.GetOptions{})
+			require.NoError(t, err)
+			require.Equal(t, tt.expectedWebhooks, gotWebhook.Webhooks)
+		})
+	}
+}
+
+type testContext struct {
+	key string
+}
+
+func (c testContext) Queue() workqueue.RateLimitingInterface {
+	return nil
+}
+
+func (c testContext) QueueKey() string {
+	return c.key
+}
+
+func (c testContext) Recorder() events.Recorder {
+	return nil
+}