Merge pull request kubernetes-retired#51 from RealHarshThakur/webhook-tree-labels

Deny requests adding tree labels

Author: k8s-ci-robot

internal/namespace/validator.go

@@ -3,6 +3,7 @@ package namespaces
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/go-logr/logr"
 	k8sadm "k8s.io/api/admission/v1"
@@ -86,6 +87,11 @@ func (v *Validator) handle(req *nsRequest) admission.Response {
 		if rsp := v.nameExistsInExternalHierarchy(req); !rsp.Allowed {
 			return rsp
 		}
+
+		if rsp := v.illegalTreeLabel(req); !rsp.Allowed {
+			return rsp
+		}
+
 	case k8sadm.Update:
 		if rsp := v.illegalIncludedNamespaceLabel(req); !rsp.Allowed {
 			return rsp
@@ -96,6 +102,11 @@ func (v *Validator) handle(req *nsRequest) admission.Response {
 		if rsp := v.conflictBetweenParentAndExternalManager(req, ns); !rsp.Allowed {
 			return rsp
 		}
+
+		if rsp := v.illegalTreeLabel(req); !rsp.Allowed {
+			return rsp
+		}
+
 	case k8sadm.Delete:
 		if rsp := v.cannotDeleteSubnamespace(req); !rsp.Allowed {
 			return rsp
@@ -108,6 +119,40 @@ func (v *Validator) handle(req *nsRequest) admission.Response {
 	return webhooks.Allow("")
 }
 
+// illegalTreeLabel checks if tree labels are being created or modified
+// by any user or service account since only HNC service account is
+// allowed to do so
+func (v *Validator) illegalTreeLabel(req *nsRequest) admission.Response {
+	msg := "Cannot set or modify tree label %q in namespace %q; these can only be modified by HNC."
+
+	oldLabels := map[string]string{}
+	if req.oldns != nil {
+		oldLabels = req.oldns.Labels
+	}
+	// Ensure the users hasn't added or changed any tree labels
+	for key, val := range req.ns.Labels {
+		if !strings.Contains(key, api.LabelTreeDepthSuffix) {
+			continue
+		}
+
+		// Check if new HNC label tree key isn't being added
+		if oldLabels[key] != val {
+			return webhooks.Deny(metav1.StatusReasonForbidden, fmt.Sprintf(msg, key, req.ns.Name))
+		}
+	}
+
+	for key := range oldLabels {
+		//  Make sure nothing's been deleted
+		if strings.Contains(key, api.LabelTreeDepthSuffix) {
+			if _, ok := req.ns.Labels[key]; !ok {
+				return webhooks.Deny(metav1.StatusReasonForbidden, fmt.Sprintf(msg, key, req.ns.Name))
+			}
+		}
+	}
+
+	return webhooks.Allow("")
+}
+
 // illegalIncludedNamespaceLabel checks if there's any illegal use of the
 // included-namespace label on namespaces. It only checks a Create or an Update
 // request.

internal/namespace/validator_test.go

@@ -339,3 +339,50 @@ func TestIllegalIncludedNamespaceNamespace(t *testing.T) {
 func logResult(t *testing.T, result *metav1.Status) {
 	t.Logf("Got reason %q, code %d, msg %q", result.Reason, result.Code, result.Message)
 }
+func TestIllegalTreeOperations(t *testing.T) {
+	f := foresttest.Create("-")
+	vns := &Validator{Forest: f}
+
+	ns := &corev1.Namespace{}
+	oldNs := &corev1.Namespace{}
+	ns.Name = "a"
+
+	ns.SetLabels(map[string]string{api.LabelTreeDepthSuffix: "1"})
+	// Change tree label value
+	oldNs.SetLabels(map[string]string{api.LabelTreeDepthSuffix: "2"})
+
+	tests := []struct {
+		name    string
+		op      k8sadm.Operation
+		allowed bool
+		oldNs   *corev1.Namespace
+	}{
+		{name: "Creation of namespace with tree label in it", op: k8sadm.Create, allowed: false},
+		{name: "Update namespace with tree label in it", op: k8sadm.Update, allowed: false},
+		{name: "Update value of a tree in existing namespace", op: k8sadm.Update, allowed: false, oldNs: oldNs},
+		{name: "No tree label updates to namespace", op: k8sadm.Update, allowed: true, oldNs: ns},
+	}
+	for _, tc := range tests {
+		g := NewWithT(t)
+
+		var req *nsRequest
+		if tc.op == k8sadm.Create {
+			req = &nsRequest{
+				ns: ns,
+				op: tc.op,
+			}
+		} else {
+			req = &nsRequest{
+				ns:    ns,
+				op:    tc.op,
+				oldns: tc.oldNs,
+			}
+		}
+
+		got := vns.illegalTreeLabel(req)
+
+		logResult(t, got.AdmissionResponse.Result)
+		g.Expect(got.AdmissionResponse.Allowed).Should(Equal(tc.allowed))
+	}
+
+}