Skip to content

Conversation

@logonoff
Copy link
Member

@logonoff logonoff commented Oct 29, 2025

The plural routes have been deprecated for a while, so let's instead redirect users to the referenceForModel routes

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Oct 29, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 29, 2025

@logonoff: This pull request references CONSOLE-4837 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set.

Details

In response to this:

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 openshift-ci bot requested review from cajieh and spadgett October 29, 2025 13:29
@openshift-ci openshift-ci bot added component/core Related to console core functionality approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 29, 2025
@logonoff
Copy link
Member Author

/label px-approved
/label docs-approved

@openshift-ci openshift-ci bot added px-approved Signifies that Product Support has signed off on this PR docs-approved Signifies that Docs has signed off on this PR labels Oct 29, 2025
@openshift-ci openshift-ci bot added the component/topology Related to topology label Oct 31, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Nov 4, 2025

@logonoff: This pull request references CONSOLE-4837 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set.

Details

In response to this:

The plural routes have been deprecated for a while, so let's instead redirect users to the referenceForModel routes

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.

@logonoff
Copy link
Member Author

logonoff commented Nov 4, 2025

/hold

need to fix e2e

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 4, 2025
@vojtechszocs
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 4, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 4, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: logonoff, vojtechszocs

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:

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

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 8, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 8, 2025

New changes are detected. LGTM label has been removed.

@openshift-ci openshift-ci bot added the kind/cypress Related to Cypress e2e integration testing label Nov 8, 2025
@coderabbitai
Copy link

coderabbitai bot commented Nov 25, 2025

Walkthrough

Resource path construction is refactored across the frontend to use model references with API group/version format (e.g., core~v1~Pod) instead of pluralized resource names (e.g., pods). This affects redirect logic, navigation paths, test URLs, and URL construction utilities.

Changes

Cohort / File(s) Summary
Model reference integration
frontend/packages/container-security/src/components/summary.tsx, frontend/public/components/app-contents.tsx
Added ProjectModel import and updated URL construction to use referenceForModel(ProjectModel) in vulnerability paths and redirects, replacing hardcoded projects path segments.
Cypress test navigation updates
frontend/packages/integration-tests-cypress/support/project.ts, frontend/packages/integration-tests-cypress/tests/app/overview.cy.ts, frontend/packages/integration-tests-cypress/tests/crud/secrets/key-value.cy.ts, frontend/packages/integration-tests-cypress/tests/dashboards/project-dashboard.cy.ts
Updated test navigation paths to use OpenShift Project model reference format (project.openshift.io~v1~Project) instead of generic projects path.
Cypress dashboard inventory updates
frontend/packages/integration-tests-cypress/tests/dashboards/cluster-dashboard.cy.ts, frontend/packages/integration-tests-cypress/tests/dashboards/project-dashboard.cy.ts
Converted inventory card item links from pluralized paths to API group/version format (e.g., Node: /k8s/cluster/nodes/k8s/cluster/core~v1~Node; Deployment: /k8s/ns/${testName}/deployments/k8s/ns/${testName}/apps~v1~Deployment).
Resource path construction logic
frontend/public/components/utils/resource-link.tsx
Simplified resourcePathFromModel to always append referenceForModel(model) instead of conditionally using plural or CRD properties, eliminating branching logic.
Test expectations alignment
frontend/public/components/utils/__tests__/resource-link.spec.ts, frontend/packages/topology/src/components/export-app/__tests__/ExportViewLogButton.spec.tsx
Updated test assertions to expect API group/version resource format in paths (e.g., Pod: core~v1~Pod; ClusterRole: rbac.authorization.k8s.io~v1~ClusterRole).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • frontend/public/components/utils/resource-link.tsx: Verify the simplified logic in resourcePathFromModel correctly handles all edge cases (namespaced vs. cluster-scoped, CRD resources) despite removing conditional branches.
  • Path consistency: Confirm all test assertions and redirect paths use the correct API group/version format across multiple test files and packages.
  • Model reference coverage: Ensure all redirects in app-contents.tsx and security components properly import and apply the ProjectModel reference.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 2243c06 and 9fada58.

📒 Files selected for processing (10)
  • frontend/packages/container-security/src/components/summary.tsx (2 hunks)
  • frontend/packages/integration-tests-cypress/support/project.ts (2 hunks)
  • frontend/packages/integration-tests-cypress/tests/app/overview.cy.ts (1 hunks)
  • frontend/packages/integration-tests-cypress/tests/crud/secrets/key-value.cy.ts (1 hunks)
  • frontend/packages/integration-tests-cypress/tests/dashboards/cluster-dashboard.cy.ts (1 hunks)
  • frontend/packages/integration-tests-cypress/tests/dashboards/project-dashboard.cy.ts (3 hunks)
  • frontend/packages/topology/src/components/export-app/__tests__/ExportViewLogButton.spec.tsx (1 hunks)
  • frontend/public/components/app-contents.tsx (2 hunks)
  • frontend/public/components/utils/__tests__/resource-link.spec.ts (1 hunks)
  • frontend/public/components/utils/resource-link.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

  • frontend/packages/integration-tests-cypress/tests/dashboards/cluster-dashboard.cy.ts
  • frontend/packages/topology/src/components/export-app/__tests__/ExportViewLogButton.spec.tsx
  • frontend/packages/integration-tests-cypress/tests/dashboards/project-dashboard.cy.ts
  • frontend/public/components/utils/resource-link.tsx
  • frontend/packages/integration-tests-cypress/tests/app/overview.cy.ts
  • frontend/packages/integration-tests-cypress/support/project.ts
  • frontend/packages/container-security/src/components/summary.tsx
  • frontend/public/components/utils/__tests__/resource-link.spec.ts
  • frontend/packages/integration-tests-cypress/tests/crud/secrets/key-value.cy.ts
  • frontend/public/components/app-contents.tsx
