From cadcb9e5a56513d836fea3f21f1e028dd8b5a4b6 Mon Sep 17 00:00:00 2001 From: Maxim Fedotov Date: Tue, 2 Nov 2021 19:01:53 +0200 Subject: [PATCH] Forbidden node labels and annotations (#464) * feat: forbidden node labels and annotations * test(e2e): forbidden node labels and annotations * build(kustomize): forbidden node labels and annotations * build(helm): forbidden node labels and annotations * build(installer): forbidden node labels and annotations * chore(make): forbidden node labels and annotations * docs: forbidden node labels and annotations * test(e2e): forbidden node labels and annotations. Use EventuallyCreation func * feat: forbidden node labels and annotations. Check kubernetes version * test(e2e): forbidden node labels and annotations. Check kubernetes version * docs: forbidden node labels and annotations. Version restrictions * feat: forbidden node labels and annotations. Do not update deepcopy functions * docs: forbidden node labels and annotations. Use blockquotes for notes Co-authored-by: Maksim Fedotov --- Makefile | 54 ++++- .../capsuleconfiguration_annotations.go | 8 + charts/capsule/README.md | 1 + .../validatingwebhookconfiguration.yaml | 26 ++ charts/capsule/values.yaml | 51 +++- config/install.yaml | 23 +- config/webhook/manifests.yaml | 19 ++ config/webhook/patch_ns_selector.yaml | 10 +- controllers/servicelabels/endpoint_slices.go | 4 +- .../use-cases/deny-wildcard-hostnames.md | 5 +- .../namespace-labels-and-annotations.md | 6 +- .../use-cases/node-labels-and-annotations.md | 41 ++++ docs/operator/use-cases/taint-services.md | 2 +- e2e/node_user_metadata_test.go | 227 ++++++++++++++++++ e2e/utils_test.go | 28 ++- main.go | 15 +- pkg/configuration/client.go | 43 ++++ pkg/configuration/configuration.go | 4 + pkg/webhook/node/errors.go | 53 ++++ pkg/webhook/node/user_metadata.go | 126 ++++++++++ pkg/webhook/route/node.go | 26 ++ pkg/webhook/utils/kubernetes_version.go | 45 +++- 22 files changed, 774 insertions(+), 43 deletions(-) create mode 100644 api/v1alpha1/capsuleconfiguration_annotations.go create mode 100644 docs/operator/use-cases/node-labels-and-annotations.md create mode 100644 e2e/node_user_metadata_test.go create mode 100644 pkg/webhook/node/errors.go create mode 100644 pkg/webhook/node/user_metadata.go create mode 100644 pkg/webhook/route/node.go diff --git a/Makefile b/Makefile index 7b2040d7..eedfac95 100644 --- a/Makefile +++ b/Makefile @@ -45,7 +45,7 @@ manager: generate fmt vet # Run against the configured Kubernetes cluster in ~/.kube/config run: generate manifests - go run ./main.go + go run . # Creates the single file to install Capsule without any external dependency installer: manifests kustomize @@ -78,6 +78,58 @@ manifests: controller-gen generate: controller-gen $(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./..." +# Setup development env +# Usage: +# LAPTOP_HOST_IP= make dev-setup +# For example: +# LAPTOP_HOST_IP=192.168.10.101 make dev-setup +define TLS_CNF +[ req ] +default_bits = 4096 +distinguished_name = req_distinguished_name +req_extensions = req_ext +[ req_distinguished_name ] +countryName = SG +stateOrProvinceName = SG +localityName = SG +organizationName = CAPSULE +commonName = CAPSULE +[ req_ext ] +subjectAltName = @alt_names +[alt_names] +IP.1 = $(LAPTOP_HOST_IP) +endef +export TLS_CNF +dev-setup: + kubectl -n capsule-system scale deployment capsule-controller-manager --replicas=0 + mkdir -p /tmp/k8s-webhook-server/serving-certs + echo "$${TLS_CNF}" > _tls.cnf + openssl req -newkey rsa:4096 -days 3650 -nodes -x509 \ + -subj "/C=SG/ST=SG/L=SG/O=CAPSULE/CN=CAPSULE" \ + -extensions req_ext \ + -config _tls.cnf \ + -keyout /tmp/k8s-webhook-server/serving-certs/tls.key \ + -out /tmp/k8s-webhook-server/serving-certs/tls.crt + rm -f _tls.cnf + export WEBHOOK_URL="https://$${LAPTOP_HOST_IP}:9443"; \ + export CA_BUNDLE=`openssl base64 -in /tmp/k8s-webhook-server/serving-certs/tls.crt | tr -d '\n'`; \ + kubectl patch MutatingWebhookConfiguration capsule-mutating-webhook-configuration \ + --type='json' -p="[\ + {'op': 'replace', 'path': '/webhooks/0/clientConfig', 'value':{'url':\"$${WEBHOOK_URL}/mutate-v1-namespace-owner-reference\",'caBundle':\"$${CA_BUNDLE}\"}}\ + ]" && \ + kubectl patch ValidatingWebhookConfiguration capsule-validating-webhook-configuration \ + --type='json' -p="[\ + {'op': 'replace', 'path': '/webhooks/0/clientConfig', 'value':{'url':\"$${WEBHOOK_URL}/cordoning\",'caBundle':\"$${CA_BUNDLE}\"}},\ + {'op': 'replace', 'path': '/webhooks/1/clientConfig', 'value':{'url':\"$${WEBHOOK_URL}/ingresses\",'caBundle':\"$${CA_BUNDLE}\"}},\ + {'op': 'replace', 'path': '/webhooks/2/clientConfig', 'value':{'url':\"$${WEBHOOK_URL}/namespaces\",'caBundle':\"$${CA_BUNDLE}\"}},\ + {'op': 'replace', 'path': '/webhooks/3/clientConfig', 'value':{'url':\"$${WEBHOOK_URL}/networkpolicies\",'caBundle':\"$${CA_BUNDLE}\"}},\ + {'op': 'replace', 'path': '/webhooks/4/clientConfig', 'value':{'url':\"$${WEBHOOK_URL}/pods\",'caBundle':\"$${CA_BUNDLE}\"}},\ + {'op': 'replace', 'path': '/webhooks/5/clientConfig', 'value':{'url':\"$${WEBHOOK_URL}/persistentvolumeclaims\",'caBundle':\"$${CA_BUNDLE}\"}},\ + {'op': 'replace', 'path': '/webhooks/6/clientConfig', 'value':{'url':\"$${WEBHOOK_URL}/services\",'caBundle':\"$${CA_BUNDLE}\"}},\ + {'op': 'replace', 'path': '/webhooks/7/clientConfig', 'value':{'url':\"$${WEBHOOK_URL}/tenants\",'caBundle':\"$${CA_BUNDLE}\"}},\ + {'op': 'replace', 'path': '/webhooks/8/clientConfig', 'value':{'url':\"$${WEBHOOK_URL}/nodes\",'caBundle':\"$${CA_BUNDLE}\"}}\ + ]"; + # Build the docker image docker-build: test docker build . -t ${IMG} --build-arg GIT_HEAD_COMMIT=$(GIT_HEAD_COMMIT) \ diff --git a/api/v1alpha1/capsuleconfiguration_annotations.go b/api/v1alpha1/capsuleconfiguration_annotations.go new file mode 100644 index 00000000..12fac835 --- /dev/null +++ b/api/v1alpha1/capsuleconfiguration_annotations.go @@ -0,0 +1,8 @@ +package v1alpha1 + +const ( + ForbiddenNodeLabelsAnnotation = "capsule.clastix.io/forbidden-node-labels" + ForbiddenNodeLabelsRegexpAnnotation = "capsule.clastix.io/forbidden-node-labels-regexp" + ForbiddenNodeAnnotationsAnnotation = "capsule.clastix.io/forbidden-node-annotations" + ForbiddenNodeAnnotationsRegexpAnnotation = "capsule.clastix.io/forbidden-node-annotations-regexp" +) diff --git a/charts/capsule/README.md b/charts/capsule/README.md index 1ba748bd..9f2d403d 100644 --- a/charts/capsule/README.md +++ b/charts/capsule/README.md @@ -80,6 +80,7 @@ Parameter | Description | Default `manager.resources.limits/cpu` | Set the memory limits assigned to the controller. | `128Mi` `mutatingWebhooksTimeoutSeconds` | Timeout in seconds for mutating webhooks. | `30` `validatingWebhooksTimeoutSeconds` | Timeout in seconds for validating webhooks. | `30` +`webhooks` | Additional configuration for capsule webhooks. | `imagePullSecrets` | Configuration for `imagePullSecrets` so that you can use a private images registry. | `[]` `serviceAccount.create` | Specifies whether a service account should be created. | `true` `serviceAccount.annotations` | Annotations to add to the service account. | `{}` diff --git a/charts/capsule/templates/validatingwebhookconfiguration.yaml b/charts/capsule/templates/validatingwebhookconfiguration.yaml index ac8109b4..7aae5999 100644 --- a/charts/capsule/templates/validatingwebhookconfiguration.yaml +++ b/charts/capsule/templates/validatingwebhookconfiguration.yaml @@ -252,3 +252,29 @@ webhooks: scope: '*' sideEffects: None timeoutSeconds: {{ .Values.validatingWebhooksTimeoutSeconds }} +- admissionReviewVersions: + - v1 + - v1beta1 + clientConfig: + caBundle: Cg== + service: + name: {{ include "capsule.fullname" . }}-webhook-service + namespace: {{ .Release.Namespace }} + path: /nodes + port: 443 + failurePolicy: {{ .Values.webhooks.nodes.failurePolicy }} + name: nodes.capsule.clastix.io + matchPolicy: Exact + namespaceSelector: {} + objectSelector: {} + rules: + - apiGroups: + - "" + apiVersions: + - v1 + operations: + - UPDATE + resources: + - nodes + sideEffects: None + timeoutSeconds: {{ .Values.validatingWebhooksTimeoutSeconds }} diff --git a/charts/capsule/values.yaml b/charts/capsule/values.yaml index 7024c7be..1c7750ad 100644 --- a/charts/capsule/values.yaml +++ b/charts/capsule/values.yaml @@ -42,8 +42,6 @@ jobs: repository: quay.io/clastix/kubectl pullPolicy: IfNotPresent tag: "v1.20.7" -mutatingWebhooksTimeoutSeconds: 30 -validatingWebhooksTimeoutSeconds: 30 imagePullSecrets: [] serviceAccount: create: true @@ -80,3 +78,52 @@ customLabels: {} # Additional annotations customAnnotations: {} + +# Webhooks configurations +webhooks: + namespaceOwnerReference: + failurePolicy: Fail + cordoning: + failurePolicy: Fail + namespaceSelector: + matchExpressions: + - key: capsule.clastix.io/tenant + operator: Exists + ingresses: + failurePolicy: Fail + namespaceSelector: + matchExpressions: + - key: capsule.clastix.io/tenant + operator: Exists + namespaces: + failurePolicy: Fail + networkpolicies: + failurePolicy: Fail + namespaceSelector: + matchExpressions: + - key: capsule.clastix.io/tenant + operator: Exists + pods: + failurePolicy: Fail + namespaceSelector: + matchExpressions: + - key: capsule.clastix.io/tenant + operator: Exists + persistentvolumeclaims: + failurePolicy: Fail + namespaceSelector: + matchExpressions: + - key: capsule.clastix.io/tenant + operator: Exists + tenants: + failurePolicy: Fail + services: + failurePolicy: Fail + namespaceSelector: + matchExpressions: + - key: capsule.clastix.io/tenant + operator: Exists + nodes: + failurePolicy: Fail +mutatingWebhooksTimeoutSeconds: 30 +validatingWebhooksTimeoutSeconds: 30 diff --git a/config/install.yaml b/config/install.yaml index 809610ec..363f5ef1 100644 --- a/config/install.yaml +++ b/config/install.yaml @@ -1587,14 +1587,33 @@ webhooks: service: name: capsule-webhook-service namespace: capsule-system - path: /pods + path: /nodes failurePolicy: Fail - name: pods.capsule.clastix.io + name: nodes.capsule.clastix.io namespaceSelector: matchExpressions: - key: capsule.clastix.io/tenant operator: Exists rules: + - apiGroups: + - "" + apiVersions: + - v1 + operations: + - UPDATE + resources: + - nodes + sideEffects: None +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: capsule-webhook-service + namespace: capsule-system + path: /pods + failurePolicy: Fail + name: pods.capsule.clastix.io + rules: - apiGroups: - "" apiVersions: diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 77cf6ffb..258d213d 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -117,6 +117,25 @@ webhooks: resources: - networkpolicies sideEffects: None +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /nodes + failurePolicy: Fail + name: nodes.capsule.clastix.io + rules: + - apiGroups: + - "" + apiVersions: + - v1 + operations: + - UPDATE + resources: + - nodes + sideEffects: None - admissionReviewVersions: - v1 clientConfig: diff --git a/config/webhook/patch_ns_selector.yaml b/config/webhook/patch_ns_selector.yaml index cd3d69d4..eacfd011 100644 --- a/config/webhook/patch_ns_selector.yaml +++ b/config/webhook/patch_ns_selector.yaml @@ -23,13 +23,13 @@ - key: capsule.clastix.io/tenant operator: Exists - op: add - path: /webhooks/5/namespaceSelector + path: /webhooks/6/namespaceSelector value: matchExpressions: - key: capsule.clastix.io/tenant operator: Exists - op: add - path: /webhooks/6/namespaceSelector + path: /webhooks/7/namespaceSelector value: matchExpressions: - key: capsule.clastix.io/tenant @@ -43,12 +43,12 @@ - op: add path: /webhooks/3/rules/0/scope value: Namespaced -- op: add - path: /webhooks/4/rules/0/scope - value: Namespaced - op: add path: /webhooks/5/rules/0/scope value: Namespaced - op: add path: /webhooks/6/rules/0/scope value: Namespaced +- op: add + path: /webhooks/7/rules/0/scope + value: Namespaced diff --git a/controllers/servicelabels/endpoint_slices.go b/controllers/servicelabels/endpoint_slices.go index 39ae4953..021be700 100644 --- a/controllers/servicelabels/endpoint_slices.go +++ b/controllers/servicelabels/endpoint_slices.go @@ -14,8 +14,8 @@ type EndpointSlicesLabelsReconciler struct { abstractServiceLabelsReconciler Log logr.Logger - VersionMinor int - VersionMajor int + VersionMinor uint + VersionMajor uint } func (r *EndpointSlicesLabelsReconciler) SetupWithManager(mgr ctrl.Manager) error { diff --git a/docs/operator/use-cases/deny-wildcard-hostnames.md b/docs/operator/use-cases/deny-wildcard-hostnames.md index f1f675de..553a5622 100644 --- a/docs/operator/use-cases/deny-wildcard-hostnames.md +++ b/docs/operator/use-cases/deny-wildcard-hostnames.md @@ -26,7 +26,4 @@ EOF Doing this, Alice will not be able to use `oil.bigorg.com`, being the tenant-owner of `gas`. # What’s next - -This ends our tour in Capsule use cases. As we improve Capsule, more use cases about multi-tenancy, policy admission control, and cluster governance will be covered in the future. - -Stay tuned! \ No newline at end of file +See how Bill, the cluster admin can protect specific labels and annotations on Nodes from modifications by Tenant Owners. [Denying specific user-defined labels or annotations on Nodes](./node-labels-and-annotations.md). diff --git a/docs/operator/use-cases/namespace-labels-and-annotations.md b/docs/operator/use-cases/namespace-labels-and-annotations.md index 36a6434b..8d1837d9 100644 --- a/docs/operator/use-cases/namespace-labels-and-annotations.md +++ b/docs/operator/use-cases/namespace-labels-and-annotations.md @@ -1,4 +1,4 @@ -# Denying user-defined labels or annotations +# Denying specific user-defined labels or annotations on Namespaces By default, capsule allows tenant owners to add and modify any label or annotation on their namespaces. @@ -13,9 +13,9 @@ kind: Tenant metadata: name: oil annotations: - capsule.clastix.io/forbidden-namespace-labels: foo.acme.net, bar.acme.net + capsule.clastix.io/forbidden-namespace-labels: foo.acme.net,bar.acme.net capsule.clastix.io/forbidden-namespace-labels-regexp: .*.acme.net - capsule.clastix.io/forbidden-namespace-annotations: foo.acme.net, bar.acme.net + capsule.clastix.io/forbidden-namespace-annotations: foo.acme.net,bar.acme.net capsule.clastix.io/forbidden-namespace-annotations-regexp: .*.acme.net spec: owners: diff --git a/docs/operator/use-cases/node-labels-and-annotations.md b/docs/operator/use-cases/node-labels-and-annotations.md new file mode 100644 index 00000000..b90a61fb --- /dev/null +++ b/docs/operator/use-cases/node-labels-and-annotations.md @@ -0,0 +1,41 @@ +# Denying specific user-defined labels or annotations on Nodes + +When using `capsule` together with [capsule-proxy](https://github.com/clastix/capsule-proxy), Bill can allow Tenant Owners to [modify Nodes](../../proxy/overview.md). + +By default, it will allow tenant owners to add and modify any label or annotation on their nodes. + +But there are some scenarios, when tenant owners should not have an ability to add or modify specific labels or annotations (there are some types of labels or annotations, which must be protected from modifications - for example, which are set by `cloud-providers` or `autoscalers`). + +Bill, the cluster admin, can deny Tenant Owners to add or modify specific labels and annotations on Nodes: + +```yaml +kubectl apply -f - << EOF +apiVersion: capsule.clastix.io/v1alpha1 +kind: CapsuleConfiguration +metadata: + name: default + annotations: + capsule.clastix.io/forbidden-node-labels: foo.acme.net,bar.acme.net + capsule.clastix.io/forbidden-node-labels-regexp: .*.acme.net + capsule.clastix.io/forbidden-node-annotations: foo.acme.net,bar.acme.net + capsule.clastix.io/forbidden-node-annotations-regexp: .*.acme.net +spec: + userGroups: + - capsule.clastix.io + - system:serviceaccounts:default +EOF +``` + +> **Important note** +> +>Due to [CVE-2021-25735](https://github.com/kubernetes/kubernetes/issues/100096) this feature is only supported for Kubernetes version older than: +>* v1.18.18 +>* v1.19.10 +>* v1.20.6 +>* v1.21.0 + +# What’s next + +This ends our tour in Capsule use cases. As we improve Capsule, more use cases about multi-tenancy, policy admission control, and cluster governance will be covered in the future. + +Stay tuned! \ No newline at end of file diff --git a/docs/operator/use-cases/taint-services.md b/docs/operator/use-cases/taint-services.md index fd427984..811c4016 100644 --- a/docs/operator/use-cases/taint-services.md +++ b/docs/operator/use-cases/taint-services.md @@ -25,4 +25,4 @@ EOF When Alice creates a service in a namespace, this will inherit the given label and/or annotation. # What’s next -See how Bill, the cluster admin, can allow Alice to use specific labels or annotations. [Allow adding labels and annotations on namespaces](./namespace-labels-and-annotations.md). +See how Bill, the cluster admin, can protect specific labels and annotations on Namespaces from modifications by Alice. [Denying specific user-defined labels or annotations on Namespaces](./namespace-labels-and-annotations.md). diff --git a/e2e/node_user_metadata_test.go b/e2e/node_user_metadata_test.go new file mode 100644 index 00000000..8ee2fdc3 --- /dev/null +++ b/e2e/node_user_metadata_test.go @@ -0,0 +1,227 @@ +//go:build e2e +// +build e2e + +// Copyright 2020-2021 Clastix Labs +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "context" + + capsulev1alpha1 "github.com/clastix/capsule/api/v1alpha1" + capsulev1beta1 "github.com/clastix/capsule/api/v1beta1" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/clastix/capsule/pkg/webhook/utils" + "fmt" +) + +var _ = Describe("modifying node labels and annotations", func() { + tnt := &capsulev1beta1.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tenant-node-user-metadata-forbidden", + }, + Spec: capsulev1beta1.TenantSpec{ + Owners: capsulev1beta1.OwnerListSpec{ + { + Name: "gatsby", + Kind: "User", + }, + }, + }, + } + + cr := &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node-modifier", + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"nodes"}, + Verbs: []string{"patch", "update", "get", "list"}, + }, + }, + } + + crb := &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node-modifier", + }, + RoleRef: rbacv1.RoleRef{ + Kind: "ClusterRole", + Name: "node-modifier", + APIGroup: "rbac.authorization.k8s.io", + }, + Subjects: []rbacv1.Subject{ + { + Kind: rbacv1.UserKind, + APIGroup: "rbac.authorization.k8s.io", + Name: "gatsby", + }, + }, + } + + JustBeforeEach(func() { + version := GetKubernetesVersion() + nodeWebhookSupported, _ := utils.NodeWebhookSupported(version) + + if !nodeWebhookSupported { + Skip(fmt.Sprintf("Node webhook is disabled for current version %s", version.String())) + } + + EventuallyCreation(func() error { + tnt.ResourceVersion = "" + return k8sClient.Create(context.TODO(), tnt) + }).Should(Succeed()) + EventuallyCreation(func() error { + cr.ResourceVersion = "" + return k8sClient.Create(context.TODO(), cr) + }).Should(Succeed()) + EventuallyCreation(func() error { + crb.ResourceVersion = "" + return k8sClient.Create(context.TODO(), crb) + }).Should(Succeed()) + }) + JustAfterEach(func() { + Expect(k8sClient.Delete(context.TODO(), tnt)).Should(Succeed()) + Expect(k8sClient.Delete(context.TODO(), crb)).Should(Succeed()) + Expect(k8sClient.Delete(context.TODO(), cr)).Should(Succeed()) + }) + + It("should allow", func() { + ModifyCapsuleConfigurationOpts(func(configuration *capsulev1alpha1.CapsuleConfiguration) { + protected := map[string]string{ + capsulev1alpha1.ForbiddenNodeLabelsAnnotation: "foo,bar", + capsulev1alpha1.ForbiddenNodeLabelsRegexpAnnotation: "^gatsby-.*$", + capsulev1alpha1.ForbiddenNodeAnnotationsAnnotation: "foo,bar", + capsulev1alpha1.ForbiddenNodeAnnotationsRegexpAnnotation: "^gatsby-.*$", + } + configuration.SetAnnotations(protected) + }) + By("adding non-forbidden labels", func() { + EventuallyCreation(func() error { + return ModifyNode(func(node *corev1.Node) error { + node.Labels["bim"] = "baz" + cs := ownerClient(tnt.Spec.Owners[0]) + + _, err := cs.CoreV1().Nodes().Update(context.Background(), node, metav1.UpdateOptions{}) + return err + }) + }).Should(Succeed()) + }) + By("modifying non-forbidden labels", func() { + EventuallyCreation(func() error { + return ModifyNode(func(node *corev1.Node) error { + node.Labels["bim"] = "bom" + cs := ownerClient(tnt.Spec.Owners[0]) + + _, err := cs.CoreV1().Nodes().Update(context.Background(), node, metav1.UpdateOptions{}) + return err + }) + }).Should(Succeed()) + }) + By("adding non-forbidden annotations", func() { + EventuallyCreation(func() error { + return ModifyNode(func(node *corev1.Node) error { + node.Annotations["bim"] = "baz" + cs := ownerClient(tnt.Spec.Owners[0]) + + _, err := cs.CoreV1().Nodes().Update(context.Background(), node, metav1.UpdateOptions{}) + return err + }) + }).Should(Succeed()) + }) + By("modifying non-forbidden annotations", func() { + EventuallyCreation(func() error { + return ModifyNode(func(node *corev1.Node) error { + node.Annotations["bim"] = "bom" + cs := ownerClient(tnt.Spec.Owners[0]) + + _, err := cs.CoreV1().Nodes().Update(context.Background(), node, metav1.UpdateOptions{}) + return err + }) + }).Should(Succeed()) + }) + }) + It("should fail", func() { + Expect(ModifyNode(func(node *corev1.Node) error { + node.Labels["foo"] = "bar" + node.Labels["gatsby-foo"] = "bar" + node.Annotations["foo"] = "bar" + node.Annotations["gatsby-foo"] = "bar" + return k8sClient.Update(context.Background(), node) + })).Should(Succeed()) + + By("adding forbidden labels using exact match", func() { + EventuallyCreation(func() error { + return ModifyNode(func(node *corev1.Node) error { + node.Labels["bar"] = "baz" + cs := ownerClient(tnt.Spec.Owners[0]) + + _, err := cs.CoreV1().Nodes().Update(context.Background(), node, metav1.UpdateOptions{}) + return err + }) + }).ShouldNot(Succeed()) + }) + By("adding forbidden labels using regex match", func() { + EventuallyCreation(func() error { + return ModifyNode(func(node *corev1.Node) error { + node.Labels["gatsby-foo"] = "baz" + cs := ownerClient(tnt.Spec.Owners[0]) + + _, err := cs.CoreV1().Nodes().Update(context.Background(), node, metav1.UpdateOptions{}) + return err + }) + }).ShouldNot(Succeed()) + }) + By("modifying forbidden labels", func() { + EventuallyCreation(func() error { + return ModifyNode(func(node *corev1.Node) error { + node.Labels["foo"] = "baz" + cs := ownerClient(tnt.Spec.Owners[0]) + + _, err := cs.CoreV1().Nodes().Update(context.Background(), node, metav1.UpdateOptions{}) + return err + }) + }).ShouldNot(Succeed()) + }) + By("adding forbidden annotations using exact match", func() { + EventuallyCreation(func() error { + return ModifyNode(func(node *corev1.Node) error { + node.Annotations["bar"] = "baz" + cs := ownerClient(tnt.Spec.Owners[0]) + + _, err := cs.CoreV1().Nodes().Update(context.Background(), node, metav1.UpdateOptions{}) + return err + }) + }).ShouldNot(Succeed()) + }) + By("adding forbidden annotations using regex match", func() { + EventuallyCreation(func() error { + return ModifyNode(func(node *corev1.Node) error { + node.Annotations["gatsby-foo"] = "baz" + cs := ownerClient(tnt.Spec.Owners[0]) + + _, err := cs.CoreV1().Nodes().Update(context.Background(), node, metav1.UpdateOptions{}) + return err + }) + }).ShouldNot(Succeed()) + }) + By("modifying forbidden annotations", func() { + EventuallyCreation(func() error { + return ModifyNode(func(node *corev1.Node) error { + node.Annotations["foo"] = "baz" + cs := ownerClient(tnt.Spec.Owners[0]) + + _, err := cs.CoreV1().Nodes().Update(context.Background(), node, metav1.UpdateOptions{}) + return err + }) + }).ShouldNot(Succeed()) + }) + }) +}) diff --git a/e2e/utils_test.go b/e2e/utils_test.go index 1910c4c5..cb599d8e 100644 --- a/e2e/utils_test.go +++ b/e2e/utils_test.go @@ -7,7 +7,6 @@ package e2e import ( "context" - "strconv" "time" . "github.com/onsi/gomega" @@ -15,6 +14,7 @@ import ( rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + versionUtil "k8s.io/apimachinery/pkg/util/version" "k8s.io/apimachinery/pkg/version" "k8s.io/client-go/kubernetes" @@ -50,6 +50,14 @@ func TenantNamespaceList(t *capsulev1beta1.Tenant, timeout time.Duration) AsyncA }, timeout, defaultPollInterval) } +func ModifyNode(fn func(node *corev1.Node) error) error { + nodeList := &corev1.NodeList{} + + Expect(k8sClient.List(context.Background(), nodeList)).ToNot(HaveOccurred()) + + return fn(&nodeList.Items[0]) +} + func EventuallyCreation(f interface{}) AsyncAssertion { return Eventually(f, defaultTimeoutInterval, defaultPollInterval) } @@ -62,7 +70,7 @@ func ModifyCapsuleConfigurationOpts(fn func(configuration *capsulev1alpha1.Capsu Expect(k8sClient.Update(context.Background(), config)).ToNot(HaveOccurred()) - time.Sleep(time.Second) + time.Sleep(1 * time.Second) } func KindInTenantRoleBindingAssertions(ns *corev1.Namespace, timeout time.Duration) (out []AsyncAssertion) { @@ -82,21 +90,21 @@ func KindInTenantRoleBindingAssertions(ns *corev1.Namespace, timeout time.Durati return } -func GetKubernetesSemVer() (major, minor int, ver string) { - var v *version.Info +func GetKubernetesVersion() *versionUtil.Version { + var serverVersion *version.Info var err error var cs kubernetes.Interface + var ver *versionUtil.Version cs, err = kubernetes.NewForConfig(cfg) Expect(err).ToNot(HaveOccurred()) - v, err = cs.Discovery().ServerVersion() + serverVersion, err = cs.Discovery().ServerVersion() Expect(err).ToNot(HaveOccurred()) - major, err = strconv.Atoi(v.Major) - Expect(err).ToNot(HaveOccurred()) - minor, err = strconv.Atoi(v.Minor) + + ver, err = versionUtil.ParseGeneric(serverVersion.String()) Expect(err).ToNot(HaveOccurred()) - ver = v.String() - return + + return ver } diff --git a/main.go b/main.go index 9e5aba7c..ef97be8b 100644 --- a/main.go +++ b/main.go @@ -20,6 +20,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" + "github.com/clastix/capsule/pkg/webhook/node" + capsulev1alpha1 "github.com/clastix/capsule/api/v1alpha1" capsulev1beta1 "github.com/clastix/capsule/api/v1beta1" configcontroller "github.com/clastix/capsule/controllers/config" @@ -119,7 +121,7 @@ func main() { os.Exit(1) } - majorVer, minorVer, _, err := utils.GetK8sVersion() + kubeVersion, err := utils.GetK8sVersion() if err != nil { setupLog.Error(err, "unable to get kubernetes version") os.Exit(1) @@ -157,7 +159,14 @@ func main() { route.Tenant(tenant.NameHandler(), tenant.IngressClassRegexHandler(), tenant.StorageClassRegexHandler(), tenant.ContainerRegistryRegexHandler(), tenant.HostnameRegexHandler(), tenant.FreezedEmitter(), tenant.ServiceAccountNameHandler()), route.OwnerReference(utils.InCapsuleGroups(cfg, ownerreference.Handler(cfg))), route.Cordoning(tenant.CordoningHandler(cfg)), + route.Node(utils.InCapsuleGroups(cfg, node.UserMetadataHandler(cfg, kubeVersion))), ) + + nodeWebhookSupported, _ := utils.NodeWebhookSupported(kubeVersion) + if !nodeWebhookSupported { + setupLog.Info("Disabling node labels verification webhook as current Kubernetes version doesn't have fix for CVE-2021-25735") + } + if err = webhook.Register(manager, webhooksList...); err != nil { setupLog.Error(err, "unable to setup webhooks") os.Exit(1) @@ -209,8 +218,8 @@ func main() { } if err = (&servicelabelscontroller.EndpointSlicesLabelsReconciler{ Log: ctrl.Log.WithName("controllers").WithName("EndpointSliceLabels"), - VersionMinor: minorVer, - VersionMajor: majorVer, + VersionMinor: kubeVersion.Minor(), + VersionMajor: kubeVersion.Major(), }).SetupWithManager(manager); err != nil { setupLog.Error(err, "unable to create controller", "controller", "EndpointSliceLabels") } diff --git a/pkg/configuration/client.go b/pkg/configuration/client.go index 20838a11..dc3c62e7 100644 --- a/pkg/configuration/client.go +++ b/pkg/configuration/client.go @@ -6,12 +6,15 @@ package configuration import ( "context" "regexp" + "strings" "github.com/pkg/errors" machineryerr "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + capsulev1beta1 "github.com/clastix/capsule/api/v1beta1" + capsulev1alpha1 "github.com/clastix/capsule/api/v1alpha1" ) @@ -62,3 +65,43 @@ func (c capsuleConfiguration) ForceTenantPrefix() bool { func (c capsuleConfiguration) UserGroups() []string { return c.retrievalFn().Spec.UserGroups } + +func (c capsuleConfiguration) hasForbiddenNodeLabelsAnnotations() bool { + if _, ok := c.retrievalFn().Annotations[capsulev1alpha1.ForbiddenNodeLabelsAnnotation]; ok { + return true + } + if _, ok := c.retrievalFn().Annotations[capsulev1alpha1.ForbiddenNodeLabelsRegexpAnnotation]; ok { + return true + } + return false +} + +func (c capsuleConfiguration) hasForbiddenNodeAnnotationsAnnotations() bool { + if _, ok := c.retrievalFn().Annotations[capsulev1alpha1.ForbiddenNodeAnnotationsAnnotation]; ok { + return true + } + if _, ok := c.retrievalFn().Annotations[capsulev1alpha1.ForbiddenNodeAnnotationsRegexpAnnotation]; ok { + return true + } + return false +} + +func (c *capsuleConfiguration) ForbiddenUserNodeLabels() *capsulev1beta1.ForbiddenListSpec { + if !c.hasForbiddenNodeLabelsAnnotations() { + return nil + } + return &capsulev1beta1.ForbiddenListSpec{ + Exact: strings.Split(c.retrievalFn().Annotations[capsulev1alpha1.ForbiddenNodeLabelsAnnotation], ","), + Regex: c.retrievalFn().Annotations[capsulev1alpha1.ForbiddenNodeLabelsRegexpAnnotation], + } +} + +func (c *capsuleConfiguration) ForbiddenUserNodeAnnotations() *capsulev1beta1.ForbiddenListSpec { + if !c.hasForbiddenNodeAnnotationsAnnotations() { + return nil + } + return &capsulev1beta1.ForbiddenListSpec{ + Exact: strings.Split(c.retrievalFn().Annotations[capsulev1alpha1.ForbiddenNodeAnnotationsAnnotation], ","), + Regex: c.retrievalFn().Annotations[capsulev1alpha1.ForbiddenNodeAnnotationsRegexpAnnotation], + } +} diff --git a/pkg/configuration/configuration.go b/pkg/configuration/configuration.go index a481e8e2..130e68b9 100644 --- a/pkg/configuration/configuration.go +++ b/pkg/configuration/configuration.go @@ -5,10 +5,14 @@ package configuration import ( "regexp" + + capsulev1beta1 "github.com/clastix/capsule/api/v1beta1" ) type Configuration interface { ProtectedNamespaceRegexp() (*regexp.Regexp, error) ForceTenantPrefix() bool UserGroups() []string + ForbiddenUserNodeLabels() *capsulev1beta1.ForbiddenListSpec + ForbiddenUserNodeAnnotations() *capsulev1beta1.ForbiddenListSpec } diff --git a/pkg/webhook/node/errors.go b/pkg/webhook/node/errors.go new file mode 100644 index 00000000..c4fcb063 --- /dev/null +++ b/pkg/webhook/node/errors.go @@ -0,0 +1,53 @@ +// Copyright 2020-2021 Clastix Labs +// SPDX-License-Identifier: Apache-2.0 + +package node + +import ( + "fmt" + "strings" + + capsulev1beta1 "github.com/clastix/capsule/api/v1beta1" +) + +func appendForbiddenError(spec *capsulev1beta1.ForbiddenListSpec) (append string) { + append += "Forbidden are " + if len(spec.Exact) > 0 { + append += fmt.Sprintf("one of the following (%s)", strings.Join(spec.Exact, ", ")) + if len(spec.Regex) > 0 { + append += " or " + } + } + if len(spec.Regex) > 0 { + append += fmt.Sprintf("matching the regex %s", spec.Regex) + } + return +} + +type nodeLabelForbiddenError struct { + spec *capsulev1beta1.ForbiddenListSpec +} + +func NewNodeLabelForbiddenError(forbiddenSpec *capsulev1beta1.ForbiddenListSpec) error { + return &nodeLabelForbiddenError{ + spec: forbiddenSpec, + } +} + +func (f nodeLabelForbiddenError) Error() string { + return fmt.Sprintf("Unable to update node as some labels are marked as forbidden by system administrator. %s", appendForbiddenError(f.spec)) +} + +type nodeAnnotationForbiddenError struct { + spec *capsulev1beta1.ForbiddenListSpec +} + +func NewNodeAnnotationForbiddenError(forbiddenSpec *capsulev1beta1.ForbiddenListSpec) error { + return &nodeAnnotationForbiddenError{ + spec: forbiddenSpec, + } +} + +func (f nodeAnnotationForbiddenError) Error() string { + return fmt.Sprintf("Unable to update node as some annotations are marked as forbidden by system administrator. %s", appendForbiddenError(f.spec)) +} diff --git a/pkg/webhook/node/user_metadata.go b/pkg/webhook/node/user_metadata.go new file mode 100644 index 00000000..bf7ffe7c --- /dev/null +++ b/pkg/webhook/node/user_metadata.go @@ -0,0 +1,126 @@ +// Copyright 2020-2021 Clastix Labs +// SPDX-License-Identifier: Apache-2.0 + +package node + +import ( + "context" + "reflect" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/version" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/clastix/capsule/pkg/configuration" + + capsulewebhook "github.com/clastix/capsule/pkg/webhook" + "github.com/clastix/capsule/pkg/webhook/utils" +) + +type userMetadataHandler struct { + configuration configuration.Configuration + version *version.Version +} + +func UserMetadataHandler(configuration configuration.Configuration, ver *version.Version) capsulewebhook.Handler { + return &userMetadataHandler{ + configuration: configuration, + version: ver, + } +} + +func (r *userMetadataHandler) OnCreate(client client.Client, decoder *admission.Decoder, recorder record.EventRecorder) capsulewebhook.Func { + return func(ctx context.Context, req admission.Request) *admission.Response { + return nil + } +} + +func (r *userMetadataHandler) OnDelete(client client.Client, decoder *admission.Decoder, recorder record.EventRecorder) capsulewebhook.Func { + return func(ctx context.Context, req admission.Request) *admission.Response { + return nil + } +} + +func (r *userMetadataHandler) getForbiddenNodeLabels(node *corev1.Node) map[string]string { + forbiddenNodeLabels := make(map[string]string) + + forbiddenLabels := r.configuration.ForbiddenUserNodeLabels() + for label, value := range node.GetLabels() { + var forbidden, matched bool + forbidden = forbiddenLabels.ExactMatch(label) + matched = forbiddenLabels.RegexMatch(label) + + if forbidden || matched { + forbiddenNodeLabels[label] = value + } + } + + return forbiddenNodeLabels +} + +func (r *userMetadataHandler) getForbiddenNodeAnnotations(node *corev1.Node) map[string]string { + forbiddenNodeAnnotations := make(map[string]string) + + forbiddenAnnotations := r.configuration.ForbiddenUserNodeAnnotations() + for annotation, value := range node.GetAnnotations() { + var forbidden, matched bool + forbidden = forbiddenAnnotations.ExactMatch(annotation) + matched = forbiddenAnnotations.RegexMatch(annotation) + + if forbidden || matched { + forbiddenNodeAnnotations[annotation] = value + } + } + + return forbiddenNodeAnnotations +} + +func (r *userMetadataHandler) OnUpdate(client client.Client, decoder *admission.Decoder, recorder record.EventRecorder) capsulewebhook.Func { + return func(ctx context.Context, req admission.Request) *admission.Response { + nodeWebhookSupported, _ := utils.NodeWebhookSupported(r.version) + + if !nodeWebhookSupported { + return nil + } + + oldNode := &corev1.Node{} + if err := decoder.DecodeRaw(req.OldObject, oldNode); err != nil { + return utils.ErroredResponse(err) + } + + newNode := &corev1.Node{} + if err := decoder.Decode(req, newNode); err != nil { + return utils.ErroredResponse(err) + } + + if r.configuration.ForbiddenUserNodeLabels() != nil { + oldNodeForbiddenLabels := r.getForbiddenNodeLabels(oldNode) + newNodeForbiddenLabels := r.getForbiddenNodeLabels(newNode) + + if !reflect.DeepEqual(oldNodeForbiddenLabels, newNodeForbiddenLabels) { + recorder.Eventf(newNode, corev1.EventTypeWarning, "ForbiddenNodeLabel", "Denied modifying forbidden labels on node") + + response := admission.Denied(NewNodeLabelForbiddenError(r.configuration.ForbiddenUserNodeLabels()).Error()) + + return &response + } + } + + if r.configuration.ForbiddenUserNodeAnnotations() != nil { + oldNodeForbiddenAnnotations := r.getForbiddenNodeAnnotations(oldNode) + newNodeForbiddenAnnotations := r.getForbiddenNodeAnnotations(newNode) + + if !reflect.DeepEqual(oldNodeForbiddenAnnotations, newNodeForbiddenAnnotations) { + recorder.Eventf(newNode, corev1.EventTypeWarning, "ForbiddenNodeLabel", "Denied modifying forbidden annotations on node") + + response := admission.Denied(NewNodeAnnotationForbiddenError(r.configuration.ForbiddenUserNodeAnnotations()).Error()) + + return &response + } + } + + return nil + } +} diff --git a/pkg/webhook/route/node.go b/pkg/webhook/route/node.go new file mode 100644 index 00000000..13dcbe20 --- /dev/null +++ b/pkg/webhook/route/node.go @@ -0,0 +1,26 @@ +// Copyright 2020-2021 Clastix Labs +// SPDX-License-Identifier: Apache-2.0 + +package route + +import ( + capsulewebhook "github.com/clastix/capsule/pkg/webhook" +) + +// +kubebuilder:webhook:path=/nodes,mutating=false,sideEffects=None,admissionReviewVersions=v1,failurePolicy=fail,groups="",resources=nodes,verbs=update,versions=v1,name=nodes.capsule.clastix.io + +type node struct { + handlers []capsulewebhook.Handler +} + +func Node(handler ...capsulewebhook.Handler) capsulewebhook.Webhook { + return &node{handlers: handler} +} + +func (n *node) GetHandlers() []capsulewebhook.Handler { + return n.handlers +} + +func (n *node) GetPath() string { + return "/nodes" +} diff --git a/pkg/webhook/utils/kubernetes_version.go b/pkg/webhook/utils/kubernetes_version.go index 2b74b0bf..7a7e8207 100644 --- a/pkg/webhook/utils/kubernetes_version.go +++ b/pkg/webhook/utils/kubernetes_version.go @@ -5,34 +5,59 @@ package utils import ( "path/filepath" - "strconv" + "k8s.io/apimachinery/pkg/util/version" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/util/homedir" ) -func GetK8sVersion() (major, minor int, ver string, err error) { +var versionsWithNodeFix = []string{"v1.18.18", "v1.19.10", "v1.20.6", "v1.21.0"} + +func NodeWebhookSupported(currentVersion *version.Version) (bool, error) { + var versions []*version.Version + + for _, v := range versionsWithNodeFix { + ver, err := version.ParseGeneric(v) + if err != nil { + return false, err + } + versions = append(versions, ver) + } + + for _, v := range versions { + if currentVersion.Major() == v.Major() { + if currentVersion.Minor() < v.Minor() { + return false, nil + } + if currentVersion.Minor() == v.Minor() && currentVersion.Patch() < v.Patch() { + return false, nil + } + } + } + + return true, nil +} + +func GetK8sVersion() (*version.Version, error) { cfg, err := rest.InClusterConfig() if err != nil { kubeconfig := filepath.Join(homedir.HomeDir(), ".kube", "config") cfg, err = clientcmd.BuildConfigFromFlags("", kubeconfig) } if err != nil { - return 0, 0, "", err + return nil, err } client, err := kubernetes.NewForConfig(cfg) if err != nil { - return 0, 0, "", err + return nil, err } - version, err := client.Discovery().ServerVersion() + v, err := client.Discovery().ServerVersion() if err != nil { - return 0, 0, "", err + return nil, err } - major, _ = strconv.Atoi(version.Major) - minor, _ = strconv.Atoi(version.Minor) - ver = version.String() - return + + return version.ParseGeneric(v.String()) }