fix: fix enforcing up to date ControlPlane's ValidatingWebhookConfigu…

…ration (#225)

CHANGELOG.md

@@ -28,6 +28,11 @@
   `apis` -> `api` and `controllers` -> `controller`.
   [#84](https://github.com/Kong/gateway-operator/pull/84)
 
+### Fixes
+
+- Fix enforcing up to date `ControlPlane`'s `ValidatingWebhookConfiguration`
+  [#225](https://github.com/Kong/gateway-operator/pull/225)
+
 ## [v1.2.3]
 
 ### Fixes

controller/controlplane/controller_reconciler_utils.go

@@ -693,17 +693,18 @@ 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
 		}
 
 		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

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)
+		})
+	}
+}