🧬 Code graph analysis (4)
frontend/packages/container-security/src/components/summary.tsx (1)
frontend/public/models/index.ts (1)
  • ProjectModel (432-445)
frontend/public/components/utils/__tests__/resource-link.spec.ts (1)
frontend/public/models/index.ts (1)
  • ClusterRoleModel (539-552)
frontend/packages/integration-tests-cypress/tests/crud/secrets/key-value.cy.ts (1)
frontend/packages/integration-tests-cypress/support/index.ts (1)
  • testName (92-95)
frontend/public/components/app-contents.tsx (1)
frontend/public/models/index.ts (1)
  • ProjectModel (432-445)
🔇 Additional comments (11)
frontend/packages/topology/src/components/export-app/__tests__/ExportViewLogButton.spec.tsx (1)

46-54: Export log href expectation updated to canonical Pod reference

The new expectation (/k8s/ns/test/core~v1~Pod/test/logs) matches the reference-based routing scheme and keeps the test aligned with the underlying URL construction.

frontend/packages/integration-tests-cypress/support/project.ts (1)

20-42: Project Cypress helpers now target ProjectModel canonical paths

Visiting /k8s/cluster/project.openshift.io~v1~Project and the corresponding detail URL keeps these commands consistent with the reference-based routing and the generic /k8s/cluster/:plural routes.

frontend/packages/container-security/src/components/summary.tsx (1)

9-81: Vulnerability URLs aligned with ProjectModel reference

Using referenceForModel(ProjectModel) in baseVulnListUrl brings project-scoped vulnerability navigation in line with the new canonical cluster project path while preserving existing namespaced vuln details behavior.

frontend/public/components/utils/resource-link.tsx (1)

20-42: resourcePathFromModel simplified to use model reference

Dropping plural/CRD branching and always using referenceForModel(model) produces consistent /k8s/(cluster|ns|all-namespaces)/<ref>[ /name ] URLs and matches the updated routing/tests.

frontend/public/components/app-contents.tsx (1)

34-35: Status/Overview project redirects normalized to ProjectModel reference

Redirect targets now use /k8s/cluster/${referenceForModel(ProjectModel)}/${ns}[ /workloads ], which is consistent with the canonical project resource routing and other refactors in this PR.

Also applies to: 133-143

frontend/public/components/utils/__tests__/resource-link.spec.ts (1)

24-37: Tests updated to assert reference-based resource paths

The revised expectations for ClusterRole and Pod now correctly mirror the unified resourcePathFromModel behavior (cluster vs. namespaced vs. all-namespaces) using the <group>~<version>~<Kind> reference.

frontend/packages/integration-tests-cypress/tests/crud/secrets/key-value.cy.ts (1)

41-47: Secret CRUD tests now select project via canonical ProjectModel URL

Ensuring the project context by visiting /k8s/cluster/project.openshift.io~v1~Project/${testName} aligns this setup step with the new project routing while keeping the existing Workloads > Secrets flow intact.

frontend/packages/integration-tests-cypress/tests/app/overview.cy.ts (1)

57-71: Overview tests updated to use canonical project workloads URL

Navigating to /k8s/cluster/project.openshift.io~v1~Project/${testName}/workloads?view=list in both tests matches the new project workloads route and keeps the overview assertions valid.

frontend/packages/integration-tests-cypress/tests/dashboards/cluster-dashboard.cy.ts (1)

96-105: LGTM! Consistent migration to canonical model references.

All inventory item links have been correctly updated to use the new {group}~{version}~{Kind} format, with appropriate scope paths (cluster vs all-namespaces).

frontend/packages/integration-tests-cypress/tests/dashboards/project-dashboard.cy.ts (2)

39-42: URL assertion updated consistently with navigation changes.

The details page URL correctly uses the new canonical model reference format.


12-12: Navigation path verified and correct.

The Project resource navigation uses the canonical project.openshift.io~v1~Project format, which is generated by referenceForModel(ProjectModel) and aligns with the codebase convention. Line 12 and the details URL assertion (lines 39-42) are consistent with this format.


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

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 26, 2025

@logonoff: The following tests 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-gcp-console 9fada58 link true /test e2e-gcp-console
ci/prow/frontend 9fada58 link true /test frontend

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. component/core Related to console core functionality component/topology Related to topology do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. docs-approved Signifies that Docs has signed off on this PR hacktoberfest-accepted jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. kind/cypress Related to Cypress e2e integration testing px-approved Signifies that Product Support has signed off on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants