Add validation of managed labels/annotations

Part of kubernetes-retired#47. This adds validation of managed labels and annotations.

Tested: Added unit tests for validations. Ran e2e-tests, but no additions/modifications to those in this PR.

Author: erikgb

internal/hierarchyconfig/validator.go

@@ -12,9 +12,12 @@ import (
 	authzv1 "k8s.io/api/authorization/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/validation"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
@@ -129,6 +132,13 @@ func (v *Validator) handle(ctx context.Context, log logr.Logger, req *request) a
 		return webhooks.Deny(metav1.StatusReasonForbidden, reason)
 	}
 
+	allErrs := validateManagedMeta(req.hc)
+	if len(allErrs) > 0 {
+		gk := schema.GroupKind{Group: api.GroupVersion.Group, Kind: "HierarchyConfiguration"}
+		err := apierrors.NewInvalid(gk, req.hc.Name, allErrs)
+		return webhooks.DenyFromAPIError(err)
+	}
+
 	// Do all checks that require holding the in-memory lock. Generate a list of server checks we
 	// should perform once the lock is released.
 	serverChecks, resp := v.checkForest(log, req.hc)
@@ -514,6 +524,49 @@ func (r *realClient) IsAdmin(ctx context.Context, ui *authnv1.UserInfo, nnm stri
 	return sar.Status.Allowed, err
 }
 
