diff --git a/build/selectorsyncset.yaml b/build/selectorsyncset.yaml index 2b6fd447..926d33de 100644 --- a/build/selectorsyncset.yaml +++ b/build/selectorsyncset.yaml @@ -454,6 +454,36 @@ objects: scope: Cluster sideEffects: None timeoutSeconds: 2 + - apiVersion: admissionregistration.k8s.io/v1 + kind: ValidatingWebhookConfiguration + metadata: + annotations: + service.beta.openshift.io/inject-cabundle: "true" + name: sre-network-operator-validation + webhooks: + - admissionReviewVersions: + - v1 + clientConfig: + service: + name: validation-webhook + namespace: openshift-validation-webhook + path: /network-operator-validation + failurePolicy: Ignore + matchPolicy: Equivalent + name: network-operator-validation.managed.openshift.io + rules: + - apiGroups: + - operator.openshift.io + apiVersions: + - '*' + operations: + - UPDATE + resources: + - network + - networks + scope: Cluster + sideEffects: None + timeoutSeconds: 2 - apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration metadata: diff --git a/pkg/webhooks/networkoperator/networkoperator.go b/pkg/webhooks/networkoperator/networkoperator.go index 5a0abd58..45d81053 100644 --- a/pkg/webhooks/networkoperator/networkoperator.go +++ b/pkg/webhooks/networkoperator/networkoperator.go @@ -18,7 +18,7 @@ import ( const ( WebhookName string = "network-operator-validation" - docString string = `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 and SRE service accounts are allowed to modify these critical fields. Regular cluster-admin users (system:admin) are explicitly blocked.` + docString string = `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.` ) var ( @@ -37,13 +37,16 @@ var ( }, } - // Users allowed to modify critical migration fields - // backplane-cluster-admin and system:admin are allowed + // Users allowed to modify critical migration fields (exact username match). + // Includes backplane-cluster-admin and CNO/MUO service accounts (username format: system:serviceaccount::). allowedUsers = []string{ "backplane-cluster-admin", + "system:serviceaccount:openshift-network-operator:cluster-network-operator", + "system:serviceaccount:openshift-managed-upgrade-operator:managed-upgrade-operator", } - // Groups allowed to modify critical migration fields + // Groups allowed to modify critical migration fields (SRE service accounts only). + // Kubernetes only assigns system:serviceaccounts and system:serviceaccounts:; CNO/MUO are in allowedUsers. sreAdminGroups = []string{ "system:serviceaccounts:openshift-backplane-srep", } @@ -56,7 +59,7 @@ type NetworkOperatorWebhook struct { // Authorized will determine if the request is allowed func (w *NetworkOperatorWebhook) Authorized(request admissionctl.Request) admissionctl.Response { // Block regular cluster-admin users (system:admin) from modifying critical migration fields - // Only backplane-cluster-admin and SRE service accounts are allowed + // Only backplane-cluster-admin, SRE, CNO, and MUO service accounts are allowed if request.Operation == admissionv1.Update { decoder := admissionctl.NewDecoder(&w.s) object := &operatorv1.Network{} @@ -85,7 +88,7 @@ func (w *NetworkOperatorWebhook) Authorized(request admissionctl.Request) admiss "userInfoGroups", request.UserInfo.Groups, ) - // Allow only backplane-cluster-admin and SRE admin groups to modify critical migration fields + // Allow only backplane-cluster-admin, SRE, CNO, and MUO service accounts to modify critical migration fields // Regular cluster-admin (system:admin) is explicitly blocked if isAllowedUserGroup(request) { log.Info("User is allowed to modify critical migration fields") @@ -284,10 +287,10 @@ func (w *NetworkOperatorWebhook) SyncSetLabelSelector() metav1.LabelSelector { return utils.DefaultLabelSelector() } -func (w *NetworkOperatorWebhook) ClassicEnabled() bool { return false } +func (w *NetworkOperatorWebhook) ClassicEnabled() bool { return true } // HypershiftEnabled will return boolean value for hypershift enabled configurations -func (w *NetworkOperatorWebhook) HypershiftEnabled() bool { return false } +func (w *NetworkOperatorWebhook) HypershiftEnabled() bool { return true } // NewWebhook creates a new webhook func NewWebhook() *NetworkOperatorWebhook { diff --git a/pkg/webhooks/networkoperator/networkoperator_test.go b/pkg/webhooks/networkoperator/networkoperator_test.go index 26155d90..6e169640 100644 --- a/pkg/webhooks/networkoperator/networkoperator_test.go +++ b/pkg/webhooks/networkoperator/networkoperator_test.go @@ -2,6 +2,7 @@ package networkoperator import ( "reflect" + "strings" "testing" "github.com/openshift/managed-cluster-validating-webhooks/pkg/webhooks/utils" @@ -418,6 +419,100 @@ func TestAuthorized(t *testing.T) { }, ExpectAllowed: true, }, + { + Name: "Cluster Network Operator (CNO) service account modifying migration.networkType should be allowed", + Request: admissionctl.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + UserInfo: authenticationv1.UserInfo{ + Username: "system:serviceaccount:openshift-network-operator:cluster-network-operator", + Groups: []string{ + "system:serviceaccounts:openshift-network-operator", + }, + }, + Kind: metav1.GroupVersionKind{ + Group: "operator.openshift.io", + Kind: "Network", + }, + Operation: admissionv1.Update, + Object: runtime.RawExtension{ + Raw: []byte(`{ + "apiVersion": "operator.openshift.io/v1", + "kind": "Network", + "metadata": { + "name": "cluster" + }, + "spec": { + "migration": { + "networkType": "OVNKubernetes" + } + } + }`), + }, + OldObject: runtime.RawExtension{ + Raw: []byte(`{ + "apiVersion": "operator.openshift.io/v1", + "kind": "Network", + "metadata": { + "name": "cluster" + }, + "spec": { + "migration": { + "networkType": "OpenShiftSDN" + } + } + }`), + }, + }, + }, + ExpectAllowed: true, + }, + { + Name: "Managed Upgrade Operator (MUO) service account modifying migration.networkType should be allowed", + Request: admissionctl.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + UserInfo: authenticationv1.UserInfo{ + Username: "system:serviceaccount:openshift-managed-upgrade-operator:managed-upgrade-operator", + Groups: []string{ + "system:serviceaccounts:openshift-managed-upgrade-operator", + }, + }, + Kind: metav1.GroupVersionKind{ + Group: "operator.openshift.io", + Kind: "Network", + }, + Operation: admissionv1.Update, + Object: runtime.RawExtension{ + Raw: []byte(`{ + "apiVersion": "operator.openshift.io/v1", + "kind": "Network", + "metadata": { + "name": "cluster" + }, + "spec": { + "migration": { + "networkType": "OVNKubernetes" + } + } + }`), + }, + OldObject: runtime.RawExtension{ + Raw: []byte(`{ + "apiVersion": "operator.openshift.io/v1", + "kind": "Network", + "metadata": { + "name": "cluster" + }, + "spec": { + "migration": { + "networkType": "OpenShiftSDN" + } + } + }`), + }, + }, + }, + ExpectAllowed: true, + }, { Name: "backplane-cluster-admin modifying migration.networkType should be allowed", Request: admissionctl.Request{ @@ -912,6 +1007,12 @@ func TestDoc(t *testing.T) { if len(docs) == 0 { t.Error("TestDoc(): expected content, received none") } + // Doc should mention all allowed identities: backplane-cluster-admin, SRE, CNO, MUO + for _, sub := range []string{"backplane-cluster-admin", "SRE", "Cluster Network Operator", "CNO", "Managed Upgrade Operator", "MUO"} { + if !strings.Contains(docs, sub) { + t.Errorf("TestDoc(): expected doc to contain %q", sub) + } + } } func TestSyncSetLabelSelector(t *testing.T) {