OCPBUGS-83807: Fix issues in NodeNetworkConfiguration topology view

[pcbailey]

This PR makes the following changes to the NodeNetworkConfiguration topology view.

• Position ovs-interface interfaces above ovs-bridge interfaces
• Position infiniband interfaces at the bottom-most level next to ethernet interfaces
• Position ovs-bridge interfaces on the same level as linux-bridge interfaces
• Hide the following interfaces:
  -- ovn-k8s-mp0
  -- Interfaces starting with genev-sys
  -- br-int

Jira: https://redhat.atlassian.net/browse/OCPBUGS-83807

Screenshots

Before

After

Summary by CodeRabbit

• Refactor
  • Improved network topology rendering to recognize more interface types and hide internal/auxiliary interfaces for clearer node views.
  • Standardized interface configuration handling to ensure more reliable display of bonded and VLAN relationships.
• Tests
  • Updated end-to-end test flow to confirm newly created policies appear in the policies listing.

[openshift-ci-robot]

@pcbailey: This pull request references Jira Issue OCPBUGS-83807, which is invalid:

• expected the bug to target the "5.0.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

This PR makes the following changes to the NodeNetworkConfiguration topology view.

• Position ovs-interface interfaces above ovs-bridge interfaces
• Position infiniband interfaces at the bottom-most level next to ethernet interfaces
• Position ovs-bridge interfaces on the same level as linux-bridge interfaces
• Hide the following interfaces:
  -- ovn-k8s-mp0
  -- Interfaces starting with genev-sys
  -- br-int

Jira: https://redhat.atlassian.net/browse/OCPBUGS-83807

Screenshots

Before

After

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: a5e996b6-139b-40cb-afe9-c8cb551102b3

📥 Commits

Reviewing files that changed from the base of the PR and between 29c4fee and c621164.

📒 Files selected for processing (5)

• cypress/e2e/NewPolicy.spec.cy.ts
• src/utils/constants.ts
• src/views/nodenetworkconfiguration/utils/constants.ts
• src/views/nodenetworkconfiguration/utils/utils.ts
• src/views/policies/list/components/CreatePolicyButtons.tsx

✅ Files skipped from review due to trivial changes (2)

• src/utils/constants.ts
• src/views/nodenetworkconfiguration/utils/constants.ts

🚧 Files skipped from review as they are similar to previous changes (3)

• src/views/policies/list/components/CreatePolicyButtons.tsx
• cypress/e2e/NewPolicy.spec.cy.ts
• src/views/nodenetworkconfiguration/utils/utils.ts

---

Walkthrough

Three files add network/interface-identifying constants and update node-topology creation to use them (including expanded interface-type levels and visibility rules). Separately, a small typing change was made to a policy UI component and an E2E test's post-create assertion was adjusted.

Changes

Network Interface Constants and Topology Enhancement

Layer / File(s)	Summary
Base interface property constants src/utils/constants.ts	Adds LINK_AGGREGATION and BASE_IFACE string constants used to read bond ports and VLAN base interface properties.
OVN and network-specific constants src/views/nodenetworkconfiguration/utils/constants.ts	Adds OVN_K8S_MP0, BR_INT, and GENEV_SYS_PREFIX constants to identify interfaces by name/prefix in OVN/bridge contexts.
Topology levels and node creation src/views/nodenetworkconfiguration/utils/utils.ts	Imports new constants; extends networkTypeLevelMap with OVS and INFINIBAND entries; updates createNodes to (1) treat configured interfaces via the renamed parameter, (2) hide nodes for interfaces matching OVN_K8S_MP0, BR_INT, or names starting with GENEV_SYS_PREFIX, and (3) read bondPorts and vlanBaseInterface via iface[LINK_AGGREGATION]?.port and iface.vlan?.[BASE_IFACE] instead of hardcoded keys.

Policy UI typing change

Layer / File(s)	Summary
Component props typing src/views/policies/list/components/CreatePolicyButtons.tsx	Adds a local CreatePolicyButtonsProps type with children: ReactNode and updates the component signature to FC<CreatePolicyButtonsProps>; updates React import to include ReactNode. Logic unchanged.

