From 8503d2ccbbe2003e486d4e068d57bb0c1ddea71a Mon Sep 17 00:00:00 2001 From: Sebastian Widmer Date: Tue, 27 Aug 2024 19:18:35 +0200 Subject: [PATCH] Replace Kyverno namespace/project policies (#116) --- config/webhook/manifests.yaml | 42 ++ main.go | 18 + .../namespace_project_organization_mutator.go | 175 ++++++++ ...space_project_organization_mutator_test.go | 424 ++++++++++++++++++ webhooks/pod_node_selector_mutator.go | 8 +- webhooks/utils_test.go | 14 + 6 files changed, 679 insertions(+), 2 deletions(-) create mode 100644 webhooks/namespace_project_organization_mutator.go create mode 100644 webhooks/namespace_project_organization_mutator_test.go diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 4a03cbb..673d191 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -25,6 +25,48 @@ webhooks: resources: - pods sideEffects: None + - admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /mutate-namespace-project-organization + failurePolicy: Fail + matchPolicy: Equivalent + name: namespace.namespace-project-organization-mutator.appuio.io + rules: + - apiGroups: + - "" + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + resources: + - namespaces + sideEffects: None + - admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /mutate-namespace-project-organization + failurePolicy: Fail + matchPolicy: Equivalent + name: project.namespace-project-organization-mutator.appuio.io + rules: + - apiGroups: + - project.openshift.io + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + resources: + - projects + sideEffects: None --- apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration diff --git a/main.go b/main.go index c48a4f8..a592b21 100644 --- a/main.go +++ b/main.go @@ -80,6 +80,9 @@ func main() { var cloudscaleLoadbalancerValidationEnabled bool flag.BoolVar(&cloudscaleLoadbalancerValidationEnabled, "cloudscale-loadbalancer-validation-enabled", false, "Enable Cloudscale Loadbalancer validation. Validates that the k8s.cloudscale.ch/loadbalancer-uuid annotation cannot be changed by unprivileged users.") + 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 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") @@ -255,6 +258,21 @@ func main() { }, }) + mgr.GetWebhookServer().Register("/mutate-namespace-project-organization", &webhook.Admission{ + Handler: &webhooks.NamespaceProjectOrganizationMutator{ + Decoder: admission.NewDecoder(mgr.GetScheme()), + Client: mgr.GetClient(), + + Skipper: skipper.NewMultiSkipper( + skipper.StaticSkipper{ShouldSkip: false}, + psk, + ), + + OrganizationLabel: conf.OrganizationLabel, + UserDefaultOrganizationAnnotation: conf.UserDefaultOrganizationAnnotation, + }, + }) + 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_project_organization_mutator.go b/webhooks/namespace_project_organization_mutator.go new file mode 100644 index 0000000..e85c534 --- /dev/null +++ b/webhooks/namespace_project_organization_mutator.go @@ -0,0 +1,175 @@ +package webhooks + +import ( + "context" + "fmt" + "net/http" + "slices" + "strings" + + "github.com/appuio/appuio-cloud-agent/skipper" + userv1 "github.com/openshift/api/user/v1" + "gomodules.xyz/jsonpatch/v2" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +//+kubebuilder:webhook:path=/mutate-namespace-project-organization,name=namespace.namespace-project-organization-mutator.appuio.io,admissionReviewVersions=v1,sideEffects=none,mutating=true,failurePolicy=Fail,groups="",resources=namespaces,verbs=create;update,versions=v1,matchPolicy=equivalent +//+kubebuilder:webhook:path=/mutate-namespace-project-organization,name=project.namespace-project-organization-mutator.appuio.io,admissionReviewVersions=v1,sideEffects=none,mutating=true,failurePolicy=Fail,groups=project.openshift.io,resources=projects,verbs=create;update,versions=v1,matchPolicy=equivalent + +// +kubebuilder:rbac:groups=user.openshift.io,resources=users;groups,verbs=get;list;watch +// +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch + +// NamespaceProjectOrganizationMutator adds the OrganizationLabel to namespace and project create requests. +type NamespaceProjectOrganizationMutator struct { + Decoder admission.Decoder + + Client client.Reader + + Skipper skipper.Skipper + + // OrganizationLabel is the label used to mark namespaces to belong to an organization + OrganizationLabel string + + // UserDefaultOrganizationAnnotation is the annotation the default organization setting for a user is stored in. + UserDefaultOrganizationAnnotation string +} + +const OpenShiftProjectRequesterAnnotation = "openshift.io/requester" + +// Handle handles the admission requests +// +// If the requestor is a service account: +// - Project requests are denied. +// - Namespace requests are checked against the organization of the service account's namespace. +// - If the organization is not set in the request, the organization of the service account's namespace is added. +// - If the service account's namespace has no organization set, the request is denied. +// +// If the requestor is an OpenShift user: +// - If there is no OrganizationLabel set on the object, the default organization of the user is used; if there is no default organization set for the user, the request is denied. +// - Namespace requests use the username of the requests user info. +// - Project requests use the annotation `openshift.io/requester` on the project object. If the annotation is not set, the request is allowed. +// - If the user is not a member of the organization, the request is denied; this is done by checking for an OpenShift group with the same name as the organization. +func (m *NamespaceProjectOrganizationMutator) Handle(ctx context.Context, req admission.Request) admission.Response { + ctx = log.IntoContext(ctx, log.FromContext(ctx). + WithName("webhook.namespace-project-organization-mutator.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, m.handle(ctx, req)) +} + +func (m *NamespaceProjectOrganizationMutator) handle(ctx context.Context, req admission.Request) admission.Response { + skip, err := m.Skipper.Skip(ctx, req) + if err != nil { + return admission.Errored(http.StatusInternalServerError, fmt.Errorf("error while checking skipper: %w", err)) + } + if skip { + return admission.Allowed("skipped") + } + + var rawObject unstructured.Unstructured + if err := m.Decoder.Decode(req, &rawObject); err != nil { + return admission.Errored(http.StatusBadRequest, fmt.Errorf("failed to decode object from request: %w", err)) + } + + if req.Kind.Kind == "Project" { + return m.handleProject(ctx, rawObject) + } + + return m.handleNamespace(ctx, req, rawObject) +} + +func (m *NamespaceProjectOrganizationMutator) handleProject(ctx context.Context, rawObject unstructured.Unstructured) admission.Response { + userName := rawObject.GetAnnotations()[OpenShiftProjectRequesterAnnotation] + if userName == "" { + // https://github.com/appuio/component-appuio-cloud/blob/196f76ede357a73b88f9314bf1d1bccc097cb6b7/component/namespace-policies.jsonnet#L54 + return admission.Allowed("Skipped: no requester annotation found") + } + if strings.HasPrefix(userName, "system:serviceaccount:") { + return admission.Denied("Service accounts are not allowed to create projects") + } + + return m.handleUserRequested(ctx, userName, rawObject) +} + +func (m *NamespaceProjectOrganizationMutator) handleNamespace(ctx context.Context, req admission.Request, rawObject unstructured.Unstructured) admission.Response { + if strings.HasPrefix(req.UserInfo.Username, "system:serviceaccount:") { + return m.handleServiceAccountNamespace(ctx, req, rawObject) + } + + return m.handleUserRequested(ctx, req.UserInfo.Username, rawObject) +} + +func (m *NamespaceProjectOrganizationMutator) handleUserRequested(ctx context.Context, userName string, rawObject unstructured.Unstructured) admission.Response { + var user userv1.User + if err := m.Client.Get(ctx, client.ObjectKey{Name: userName}, &user); err != nil { + return admission.Errored(http.StatusInternalServerError, fmt.Errorf("failed to get user %s: %w", userName, err)) + } + + org := rawObject.GetLabels()[m.OrganizationLabel] + defaultOrgAdded := false + if org == "" { + org = user.Annotations[m.UserDefaultOrganizationAnnotation] + defaultOrgAdded = true + } + if org == "" { + return admission.Denied("No organization label found and no default organization set") + } + + var group userv1.Group + if err := m.Client.Get(ctx, types.NamespacedName{Name: org}, &group); client.IgnoreNotFound(err) != nil { + return admission.Errored(http.StatusInternalServerError, fmt.Errorf("failed to get group: %w", err)) + } + + if !slices.Contains(group.Users, userName) { + return admission.Denied("Requester is not a member of the organization") + } + + if defaultOrgAdded { + return admission.Patched("added default organization", m.orgLabelPatch(org)) + } + + return admission.Allowed("Requester is member of organization") +} + +func (m *NamespaceProjectOrganizationMutator) handleServiceAccountNamespace(ctx context.Context, req admission.Request, rawObject unstructured.Unstructured) admission.Response { + p := strings.Split(req.UserInfo.Username, ":") + if len(p) != 4 { + return admission.Errored(http.StatusUnprocessableEntity, fmt.Errorf("invalid service account name: %s, expected 4 segments", req.UserInfo.Username)) + } + nsName := p[2] + var ns corev1.Namespace + if err := m.Client.Get(ctx, client.ObjectKey{Name: nsName}, &ns); err != nil { + return admission.Errored(http.StatusInternalServerError, fmt.Errorf("failed to get namespace %s for service account %s: %w", nsName, req.UserInfo.Username, err)) + } + nsOrg := ns.Labels[m.OrganizationLabel] + if nsOrg == "" { + return admission.Denied("No organization label found for the service accounts namespace") + } + + requestedOrg := rawObject.GetLabels()[m.OrganizationLabel] + if requestedOrg != "" && requestedOrg != nsOrg { + return admission.Denied("Service accounts are not allowed to use organizations other than the one of their namespace.") + } + + if requestedOrg == "" { + return admission.Patched("added organization label", m.orgLabelPatch(nsOrg)) + } + + return admission.Allowed("service account may use the organization of its namespace") +} + +// orgLabelPatch returns a JSON patch operation to add the `OrganizationLabel` with value `org` to an object. +func (m *NamespaceProjectOrganizationMutator) orgLabelPatch(org string) jsonpatch.Operation { + return jsonpatch.Operation{ + Operation: "add", + Path: "/" + strings.Join([]string{"metadata", "labels", escapeJSONPointerSegment(m.OrganizationLabel)}, "/"), + Value: org, + } +} diff --git a/webhooks/namespace_project_organization_mutator_test.go b/webhooks/namespace_project_organization_mutator_test.go new file mode 100644 index 0000000..15b0672 --- /dev/null +++ b/webhooks/namespace_project_organization_mutator_test.go @@ -0,0 +1,424 @@ +package webhooks + +import ( + "context" + "fmt" + "testing" + + projectv1 "github.com/openshift/api/project/v1" + userv1 "github.com/openshift/api/user/v1" + "github.com/stretchr/testify/require" + "gomodules.xyz/jsonpatch/v2" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/appuio/appuio-cloud-agent/skipper" +) + +const testDefaultOrgAnnotation = "example.com/default-organization" + +func Test_NamespaceProjectOrganizationMutator_Handle(t *testing.T) { + const orgLabel = "example.com/organization" + + testCases := []struct { + name string + + object client.Object + + additionalObjects func(t *testing.T) []client.Object + + user string + + allowed bool + orgPatch string + }{ + { + name: "Project: request with org label set", + + object: newProjectRequest("project", map[string]string{ + orgLabel: "some-org", + }, nil), + additionalObjects: func(*testing.T) []client.Object { + return []client.Object{ + newUser("user", "blub"), + newGroup("some-org", "user"), + } + }, + + user: "user", + allowed: true, + }, + { + name: "Namespace: request with org label set", + + object: newNamespace("project", map[string]string{ + orgLabel: "some-org", + }, nil), + additionalObjects: func(*testing.T) []client.Object { + return []client.Object{ + newUser("user", "blub"), + newGroup("some-org", "user"), + } + }, + + user: "user", + allowed: true, + }, + + { + name: "Project: user with default org, no org set on object", + + object: newProjectRequest("project", nil, nil), + additionalObjects: func(*testing.T) []client.Object { + return []client.Object{ + newUser("user", "some-org"), + newGroup("some-org", "user"), + } + }, + + user: "user", + allowed: true, + orgPatch: "some-org", + }, + { + name: "Namespace: user with default org, no org set on object", + + object: newNamespace("project", nil, nil), + additionalObjects: func(*testing.T) []client.Object { + return []client.Object{ + newUser("user", "some-org"), + newGroup("some-org", "user"), + } + }, + + user: "user", + allowed: true, + orgPatch: "some-org", + }, + + { + name: "Project: request with org label set, user not in org", + + object: newProjectRequest("project", map[string]string{orgLabel: "other-org"}, nil), + additionalObjects: func(*testing.T) []client.Object { + return []client.Object{ + newUser("user", ""), + newGroup("other-org"), + } + }, + + user: "user", + allowed: false, + }, + { + name: "Namespace: request with org label set, user not in org", + + object: newNamespace("project", map[string]string{orgLabel: "other-org"}, nil), + additionalObjects: func(*testing.T) []client.Object { + return []client.Object{ + newUser("user", ""), + newGroup("other-org"), + } + }, + + user: "user", + allowed: false, + }, + + { + name: "Project: default org, user not in org", + + object: newProjectRequest("project", nil, nil), + additionalObjects: func(*testing.T) []client.Object { + return []client.Object{ + newUser("user", "user-default-org"), + newGroup("user-default-org"), + } + }, + + user: "user", + allowed: false, + }, + { + name: "Namespace: default org, user not in org", + + object: newNamespace("project", nil, nil), + additionalObjects: func(*testing.T) []client.Object { + return []client.Object{ + newUser("user", "user-default-org"), + newGroup("user-default-org"), + } + }, + + user: "user", + allowed: false, + }, + + { + name: "Project: default org, user not in org", + + object: newProjectRequest("project", nil, nil), + additionalObjects: func(*testing.T) []client.Object { + return []client.Object{ + newUser("user", "user-default-org"), + newGroup("user-default-org"), + } + }, + + user: "user", + allowed: false, + }, + { + name: "Namespace: default org, user not in org", + + object: newNamespace("project", nil, nil), + additionalObjects: func(*testing.T) []client.Object { + return []client.Object{ + newUser("user", "user-default-org"), + newGroup("user-default-org"), + } + }, + + user: "user", + allowed: false, + }, + + { + name: "Project: request non-existing org", + + object: newProjectRequest("project", map[string]string{orgLabel: "non-existing"}, nil), + additionalObjects: func(*testing.T) []client.Object { + return []client.Object{ + newUser("user", ""), + } + }, + + user: "user", + allowed: false, + }, + { + name: "Namespace: request non-existing org", + + object: newNamespace("project", map[string]string{orgLabel: "non-existing"}, nil), + additionalObjects: func(*testing.T) []client.Object { + return []client.Object{ + newUser("user", ""), + } + }, + + user: "user", + allowed: false, + }, + + { + name: "Project: no org set, no default org", + + object: newProjectRequest("project", nil, nil), + additionalObjects: func(*testing.T) []client.Object { + return []client.Object{ + newUser("user", ""), + } + }, + + user: "user", + allowed: false, + }, + { + name: "Namespace: no org set, no default org", + + object: newNamespace("project", nil, nil), + additionalObjects: func(*testing.T) []client.Object { + return []client.Object{ + newUser("user", ""), + } + }, + + user: "user", + allowed: false, + }, + + { + name: "Project: request from service account", + + object: newProjectRequest("new-project", map[string]string{orgLabel: "some-org"}, nil), + additionalObjects: func(*testing.T) []client.Object { + return []client.Object{ + newNamespace("project", map[string]string{ + orgLabel: "some-org", + }, nil), + newServiceAccount("ns-creator", "project"), + } + }, + + user: "system:serviceaccount:project:ns-creator", + allowed: false, + }, + { + name: "Namespace: request from service account", + + object: newNamespace("new-project", map[string]string{orgLabel: "some-org"}, nil), + additionalObjects: func(*testing.T) []client.Object { + return []client.Object{ + newNamespace("project", map[string]string{ + orgLabel: "some-org", + }, nil), + newServiceAccount("ns-creator", "project"), + } + }, + + user: "system:serviceaccount:project:ns-creator", + allowed: true, + }, + { + name: "Namespace: request from service account, no label on object", + + object: newNamespace("new-project", nil, nil), + additionalObjects: func(*testing.T) []client.Object { + return []client.Object{ + newNamespace("project", map[string]string{ + orgLabel: "some-org", + }, nil), + newServiceAccount("ns-creator", "project"), + } + }, + + user: "system:serviceaccount:project:ns-creator", + allowed: true, + orgPatch: "some-org", + }, + { + name: "Namespace: request from service account for other organization", + + object: newNamespace("new-project", map[string]string{ + orgLabel: "other-org", + }, nil), + additionalObjects: func(*testing.T) []client.Object { + return []client.Object{ + newNamespace("project", map[string]string{ + orgLabel: "some-org", + }, nil), + newServiceAccount("ns-creator", "project"), + } + }, + + user: "system:serviceaccount:project:ns-creator", + allowed: false, + }, + { + name: "Namespace: request from service account in namespace without org label and no org label set on object", + + object: newNamespace("new-project", nil, nil), + additionalObjects: func(*testing.T) []client.Object { + return []client.Object{ + newNamespace("project", nil, nil), + newServiceAccount("ns-creator", "project"), + } + }, + + user: "system:serviceaccount:project:ns-creator", + allowed: false, + }, + { + name: "Namespace: request from service account in namespace without org label and org label set on object", + + object: newNamespace("new-project", map[string]string{orgLabel: "some-org"}, nil), + additionalObjects: func(*testing.T) []client.Object { + return []client.Object{ + newNamespace("project", nil, nil), + newServiceAccount("ns-creator", "project"), + } + }, + + user: "system:serviceaccount:project:ns-creator", + allowed: false, + }, + + { + name: "Project: no openshift.io/requester annotation", + + object: newProjectRequest("new-project", map[string]string{orgLabel: "some-org"}, nil), + + user: "", + allowed: true, + }, + } + + for _, tc := range testCases { + t.Run(fmt.Sprintf("%s (Expect allowed: %v)", tc.name, tc.allowed), func(t *testing.T) { + t.Parallel() + + var initObjs []client.Object + if tc.additionalObjects != nil { + initObjs = tc.additionalObjects(t) + } + + c, scheme, decoder := prepareClient(t, initObjs...) + + subject := NamespaceProjectOrganizationMutator{ + Decoder: decoder, + Client: c, + Skipper: skipper.StaticSkipper{}, + + OrganizationLabel: orgLabel, + UserDefaultOrganizationAnnotation: testDefaultOrgAnnotation, + } + + // OpenShift project requests are a special case, we can't trust the user info in the request but OpenShift adds the original requester as an annotation + if p, ok := tc.object.(*projectv1.ProjectRequest); ok { + if p.Annotations == nil { + p.Annotations = make(map[string]string) + } + p.Annotations[OpenShiftProjectRequesterAnnotation] = tc.user + } + amr := admissionRequestForObject(t, tc.object, scheme) + amr.UserInfo.Username = tc.user + amr.UserInfo.Groups = []string{} + resp := subject.Handle(context.Background(), amr) + t.Log("Response:", resp.Result.Reason, resp.Result.Message) + require.Equal(t, tc.allowed, resp.Allowed) + + if tc.orgPatch != "" { + requireOrgPatch(t, tc.orgPatch, resp.Patches) + } else { + require.Empty(t, resp.Patches) + } + }) + } +} + +func requireOrgPatch(t *testing.T, org string, ps []jsonpatch.Operation) { + require.Len(t, ps, 1) + require.Equal(t, jsonpatch.NewOperation("add", "/metadata/labels/example.com~1organization", org), ps[0]) +} + +func newGroup(name string, users ...string) *userv1.Group { + return &userv1.Group{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Users: users, + } +} + +func newUser(name string, defaultOrg string) *userv1.User { + u := &userv1.User{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Annotations: map[string]string{}, + }, + } + if defaultOrg != "" { + u.Annotations[testDefaultOrgAnnotation] = defaultOrg + } + return u +} + +func newServiceAccount(name, namespace string) *corev1.ServiceAccount { + return &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + } +} diff --git a/webhooks/pod_node_selector_mutator.go b/webhooks/pod_node_selector_mutator.go index bd7a505..01d7dad 100644 --- a/webhooks/pod_node_selector_mutator.go +++ b/webhooks/pod_node_selector_mutator.go @@ -88,7 +88,7 @@ func (v *PodNodeSelectorMutator) Handle(ctx context.Context, req admission.Reque if hasNodeSel { for k, v := range defaults { if _, exists := nodeSel[k]; !exists { - patches = append(patches, jsonpatch.NewOperation("add", "/spec/nodeSelector/"+escapeJSONPointer(k), v)) + patches = append(patches, jsonpatch.NewOperation("add", "/spec/nodeSelector/"+escapeJSONPointerSegment(k), v)) } } } else { @@ -117,6 +117,10 @@ func (v *PodNodeSelectorMutator) defaultLabels(ns corev1.Namespace) (labels.Set, return d, nil } -func escapeJSONPointer(s string) string { +// escapeJSONPointerSegment escapes a JSON pointer segment. +// It replaces `~“ with `~0` and `/“ with `~1`. +// See https://tools.ietf.org/html/rfc6901#section-3. +// example.com/label~test becomes example.com~1label~0test +func escapeJSONPointerSegment(s string) string { return strings.ReplaceAll(strings.ReplaceAll(s, "~", "~0"), "/", "~1") } diff --git a/webhooks/utils_test.go b/webhooks/utils_test.go index bcda68c..979a844 100644 --- a/webhooks/utils_test.go +++ b/webhooks/utils_test.go @@ -85,6 +85,20 @@ func newNamespace(name string, labels, annotations map[string]string) *corev1.Na } } +func newProjectRequest(name string, labels, annotations map[string]string) *projectv1.ProjectRequest { + return &projectv1.ProjectRequest{ + TypeMeta: metav1.TypeMeta{ + Kind: "Project", + APIVersion: "project.openshift.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: labels, + Annotations: annotations, + }, + } +} + func newService(name string, labels, annotations map[string]string) *corev1.Service { return &corev1.Service{ TypeMeta: metav1.TypeMeta{