Merge pull request #39 from yiqigao217/mwh

Add MWH and update VWH on included-namespace label

Author: k8s-ci-robot

cmd/manager/main.go

@@ -41,6 +41,7 @@ import (
 	v1a2 "sigs.k8s.io/hierarchical-namespaces/api/v1alpha2"
 	"sigs.k8s.io/hierarchical-namespaces/internal/config"
 	"sigs.k8s.io/hierarchical-namespaces/internal/forest"
+	"sigs.k8s.io/hierarchical-namespaces/internal/mutators"
 	"sigs.k8s.io/hierarchical-namespaces/internal/reconcilers"
 	"sigs.k8s.io/hierarchical-namespaces/internal/stats"
 	"sigs.k8s.io/hierarchical-namespaces/internal/validators"
@@ -213,6 +214,10 @@ func startControllers(mgr ctrl.Manager, certsCreated chan struct{}) {
 		validators.Create(mgr, f)
 	}
 
+	// Create mutating admission controllers.
+	setupLog.Info("Registering mutating webhook")
+	mutators.Create(mgr)
+
 	// Create all reconciling controllers
 	setupLog.Info("Creating controllers", "maxReconciles", maxReconciles)
 	if err := reconcilers.Create(mgr, f, maxReconciles, false); err != nil {

config/webhook/manifests.yaml

@@ -1,4 +1,32 @@
 
+---
+apiVersion: admissionregistration.k8s.io/v1
+kind: MutatingWebhookConfiguration
+metadata:
+  creationTimestamp: null
+  name: mutating-webhook-configuration
+webhooks:
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /mutate-namespace
+  failurePolicy: Ignore
+  name: namespacelabel.hnc.x-k8s.io
+  rules:
+  - apiGroups:
+    - ""
+    apiVersions:
+    - v1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - namespaces
+  sideEffects: None
+
 ---
 apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration
@@ -8,7 +36,6 @@ metadata:
 webhooks:
 - admissionReviewVersions:
   - v1
-  - v1beta1
   clientConfig:
     service:
       name: webhook-service
@@ -29,7 +56,6 @@ webhooks:
   sideEffects: None
 - admissionReviewVersions:
   - v1
-  - v1beta1
   clientConfig:
     service:
       name: webhook-service
@@ -50,7 +76,6 @@ webhooks:
   sideEffects: None
 - admissionReviewVersions:
   - v1
-  - v1beta1
   clientConfig:
     service:
       name: webhook-service
@@ -72,7 +97,6 @@ webhooks:
   sideEffects: None
 - admissionReviewVersions:
   - v1
-  - v1beta1
   clientConfig:
     service:
       name: webhook-service
@@ -94,7 +118,6 @@ webhooks:
   sideEffects: None
 - admissionReviewVersions:
   - v1
-  - v1beta1
   clientConfig:
     service:
       name: webhook-service

internal/mutators/namespace.go

@@ -0,0 +1,74 @@
+package mutators
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+
+	"github.com/go-logr/logr"
+	corev1 "k8s.io/api/core/v1"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+
+	api "sigs.k8s.io/hierarchical-namespaces/api/v1alpha2"
+	"sigs.k8s.io/hierarchical-namespaces/internal/config"
+)
+
+// NamespaceMutatorServingPath is where the mutator will run. Must be kept in
+// sync with the kubebuilder markers below.
+const (
+	NamespaceMutatorServingPath = "/mutate-namespace"
+)
+
+// Note: the mutating webhook FAILS OPEN. This means that if the webhook goes
+// down, all further changes are allowed. (An empty line has to be kept below
+// the kubebuilder marker for the controller-gen to generate manifests.)
+//
+// +kubebuilder:webhook:admissionReviewVersions=v1,path=/mutate-namespace,mutating=true,failurePolicy=ignore,groups="",resources=namespaces,sideEffects=None,verbs=create;update,versions=v1,name=namespacelabel.hnc.x-k8s.io
+
+type Namespace struct {
+	Log     logr.Logger
+	decoder *admission.Decoder
+}
+
+// Handle implements the mutating webhook.
+func (m *Namespace) Handle(ctx context.Context, req admission.Request) admission.Response {
+	log := m.Log.WithValues("namespace", req.Name)
+	ns := &corev1.Namespace{}
+	err := m.decoder.Decode(req, ns)
+	if err != nil {
+		return admission.Errored(http.StatusBadRequest, err)
+	}
+
+	m.handle(log, ns)
+	marshaledNS, err := json.Marshal(ns)
+	if err != nil {
+		return admission.Errored(http.StatusInternalServerError, err)
+	}
+	return admission.PatchResponseFromRaw(req.Object.Raw, marshaledNS)
+}
+
+// handle implements the non-boilerplate logic of this mutator, allowing it to
+// be more easily unit tested (ie without constructing a full admission.Request).
+// Currently, we only add `included-namespace` label to non-excluded namespaces
+// if the label is missing.
+func (m *Namespace) handle(log logr.Logger, ns *corev1.Namespace) {
+	// Early exit if the namespace is excluded.
+	if config.ExcludedNamespaces[ns.Name] {
+		return
+	}
+
+	// Add label if the namespace doesn't have it.
+	if _, hasLabel := ns.Labels[api.LabelIncludedNamespace]; !hasLabel {
+		if ns.Labels == nil {
+			ns.Labels = map[string]string{}
+		}
+		log.Info("Not an excluded namespace; set included-namespace label.")
+		ns.Labels[api.LabelIncludedNamespace] = "true"
+	}
+}
+
+// InjectDecoder injects the decoder.
+func (m *Namespace) InjectDecoder(d *admission.Decoder) error {
+	m.decoder = d
+	return nil
+}

internal/mutators/namespace_test.go

@@ -0,0 +1,49 @@
+package mutators
+
+import (
+	"testing"
+
+	. "github.com/onsi/gomega"
+	corev1 "k8s.io/api/core/v1"
+	"sigs.k8s.io/controller-runtime/pkg/log/zap"
+
+	api "sigs.k8s.io/hierarchical-namespaces/api/v1alpha2"
+	"sigs.k8s.io/hierarchical-namespaces/internal/config"
+)
+
+func TestMutateNamespaceIncludedLabel(t *testing.T) {
+	m := &Namespace{}
+	l := zap.New()
+	config.ExcludedNamespaces = map[string]bool{"excluded": true}
+
+	tests := []struct {
+		name       string
+		nsn        string
+		lval       string
+		expectlval string
+	}{
+		{name: "Set label on non-excluded namespace without label", nsn: "a", expectlval: "true"},
+		{name: "No operation on non-excluded namespace with label (e.g. from a yaml file)", nsn: "a", lval: "true", expectlval: "true"},
+		{name: "No operation on non-excluded namespace with label with wrong value(e.g. from a yaml file)", nsn: "a", lval: "wrong", expectlval: "wrong"},
+		{name: "No operation on excluded namespace without label", nsn: "excluded"},
+		{name: "No operation on excluded namespace with label (e.g. from a yaml file)", nsn: "excluded", lval: "true", expectlval: "true"},
+		{name: "No operation on excluded namespace with label with wrong value (e.g. from a yaml file)", nsn: "excluded", lval: "wrong", expectlval: "wrong"},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			nsInst := &corev1.Namespace{}
+			nsInst.Name = tc.nsn
+			if tc.lval != "" {
+				nsInst.SetLabels(map[string]string{api.LabelIncludedNamespace: tc.lval})
+			}
+
+			// Test
+			m.handle(l, nsInst)
+
+			// Report
+			g.Expect(nsInst.Labels[api.LabelIncludedNamespace]).Should(Equal(tc.expectlval))
+		})
+	}
+}