E2E test post-create assertion

Layer / File(s)	Summary
Post-create verification update cypress/e2e/NewPolicy.spec.cy.ts	Replaces the previous cy.contains('h1', testPolicyName) verification with a redirect back to the policy list page and cy.contains('a', testPolicyName) to assert the created policy appears in the list.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Test Structure And Quality	⚠️ Warning	Test cleanup not in afterEach (resource leak risk), hardcoded cy.wait(1000) instead of explicit timeouts, assertions lack diagnostic messages, inconsistent with codebase patterns.	Move cleanup to afterEach block, use cy.wait(['@alias'], { timeout: 40000 }), add failure messages to assertions, align with StatusList.spec.cy.ts pattern with fixtures and explicit timeouts.

✅ Passed checks (14 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly references the bug ticket (OCPBUGS-83807) and accurately summarizes the main objective: fixing issues in the NodeNetworkConfiguration topology view.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	This is a TypeScript/React project with no Go files or Ginkgo tests. The check for Ginkgo test name stability is not applicable to this codebase.
Microshift Test Compatibility	✅ Passed	PR contains only Cypress E2E tests and TypeScript components, not Ginkgo tests. Custom check only applies to Ginkgo Go e2e tests, so it is not applicable here.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	This PR does not add new Ginkgo e2e tests. It's a TypeScript/Node.js console plugin project with Cypress e2e tests, not Go tests with Ginkgo. The custom check is not applicable.
Topology-Aware Scheduling Compatibility	✅ Passed	PR modifies only UI constants, topology view utilities, and React components; no deployment manifests, operator code, or scheduling constraints are introduced.
Ote Binary Stdout Contract	✅ Passed	This is a TypeScript/React frontend plugin with no Go code. The custom check for OTE binary stdout contracts is not applicable to this TypeScript codebase with Cypress/Jest tests.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	This is a TypeScript/React console plugin using Cypress tests, not a Go/Ginkgo project. No Ginkgo tests are present in the repository.
No-Weak-Crypto	✅ Passed	PR contains no cryptographic code. Changes are limited to network topology constants, UI logic updates, and TypeScript type definitions.
Container-Privileges	✅ Passed	No privileged container settings found. Kubernetes manifests and Dockerfiles lack privileged: true, hostPID/Network/IPC, SYS_ADMIN, allowPrivilegeEscalation, or root execution without justification.
No-Sensitive-Data-In-Logs	✅ Passed	No logging statements exposing sensitive data. All modified files contain only constants, types, and logic with no logging of passwords, tokens, API keys, or PII.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[pcbailey]

/jira refresh

[openshift-ci-robot]

@pcbailey: This pull request references Jira Issue OCPBUGS-83807, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (5.0.0) matches configured target version for branch (5.0.0)
• bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @lkladnit

Details

In response to this:

/jira refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci-robot]

