From d62f58558b91a6b0f9799db52fb8cab6f6be21e7 Mon Sep 17 00:00:00 2001 From: Sebastian Widmer Date: Wed, 28 Aug 2024 09:37:31 +0200 Subject: [PATCH] Validate namespace metadata (#117) Replaces https://hub.syn.tools/appuio-cloud/references/policies/02_disallow_reserved_namespaces.html and https://hub.syn.tools/appuio-cloud/references/policies/02_validate_namespace_metadata.html Kyverno policies. --- config.go | 10 ++ config.yaml | 10 ++ config/webhook/manifests.yaml | 42 ++++++ main.go | 19 ++- webhooks/namespace_metadata_validator.go | 129 ++++++++++++++++ webhooks/namespace_metadata_validator_test.go | 139 ++++++++++++++++++ webhooks/service_cloudscale_lb_validator.go | 2 +- 7 files changed, 349 insertions(+), 2 deletions(-) create mode 100644 webhooks/namespace_metadata_validator.go create mode 100644 webhooks/namespace_metadata_validator_test.go diff --git a/config.go b/config.go index 320e02f..f5975f2 100644 --- a/config.go +++ b/config.go @@ -53,6 +53,16 @@ type Config struct { // DefaultOrganizationClusterRoles is a map containing the configuration for rolebindings that are created by default in each organization namespace. // The keys are the name of default rolebindings to create and the values are the names of the clusterroles they bind to. DefaultOrganizationClusterRoles map[string]string + + // ReservedNamespaces is a list of namespaces that are reserved and can't be created by users. + // Supports '*' and '?' wildcards. + ReservedNamespaces []string + // AllowedAnnotations is a list of annotations that are allowed on namespaces. + // Supports '*' and '?' wildcards. + AllowedAnnotations []string + // AllowedLabels is a list of labels that are allowed on namespaces. + // Supports '*' and '?' wildcards. + AllowedLabels []string } func ConfigFromFile(path string) (c Config, warn []string, err error) { diff --git a/config.yaml b/config.yaml index c9a9997..685c936 100644 --- a/config.yaml +++ b/config.yaml @@ -37,3 +37,13 @@ DefaultNodeSelector: # The keys are the name of default rolebindings to create and the values are the names of the clusterroles they bind to. DefaultOrganizationClusterRoles: admin: admin + +# ReservedNamespaces is a list of namespaces that are reserved and can't be created by users. +# Supports '*' and '?' wildcards. +ReservedNamespaces: [default, kube-*, openshift-*] +# AllowedAnnotations is a list of annotations that are allowed on namespaces. +# Supports '*' and '?' wildcards. +AllowedAnnotations: [appuio.io/default-node-selector] +# AllowedLabels is a list of labels that are allowed on namespaces. +# Supports '*' and '?' wildcards. +AllowedLabels: [appuio.io/organization] diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 673d191..a7ae5c1 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -73,6 +73,48 @@ kind: ValidatingWebhookConfiguration metadata: name: validating-webhook-configuration webhooks: + - admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-namespace-metadata + failurePolicy: Fail + matchPolicy: Equivalent + name: validate-namespace-metadata-projectrequests.appuio.io + rules: + - apiGroups: + - project.openshift.io + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + resources: + - projectrequests + sideEffects: None + - admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-namespace-metadata + failurePolicy: Fail + matchPolicy: Equivalent + name: validate-namespace-metadata.appuio.io + rules: + - apiGroups: + - "" + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + resources: + - namespaces + sideEffects: None - admissionReviewVersions: - v1 clientConfig: diff --git a/main.go b/main.go index a592b21..106b9f7 100644 --- a/main.go +++ b/main.go @@ -83,6 +83,9 @@ func main() { var namespaceProjectOrganizationMutatorEnabled bool flag.BoolVar(&namespaceProjectOrganizationMutatorEnabled, "namespace-project-organization-mutator-enabled", false, "Enable the NamespaceProjectOrganizationMutator webhook. Adds the organization label to namespace and project create requests.") + var namespaceMetadataValidatorEnabled bool + flag.BoolVar(&namespaceMetadataValidatorEnabled, "namespace-metadata-validator-enabled", false, "Enable the NamespaceMetadataValidator webhook. Validates the metadata of a namespace.") + var qps, burst int flag.IntVar(&qps, "qps", 20, "QPS to use for the controller-runtime client") flag.IntVar(&burst, "burst", 100, "Burst to use for the controller-runtime client") @@ -264,7 +267,7 @@ func main() { Client: mgr.GetClient(), Skipper: skipper.NewMultiSkipper( - skipper.StaticSkipper{ShouldSkip: false}, + skipper.StaticSkipper{ShouldSkip: !namespaceProjectOrganizationMutatorEnabled}, psk, ), @@ -273,6 +276,20 @@ func main() { }, }) + mgr.GetWebhookServer().Register("/validate-namespace-metadata", &webhook.Admission{ + Handler: &webhooks.NamespaceMetadataValidator{ + Decoder: admission.NewDecoder(mgr.GetScheme()), + Skipper: skipper.NewMultiSkipper( + skipper.StaticSkipper{ShouldSkip: !namespaceMetadataValidatorEnabled}, + psk, + ), + + ReservedNamespaces: conf.ReservedNamespaces, + AllowedAnnotations: conf.AllowedAnnotations, + AllowedLabels: conf.AllowedLabels, + }, + }) + if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { setupLog.Error(err, "unable to setup health endpoint") os.Exit(1) diff --git a/webhooks/namespace_metadata_validator.go b/webhooks/namespace_metadata_validator.go new file mode 100644 index 0000000..1f683c1 --- /dev/null +++ b/webhooks/namespace_metadata_validator.go @@ -0,0 +1,129 @@ +package webhooks + +import ( + "context" + "fmt" + "net/http" + "slices" + + "github.com/minio/pkg/wildcard" + "go.uber.org/multierr" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/util/sets" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/appuio/appuio-cloud-agent/skipper" +) + +// +kubebuilder:webhook:path=/validate-namespace-metadata,name=validate-namespace-metadata.appuio.io,admissionReviewVersions=v1,sideEffects=none,mutating=false,failurePolicy=Fail,groups="",resources=namespaces,verbs=create;update,versions=v1,matchPolicy=equivalent +// +kubebuilder:webhook:path=/validate-namespace-metadata,name=validate-namespace-metadata-projectrequests.appuio.io,admissionReviewVersions=v1,sideEffects=none,mutating=false,failurePolicy=Fail,groups=project.openshift.io,resources=projectrequests,verbs=create;update,versions=v1,matchPolicy=equivalent + +// NamespaceMetadataValidator validates the metadata of a namespace. +type NamespaceMetadataValidator struct { + Decoder admission.Decoder + + Skipper skipper.Skipper + + // ReservedNamespace is a list of namespaces that are reserved and do not count towards the quota. + // Supports '*' and '?' wildcards. + ReservedNamespaces []string + // AllowedAnnotations is a list of annotations that are allowed on the namespace. + // Supports '*' and '?' wildcards. + AllowedAnnotations []string + // AllowedLabels is a list of labels that are allowed on the namespace. + // Supports '*' and '?' wildcards. + AllowedLabels []string +} + +// Handle handles the admission requests +func (v *NamespaceMetadataValidator) Handle(ctx context.Context, req admission.Request) admission.Response { + ctx = log.IntoContext(ctx, log.FromContext(ctx). + WithName("webhook.validate-namespace-metadata.appuio.io"). + WithValues("id", req.UID, "user", req.UserInfo.Username). + WithValues("namespace", req.Namespace, "name", req.Name, + "group", req.Kind.Group, "version", req.Kind.Version, "kind", req.Kind.Kind)) + + return logAdmissionResponse(ctx, v.handle(ctx, req)) +} + +func (v *NamespaceMetadataValidator) handle(ctx context.Context, req admission.Request) admission.Response { + skip, err := v.Skipper.Skip(ctx, req) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + if skip { + return admission.Allowed("skipped") + } + + for _, ns := range v.ReservedNamespaces { + if wildcard.Match(ns, req.Name) { + return admission.Denied("Changing or creating reserved namespaces is not allowed.") + } + } + + var oldObj unstructured.Unstructured + if len(req.OldObject.Raw) > 0 { + if err := v.Decoder.DecodeRaw(req.OldObject, &oldObj); err != nil { + return admission.Errored(http.StatusBadRequest, fmt.Errorf("failed to decode old object: %w", err)) + } + } + + var newObj unstructured.Unstructured + if err := v.Decoder.Decode(req, &newObj); err != nil { + return admission.Errored(http.StatusBadRequest, fmt.Errorf("failed to decode object from request: %w", err)) + } + + if err := validateChangedMap(oldObj.GetAnnotations(), newObj.GetAnnotations(), v.AllowedAnnotations, "annotation"); err != nil { + return admission.Denied(formatDeniedMessage(err, "annotations", v.AllowedAnnotations, newObj.GetAnnotations(), oldObj.GetAnnotations())) + } + if err := validateChangedMap(oldObj.GetLabels(), newObj.GetLabels(), v.AllowedLabels, "label"); err != nil { + return admission.Denied(formatDeniedMessage(err, "labels", v.AllowedLabels, newObj.GetLabels(), oldObj.GetLabels())) + } + + return admission.Allowed("allowed") +} + +func formatDeniedMessage(err error, errMapRef string, allowed []string, newMap, oldMap map[string]string) string { + msg := `The request was denied: + %v +The following %s can be modified: + %s +%s given: + %s +%s before modification: + %s +` + + return fmt.Sprintf(msg, err, errMapRef, allowed, errMapRef, newMap, errMapRef, oldMap) +} + +func validateChangedMap(old, new map[string]string, allowedKeys []string, errObjectRef string) error { + changed := changedKeys(old, new) + errs := make([]error, 0, len(changed)) + for _, k := range changed { + allowed := slices.ContainsFunc(allowedKeys, func(a string) bool { return wildcard.Match(a, k) }) + if !allowed { + errs = append(errs, fmt.Errorf("%s %q is not allowed to be changed", errObjectRef, k)) + } + } + + return multierr.Combine(errs...) +} + +func changedKeys(a, b map[string]string) []string { + changed := sets.New[string]() + + for k, v := range a { + if bV, ok := b[k]; !ok || v != bV { + changed.Insert(k) + } + } + for k, v := range b { + if aV, ok := a[k]; !ok || v != aV { + changed.Insert(k) + } + } + + return sets.List(changed) +} diff --git a/webhooks/namespace_metadata_validator_test.go b/webhooks/namespace_metadata_validator_test.go new file mode 100644 index 0000000..5d77ead --- /dev/null +++ b/webhooks/namespace_metadata_validator_test.go @@ -0,0 +1,139 @@ +package webhooks + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/appuio/appuio-cloud-agent/skipper" +) + +func Test_NamespaceMetadataValidator_Handle(t *testing.T) { + + testCases := []struct { + name string + + reservedNamespaces []string + allowedAnnotations []string + allowedLabels []string + + object client.Object + oldObj client.Object + + allowed bool + }{ + { + name: "new namespace with allowed name", + object: newNamespace("test-namespace", nil, nil), + + reservedNamespaces: []string{"appuio*"}, + + allowed: true, + }, + { + name: "new project with allowed name", + object: newProjectRequest("test-project", nil, nil), + + reservedNamespaces: []string{"appuio*"}, + + allowed: true, + }, + { + name: "new namespace with reserved name", + object: newNamespace("appuio-blub", nil, nil), + + reservedNamespaces: []string{"test", "appuio*"}, + + allowed: false, + }, + { + name: "new project with reserved name", + object: newProjectRequest("appuio-blub", nil, nil), + + reservedNamespaces: []string{"test", "appuio*"}, + + allowed: false, + }, + { + name: "new namespace with allowed annotation", + object: newNamespace("test-namespace", nil, map[string]string{"allowed": ""}), + + allowedAnnotations: []string{"allowed"}, + allowed: true, + }, + { + name: "new namespace with disallowed annotation", + object: newNamespace("test-namespace", nil, map[string]string{"disallowed": ""}), + + allowedAnnotations: []string{"allowed"}, + allowed: false, + }, + { + name: "new namespace with allowed label", + object: newNamespace("test-namespace", map[string]string{"allowed-kajshd": "", "custom/x": "asd"}, nil), + + allowedLabels: []string{"allowed*", "custom/*"}, + allowed: true, + }, + { + name: "new namespace with disallowed label", + object: newNamespace("test-namespace", map[string]string{"disallowed": ""}, nil), + + allowedLabels: []string{"allowed"}, + allowed: false, + }, + { + name: "update namespace with allowed annotation", + object: newNamespace("test-namespace", nil, map[string]string{"pre-existing": "s", "allowed": ""}), + oldObj: newNamespace("test-namespace", nil, map[string]string{"pre-existing": "s"}), + + allowedAnnotations: []string{"allowed"}, + allowed: true, + }, + { + name: "update namespace with disallowed annotation", + object: newNamespace("test-namespace", nil, map[string]string{"pre-existing": "s", "disallowed": "a"}), + oldObj: newNamespace("test-namespace", nil, map[string]string{"pre-existing": "s", "disallowed": "b"}), + allowedAnnotations: []string{"allowed"}, + allowed: false, + }, + { + name: "remove disallowed annotation", + object: newNamespace("test-namespace", nil, map[string]string{"pre-existing": "s"}), + oldObj: newNamespace("test-namespace", nil, map[string]string{"pre-existing": "s", "disallowed": "b", "disallowed2": "", "allowed": ""}), + allowedAnnotations: []string{"allowed"}, + allowed: false, + }, + { + name: "remove disallowed annotation", + object: newNamespace("test-namespace", nil, map[string]string{"pre-existing": "s"}), + oldObj: newNamespace("test-namespace", nil, map[string]string{"pre-existing": "s", "disallowed": ""}), + allowedAnnotations: []string{"allowed"}, + allowed: false, + }, + } + + _, scheme, decoder := prepareClient(t) + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + subject := &NamespaceMetadataValidator{ + Decoder: decoder, + Skipper: skipper.StaticSkipper{}, + ReservedNamespaces: tc.reservedNamespaces, + AllowedAnnotations: tc.allowedAnnotations, + AllowedLabels: tc.allowedLabels, + } + + amr := admissionRequestForObjectWithOldObject(t, tc.object, tc.oldObj, scheme) + + resp := subject.Handle(context.Background(), amr) + t.Log("Response:", resp.Result.Reason, resp.Result.Message) + require.Equal(t, tc.allowed, resp.Allowed) + }) + } +} diff --git a/webhooks/service_cloudscale_lb_validator.go b/webhooks/service_cloudscale_lb_validator.go index 6c10382..46b396e 100644 --- a/webhooks/service_cloudscale_lb_validator.go +++ b/webhooks/service_cloudscale_lb_validator.go @@ -55,7 +55,7 @@ func (v *ServiceCloudscaleLBValidator) handle(ctx context.Context, req admission } var oldService corev1.Service - if req.OldObject.Raw != nil { + if len(req.OldObject.Raw) > 0 { if err := v.Decoder.DecodeRaw(req.OldObject, &oldService); err != nil { l.Error(err, "failed to decode old object") return admission.Errored(http.StatusBadRequest, err)