From cfaceaf8116c9d0278ba5c16991b2d2d3f7146b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Ma=C5=82ek?= Date: Thu, 25 Apr 2024 12:11:15 +0200 Subject: [PATCH] fix: fix enforcing up to date ControlPlane's ValidatingWebhookConfiguration --- .../controller_reconciler_utils.go | 7 +- .../controller_reconciler_utils_test.go | 168 ++++++++++++++++++ 2 files changed, 172 insertions(+), 3 deletions(-) create mode 100644 controller/controlplane/controller_reconciler_utils_test.go diff --git a/controller/controlplane/controller_reconciler_utils.go b/controller/controlplane/controller_reconciler_utils.go index 9fd56466f..d4a9ae940 100644 --- a/controller/controlplane/controller_reconciler_utils.go +++ b/controller/controlplane/controller_reconciler_utils.go @@ -693,9 +693,10 @@ func (r *Reconciler) ensureValidatingWebhookConfiguration( if count == 1 { var updated bool webhookConfiguration := validatingWebhookConfigurations[0] - oldWebhookConfiguration := webhookConfiguration.DeepCopy() + old := webhookConfiguration.DeepCopy() + + updated, webhookConfiguration.ObjectMeta = k8sutils.EnsureObjectMetaIsUpdated(webhookConfiguration.ObjectMeta, generatedWebhookConfiguration.ObjectMeta) - updated, generatedWebhookConfiguration.ObjectMeta = k8sutils.EnsureObjectMetaIsUpdated(webhookConfiguration.ObjectMeta, generatedWebhookConfiguration.ObjectMeta) if !cmp.Equal(webhookConfiguration.Webhooks, generatedWebhookConfiguration.Webhooks) { webhookConfiguration.Webhooks = generatedWebhookConfiguration.Webhooks updated = true @@ -703,7 +704,7 @@ func (r *Reconciler) ensureValidatingWebhookConfiguration( if updated { log.Debug(logger, "patching existing ValidatingWebhookConfiguration", webhookConfiguration) - return op.Updated, r.Client.Patch(ctx, &webhookConfiguration, client.MergeFrom(oldWebhookConfiguration)) + return op.Updated, r.Client.Patch(ctx, &webhookConfiguration, client.MergeFrom(old)) } return op.Noop, nil diff --git a/controller/controlplane/controller_reconciler_utils_test.go b/controller/controlplane/controller_reconciler_utils_test.go new file mode 100644 index 000000000..4c68823d8 --- /dev/null +++ b/controller/controlplane/controller_reconciler_utils_test.go @@ -0,0 +1,168 @@ +package controlplane + +import ( + "context" + "testing" + + "github.com/samber/lo" + "github.com/stretchr/testify/require" + admregv1 "k8s.io/api/admissionregistration/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + fakectrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" + + operatorv1beta1 "github.com/kong/gateway-operator/api/v1beta1" + "github.com/kong/gateway-operator/controller/pkg/op" + "github.com/kong/gateway-operator/pkg/consts" + "github.com/kong/gateway-operator/pkg/utils/kubernetes/resources" +) + +func Test_ensureValidatingWebhookConfiguration(t *testing.T) { + const webhookSvcName = "webhook-svc" + + testCases := []struct { + name string + cp *operatorv1beta1.ControlPlane + webhook *admregv1.ValidatingWebhookConfiguration + + testBody func(*testing.T, *Reconciler, *operatorv1beta1.ControlPlane) + }{ + { + name: "creating validating webhook configuration", + cp: &operatorv1beta1.ControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cp", + }, + Spec: operatorv1beta1.ControlPlaneSpec{ + ControlPlaneOptions: operatorv1beta1.ControlPlaneOptions{ + Deployment: operatorv1beta1.ControlPlaneDeploymentOptions{ + Replicas: lo.ToPtr(int32(1)), + PodTemplateSpec: &corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + resources.GenerateControlPlaneContainer(consts.DefaultControlPlaneImage), + }, + }, + }, + }, + }, + }, + }, + testBody: func(t *testing.T, r *Reconciler, cp *operatorv1beta1.ControlPlane) { + var ( + ctx = context.Background() + webhooks admregv1.ValidatingWebhookConfigurationList + ) + require.NoError(t, r.Client.List(ctx, &webhooks)) + require.Empty(t, webhooks.Items) + + certSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cert-cecret", + }, + Data: map[string][]byte{ + "ca.crt": []byte("ca"), // dummy + }, + } + + res, err := r.ensureValidatingWebhookConfiguration(ctx, cp, certSecret, webhookSvcName) + require.NoError(t, err) + require.Equal(t, res, op.Created) + + require.NoError(t, r.Client.List(ctx, &webhooks)) + require.Len(t, webhooks.Items, 1) + + res, err = r.ensureValidatingWebhookConfiguration(ctx, cp, certSecret, webhookSvcName) + require.NoError(t, err) + require.Equal(t, res, op.Noop) + }, + }, + { + name: "updating validating webhook configuration enforces ObjectMeta", + cp: &operatorv1beta1.ControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cp", + }, + Spec: operatorv1beta1.ControlPlaneSpec{ + ControlPlaneOptions: operatorv1beta1.ControlPlaneOptions{ + Deployment: operatorv1beta1.ControlPlaneDeploymentOptions{ + Replicas: lo.ToPtr(int32(1)), + PodTemplateSpec: &corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + resources.GenerateControlPlaneContainer(consts.DefaultControlPlaneImage), + }, + }, + }, + }, + }, + }, + }, + testBody: func(t *testing.T, r *Reconciler, cp *operatorv1beta1.ControlPlane) { + var ( + ctx = context.Background() + webhooks admregv1.ValidatingWebhookConfigurationList + ) + require.NoError(t, r.Client.List(ctx, &webhooks)) + require.Empty(t, webhooks.Items) + + certSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cert-cecret", + }, + Data: map[string][]byte{ + "ca.crt": []byte("ca"), // dummy + }, + } + + res, err := r.ensureValidatingWebhookConfiguration(ctx, cp, certSecret, webhookSvcName) + require.NoError(t, err) + require.Equal(t, res, op.Created) + + require.NoError(t, r.Client.List(ctx, &webhooks)) + require.Len(t, webhooks.Items, 1, "webhook configuration should be created") + + res, err = r.ensureValidatingWebhookConfiguration(ctx, cp, certSecret, webhookSvcName) + require.NoError(t, err) + require.Equal(t, res, op.Noop) + + t.Log("updating webhook configuration outside of the controller") + { + w := webhooks.Items[0] + w.ObjectMeta.Labels["foo"] = "bar" + require.NoError(t, r.Client.Update(ctx, &w)) + } + + t.Log("running ensureValidatingWebhookConfiguration to enforce ObjectMeta") + res, err = r.ensureValidatingWebhookConfiguration(ctx, cp, certSecret, webhookSvcName) + require.NoError(t, err) + require.Equal(t, res, op.Updated) + + require.NoError(t, r.Client.List(ctx, &webhooks)) + require.Len(t, webhooks.Items, 1) + require.NotContains(t, webhooks.Items[0].Labels, "foo", + "labels should be updated by the controller so that changes applied by 3rd parties are overwritten", + ) + }, + }, + } + + for _, tc := range testCases { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + fakeClient := fakectrlruntimeclient. + NewClientBuilder(). + WithScheme(scheme.Scheme). + WithObjects(tc.cp). + Build() + + r := &Reconciler{ + Client: fakeClient, + } + + tc.testBody(t, r, tc.cp) + }) + } +}