@pcbailey: This pull request references Jira Issue OCPBUGS-83807, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (5.0.0) matches configured target version for branch (5.0.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @lkladnit

Details

In response to this:

This PR makes the following changes to the NodeNetworkConfiguration topology view.

• Position ovs-interface interfaces above ovs-bridge interfaces
• Position infiniband interfaces at the bottom-most level next to ethernet interfaces
• Position ovs-bridge interfaces on the same level as linux-bridge interfaces
• Hide the following interfaces:
  -- ovn-k8s-mp0
  -- Interfaces starting with genev-sys
  -- br-int

Jira: https://redhat.atlassian.net/browse/OCPBUGS-83807

Screenshots

Before

After

Summary by CodeRabbit

• Refactor
• Enhanced network topology visualization to recognize more interface types and hide internal/auxiliary interfaces (improves clarity of node rendering).
• Standardized handling of interface configuration via shared constants for more reliable display of bond/VLAN relationships.
• Tests
• Updated end-to-end test flow to verify newly created policies from the policies listing.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In `@cypress/e2e/NewPolicy.spec.cy.ts`:
- Around line 52-55: The test visits the list page and asserts the policy link
exists but calls deletePolicyFromDetailsPage(testPolicyName) which expects to be
on the policy details page; update the flow to navigate into the details view
first by clicking the policy link (use the existing selector cy.contains('a',
testPolicyName) and invoke .click() or an equivalent navigation helper) and wait
for the details page to load before calling
deletePolicyFromDetailsPage(testPolicyName) so the details-page modal/actions
are present.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 3ca73ec9-3687-4eba-9f37-690fd8410b7d

📥 Commits

Reviewing files that changed from the base of the PR and between 5ccfd4e and 29c4fee.

📒 Files selected for processing (5)

• cypress/e2e/NewPolicy.spec.cy.ts
• src/utils/constants.ts
• src/views/nodenetworkconfiguration/utils/constants.ts
• src/views/nodenetworkconfiguration/utils/utils.ts
• src/views/policies/list/components/CreatePolicyButtons.tsx

✅ Files skipped from review due to trivial changes (2)

• src/views/nodenetworkconfiguration/utils/constants.ts
• src/utils/constants.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/views/nodenetworkconfiguration/utils/utils.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Navigate to policy details before calling details-page delete helper.

After visiting the list page, the test only checks the link exists, but then calls deletePolicyFromDetailsPage(...), which expects details-page actions/modal elements. This can break cleanup and fail the test.

Suggested fix

-    cy.visit('/k8s/cluster/nmstate.io~v1~NodeNetworkConfigurationPolicy');
-    cy.contains('a', testPolicyName);
+    cy.visit('/k8s/cluster/nmstate.io~v1~NodeNetworkConfigurationPolicy');
+    cy.contains('a', testPolicyName).should('be.visible').click();

     deletePolicyFromDetailsPage(testPolicyName);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	cy.visit('/k8s/cluster/nmstate.io~v1~NodeNetworkConfigurationPolicy');
	cy.contains('a', testPolicyName);
	deletePolicyFromDetailsPage(testPolicyName);
	cy.visit('/k8s/cluster/nmstate.io~v1~NodeNetworkConfigurationPolicy');
	cy.contains('a', testPolicyName).should('be.visible').click();
	deletePolicyFromDetailsPage(testPolicyName);

🤖 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 `@cypress/e2e/NewPolicy.spec.cy.ts` around lines 52 - 55, The test visits the
list page and asserts the policy link exists but calls
deletePolicyFromDetailsPage(testPolicyName) which expects to be on the policy
details page; update the flow to navigate into the details view first by
clicking the policy link (use the existing selector cy.contains('a',
testPolicyName) and invoke .click() or an equivalent navigation helper) and wait
for the details page to load before calling
deletePolicyFromDetailsPage(testPolicyName) so the details-page modal/actions
are present.

[upalatucci]

@pcbailey this needs to be fixed

[coderabbitai]

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

[openshift-merge-bot]

/retest-required

Remaining retests: 0 against base HEAD d7ee05d and 2 for PR HEAD 29c4fee in total

[upalatucci]

/retest

[openshift-merge-bot]

/retest-required

Remaining retests: 0 against base HEAD 77a4491 and 1 for PR HEAD 29c4fee in total

[openshift-merge-bot]

/retest-required

Remaining retests: 0 against base HEAD 11466ba and 0 for PR HEAD 29c4fee in total

[openshift-merge-bot]

/hold

Revision 29c4fee was retested 3 times: holding

[pcbailey]

/hold cancel

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: avivtur, lkladnit, pcbailey

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [avivtur,pcbailey]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-merge-bot]

/retest-required

Remaining retests: 0 against base HEAD 11466ba and 2 for PR HEAD c621164 in total

[pcbailey]

/retest

[ronlavi2412]

/retest

[pcbailey]

/retest

[openshift-merge-bot]

/retest-required

Remaining retests: 0 against base HEAD 0e0f374 and 1 for PR HEAD c621164 in total

[pcbailey]

/retest

[openshift-ci]

@pcbailey: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-tests	c621164	link	true	/test e2e-tests

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.