internal/mutators/setup.go

@@ -0,0 +1,14 @@
+package mutators
+
+import (
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
+)
+
+// Create creates the mutator. This function is called from main.go.
+func Create(mgr ctrl.Manager) {
+	// Create mutator for namespace `included-namespace` label.
+	mgr.GetWebhookServer().Register(NamespaceMutatorServingPath, &webhook.Admission{Handler: &Namespace{
+		Log: ctrl.Log.WithName("mutators").WithName("Namespace"),
+	}})
+}

internal/reconcilers/hierarchy_config.go

@@ -298,7 +298,7 @@ func (r *HierarchyConfigReconciler) syncWithForest(log logr.Logger, nsInst *core
 	r.syncConditions(log, inst, ns, deletingCRD, hadCrit)
 
 	// Sync the tree labels. This should go last since it can depend on the conditions.
-	nsCustomerLabelUpdated := r.syncLabel(log, nsInst, ns)
+	nsCustomerLabelUpdated := r.syncTreeLabels(log, nsInst, ns)
 
 	return initial || nsCustomerLabelUpdated
 }
@@ -461,8 +461,8 @@ func (r *HierarchyConfigReconciler) syncAnchors(log logr.Logger, ns *forest.Name
 	}
 }
 
-// Sync namespace tree labels and other labels. Return true if the labels are updated.
-func (r *HierarchyConfigReconciler) syncLabel(log logr.Logger, nsInst *corev1.Namespace, ns *forest.Namespace) bool {
+// Sync namespace tree labels. Return true if the labels are updated.
+func (r *HierarchyConfigReconciler) syncTreeLabels(log logr.Logger, nsInst *corev1.Namespace, ns *forest.Namespace) bool {
 	if ns.IsExternal() {
 		metadata.SetLabel(nsInst, nsInst.Name+api.LabelTreeDepthSuffix, "0")
 		return false
@@ -504,7 +504,7 @@ func (r *HierarchyConfigReconciler) syncLabel(log logr.Logger, nsInst *corev1.Na
 	// Update the labels in the forest so that we can quickly access the labels and
 	// compare if they match the given selector
 	if ns.SetLabels(nsInst.Labels) {
-		log.Info("Namespace labels have been updated.")
+		log.Info("Namespace tree labels have been updated.")
 		return true
 	}
 	return false

internal/validators/anchor.go

@@ -23,7 +23,7 @@ const (
 // Note: the validating webhook FAILS CLOSE. This means that if the webhook goes down, all further
 // changes are forbidden.
 //
-// +kubebuilder:webhook:admissionReviewVersions=v1;v1beta1,path=/validate-hnc-x-k8s-io-v1alpha2-subnamespaceanchors,mutating=false,failurePolicy=fail,groups="hnc.x-k8s.io",resources=subnamespaceanchors,sideEffects=None,verbs=create;delete,versions=v1alpha2,name=subnamespaceanchors.hnc.x-k8s.io
+// +kubebuilder:webhook:admissionReviewVersions=v1,path=/validate-hnc-x-k8s-io-v1alpha2-subnamespaceanchors,mutating=false,failurePolicy=fail,groups="hnc.x-k8s.io",resources=subnamespaceanchors,sideEffects=None,verbs=create;delete,versions=v1alpha2,name=subnamespaceanchors.hnc.x-k8s.io
 
 type Anchor struct {
 	Log     logr.Logger

internal/validators/hierarchy.go

@@ -34,7 +34,7 @@ const (
 // changes to the hierarchy are forbidden. However, new objects will still be propagated according
 // to the existing hierarchy (unless the reconciler is down too).
 //
-// +kubebuilder:webhook:admissionReviewVersions=v1;v1beta1,path=/validate-hnc-x-k8s-io-v1alpha2-hierarchyconfigurations,mutating=false,failurePolicy=fail,groups="hnc.x-k8s.io",resources=hierarchyconfigurations,sideEffects=None,verbs=create;update,versions=v1alpha2,name=hierarchyconfigurations.hnc.x-k8s.io
+// +kubebuilder:webhook:admissionReviewVersions=v1,path=/validate-hnc-x-k8s-io-v1alpha2-hierarchyconfigurations,mutating=false,failurePolicy=fail,groups="hnc.x-k8s.io",resources=hierarchyconfigurations,sideEffects=None,verbs=create;update,versions=v1alpha2,name=hierarchyconfigurations.hnc.x-k8s.io
 
 type Hierarchy struct {
 	Log     logr.Logger

internal/validators/hncconfig.go

@@ -28,7 +28,7 @@ const (
 // Note: the validating webhook FAILS CLOSE. This means that if the webhook goes down, all further
 // changes are denied.
 //
-// +kubebuilder:webhook:admissionReviewVersions=v1;v1beta1,path=/validate-hnc-x-k8s-io-v1alpha2-hncconfigurations,mutating=false,failurePolicy=fail,groups="hnc.x-k8s.io",resources=hncconfigurations,sideEffects=None,verbs=create;update;delete,versions=v1alpha2,name=hncconfigurations.hnc.x-k8s.io
+// +kubebuilder:webhook:admissionReviewVersions=v1,path=/validate-hnc-x-k8s-io-v1alpha2-hncconfigurations,mutating=false,failurePolicy=fail,groups="hnc.x-k8s.io",resources=hncconfigurations,sideEffects=None,verbs=create;update;delete,versions=v1alpha2,name=hncconfigurations.hnc.x-k8s.io
 
 type HNCConfig struct {
 	Log        logr.Logger