+func validateManagedMeta(hc *api.HierarchyConfiguration) field.ErrorList {
+	allErrs := field.ErrorList{}
+
+	fldPath := field.NewPath("spec", "labels")
+	for _, l := range hc.Spec.Labels {
+		if fldErr := validateMetaKey(l.Key, fldPath); fldErr != nil {
+			allErrs = append(allErrs, fldErr)
+		} else if !config.IsManagedLabel(l.Key) { // Only validate managed if key is valid
+			fldErr := field.Invalid(fldPath, l.Key, "not a managed label and cannot be configured")
+			allErrs = append(allErrs, fldErr)
+		}
+		if fldErr := validateLabelValue(l.Key, l.Value, fldPath); fldErr != nil {
+			allErrs = append(allErrs, fldErr)
+		}
+	}
+
+	fldPath = field.NewPath("spec", "annotations")
+	for _, a := range hc.Spec.Annotations {
+		if fldErr := validateMetaKey(a.Key, fldPath); fldErr != nil {
+			allErrs = append(allErrs, fldErr)
+		} else if !config.IsManagedAnnotation(a.Key) { // Only validate managed if key is valid
+			fldErr := field.Invalid(fldPath, a.Key, "not a managed annotation and cannot be configured")
+			allErrs = append(allErrs, fldErr)
+		}
+		// No validation of annotation values; only limitation seems to be the total length (256Kb) of all annotations
+	}
+	return allErrs
+}
+
+func validateMetaKey(k string, path *field.Path) *field.Error {
+	if errs := validation.IsQualifiedName(k); len(errs) != 0 {
+		return field.Invalid(path, k, strings.Join(errs, "; "))
+	}
+	return nil
+}
+
+func validateLabelValue(k, v string, path *field.Path) *field.Error {
+	if errs := validation.IsValidLabelValue(v); len(errs) != 0 {
+		return field.Invalid(path.Key(k), v, strings.Join(errs, "; "))
+	}
+	return nil
+}
+
 // allow is a replacement for controller-runtime's admission.Allowed() that allows you to set the
 // message (human-readable) as opposed to the reason (machine-readable).
 func allow(msg string) admission.Response {

internal/hierarchyconfig/validator_test.go

@@ -17,6 +17,69 @@ import (
 	"sigs.k8s.io/hierarchical-namespaces/internal/objects"
 )
 
+func TestManagedMeta(t *testing.T) {
+	f := foresttest.Create("-") // a
+	h := &Validator{Forest: f}
+	l := zap.New()
+	// For this test we accept any label or annotation not starting with 'h',
+	// to allow almost any meta - except the hnc.x-k8s.io labels/annotations,
+	// which cannot be managed anyway. And allows us to use that for testing.
+	if err := config.SetManagedMeta([]string{"[^h].*"}, []string{"[^h].*"}); err != nil {
+		t.Fatal(err)
+	}
+	defer config.SetManagedMeta(nil, nil)
+
+	tests := []struct {
+		name        string
+		labels      []api.MetaKVP
+		annotations []api.MetaKVP
+		allowed     bool
+	}{
+		{name: "ok: managed label", labels: []api.MetaKVP{{Key: "label.com/team"}}, allowed: true},
+		{name: "invalid: unmanaged label", labels: []api.MetaKVP{{Key: api.LabelIncludedNamespace}}},
+		{name: "ok: managed annotation", annotations: []api.MetaKVP{{Key: "annot.com/log-index"}}, allowed: true},
+		{name: "invalid: unmanaged annotation", annotations: []api.MetaKVP{{Key: api.AnnotationManagedBy}}},
+
+		{name: "ok: prefixed label key", labels: []api.MetaKVP{{Key: "foo.bar/team", Value: "v"}}, allowed: true},
+		{name: "ok: bare label key", labels: []api.MetaKVP{{Key: "team", Value: "v"}}, allowed: true},
+		{name: "invalid: label prefix key", labels: []api.MetaKVP{{Key: "foo;bar/team", Value: "v"}}},
+		{name: "invalid: label name key", labels: []api.MetaKVP{{Key: "foo.bar/-team", Value: "v"}}},
+		{name: "invalid: empty label key", labels: []api.MetaKVP{{Key: "", Value: "v"}}},
+
+		{name: "ok: label value", labels: []api.MetaKVP{{Key: "k", Value: "foo"}}, allowed: true},
+		{name: "ok: empty label value", labels: []api.MetaKVP{{Key: "k", Value: ""}}, allowed: true},
+		{name: "ok: label value special char", labels: []api.MetaKVP{{Key: "k", Value: "f-oo"}}, allowed: true},
+		{name: "invalid: label value", labels: []api.MetaKVP{{Key: "k", Value: "-foo"}}},
+
+		{name: "ok: prefixed annotation key", annotations: []api.MetaKVP{{Key: "foo.bar/team", Value: "v"}}, allowed: true},
+		{name: "ok: bare annotation key", annotations: []api.MetaKVP{{Key: "team", Value: "v"}}, allowed: true},
+		{name: "invalid: annotation prefix key", annotations: []api.MetaKVP{{Key: "foo;bar/team", Value: "v"}}},
+		{name: "invalid: annotation name key", annotations: []api.MetaKVP{{Key: "foo.bar/-team", Value: "v"}}},
+		{name: "invalid: empty annotation key", annotations: []api.MetaKVP{{Key: "", Value: "v"}}},
+
+		{name: "ok: annotation value", annotations: []api.MetaKVP{{Key: "k", Value: "foo"}}, allowed: true},
+		{name: "ok: empty annotation value", annotations: []api.MetaKVP{{Key: "k", Value: ""}}, allowed: true},
+		{name: "ok: special annotation value", annotations: []api.MetaKVP{{Key: "k", Value: ";$+:;/*'\""}}, allowed: true},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			// Setup
+			g := NewWithT(t)
+			hc := &api.HierarchyConfiguration{Spec: api.HierarchyConfigurationSpec{}}
+			hc.ObjectMeta.Name = api.Singleton
+			hc.ObjectMeta.Namespace = "a"
+			hc.Spec.Labels = tc.labels
+			hc.Spec.Annotations = tc.annotations
+			req := &request{hc: hc}
+
+			got := h.handle(context.Background(), l, req)
+
+			logResult(t, got.AdmissionResponse.Result)
+			g.Expect(got.AdmissionResponse.Allowed).Should(Equal(tc.allowed))
+		})
+	}
+}
+
 func TestStructure(t *testing.T) {
 	f := foresttest.Create("-a-") // a <- b; c
 	h := &Validator{Forest: f}