templates: MutatingWebhookConfig filter to CREATE POD events only (#1904

)

charts/osm/templates/mutatingwebhook.yaml

@@ -27,4 +27,13 @@ webhooks:
       - key: "name"
         operator: NotIn
         values:
-        - {{.Release.Namespace}}
+        - {{.Release.Namespace}}
+  rules:
+    - apiGroups:
+        - ""
+      apiVersions:
+        - v1
+      operations:
+        - CREATE
+      resources:
+        - pods

pkg/injector/webhook.go

@@ -11,9 +11,8 @@ import (
 	"time"
 
 	"github.com/google/uuid"
-	"github.com/rs/zerolog"
-
 	"github.com/pkg/errors"
+	"github.com/rs/zerolog"
 	"k8s.io/api/admission/v1beta1"
 	admissionv1beta1 "k8s.io/api/admissionregistration/v1beta1"
 	corev1 "k8s.io/api/core/v1"
@@ -22,6 +21,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes"
+	admissionRegistrationTypes "k8s.io/client-go/kubernetes/typed/admissionregistration/v1beta1"
 
 	"github.com/openservicemesh/osm/pkg/catalog"
 	"github.com/openservicemesh/osm/pkg/certificate"
@@ -73,8 +73,11 @@ func NewWebhook(config Config, kubeClient kubernetes.Interface, certManager cert
 		configurator:   cfg,
 	}
 
+	// Start the MutatingWebhook web server
 	go wh.run(stop)
-	if err = patchMutatingWebhookConfiguration(cert, meshName, osmNamespace, webhookConfigName, wh.kubeClient); err != nil {
+
+	// Update the MutatingWebhookConfig with the OSM CA bundle
+	if err = updateMutatingWebhookCABundle(cert, webhookConfigName, wh.kubeClient); err != nil {
 		return errors.Errorf("Error configuring MutatingWebhookConfiguration: %+v", err)
 	}
 	return nil
@@ -315,7 +318,7 @@ func isAnnotatedForInjection(annotations map[string]string) (exists bool, enable
 		case "disabled", "no", "false":
 			enabled = false
 		default:
-			err = errors.Errorf("Invalid annotion value specified for annotation %q: %s", constants.SidecarInjectionAnnotation, inject)
+			err = errors.Errorf("Invalid annotation value specified for annotation %q: %s", constants.SidecarInjectionAnnotation, inject)
 		}
 	}
 	return
@@ -335,11 +338,9 @@ func patchAdmissionResponse(resp *v1beta1.AdmissionResponse, patchBytes []byte)
 	resp.PatchType = &pt
 }
 
