From e270e2b5d61009ff595c2443732c126fdd04fa2a Mon Sep 17 00:00:00 2001 From: Shawn Bai Date: Mon, 1 Jun 2026 16:10:46 +1000 Subject: [PATCH] ROSAENG-963: Prevent customer CREATE of ServiceAccounts in managed namespaces --- build/selectorsyncset.yaml | 1 + config/package/resources.yaml.gotmpl | 1 + docs/webhooks-short.json | 22 ++++- docs/webhooks.json | 91 +++++++++++++++++- pkg/webhooks/serviceaccount/serviceaccount.go | 17 +++- .../serviceaccount/serviceaccount_test.go | 96 +++++++++++++++++++ 6 files changed, 220 insertions(+), 8 deletions(-) diff --git a/build/selectorsyncset.yaml b/build/selectorsyncset.yaml index 18dc8370..6ff7f926 100644 --- a/build/selectorsyncset.yaml +++ b/build/selectorsyncset.yaml @@ -801,6 +801,7 @@ objects: apiVersions: - v1 operations: + - CREATE - DELETE resources: - serviceaccounts diff --git a/config/package/resources.yaml.gotmpl b/config/package/resources.yaml.gotmpl index fbe78f75..6e34154c 100644 --- a/config/package/resources.yaml.gotmpl +++ b/config/package/resources.yaml.gotmpl @@ -479,6 +479,7 @@ webhooks: apiVersions: - v1 operations: + - CREATE - DELETE resources: - serviceaccounts diff --git a/docs/webhooks-short.json b/docs/webhooks-short.json index 52ae2625..ba2637eb 100644 --- a/docs/webhooks-short.json +++ b/docs/webhooks-short.json @@ -15,10 +15,22 @@ "webhookName": "customresourcedefinitions-validation", "documentString": "Managed OpenShift Customers may not change CustomResourceDefinitions managed by Red Hat." }, + { + "webhookName": "hcpnamespace-validation", + "documentString": "Validates HCP namespace deletion operations are only performed by authorized service accounts" + }, { "webhookName": "hiveownership-validation", "documentString": "Managed OpenShift customers may not edit certain managed resources. A managed resource has a \"hive.openshift.io/managed\": \"true\" label." }, + { + "webhookName": "hostedcluster-validation", + "documentString": "Validates HostedCluster deletion operations are only performed by authorized service accounts" + }, + { + "webhookName": "hostedcontrolplane-validation", + "documentString": "Validates HostedControlPlane deletion operations are only performed by authorized service accounts" + }, { "webhookName": "imagecontentpolicies-validation", "documentString": "Managed OpenShift customers may not create ImageContentSourcePolicy, ImageDigestMirrorSet, or ImageTagMirrorSet resources that configure mirrors that would conflict with system registries (e.g. quay.io, registry.redhat.io, registry.access.redhat.com, etc). For more details, see https://docs.openshift.com/" @@ -31,13 +43,17 @@ "webhookName": "ingresscontroller-validation", "documentString": "Managed OpenShift Customer may create IngressControllers without necessary taints. This can cause those workloads to be provisioned on master nodes." }, + { + "webhookName": "manifestworks-validation", + "documentString": "Validates ManifestWorks deletion operations are only performed by authorized service accounts" + }, { "webhookName": "namespace-validation", "documentString": "Managed OpenShift Customers may not modify namespaces specified in the [openshift-monitoring/managed-namespaces openshift-monitoring/ocp-namespaces] ConfigMaps because customer workloads should be placed in customer-created namespaces. Customers may not create namespaces identified by this regular expression (^com$|^io$|^in$) because it could interfere with critical DNS resolution. Additionally, customers may not set or change the values of these Namespace labels [managed.openshift.io/storage-pv-quota-exempt managed.openshift.io/service-lb-quota-exempt]." }, { "webhookName": "network-operator-validation", - "documentString": "Managed OpenShift customers may not modify critical fields in the network.operator CRD (such as spec.migration.networkType) because it can disrupt Cluster Network Operator operations and CNI migrations. Even cluster-admin users are blocked from modifying these critical fields." + "documentString": "Managed OpenShift customers may not modify critical fields in the network.operator CRD (such as spec.migration.networkType) because it can disrupt Cluster Network Operator operations and CNI migrations. Only backplane-cluster-admin, SRE, Cluster Network Operator (CNO), and Managed Upgrade Operator (MUO) service accounts are allowed to modify these critical fields. Regular cluster-admin users (system:admin) are explicitly blocked." }, { "webhookName": "networkpolicies-validation", @@ -61,7 +77,7 @@ }, { "webhookName": "regular-user-validation", - "documentString": "Managed OpenShift customers may not manage any objects in the following APIGroups [upgrade.managed.openshift.io config.openshift.io operator.openshift.io network.openshift.io admissionregistration.k8s.io addons.managed.openshift.io cloudingress.managed.openshift.io managed.openshift.io splunkforwarder.managed.openshift.io autoscaling.openshift.io machineconfiguration.openshift.io cloudcredential.openshift.io machine.openshift.io ocmagent.managed.openshift.io], nor may Managed OpenShift customers alter the APIServer, KubeAPIServer, OpenShiftAPIServer, ClusterVersion, Proxy or SubjectPermission objects." + "documentString": "Managed OpenShift customers may not manage any objects in the following APIGroups [machineconfiguration.openshift.io operator.openshift.io network.openshift.io cloudcredential.openshift.io cloudingress.managed.openshift.io managed.openshift.io splunkforwarder.managed.openshift.io autoscaling.openshift.io config.openshift.io machine.openshift.io admissionregistration.k8s.io addons.managed.openshift.io ocmagent.managed.openshift.io upgrade.managed.openshift.io], nor may Managed OpenShift customers alter the APIServer, KubeAPIServer, OpenShiftAPIServer, ClusterVersion, Proxy or SubjectPermission objects." }, { "webhookName": "scc-validation", @@ -77,7 +93,7 @@ }, { "webhookName": "serviceaccount-validation", - "documentString": "Managed OpenShift Customers may not delete the service accounts under the managed namespaces。" + "documentString": "Managed OpenShift Customers may not create or delete service accounts in managed namespaces (excluding exception namespaces: default, openshift-logging, openshift-user-workload-monitoring, openshift-operators)." }, { "webhookName": "techpreviewnoupgrade-validation", diff --git a/docs/webhooks.json b/docs/webhooks.json index 51764813..22e64a69 100644 --- a/docs/webhooks.json +++ b/docs/webhooks.json @@ -86,6 +86,27 @@ ], "documentString": "Managed OpenShift Customers may not change CustomResourceDefinitions managed by Red Hat." }, + { + "webhookName": "hcpnamespace-validation", + "rules": [ + { + "operations": [ + "DELETE" + ], + "apiGroups": [ + "" + ], + "apiVersions": [ + "*" + ], + "resources": [ + "namespaces" + ], + "scope": "Cluster" + } + ], + "documentString": "Validates HCP namespace deletion operations are only performed by authorized service accounts" + }, { "webhookName": "hiveownership-validation", "rules": [ @@ -113,6 +134,48 @@ }, "documentString": "Managed OpenShift customers may not edit certain managed resources. A managed resource has a \"hive.openshift.io/managed\": \"true\" label." }, + { + "webhookName": "hostedcluster-validation", + "rules": [ + { + "operations": [ + "DELETE" + ], + "apiGroups": [ + "hypershift.openshift.io" + ], + "apiVersions": [ + "*" + ], + "resources": [ + "hostedclusters" + ], + "scope": "Namespaced" + } + ], + "documentString": "Validates HostedCluster deletion operations are only performed by authorized service accounts" + }, + { + "webhookName": "hostedcontrolplane-validation", + "rules": [ + { + "operations": [ + "DELETE" + ], + "apiGroups": [ + "hypershift.openshift.io" + ], + "apiVersions": [ + "*" + ], + "resources": [ + "hostedcontrolplanes" + ], + "scope": "Namespaced" + } + ], + "documentString": "Validates HostedControlPlane deletion operations are only performed by authorized service accounts" + }, { "webhookName": "imagecontentpolicies-validation", "rules": [ @@ -198,6 +261,27 @@ ], "documentString": "Managed OpenShift Customer may create IngressControllers without necessary taints. This can cause those workloads to be provisioned on master nodes." }, + { + "webhookName": "manifestworks-validation", + "rules": [ + { + "operations": [ + "DELETE" + ], + "apiGroups": [ + "work.open-cluster-management.io" + ], + "apiVersions": [ + "*" + ], + "resources": [ + "manifestworks" + ], + "scope": "Namespaced" + } + ], + "documentString": "Validates ManifestWorks deletion operations are only performed by authorized service accounts" + }, { "webhookName": "namespace-validation", "rules": [ @@ -241,7 +325,7 @@ "scope": "Cluster" } ], - "documentString": "Managed OpenShift customers may not modify critical fields in the network.operator CRD (such as spec.migration.networkType) because it can disrupt Cluster Network Operator operations and CNI migrations. Even cluster-admin users are blocked from modifying these critical fields." + "documentString": "Managed OpenShift customers may not modify critical fields in the network.operator CRD (such as spec.migration.networkType) because it can disrupt Cluster Network Operator operations and CNI migrations. Only backplane-cluster-admin, SRE, Cluster Network Operator (CNO), and Managed Upgrade Operator (MUO) service accounts are allowed to modify these critical fields. Regular cluster-admin users (system:admin) are explicitly blocked." }, { "webhookName": "networkpolicies-validation", @@ -498,7 +582,7 @@ "scope": "*" } ], - "documentString": "Managed OpenShift customers may not manage any objects in the following APIGroups [splunkforwarder.managed.openshift.io autoscaling.openshift.io ocmagent.managed.openshift.io upgrade.managed.openshift.io config.openshift.io machineconfiguration.openshift.io operator.openshift.io network.openshift.io cloudcredential.openshift.io machine.openshift.io admissionregistration.k8s.io addons.managed.openshift.io cloudingress.managed.openshift.io managed.openshift.io], nor may Managed OpenShift customers alter the APIServer, KubeAPIServer, OpenShiftAPIServer, ClusterVersion, Proxy or SubjectPermission objects." + "documentString": "Managed OpenShift customers may not manage any objects in the following APIGroups [addons.managed.openshift.io cloudingress.managed.openshift.io splunkforwarder.managed.openshift.io upgrade.managed.openshift.io operator.openshift.io network.openshift.io machine.openshift.io managed.openshift.io ocmagent.managed.openshift.io autoscaling.openshift.io config.openshift.io machineconfiguration.openshift.io cloudcredential.openshift.io admissionregistration.k8s.io], nor may Managed OpenShift customers alter the APIServer, KubeAPIServer, OpenShiftAPIServer, ClusterVersion, Proxy or SubjectPermission objects." }, { "webhookName": "scc-validation", @@ -570,6 +654,7 @@ "rules": [ { "operations": [ + "CREATE", "DELETE" ], "apiGroups": [ @@ -584,7 +669,7 @@ "scope": "Namespaced" } ], - "documentString": "Managed OpenShift Customers may not delete the service accounts under the managed namespaces。" + "documentString": "Managed OpenShift Customers may not create or delete service accounts in managed namespaces (excluding exception namespaces: default, openshift-logging, openshift-user-workload-monitoring, openshift-operators)." }, { "webhookName": "techpreviewnoupgrade-validation", diff --git a/pkg/webhooks/serviceaccount/serviceaccount.go b/pkg/webhooks/serviceaccount/serviceaccount.go index caa9cee9..043d64a1 100644 --- a/pkg/webhooks/serviceaccount/serviceaccount.go +++ b/pkg/webhooks/serviceaccount/serviceaccount.go @@ -21,7 +21,7 @@ import ( const ( WebhookName string = "serviceaccount-validation" - docString string = `Managed OpenShift Customers may not delete the service accounts under the managed namespaces。` + docString string = `Managed OpenShift Customers may not create or delete service accounts in managed namespaces (excluding exception namespaces: default, openshift-logging, openshift-user-workload-monitoring, openshift-operators).` ) var ( @@ -30,7 +30,7 @@ var ( scope = admissionregv1.NamespacedScope rules = []admissionregv1.RuleWithOperations{ { - Operations: []admissionregv1.OperationType{"DELETE"}, + Operations: []admissionregv1.OperationType{"CREATE", "DELETE"}, Rule: admissionregv1.Rule{ APIGroups: []string{""}, APIVersions: []string{"v1"}, @@ -115,6 +115,15 @@ func (s *serviceAccountWebhook) authorized(request admissionctl.Request) admissi } if isProtectedNamespace(request) && !isAllowedUserGroup(request) { + // Block CREATE operations in protected namespaces + if request.Operation == admissionv1.Create { + log.Info(fmt.Sprintf("CREATE operation denied for service account in protected namespace: %v", request.Namespace)) + ret = admissionctl.Denied(fmt.Sprintf("Creating service accounts in managed namespace %v is not allowed", request.Namespace)) + ret.UID = request.AdmissionRequest.UID + return ret + } + + // Block DELETE operations in protected namespaces (existing logic) if request.Operation == admissionv1.Delete && !isAllowedServiceAccount(sa) { log.Info(fmt.Sprintf("Deleting operation detected on proteced serviceaccount: %v", sa.Name)) ret = admissionctl.Denied(fmt.Sprintf("Deleting protected service account under namespace %v is not allowed", request.Namespace)) @@ -134,8 +143,12 @@ func (s *serviceAccountWebhook) renderServiceAccount(request admissionctl.Reques sa := &corev1.ServiceAccount{} var err error + // For DELETE: use OldObject (the object being deleted) + // For CREATE: use Object (the object being created) if len(request.OldObject.Raw) > 0 { err = decoder.DecodeRaw(request.OldObject, sa) + } else if len(request.Object.Raw) > 0 { + err = decoder.Decode(request, sa) } if err != nil { return nil, err diff --git a/pkg/webhooks/serviceaccount/serviceaccount_test.go b/pkg/webhooks/serviceaccount/serviceaccount_test.go index 7d68636c..bb58083f 100644 --- a/pkg/webhooks/serviceaccount/serviceaccount_test.go +++ b/pkg/webhooks/serviceaccount/serviceaccount_test.go @@ -148,3 +148,99 @@ func TestSADeletion(t *testing.T) { } runServiceAccountTests(t, tests) } + +func TestSACreation(t *testing.T) { + tests := []serviceAccountTestSuites{ + { + targetSA: "my-custom-sa", + testID: "user-cannot-create-sa-in-protected-ns", + username: "user1", + userGroups: []string{"system:authenticated", "system:authenticated:oauth"}, + namespace: "openshift-ingress-operator", + operation: admissionv1.Create, + shouldBeAllowed: false, + }, + { + targetSA: "my-custom-sa", + testID: "user-can-create-sa-in-normal-ns", + username: "user1", + userGroups: []string{"system:authenticated", "system:authenticated:oauth"}, + namespace: "my-namespace", + operation: admissionv1.Create, + shouldBeAllowed: true, + }, + { + targetSA: "my-custom-sa", + testID: "user-can-create-sa-in-default-ns", + username: "user1", + userGroups: []string{"system:authenticated", "system:authenticated:oauth"}, + namespace: "default", + operation: admissionv1.Create, + shouldBeAllowed: true, + }, + { + targetSA: "my-custom-sa", + testID: "user-can-create-sa-in-openshift-logging", + username: "user1", + userGroups: []string{"system:authenticated", "system:authenticated:oauth"}, + namespace: "openshift-logging", + operation: admissionv1.Create, + shouldBeAllowed: true, + }, + { + targetSA: "my-custom-sa", + testID: "user-can-create-sa-in-openshift-operators", + username: "user1", + userGroups: []string{"system:authenticated", "system:authenticated:oauth"}, + namespace: "openshift-operators", + operation: admissionv1.Create, + shouldBeAllowed: true, + }, + { + targetSA: "my-custom-sa", + testID: "user-can-create-sa-in-openshift-user-workload-monitoring", + username: "user1", + userGroups: []string{"system:authenticated", "system:authenticated:oauth"}, + namespace: "openshift-user-workload-monitoring", + operation: admissionv1.Create, + shouldBeAllowed: true, + }, + { + targetSA: "my-custom-sa", + testID: "sre-can-create-sa-in-protected-ns", + username: "user1", + userGroups: []string{"system:serviceaccounts:openshift-backplane-srep"}, + namespace: "openshift-ingress-operator", + operation: admissionv1.Create, + shouldBeAllowed: true, + }, + { + targetSA: "my-custom-sa", + testID: "backplane-admin-can-create-sa-in-protected-ns", + username: "backplane-cluster-admin", + userGroups: []string{"system:authenticated"}, + namespace: "openshift-ingress-operator", + operation: admissionv1.Create, + shouldBeAllowed: true, + }, + { + targetSA: "my-custom-sa", + testID: "system-user-can-create-sa-in-protected-ns", + username: "system:serviceaccount:kube-system:controller", + userGroups: []string{"system:serviceaccounts"}, + namespace: "openshift-ingress-operator", + operation: admissionv1.Create, + shouldBeAllowed: true, + }, + { + targetSA: "my-custom-sa", + testID: "kube-user-can-create-sa-in-protected-ns", + username: "kube:admin", + userGroups: []string{"system:authenticated"}, + namespace: "openshift-ingress-operator", + operation: admissionv1.Create, + shouldBeAllowed: true, + }, + } + runServiceAccountTests(t, tests) +}