From 810e520684f85b3dedaa7ae84988cbd06c4dae06 Mon Sep 17 00:00:00 2001 From: Sairatnam Trinagari Date: Sun, 22 Feb 2026 23:54:31 +0530 Subject: [PATCH 1/5] Fixing the bug by adding service account --- .../networkoperator/networkoperator.go | 10 ++-- .../networkoperator/networkoperator_test.go | 47 +++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/pkg/webhooks/networkoperator/networkoperator.go b/pkg/webhooks/networkoperator/networkoperator.go index 5a0abd58..79f4313a 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 service accounts, and the Cluster Network Operator (CNO) service account are allowed to modify these critical fields. Regular cluster-admin users (system:admin) are explicitly blocked.` ) var ( @@ -43,9 +43,11 @@ var ( "backplane-cluster-admin", } - // Groups allowed to modify critical migration fields + // Groups allowed to modify critical migration fields (SRE and the Cluster Network Operator) sreAdminGroups = []string{ "system:serviceaccounts:openshift-backplane-srep", + // CNO runs in openshift-network-operator and must be able to patch network.operator for CNI migration + "system:serviceaccounts:openshift-network-operator", } ) @@ -284,10 +286,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..d6ae38c1 100644 --- a/pkg/webhooks/networkoperator/networkoperator_test.go +++ b/pkg/webhooks/networkoperator/networkoperator_test.go @@ -418,6 +418,53 @@ 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: "backplane-cluster-admin modifying migration.networkType should be allowed", Request: admissionctl.Request{ From 536325234bcaf237450f23918231ae6f82ee8a69 Mon Sep 17 00:00:00 2001 From: Sairatnam Trinagari Date: Mon, 23 Feb 2026 15:18:11 +0530 Subject: [PATCH 2/5] Adding MOU to the list of exclusions --- .../networkoperator/networkoperator.go | 4 +- .../networkoperator/networkoperator_test.go | 47 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/pkg/webhooks/networkoperator/networkoperator.go b/pkg/webhooks/networkoperator/networkoperator.go index 79f4313a..49e2cd46 100644 --- a/pkg/webhooks/networkoperator/networkoperator.go +++ b/pkg/webhooks/networkoperator/networkoperator.go @@ -43,11 +43,13 @@ var ( "backplane-cluster-admin", } - // Groups allowed to modify critical migration fields (SRE and the Cluster Network Operator) + // Groups allowed to modify critical migration fields (SRE, CNO, and Managed Upgrade Operator) sreAdminGroups = []string{ "system:serviceaccounts:openshift-backplane-srep", // CNO runs in openshift-network-operator and must be able to patch network.operator for CNI migration "system:serviceaccounts:openshift-network-operator", + // Managed Upgrade Operator may need to modify network.operator during upgrades + "system:serviceaccounts:openshift-managed-upgrade-operator", } ) diff --git a/pkg/webhooks/networkoperator/networkoperator_test.go b/pkg/webhooks/networkoperator/networkoperator_test.go index d6ae38c1..f4113832 100644 --- a/pkg/webhooks/networkoperator/networkoperator_test.go +++ b/pkg/webhooks/networkoperator/networkoperator_test.go @@ -465,6 +465,53 @@ func TestAuthorized(t *testing.T) { }, ExpectAllowed: true, }, + { + Name: "Managed Upgrade Operator 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{ From f2c7a605de1153d6de82588ce7ec3bc44f9d3f60 Mon Sep 17 00:00:00 2001 From: Sairatnam Trinagari Date: Tue, 24 Feb 2026 08:56:46 +0530 Subject: [PATCH 3/5] Minor changes, rebulit syncset --- build/selectorsyncset.yaml | 30 +++++++++++++++++++ .../networkoperator/networkoperator.go | 8 ++--- 2 files changed, 34 insertions(+), 4 deletions(-) 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 49e2cd46..f61558ca 100644 --- a/pkg/webhooks/networkoperator/networkoperator.go +++ b/pkg/webhooks/networkoperator/networkoperator.go @@ -47,9 +47,9 @@ var ( sreAdminGroups = []string{ "system:serviceaccounts:openshift-backplane-srep", // CNO runs in openshift-network-operator and must be able to patch network.operator for CNI migration - "system:serviceaccounts:openshift-network-operator", + "system:serviceaccounts:openshift-network-operator:cluster-network-operator", // Managed Upgrade Operator may need to modify network.operator during upgrades - "system:serviceaccounts:openshift-managed-upgrade-operator", + "system:serviceaccounts:openshift-managed-upgrade-operator:managed-upgrade-operator", } ) @@ -60,7 +60,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{} @@ -89,7 +89,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") From b267837cc1f292a4b92e6eba881e899b82bcef12 Mon Sep 17 00:00:00 2001 From: Sairatnam Trinagari Date: Tue, 24 Feb 2026 09:05:29 +0530 Subject: [PATCH 4/5] Fixing test cases --- pkg/webhooks/networkoperator/networkoperator.go | 2 +- pkg/webhooks/networkoperator/networkoperator_test.go | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/pkg/webhooks/networkoperator/networkoperator.go b/pkg/webhooks/networkoperator/networkoperator.go index f61558ca..7005d5bb 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, SRE service accounts, and the Cluster Network Operator (CNO) service account 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 ( diff --git a/pkg/webhooks/networkoperator/networkoperator_test.go b/pkg/webhooks/networkoperator/networkoperator_test.go index f4113832..b1b3680a 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" @@ -426,6 +427,7 @@ func TestAuthorized(t *testing.T) { Username: "system:serviceaccount:openshift-network-operator:cluster-network-operator", Groups: []string{ "system:serviceaccounts:openshift-network-operator", + "system:serviceaccounts:openshift-network-operator:cluster-network-operator", }, }, Kind: metav1.GroupVersionKind{ @@ -466,13 +468,14 @@ func TestAuthorized(t *testing.T) { ExpectAllowed: true, }, { - Name: "Managed Upgrade Operator service account modifying migration.networkType should be allowed", + 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", + "system:serviceaccounts:openshift-managed-upgrade-operator:managed-upgrade-operator", }, }, Kind: metav1.GroupVersionKind{ @@ -1006,6 +1009,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) { From 2d91c2307432a8afec5f24349e505b5d2ab89a48 Mon Sep 17 00:00:00 2001 From: Sairatnam Trinagari Date: Tue, 24 Feb 2026 12:39:56 +0530 Subject: [PATCH 5/5] Making changes as per review --- pkg/webhooks/networkoperator/networkoperator.go | 13 ++++++------- .../networkoperator/networkoperator_test.go | 2 -- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/pkg/webhooks/networkoperator/networkoperator.go b/pkg/webhooks/networkoperator/networkoperator.go index 7005d5bb..45d81053 100644 --- a/pkg/webhooks/networkoperator/networkoperator.go +++ b/pkg/webhooks/networkoperator/networkoperator.go @@ -37,19 +37,18 @@ 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 (SRE, CNO, and Managed Upgrade Operator) + // 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", - // CNO runs in openshift-network-operator and must be able to patch network.operator for CNI migration - "system:serviceaccounts:openshift-network-operator:cluster-network-operator", - // Managed Upgrade Operator may need to modify network.operator during upgrades - "system:serviceaccounts:openshift-managed-upgrade-operator:managed-upgrade-operator", } ) diff --git a/pkg/webhooks/networkoperator/networkoperator_test.go b/pkg/webhooks/networkoperator/networkoperator_test.go index b1b3680a..6e169640 100644 --- a/pkg/webhooks/networkoperator/networkoperator_test.go +++ b/pkg/webhooks/networkoperator/networkoperator_test.go @@ -427,7 +427,6 @@ func TestAuthorized(t *testing.T) { Username: "system:serviceaccount:openshift-network-operator:cluster-network-operator", Groups: []string{ "system:serviceaccounts:openshift-network-operator", - "system:serviceaccounts:openshift-network-operator:cluster-network-operator", }, }, Kind: metav1.GroupVersionKind{ @@ -475,7 +474,6 @@ func TestAuthorized(t *testing.T) { Username: "system:serviceaccount:openshift-managed-upgrade-operator:managed-upgrade-operator", Groups: []string{ "system:serviceaccounts:openshift-managed-upgrade-operator", - "system:serviceaccounts:openshift-managed-upgrade-operator:managed-upgrade-operator", }, }, Kind: metav1.GroupVersionKind{