Skip to content

OCPBUGS-87991: validate additionalNetworks name format in KubeVirt NodePools #8710

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chdeshpa-hue wants to merge 1 commit into
base: main
Choose a base branch
from
Open

OCPBUGS-87991: validate additionalNetworks name format in KubeVirt NodePools #8710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion api/hypershift/v1beta1/kubevirt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ type KubevirtNodePoolPlatform struct {
// additionalNetworks specify the extra networks attached to the nodes
//
// +optional
// +listType=map
// +listMapKey=name
// +kubebuilder:validation:MaxItems=20
AdditionalNetworks []KubevirtNetwork `json:"additionalNetworks,omitempty"`

Expand Down Expand Up @@ -199,7 +201,8 @@ type KubevirtNetwork struct {
// name specify the network attached to the nodes
// it is a value with the format "[namespace]/[name]" to reference the
// multus network attachment definition
// +kubebuilder:validation:MaxLength=255
// +kubebuilder:validation:MaxLength=55
// +kubebuilder:validation:XValidation:rule="self.matches('^[^/\\\\s]+/[^/\\\\s]+$')",message="name must be in the format <namespace>/<name> to reference a multus network attachment definition"
Comment on lines +204 to +205

@coderabbitai coderabbitai Bot Jun 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add envtest coverage for the new CEL/list-map validation contract.

This introduces admission-time validation behavior (XValidation) and schema semantics (listType=map/listMapKey=name), but no corresponding envtest evidence is present in the provided changes.

As per coding guidelines, "All API CEL validations must be covered with envtests, see test/envtest/README.md for details."

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@api/hypershift/v1beta1/kubevirt.go` around lines 204 - 205, The XValidation
rule introduced in api/hypershift/v1beta1/kubevirt.go for validating the multus
network attachment definition reference format lacks corresponding envtest
coverage. Following the coding guidelines that require all API CEL validations
to be covered with envtests, add envtest cases to validate both valid and
invalid inputs for the format constraint that ensures the field matches the
pattern for namespace/name format. Refer to test/envtest/README.md for guidance
on structuring these tests, and ensure the tests verify that the validation rule
correctly accepts properly formatted namespace/name references and rejects
improperly formatted ones.

Source: Coding guidelines

// +required
Name string `json:"name"`
}
Expand Down
9 changes: 8 additions & 1 deletion ...z_generated.featuregated-crd-manifests/nodepools.hypershift.openshift.io/AAA_ungated.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1082,13 +1082,20 @@ spec:
name specify the network attached to the nodes
it is a value with the format "[namespace]/[name]" to reference the
multus network attachment definition
maxLength: 255
maxLength: 55
type: string
x-kubernetes-validations:
- message: name must be in the format <namespace>/<name>
to reference a multus network attachment definition
rule: self.matches('^[^/\\s]+/[^/\\s]+$')
required:
- name
type: object
maxItems: 20
type: array
x-kubernetes-list-map-keys:
- name
x-kubernetes-list-type: map
attachDefaultNetwork:
default: true
description: |-
Expand Down
9 changes: 8 additions & 1 deletion ...z_generated.featuregated-crd-manifests/nodepools.hypershift.openshift.io/GCPPlatform.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1349,13 +1349,20 @@ spec:
name specify the network attached to the nodes
it is a value with the format "[namespace]/[name]" to reference the
multus network attachment definition
maxLength: 255
maxLength: 55
type: string
x-kubernetes-validations:
- message: name must be in the format <namespace>/<name>
to reference a multus network attachment definition
rule: self.matches('^[^/\\s]+/[^/\\s]+$')
required:
- name
type: object
maxItems: 20
type: array
x-kubernetes-list-map-keys:
- name
x-kubernetes-list-type: map
attachDefaultNetwork:
default: true
description: |-
Expand Down
9 changes: 8 additions & 1 deletion .../zz_generated.featuregated-crd-manifests/nodepools.hypershift.openshift.io/OSStreams.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1115,13 +1115,20 @@ spec:
name specify the network attached to the nodes
it is a value with the format "[namespace]/[name]" to reference the
multus network attachment definition
maxLength: 255
maxLength: 55
type: string
x-kubernetes-validations:
- message: name must be in the format <namespace>/<name>
to reference a multus network attachment definition
rule: self.matches('^[^/\\s]+/[^/\\s]+$')
required:
- name
type: object
maxItems: 20
type: array
x-kubernetes-list-map-keys:
- name
x-kubernetes-list-type: map
attachDefaultNetwork:
default: true
description: |-
Expand Down
9 changes: 8 additions & 1 deletion .../zz_generated.featuregated-crd-manifests/nodepools.hypershift.openshift.io/OpenStack.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1082,13 +1082,20 @@ spec:
name specify the network attached to the nodes
it is a value with the format "[namespace]/[name]" to reference the
multus network attachment definition
maxLength: 255
maxLength: 55
type: string
x-kubernetes-validations:
- message: name must be in the format <namespace>/<name>
to reference a multus network attachment definition
rule: self.matches('^[^/\\s]+/[^/\\s]+$')
required:
- name
type: object
maxItems: 20
type: array
x-kubernetes-list-map-keys:
- name
x-kubernetes-list-type: map
attachDefaultNetwork:
default: true
description: |-
Expand Down
269 changes: 269 additions & 0 deletions ...operator/tests/nodepools.hypershift.openshift.io/stable.nodepools.kubevirt.testsuite.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,269 @@
apiVersion: apiextensions.k8s.io/v1
name: "NodePool KubeVirt additionalNetworks validation"
crdName: nodepools.hypershift.openshift.io
version: v1beta1
tests:
onCreate:
# --- additionalNetworks name format validation ---
- name: when additionalNetworks name is valid namespace/name format it should pass
initial: |
apiVersion: hypershift.openshift.io/v1beta1
kind: NodePool
spec:
arch: amd64
clusterName: some-cluster
management:
autoRepair: false
upgradeType: Replace
release:
image: quay.io/openshift-release-dev/ocp-release:4.17.0-rc.0-x86_64
replicas: 0
platform:
kubevirt:
rootVolume:
type: Persistent
persistent:
size: 32Gi
additionalNetworks:
- name: "my-ns/my-nad"
type: KubeVirt

- name: when additionalNetworks has multiple valid networks it should pass
initial: |
apiVersion: hypershift.openshift.io/v1beta1
kind: NodePool
spec:
arch: amd64
clusterName: some-cluster
management:
autoRepair: false
upgradeType: Replace
release:
image: quay.io/openshift-release-dev/ocp-release:4.17.0-rc.0-x86_64
replicas: 0
platform:
kubevirt:
rootVolume:
type: Persistent
persistent:
size: 32Gi
additionalNetworks:
- name: "ns-a/nad-1"
- name: "ns-b/nad-2"
type: KubeVirt

- name: when additionalNetworks name has no slash it should fail
initial: |
apiVersion: hypershift.openshift.io/v1beta1
kind: NodePool
spec:
arch: amd64
clusterName: some-cluster
management:
autoRepair: false
upgradeType: Replace
release:
image: quay.io/openshift-release-dev/ocp-release:4.17.0-rc.0-x86_64
replicas: 0
platform:
kubevirt:
rootVolume:
type: Persistent
persistent:
size: 32Gi
additionalNetworks:
- name: "just-a-name"
type: KubeVirt
expectedError: "name must be in the format <namespace>/<name> to reference a multus network attachment definition"

- name: when additionalNetworks name has multiple slashes it should fail
initial: |
apiVersion: hypershift.openshift.io/v1beta1
kind: NodePool
spec:
arch: amd64
clusterName: some-cluster
management:
autoRepair: false
upgradeType: Replace
release:
image: quay.io/openshift-release-dev/ocp-release:4.17.0-rc.0-x86_64
replicas: 0
platform:
kubevirt:
rootVolume:
type: Persistent
persistent:
size: 32Gi
additionalNetworks:
- name: "ns/sub/name"
type: KubeVirt
expectedError: "name must be in the format <namespace>/<name> to reference a multus network attachment definition"

- name: when additionalNetworks name has empty namespace it should fail
initial: |
apiVersion: hypershift.openshift.io/v1beta1
kind: NodePool
spec:
arch: amd64
clusterName: some-cluster
management:
autoRepair: false
upgradeType: Replace
release:
image: quay.io/openshift-release-dev/ocp-release:4.17.0-rc.0-x86_64
replicas: 0
platform:
kubevirt:
rootVolume:
type: Persistent
persistent:
size: 32Gi
additionalNetworks:
- name: "/my-nad"
type: KubeVirt
expectedError: "name must be in the format <namespace>/<name> to reference a multus network attachment definition"

- name: when additionalNetworks name has empty name segment it should fail
initial: |
apiVersion: hypershift.openshift.io/v1beta1
kind: NodePool
spec:
arch: amd64
clusterName: some-cluster
management:
autoRepair: false
upgradeType: Replace
release:
image: quay.io/openshift-release-dev/ocp-release:4.17.0-rc.0-x86_64
replicas: 0
platform:
kubevirt:
rootVolume:
type: Persistent
persistent:
size: 32Gi
additionalNetworks:
- name: "my-ns/"
type: KubeVirt
expectedError: "name must be in the format <namespace>/<name> to reference a multus network attachment definition"

- name: when additionalNetworks name has whitespace in namespace it should fail
initial: |
apiVersion: hypershift.openshift.io/v1beta1
kind: NodePool
spec:
arch: amd64
clusterName: some-cluster
management:
autoRepair: false
upgradeType: Replace
release:
image: quay.io/openshift-release-dev/ocp-release:4.17.0-rc.0-x86_64
replicas: 0
platform:
kubevirt:
rootVolume:
type: Persistent
persistent:
size: 32Gi
additionalNetworks:
- name: " /name"
type: KubeVirt
expectedError: "name must be in the format <namespace>/<name> to reference a multus network attachment definition"

- name: when additionalNetworks name has whitespace in name segment it should fail
initial: |
apiVersion: hypershift.openshift.io/v1beta1
kind: NodePool
spec:
arch: amd64
clusterName: some-cluster
management:
autoRepair: false
upgradeType: Replace
release:
image: quay.io/openshift-release-dev/ocp-release:4.17.0-rc.0-x86_64
replicas: 0
platform:
kubevirt:
rootVolume:
type: Persistent
persistent:
size: 32Gi
additionalNetworks:
- name: "my-ns/my nad"
type: KubeVirt
expectedError: "name must be in the format <namespace>/<name> to reference a multus network attachment definition"

- name: when additionalNetworks has duplicate names it should fail
initial: |
apiVersion: hypershift.openshift.io/v1beta1
kind: NodePool
spec:
arch: amd64
clusterName: some-cluster
management:
autoRepair: false
upgradeType: Replace
release:
image: quay.io/openshift-release-dev/ocp-release:4.17.0-rc.0-x86_64
replicas: 0
platform:
kubevirt:
rootVolume:
type: Persistent
persistent:
size: 32Gi
additionalNetworks:
- name: "ns/nad1"
- name: "ns/nad1"
type: KubeVirt
expectedError: "Duplicate value"

- name: when additionalNetworks name is at max length 55 it should pass
initial: |
apiVersion: hypershift.openshift.io/v1beta1
kind: NodePool
spec:
arch: amd64
clusterName: some-cluster
management:
autoRepair: false
upgradeType: Replace
release:
image: quay.io/openshift-release-dev/ocp-release:4.17.0-rc.0-x86_64
replicas: 0
platform:
kubevirt:
rootVolume:
type: Persistent
persistent:
size: 32Gi
additionalNetworks:
- name: "my-namespace-name-that-is-long/my-nad-name-also-long-ab"
type: KubeVirt

- name: when additionalNetworks name exceeds max length 55 it should fail
initial: |
apiVersion: hypershift.openshift.io/v1beta1
kind: NodePool
spec:
arch: amd64
clusterName: some-cluster
management:
autoRepair: false
upgradeType: Replace
release:
image: quay.io/openshift-release-dev/ocp-release:4.17.0-rc.0-x86_64
replicas: 0
platform:
kubevirt:
rootVolume:
type: Persistent
persistent:
size: 32Gi
additionalNetworks:
- name: "my-namespace-name-that-is-long/my-nad-name-also-long-abc"
type: KubeVirt
expectedError: "Too long"
9 changes: 8 additions & 1 deletion ...ts/crds/hypershift-operator/zz_generated.crd-manifests/nodepools-CustomNoUpgrade.crd.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading