Merge pull request #3206 from wfernandes/cert-manager-sideEffects

🐛 Set sideEffects:None for cert-manager MutatingWebhookConfiguration

Author: k8s-ci-robot

cmd/clusterctl/client/cluster/cert_manager_test.go

@@ -21,11 +21,111 @@ import (
 	"time"
 
 	. "github.com/onsi/gomega"
+	admissionregistration "k8s.io/api/admissionregistration/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/client/config"
+	"sigs.k8s.io/cluster-api/cmd/clusterctl/internal/scheme"
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/internal/test"
 )
 
+func Test_certManagerClient_getManifestObjects(t *testing.T) {
+
+	tests := []struct {
+		name      string
+		expectErr bool
+		assert    func(*testing.T, []unstructured.Unstructured)
+	}{
+		{
+			name:      "it should not contain the cert-manager-leaderelection ClusterRoleBinding",
+			expectErr: false,
+			assert: func(t *testing.T, objs []unstructured.Unstructured) {
+				for _, o := range objs {
+					if o.GetKind() == "ClusterRoleBinding" && o.GetName() == "cert-manager-leaderelection" {
+						t.Error("should not find cert-manager-leaderelection ClusterRoleBinding")
+					}
+				}
+			},
+		},
+		{
+			name:      "the MutatingWebhookConfiguration should have sideEffects set to None ",
+			expectErr: false,
+			assert: func(t *testing.T, objs []unstructured.Unstructured) {
+				found := false
+				for i := range objs {
+					o := objs[i]
+					if o.GetKind() == "MutatingWebhookConfiguration" && o.GetName() == "cert-manager-webhook" {
+						w := &admissionregistration.MutatingWebhookConfiguration{}
+						err := scheme.Scheme.Convert(&o, w, nil)
+						if err != nil {
+							t.Errorf("did not expect err, got %s", err)
+						}
+						if len(w.Webhooks) != 1 {
+							t.Error("expected 1 webhook to be configured")
+						}
+						wh := w.Webhooks[0]
+						if wh.SideEffects != nil && *wh.SideEffects == admissionregistration.SideEffectClassNone {
+							found = true
+						}
+					}
+				}
+				if !found {
+					t.Error("Expected to find cert-manager-webhook MutatingWebhookConfiguration with sideEffects=None")
+				}
+			},
+		},
+		{
+			name:      "the ValidatingWebhookConfiguration should have sideEffects set to None ",
+			expectErr: false,
+			assert: func(t *testing.T, objs []unstructured.Unstructured) {
+				found := false
+				for i := range objs {
+					o := objs[i]
+					if o.GetKind() == "ValidatingWebhookConfiguration" && o.GetName() == "cert-manager-webhook" {
+						w := &admissionregistration.ValidatingWebhookConfiguration{}
+						err := scheme.Scheme.Convert(&o, w, nil)
+						if err != nil {
+							t.Errorf("did not expect err, got %s", err)
+						}
+						if len(w.Webhooks) != 1 {
+							t.Error("expected 1 webhook to be configured")
+						}
+						wh := w.Webhooks[0]
+						if wh.SideEffects != nil && *wh.SideEffects == admissionregistration.SideEffectClassNone {
+							found = true
+						}
+					}
+				}
+				if !found {
+					t.Error("Expected to find cert-manager-webhook ValidatingWebhookConfiguration with sideEffects=None")
+				}
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			pollImmediateWaiter := func(interval, timeout time.Duration, condition wait.ConditionFunc) error {
+				return nil
+			}
+			fakeConfigClient := newFakeConfig("")
+
+			cm := newCertMangerClient(fakeConfigClient, nil, pollImmediateWaiter)
+			objs, err := cm.getManifestObjs()
+
+			if tt.expectErr {
+				g.Expect(err).To(HaveOccurred())
+				return
+			}
+			g.Expect(err).ToNot(HaveOccurred())
+			tt.assert(t, objs)
+		})
+	}
+
+}
+
 func Test_GetTimeout(t *testing.T) {
 
 	pollImmediateWaiter := func(interval, timeout time.Duration, condition wait.ConditionFunc) error {

cmd/clusterctl/config/assets/cert-manager.yaml

@@ -1,8 +1,10 @@
 # This is manually edited cert-manager v0.11.0 release.
-# We've manually removed the ClusterRoleBinding cert-manager-leaderelection.
-# OTHER NOTES:
+# DIFFERENCES:
+# 1. We've manually removed the ClusterRoleBinding cert-manager-leaderelection.
 # See https://github.com/kubernetes-sigs/cluster-api/issues/2928
 # See https://github.com/jetstack/cert-manager/pull/2207
+# 2. Added `sideEffects: None` to MutatingWebhookConfiguration.
+# See https://github.com/kubernetes-sigs/cluster-api/issues/3204
 ---
 apiVersion: apiextensions.k8s.io/v1beta1
 kind: CustomResourceDefinition
@@ -6271,6 +6273,7 @@ webhooks:
           - challenges
           - certificaterequests
     failurePolicy: Fail
+    sideEffects: None
     clientConfig:
       service:
         name: kubernetes