-func patchMutatingWebhookConfiguration(cert certificate.Certificater, meshName, osmNamespace, webhookConfigName string, clientSet kubernetes.Interface) error {
-	if err := hookExists(clientSet, webhookConfigName); err != nil {
-		log.Error().Err(err).Msgf("Error getting MutatingWebhookConfiguration %s", webhookConfigName)
-	}
-	updatedWH := admissionv1beta1.MutatingWebhookConfiguration{
+// getPartialMutatingWebhookConfiguration returns only the portion of the MutatingWebhookConfiguration that needs to be updated.
+func getPartialMutatingWebhookConfiguration(cert certificate.Certificater, webhookConfigName string) admissionv1beta1.MutatingWebhookConfiguration {
+	return admissionv1beta1.MutatingWebhookConfiguration{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: webhookConfigName,
 		},
@@ -349,36 +350,34 @@ func patchMutatingWebhookConfiguration(cert certificate.Certificater, meshName,
 				ClientConfig: admissionv1beta1.WebhookClientConfig{
 					CABundle: cert.GetCertificateChain(),
 				},
-				Rules: []admissionv1beta1.RuleWithOperations{
-					{
-						Operations: []admissionv1beta1.OperationType{admissionv1beta1.Create},
-						Rule: admissionv1beta1.Rule{
-							APIGroups:   []string{""},
-							APIVersions: []string{"v1"},
-							Resources:   []string{"pods"},
-						},
-					},
-				},
 			},
 		},
 	}
-	data, err := json.Marshal(updatedWH)
+}
+
+// updateMutatingWebhookCABundle updates the existing MutatingWebhookConfiguration with the CA this OSM instance runs with.
+// It is necessary to perform this patch because the original MutatingWebhookConfig YAML does not contain the root certificate.
+func updateMutatingWebhookCABundle(cert certificate.Certificater, webhookName string, clientSet kubernetes.Interface) error {
+	mwc := clientSet.AdmissionregistrationV1beta1().MutatingWebhookConfigurations()
+	if err := webhookExists(mwc, webhookName); err != nil {
+		log.Error().Err(err).Msgf("Error getting MutatingWebhookConfiguration %s; Will not update CA Bundle for webhook", webhookName)
+	}
+
+	patchJSON, err := json.Marshal(getPartialMutatingWebhookConfiguration(cert, webhookName))
 	if err != nil {
 		return err
 	}
 
-	_, err = clientSet.AdmissionregistrationV1beta1().MutatingWebhookConfigurations().Patch(
-		context.Background(), webhookConfigName, types.StrategicMergePatchType, data, metav1.PatchOptions{})
-	if err != nil {
-		log.Error().Err(err).Msgf("Error configuring MutatingWebhookConfiguration %s", webhookConfigName)
+	if _, err = mwc.Patch(context.Background(), webhookName, types.StrategicMergePatchType, patchJSON, metav1.PatchOptions{}); err != nil {
+		log.Error().Err(err).Msgf("Error updating CA Bundle for MutatingWebhookConfiguration %s", webhookName)
 		return err
 	}
 
-	log.Info().Msgf("Configured MutatingWebhookConfiguration %s", webhookConfigName)
+	log.Info().Msgf("Finished updating CA Bundle for MutatingWebhookConfiguration %s", webhookName)
 	return nil
 }
 
-func hookExists(clientSet kubernetes.Interface, webhookConfigName string) error {
-	_, err := clientSet.AdmissionregistrationV1beta1().MutatingWebhookConfigurations().Get(context.Background(), webhookConfigName, metav1.GetOptions{})
+func webhookExists(mwc admissionRegistrationTypes.MutatingWebhookConfigurationInterface, webhookName string) error {
+	_, err := mwc.Get(context.Background(), webhookName, metav1.GetOptions{})
 	return err
 }

pkg/injector/webhook_test.go

@@ -30,10 +30,8 @@ import (
 )
 
 var _ = Describe("Test MutatingWebhookConfiguration patch", func() {
-	Context("find and patches webhook", func() {
+	Context("find and patches the mutating webhook and updates the CABundle", func() {
 		cert := mockCertificate{}
-		meshName := "--meshName--"
-		osmNamespace := "--namespace--"
 		webhookName := "--webhookName--"
 		//TODO:seed a test webhook
 		testWebhookServiceNamespace := "test-namespace"
@@ -62,18 +60,21 @@ var _ = Describe("Test MutatingWebhookConfiguration patch", func() {
 			},
 		})
 
+		mwc := kubeClient.AdmissionregistrationV1beta1().MutatingWebhookConfigurations()
+
 		It("checks if the hook exists", func() {
-			err := hookExists(kubeClient, webhookName)
+			err := webhookExists(mwc, webhookName)
 			Expect(err).ToNot(HaveOccurred())
 		})
 
 		It("checks if a non existent hook exists", func() {
-			err := hookExists(kubeClient, webhookName+"blah")
+
+			err := webhookExists(mwc, webhookName+"blah")
 			Expect(err).To(HaveOccurred())
 		})
 
 		It("patches a webhook", func() {
-			err := patchMutatingWebhookConfiguration(cert, meshName, osmNamespace, webhookName, kubeClient)
+			err := updateMutatingWebhookCABundle(cert, webhookName, kubeClient)
 			Expect(err).ToNot(HaveOccurred())
 
 		})
@@ -90,13 +91,6 @@ var _ = Describe("Test MutatingWebhookConfiguration patch", func() {
 			Expect(webhook.Webhooks[0].ClientConfig.Service.Name).To(Equal(testWebhookServiceName))
 			Expect(webhook.Webhooks[0].ClientConfig.Service.Path).To(Equal(&testWebhookServicePath))
 			Expect(webhook.Webhooks[0].ClientConfig.CABundle).To(Equal([]byte("chain")))
-			Expect(len(webhook.Webhooks[0].Rules)).To(Equal(1))
-			rule := webhook.Webhooks[0].Rules[0]
-			Expect(len(rule.Operations)).To(Equal(1))
-			Expect(rule.Operations[0]).To(Equal(admissionv1beta1.Create))
-			Expect(rule.Rule.APIGroups).To(Equal([]string{""}))
-			Expect(rule.Rule.APIVersions).To(Equal([]string{"v1"}))
-			Expect(rule.Rule.Resources).To(Equal([]string{"pods"}))
 		})
 	})
 })
@@ -611,4 +605,26 @@ var _ = Describe("Testing Injector Functions", func() {
 		}
 		Expect(actual).To(Equal(&expected))
 	})
+
+	It("creates partial mutating webhook configuration", func() {
+		cert := mockCertificate{}
+		webhookConfigName := "-webhook-config-name-"
+
+		actual := getPartialMutatingWebhookConfiguration(cert, webhookConfigName)
+
+		expected := admissionv1beta1.MutatingWebhookConfiguration{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "-webhook-config-name-",
+			},
+			Webhooks: []admissionv1beta1.MutatingWebhook{
+				{
+					Name: "osm-inject.k8s.io",
+					ClientConfig: admissionv1beta1.WebhookClientConfig{
+						CABundle: cert.GetCertificateChain(),
+					},
+				},
+			},
+		}
+		Expect(actual).To(Equal(expected))
+	})
 })