Conversation
📝 WalkthroughWalkthroughThis PR introduces namespace-scoped Overview page functionality with smart RBAC-based routing, refactors CI/CD pipelines to use Docker multi-platform builds, adds configuration and metrics query utilities, and migrates React Router navigation patterns to v5-compat, with comprehensive e2e test coverage for namespace-aware access patterns. Changes
Sequence Diagram(s)sequenceDiagram
actor User as Namespace-Scoped User
participant Browser
participant Component as KuadrantOverviewPage
participant K8sAPI as Kubernetes API
participant AuthChecker as Access Checker
participant Config as Config Loader
participant Prometheus
User->>Browser: Navigate to /kuadrant/overview
Browser->>Component: Mount component
Component->>Config: fetchConfig()
Config-->>Component: KuadrantConfig (namespace, metrics suffix)
Component->>AuthChecker: checkAccess(gateways/list, undefined)
AuthChecker->>K8sAPI: Check cluster-wide permission
K8sAPI-->>AuthChecker: Permission denied
AuthChecker-->>Component: isAllowed = false
Component->>Browser: navigate(/kuadrant/ns/default/overview)
Browser->>Component: Re-mount with ns param
Component->>Component: watchNamespace = 'default'
Component->>K8sAPI: Watch Gateways in 'default' namespace
K8sAPI-->>Component: Gateway resources (scoped)
Component->>Prometheus: buildTotalRequestsQuery('default')
Prometheus-->>Component: Metrics data (filtered)
Component->>Browser: Render overview (namespace-scoped)
User->>Browser: Select different namespace (kuadrant-test)
Browser->>Component: handleNamespaceChange('kuadrant-test')
Component->>Browser: navigate(/kuadrant/ns/kuadrant-test/overview)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
I think this might be the first cherry picked release of console plugin, at least since 0.3, every v0.3.x release was tagged directly on main. Looking at how kuadrant-operator does it: one branch per minor (release-v1.4), and patch releases are PRs into that branch. So I created a release-v0.3 branch, meant to be the long-lived branch for all 0.3.x patches. Let me know if that makes sense and if you agree. |
Bump ajv from 6.12.6 to 6.14.0 Signed-off-by: Thomas Maas <thomas@webtypes.com>
Compatible Navigation with OpenShift Console plugin Signed-off-by: Thomas Maas <thomas@webtypes.com>
…-the-connectivity-link-release-notes-and-cert-manager-operator-pages-under-getting-started-resources-are-not-found 324 on ocp versions 420 and 421 the connectivity link release notes and cert manager operator pages under getting started resources are not found Signed-off-by: Thomas Maas <thomas@webtypes.com>
dev image build: latest and git sha tag sharing digest Signed-off-by: Thomas Maas <thomas@webtypes.com>
…ility image build add s390x to dev and release multi arch builds Signed-off-by: Thomas Maas <thomas@webtypes.com>
Fix metrics Signed-off-by: Thomas Maas <thomas@webtypes.com>
🔨 🔧 KuadrantPoliciesPage.tsx: fix page refresh Signed-off-by: Thomas Maas <thomas@webtypes.com>
missing i18n keys Signed-off-by: Thomas Maas <thomas@webtypes.com>
chore(deps): fix CVE-2026-4800 (lodash) and CVE-2026-29063 (immutable) Signed-off-by: Thomas Maas <thomas@webtypes.com>
Gh 298 Signed-off-by: Thomas Maas <thomas@webtypes.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Maas <thomas@webtypes.com>
|
there is a little technical issue here. You cherry-picked "Merge pull request" commits to the patch branch and those are not signed off, so the DCO check is failing. |
thomasmaas
force-pushed
the
patch-v0.3.7
branch
from
5dc7724 to
6abed8e
Compare
I am more in favor of moving forward to the next minor release (v0.4.0) than keeping the release-v0.3 branch for the console plugin. The kuadrant operator represents a full product and should maintain minor release branches. However, for dependencies, like the console plugin, moving forward seems easier to manage. Anyway, it's not a big deal to me. You are cherry-picking, which is fine and the next minor release will contain those fixes as well. |
thomasmaas
left a comment
thomasmaas
left a comment
There was a problem hiding this comment.
Cherry-pick conflict resolutions
Two cherry-picks had merge conflicts. Here's how they were resolved:
1. PR #329 (Fix metrics) — kuadrant-dev-setup/ files
Three files (kuadrant-dev-setup/Makefile, kuadrant-dev-setup/kuadrant/kuadrant-instance.yaml, kuadrant-dev-setup/kuadrant/subscription.yaml) were modified by #329 but don't exist on the release branch — they were introduced by #330 (kuadrant dev setup), which is not included in this patch. Resolution: removed the conflicting files. The kuadrant-dev-setup/cluster-monitoring.yaml file from #329 was kept as it's a new file.
2. PR #352 (Namespace-scoped overview) — src/components/KuadrantOverviewPage.tsx
Two identical conflicts at the ResourceList components for Gateways (line ~993) and HTTPRoutes (line ~1143):
- HEAD had:
namespace="#ALL_NS#"andemtpyResourceName(typo) - Incoming had:
namespace={watchNamespace}andemptyResourceName(typo fixed)
Resolution: took the incoming version in both cases — watchNamespace is the whole point of #352 (namespace scoping), and emptyResourceName is the correct spelling.
|
@eguzki I see how for a single active minor, tagging from main is simpler but we're moving to a place where we will need to do patch releases on multiple minors. Eg once kuadrant-operator v1.5.0 ships with console-plugin v0.4.x, any CVE or bug fix needed in v1.4.x requires a console-plugin v0.3.x patch. Plus it would be nice to have all repo's behave the same when it comes to releases; less surprises. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/dnspolicy/useDNSPolicyActions.tsx (1)
86-86:⚠️ Potential issue | 🔴 CriticalReplace undefined
historywithnavigatein dependencies.Line 86 references
historyin the dependency array, buthistoryis not imported. The component usesnavigatefrom theuseNavigatehook (line 18) for route navigation (lines 71–73), sonavigateshould be in the dependencies instead.Fix
- }, [history, obj, dnsPolicyModel, launchAnnotationsModal, launchDeleteModal, launchLabelsModal]); + }, [navigate, obj, dnsPolicyModel, launchAnnotationsModal, launchDeleteModal, launchLabelsModal]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dnspolicy/useDNSPolicyActions.tsx` at line 86, The dependency array for the hook includes an undefined symbol `history`; replace it with the `navigate` function returned by `useNavigate` so React correctly tracks changes — locate the hook whose dependency array contains `history` (in src/components/dnspolicy/useDNSPolicyActions.tsx) and update the dependencies to include `navigate` (the value from `useNavigate`) instead of `history`; ensure any references inside the effect/callback use the `navigate` identifier so ESLint/react-hooks no longer warn.src/components/tlspolicy/useTLSPolicyActions.tsx (1)
86-86:⚠️ Potential issue | 🔴 CriticalReplace undefined
historydependency withnavigate.Line 86 references
historyin the dependency array, but this variable is never imported or defined. The function usesnavigate(fromuseNavigatehook) at lines 71–73, sonavigatemust be included in the dependencies instead.Proposed fix
- }, [history, obj, tlsPolicyModel, launchAnnotationsModal, launchDeleteModal, launchLabelsModal]); + }, [navigate, obj, tlsPolicyModel, launchAnnotationsModal, launchDeleteModal, launchLabelsModal]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/tlspolicy/useTLSPolicyActions.tsx` at line 86, The dependency array for the effect in useTLSPolicyActions incorrectly lists an undefined symbol history; replace history with the actual navigate function from the useNavigate hook: update the dependency array in the effect that references navigate (in useTLSPolicyActions) to include navigate instead of history, ensuring the effect lists navigate, obj, tlsPolicyModel, launchAnnotationsModal, launchDeleteModal, and launchLabelsModal so React's hook dependencies are accurate.src/components/KuadrantPoliciesPage.tsx (1)
399-402:⚠️ Potential issue | 🟠 MajorHooks called inside a loop – Rules of Hooks violation.
useResourceRBACcallsuseAccessReviewat the top level, butuseResourceRBACitself is invoked inside areducecallback at lines 399–402. React hooks cannot be called from within callbacks, loops, or conditions. AlthoughpolicyKindsis currently stable (derived from a constantRESOURCESregistry), this pattern violates React's Rules of Hooks and is fragile. If code changes in the future to alter the number of policy kinds dynamically, this will cause runtime errors.Call
useResourceRBACat the component's top level for each known policy kind, or refactor to extract the hook calls into a separate custom hook that doesn't depend on a dynamic array length.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/KuadrantPoliciesPage.tsx` around lines 399 - 402, The current implementation calls useResourceRBAC inside a reduce over policyKinds which violates the Rules of Hooks (useResourceRBAC calls useAccessReview), so refactor so hook calls are at the component top level: either explicitly call useResourceRBAC once per known policy kind (e.g., call useResourceRBAC(PolicyKindA, nsForCheck), useResourceRBAC(PolicyKindB, nsForCheck), etc.) and assemble resourceRBAC from those results, or create a new custom hook (e.g., useResourceRBACMap) that internally calls useAccessReview a fixed number of times for each known entry in RESOURCES and returns the RBACMap; ensure the number and order of hook calls is constant and no hooks are invoked inside loops or callbacks.
🧹 Nitpick comments (4)
src/utils/configLoader.ts (1)
32-34: Acknowledge static analysis warning — trusted source context.The static analysis flagged the
innerHTMLassignment as a potential XSS vector. In this case, the script is fetched from/api/plugins/kuadrant-console-plugin/config.js, which is a same-origin, deployment-controlled resource generated byentrypoint.shfrom environment variables—not user input.While the current implementation is safe given the trusted source, consider using a safer alternative for future maintainability:
🔧 Optional: Use script.src instead of innerHTML
- const script = await response.text(); - - const configScript = document.createElement('script'); - configScript.innerHTML = script; - document.head.appendChild(configScript); + // Create script element with src attribute instead of innerHTML + const configScript = document.createElement('script'); + configScript.src = '/api/plugins/kuadrant-console-plugin/config.js'; + + await new Promise<void>((resolve, reject) => { + configScript.onload = () => resolve(); + configScript.onerror = () => reject(new Error('Failed to load config.js')); + document.head.appendChild(configScript); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/configLoader.ts` around lines 32 - 34, The static analyzer warns about assigning to configScript.innerHTML (created via document.createElement('script')) as a potential XSS; since the script comes from a trusted same‑origin endpoint, you can eliminate the warning and be safer for future changes by loading it via a src URL instead of injecting raw HTML: replace the configScript.innerHTML = script assignment with setting configScript.src to the trusted endpoint ('/api/plugins/kuadrant-console-plugin/config.js'), attach onload/onerror handlers if needed, and then document.head.appendChild(configScript) (adjust any code that currently relies on the inline string).e2e/tests/rbac.spec.ts (2)
500-504: Fixed timeout for stability check may be unnecessary.The second URL assertion with
waitForTimeout(2000)to verify "URL doesn't revert" could be replaced with a more targeted check or removed if the first assertion is sufficient.♻️ Suggested improvement
If verifying URL stability is important, consider using
page.waitForLoadState('networkidle')instead of a fixed timeout:- // verify URL doesn't revert back to cluster-wide view - await page.waitForTimeout(2000); + // verify URL remains stable after any async operations + await page.waitForLoadState('networkidle'); await expect(page).toHaveURL('/kuadrant/ns/kuadrant-test/overview', { timeout: 5_000, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/tests/rbac.spec.ts` around lines 500 - 504, The fixed 2s pause using page.waitForTimeout(2000) followed by expect(page).toHaveURL('/kuadrant/ns/kuadrant-test/overview') is unnecessary and brittle; either remove the second stability check if the initial expect(page).toHaveURL(...) already asserts the target URL, or replace the waitForTimeout call with a targeted wait such as await page.waitForLoadState('networkidle') before calling expect(page).toHaveURL to ensure network/activity quiescence; update the code around page.waitForTimeout and expect(page).toHaveURL accordingly.
470-493: Consider replacingwaitForTimeoutwith explicit waits.Using fixed timeouts (
waitForTimeout(1000),waitForTimeout(2000)) can lead to flaky tests. Prefer waiting for specific conditions.♻️ Suggested improvement
// click to open dropdown await namespaceButton.click(); - await page.waitForTimeout(1000); + // Wait for dropdown to be visible + await page.waitForSelector('[role="listbox"], [role="menu"], .pf-v6-c-menu', { state: 'visible' }); // ... select namespace ... await namespaceOption.click(); - await page.waitForTimeout(2000); + // Wait for navigation to complete + await page.waitForURL('/kuadrant/ns/kuadrant-test/overview', { timeout: 15_000 });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/tests/rbac.spec.ts` around lines 470 - 493, Replace the fixed sleeps around namespace selection with explicit waits: after clicking namespaceButton, wait for the dropdown/options to appear using a selector from namespaceOptionSelectors via page.waitForSelector or await page.locator(selector).waitFor({ state: 'visible' }); and before clicking namespaceOption, wait for that specific locator's visibility or be enabled (e.g., await namespaceOption.waitFor({ state: 'visible' }) or waitForElementState('stable') ) instead of page.waitForTimeout(1000/2000); keep the existing namespaceButton, namespaceOptionSelectors, namespaceOption identifiers so the logic of choosing the first visible option remains the same.src/components/KuadrantPoliciesPage.tsx (1)
481-485: Consider memoizing the context value.The
contextValueobject is recreated on every render, which could cause unnecessary re-renders of context consumers. Consider usinguseMemoto stabilise the reference.♻️ Suggested improvement
- const contextValue = { - activeNamespace, - defaultColumns, - resourceRBAC, - }; + const contextValue = React.useMemo( + () => ({ + activeNamespace, + defaultColumns, + resourceRBAC, + }), + [activeNamespace, defaultColumns, resourceRBAC] + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/KuadrantPoliciesPage.tsx` around lines 481 - 485, The contextValue object in KuadrantPoliciesPage (currently built from activeNamespace, defaultColumns, resourceRBAC) is recreated every render causing unnecessary context consumer updates; wrap its creation in React.useMemo and return a memoized object keyed on [activeNamespace, defaultColumns, resourceRBAC] so the reference is stable across renders—update the code that declares contextValue to use useMemo inside the KuadrantPoliciesPage component.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/dev-build.yaml:
- Around line 27-32: The "Login to container registry" step currently uses
docker/login-action@v2 and runs unconditionally; change it to use
docker/login-action@v3 (or v4) and add the same guard as the build/push step
(if: github.repository_owner == 'kuadrant') so the login only runs for the
kuadrant repository and avoids failing on forks without registry secrets; update
the step reference (the step name "Login to container registry" and the action
reference docker/login-action@v2) accordingly.
In `@CLAUDE.md`:
- Line 96: Fix the minor typo in the sentence containing "Topology
andPrometheus" by inserting a space so it reads "Topology and Prometheus";
locate the sentence phrase "Topology andPrometheus metrics for gateway traffic
monitoring" and update it to "Topology and Prometheus metrics for gateway
traffic monitoring".
In `@entrypoint.sh`:
- Line 8: The fallback value for METRICS_WORKLOAD_SUFFIX is missing the leading
hyphen, causing inconsistent defaults; update the environment variable
assignment for METRICS_WORKLOAD_SUFFIX so its default value is
"-openshift-default" instead of "openshift-default" (i.e., change the assignment
that sets METRICS_WORKLOAD_SUFFIX:
'${METRICS_WORKLOAD_SUFFIX:-openshift-default}' to use the hyphen-prefixed
default), ensuring all runtime code that expects the "-openshift-default" suffix
receives the correct value.
In `@src/components/KuadrantOverviewPage.tsx`:
- Around line 973-975: The tooltip strings in KuadrantOverviewPage are
hard-coded (e.g. the Tooltip content "You do not have permission to create a
Gateway" used alongside resourceRBAC['Gateway']?.create and the similar strings
around the create Button with class kuadrant-overview-create-button); update the
component to use the useTranslation hook and replace those literal strings with
t('...') calls using new i18n keys (e.g. gateway.permission.create.tooltip),
then add the corresponding keys/translations to your i18n resource files so the
tooltips are localisable.
- Around line 551-566: The helpers getTotalRequests, getSuccessfulRequests and
getErrorRate treat 0 as missing because they rely on falsy checks; change them
to use explicit nullish handling: for getTotalRequests use const total =
totalRequestsByGateway[key]?.total ?? 0 and return Math.round(total); for
getSuccessfulRequests read const total = totalRequestsByGateway[key]?.total;
const errors = totalRequestsByGateway[key]?.errors ?? 0 and if total == null
return 0 (or preserve existing missing-vs-zero behavior by returning 0 when
total is null?), otherwise compute success = Math.round((total ?? 0) - errors);
for getErrorRate read const total = totalRequestsByGateway[key]?.total; const
errors = totalRequestsByGateway[key]?.errors; if total == null return '-';
otherwise compute rate = total === 0 ? 0 : (errors ?? 0) / total * 100 and
return rate.toFixed(1); apply the same nullish-based fixes to the corresponding
helpers/cells referenced around lines 688-707.
---
Outside diff comments:
In `@src/components/dnspolicy/useDNSPolicyActions.tsx`:
- Line 86: The dependency array for the hook includes an undefined symbol
`history`; replace it with the `navigate` function returned by `useNavigate` so
React correctly tracks changes — locate the hook whose dependency array contains
`history` (in src/components/dnspolicy/useDNSPolicyActions.tsx) and update the
dependencies to include `navigate` (the value from `useNavigate`) instead of
`history`; ensure any references inside the effect/callback use the `navigate`
identifier so ESLint/react-hooks no longer warn.
In `@src/components/KuadrantPoliciesPage.tsx`:
- Around line 399-402: The current implementation calls useResourceRBAC inside a
reduce over policyKinds which violates the Rules of Hooks (useResourceRBAC calls
useAccessReview), so refactor so hook calls are at the component top level:
either explicitly call useResourceRBAC once per known policy kind (e.g., call
useResourceRBAC(PolicyKindA, nsForCheck), useResourceRBAC(PolicyKindB,
nsForCheck), etc.) and assemble resourceRBAC from those results, or create a new
custom hook (e.g., useResourceRBACMap) that internally calls useAccessReview a
fixed number of times for each known entry in RESOURCES and returns the RBACMap;
ensure the number and order of hook calls is constant and no hooks are invoked
inside loops or callbacks.
In `@src/components/tlspolicy/useTLSPolicyActions.tsx`:
- Line 86: The dependency array for the effect in useTLSPolicyActions
incorrectly lists an undefined symbol history; replace history with the actual
navigate function from the useNavigate hook: update the dependency array in the
effect that references navigate (in useTLSPolicyActions) to include navigate
instead of history, ensuring the effect lists navigate, obj, tlsPolicyModel,
launchAnnotationsModal, launchDeleteModal, and launchLabelsModal so React's hook
dependencies are accurate.
---
Nitpick comments:
In `@e2e/tests/rbac.spec.ts`:
- Around line 500-504: The fixed 2s pause using page.waitForTimeout(2000)
followed by expect(page).toHaveURL('/kuadrant/ns/kuadrant-test/overview') is
unnecessary and brittle; either remove the second stability check if the initial
expect(page).toHaveURL(...) already asserts the target URL, or replace the
waitForTimeout call with a targeted wait such as await
page.waitForLoadState('networkidle') before calling expect(page).toHaveURL to
ensure network/activity quiescence; update the code around page.waitForTimeout
and expect(page).toHaveURL accordingly.
- Around line 470-493: Replace the fixed sleeps around namespace selection with
explicit waits: after clicking namespaceButton, wait for the dropdown/options to
appear using a selector from namespaceOptionSelectors via page.waitForSelector
or await page.locator(selector).waitFor({ state: 'visible' }); and before
clicking namespaceOption, wait for that specific locator's visibility or be
enabled (e.g., await namespaceOption.waitFor({ state: 'visible' }) or
waitForElementState('stable') ) instead of page.waitForTimeout(1000/2000); keep
the existing namespaceButton, namespaceOptionSelectors, namespaceOption
identifiers so the logic of choosing the first visible option remains the same.
In `@src/components/KuadrantPoliciesPage.tsx`:
- Around line 481-485: The contextValue object in KuadrantPoliciesPage
(currently built from activeNamespace, defaultColumns, resourceRBAC) is
recreated every render causing unnecessary context consumer updates; wrap its
creation in React.useMemo and return a memoized object keyed on
[activeNamespace, defaultColumns, resourceRBAC] so the reference is stable
across renders—update the code that declares contextValue to use useMemo inside
the KuadrantPoliciesPage component.
In `@src/utils/configLoader.ts`:
- Around line 32-34: The static analyzer warns about assigning to
configScript.innerHTML (created via document.createElement('script')) as a
potential XSS; since the script comes from a trusted same‑origin endpoint, you
can eliminate the warning and be safer for future changes by loading it via a
src URL instead of injecting raw HTML: replace the configScript.innerHTML =
script assignment with setting configScript.src to the trusted endpoint
('/api/plugins/kuadrant-console-plugin/config.js'), attach onload/onerror
handlers if needed, and then document.head.appendChild(configScript) (adjust any
code that currently relies on the inline string).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a2a6e885-b6b9-479c-9c8a-2c5a16c58a97
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (31)
.github/workflows/dev-build.yaml.github/workflows/release-build.yamlCLAUDE.mdcharts/openshift-console-plugin/templates/deployment.yamlcharts/openshift-console-plugin/values.yamlconsole-extensions.jsondocs/designs/2026-03-27-namespace-scoped-overview-design.mddownstream.jse2e/tests/rbac.spec.tsentrypoint.shinstall.yamlkuadrant-dev-setup/cluster-monitoring.yamllocales/en/plugin__kuadrant-console-plugin.jsonpackage.jsonsrc/components/DropdownWithKebab.tsxsrc/components/GatewayPoliciesPage.tsxsrc/components/HTTPRoutePoliciesPage.tsxsrc/components/KuadrantCreateUpdate.tsxsrc/components/KuadrantDNSPolicyCreatePage.tsxsrc/components/KuadrantOverviewPage.tsxsrc/components/KuadrantPoliciesPage.tsxsrc/components/KuadrantStatusAlert.tsxsrc/components/KuadrantTLSCreatePage.tsxsrc/components/PolicyTopologyPage.tsxsrc/components/dnspolicy/useDNSPolicyActions.tsxsrc/components/tlspolicy/useTLSPolicyActions.tsxsrc/constants/links.tssrc/utils/cancel.tsxsrc/utils/configLoader.tssrc/utils/metricsQueries.tssrc/utils/topology/configLoader.ts
💤 Files with no reviewable changes (1)
- src/utils/topology/configLoader.ts
| - name: Login to container registry | ||
| uses: docker/login-action@v2 | ||
| with: | ||
| image: ${{ env.REGISTRY }}/${{ env.IMAGE_BASE_NAME }} | ||
| tags: latest-${{ matrix.arch }} | ||
| archs: ${{ matrix.arch }} | ||
| containerfiles: | | ||
| ./Dockerfile | ||
|
|
||
| - name: Push Architecture-Specific Image | ||
| uses: redhat-actions/push-to-registry@v2 | ||
| username: ${{ secrets.IMG_REGISTRY_USERNAME }} | ||
| password: ${{ secrets.IMG_REGISTRY_TOKEN }} | ||
| registry: ${{ env.IMG_REGISTRY_HOST }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "dev-build.yaml" -type fRepository: Kuadrant/kuadrant-console-plugin
Length of output: 110
🏁 Script executed:
cat -n .github/workflows/dev-build.yaml | head -60Repository: Kuadrant/kuadrant-console-plugin
Length of output: 2260
🌐 Web query:
docker/login-action@v2 GitHub Actions compatibility current runners 2024 2025
💡 Result:
docker/login-action@v2 is compatible with current GitHub Actions hosted runners as of April 2026, but it is outdated and runs on Node.js 16, which has been deprecated since 2023-2024. Current runners (v2.33x+) support Node 20 (default until June 2026) and Node 24. Node 16 actions like v2 will continue to run with opt-out env vars (ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION=true), but may fail post-September 2026 when Node 20 is fully removed. Official docs and examples use v4.1.0 (Node 24, requires runner v2.327.1+). Upgrade to v3+ (Node 20, runner v2.308+) or v4+ for full compatibility without warnings or future breakage. In 2024-early 2025, v2 worked fine; by 2025-2026, Dependabot recommends upgrades in projects like actions/runner.
Citations:
- 1: https://github.com/docker/login-action
- 2: Bump docker/login-action from 2 to 3 actions/runner#3673
- 3: https://github.com/docker/login-action/releases
- 4: https://github.com/marketplace/actions/docker-login
- 5: docker/login-action@v1.14.1...v2.0.0
- 6: docker/login-action@v1...v2
- 7: node 24 as default runtime docker/login-action#929
- 8: Bump docker/login-action from 3 to 4 actions/runner#4278
- 9: https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/
- 10: https://github.blog/changelog/2024-03-06-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default
🌐 Web query:
actionlint docker/login-action version compatibility warnings
💡 Result:
No specific version compatibility warnings or issues were found between actionlint and docker/login-action. Actionlint performs comprehensive static analysis on GitHub Actions workflows, including checks for popular actions like docker/login-action such as input validation at 'with:', outdated versions at 'uses:', and general syntax/type checks. Docker/login-action is actively maintained with recent releases up to v4.1.0 (as of 2026-04-02), and major version bumps (e.g., v2 to v3, v3 to v4) are handled via Dependabot without actionlint-related complaints in searched issues/PRs. Use the latest versions: actionlint v1.7.12+ and docker/login-action@v4 for optimal compatibility. To verify workflows, run actionlint locally or in CI.
Citations:
- 1: https://github.com/rhysd/actionlint/
- 2: https://github.com/rhysd/actionlint/tree/main/docs/checks.md
- 3: https://redirect.github.com/rhysd/actionlint/blob/main/docs/checks.md
- 4: https://github.com/rhysd/actionlint/blob/main/docs/checks.md
- 5: https://github.com/docker/login-action
- 6: Bump docker/login-action from 2 to 3 jorisroovers/gitlint#529
- 7: Bump docker/login-action from 3 to 4 actions/container-prebuilt-action#36
Guard this login step and bump docker/login-action to v3 or later.
This login step (line 28) runs unconditionally whilst the build/push step (line 46) is guarded by if: github.repository_owner == 'kuadrant'. For non-Kuadrant forks triggered via workflow_dispatch, the login step will attempt to execute before being skipped, causing failure due to missing registry credentials. Additionally, docker/login-action@v2 runs on Node.js 16, which is deprecated. Upgrade to v3 (Node 20, supported through June 2026) or v4 for forward compatibility.
Suggested patch
- name: Login to container registry
+ if: github.repository_owner == 'kuadrant'
- uses: docker/login-action@v2
+ uses: docker/login-action@v3
with:
username: ${{ secrets.IMG_REGISTRY_USERNAME }}
password: ${{ secrets.IMG_REGISTRY_TOKEN }}
registry: ${{ env.IMG_REGISTRY_HOST }}🧰 Tools
🪛 actionlint (1.7.12)
[error] 28-28: the runner of "docker/login-action@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/dev-build.yaml around lines 27 - 32, The "Login to
container registry" step currently uses docker/login-action@v2 and runs
unconditionally; change it to use docker/login-action@v3 (or v4) and add the
same guard as the build/push step (if: github.repository_owner == 'kuadrant') so
the login only runs for the kuadrant repository and avoids failing on forks
without registry secrets; update the step reference (the step name "Login to
container registry" and the action reference docker/login-action@v2)
accordingly.
|
|
||
| ### 5. Configuration | ||
|
|
||
| The plugin supports configurable Topology andPrometheus metrics for gateway traffic monitoring. This allows the console to work with different Gateway API implementations (OpenShift 4.19+, OSSM, etc.). |
There was a problem hiding this comment.
Minor typo: missing space.
There's a missing space between "and" and "Prometheus".
📝 Suggested fix
-The plugin supports configurable Topology andPrometheus metrics for gateway traffic monitoring. This allows the console to work with different Gateway API implementations (OpenShift 4.19+, OSSM, etc.).
+The plugin supports configurable Topology and Prometheus metrics for gateway traffic monitoring. This allows the console to work with different Gateway API implementations (OpenShift 4.19+, OSSM, etc.).📝 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.
| The plugin supports configurable Topology andPrometheus metrics for gateway traffic monitoring. This allows the console to work with different Gateway API implementations (OpenShift 4.19+, OSSM, etc.). | |
| The plugin supports configurable Topology and Prometheus metrics for gateway traffic monitoring. This allows the console to work with different Gateway API implementations (OpenShift 4.19+, OSSM, etc.). |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLAUDE.md` at line 96, Fix the minor typo in the sentence containing
"Topology andPrometheus" by inserting a space so it reads "Topology and
Prometheus"; locate the sentence phrase "Topology andPrometheus metrics for
gateway traffic monitoring" and update it to "Topology and Prometheus metrics
for gateway traffic monitoring".
| TOPOLOGY_CONFIGMAP_NAMESPACE: '${TOPOLOGY_CONFIGMAP_NAMESPACE}' | ||
| TOPOLOGY_CONFIGMAP_NAME: '${TOPOLOGY_CONFIGMAP_NAME:-topology}', | ||
| TOPOLOGY_CONFIGMAP_NAMESPACE: '${TOPOLOGY_CONFIGMAP_NAMESPACE:-kuadrant-system}', | ||
| METRICS_WORKLOAD_SUFFIX: '${METRICS_WORKLOAD_SUFFIX:-openshift-default}' |
There was a problem hiding this comment.
Fallback value mismatch for METRICS_WORKLOAD_SUFFIX.
Line 8 uses openshift-default, but the rest of the codebase expects -openshift-default. This causes inconsistent runtime config when the env var is not set.
💡 Proposed fix
- METRICS_WORKLOAD_SUFFIX: '${METRICS_WORKLOAD_SUFFIX:-openshift-default}'
+ METRICS_WORKLOAD_SUFFIX: '${METRICS_WORKLOAD_SUFFIX:--openshift-default}'📝 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.
| METRICS_WORKLOAD_SUFFIX: '${METRICS_WORKLOAD_SUFFIX:-openshift-default}' | |
| METRICS_WORKLOAD_SUFFIX: '${METRICS_WORKLOAD_SUFFIX:--openshift-default}' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@entrypoint.sh` at line 8, The fallback value for METRICS_WORKLOAD_SUFFIX is
missing the leading hyphen, causing inconsistent defaults; update the
environment variable assignment for METRICS_WORKLOAD_SUFFIX so its default value
is "-openshift-default" instead of "openshift-default" (i.e., change the
assignment that sets METRICS_WORKLOAD_SUFFIX:
'${METRICS_WORKLOAD_SUFFIX:-openshift-default}' to use the hyphen-prefixed
default), ensuring all runtime code that expects the "-openshift-default" suffix
receives the correct value.
| const getTotalRequests = (obj: { metadata: { namespace: string; name: string } }): number => { | ||
| const key = `${obj.metadata.namespace}/${obj.metadata.name}-istio`; | ||
| const key = buildGatewayKey(obj.metadata.namespace, obj.metadata.name, metricsWorkloadSuffix); | ||
| const total = totalRequestsByGateway[key]?.total; | ||
| return Number.isFinite(total) ? Math.round(total) : 0; | ||
| }; | ||
| const getSuccessfulRequests = (obj: { | ||
| metadata: { namespace: string; name: string }; | ||
| }): number => { | ||
| const key = `${obj.metadata.namespace}/${obj.metadata.name}-istio`; | ||
| const key = buildGatewayKey(obj.metadata.namespace, obj.metadata.name, metricsWorkloadSuffix); | ||
| const success = totalRequestsByGateway[key]?.total - totalRequestsByGateway[key]?.errors; | ||
| return Number.isFinite(success) ? Math.round(success) : 0; | ||
| }; | ||
| const getErrorRate = (obj: { metadata: { namespace: string; name: string } }): string => { | ||
| const key = `${obj.metadata.namespace}/${obj.metadata.name}-istio`; | ||
| const key = buildGatewayKey(obj.metadata.namespace, obj.metadata.name, metricsWorkloadSuffix); | ||
| const rate = (totalRequestsByGateway[key]?.errors / totalRequestsByGateway[key]?.total) * 100; | ||
| return Number.isFinite(rate) ? rate.toFixed(1) : '-'; |
There was a problem hiding this comment.
Keep zero-traffic gateways distinct from missing metrics.
The current fallbacks treat valid 0 values as empty, so a quiet gateway renders - for totals/successes and -% for error rate. Please switch these helpers/cells to explicit nullish handling instead of falsy checks.
💡 Possible fix
- const getTotalRequests = (obj: { metadata: { namespace: string; name: string } }): number => {
+ const getTotalRequests = (
+ obj: { metadata: { namespace: string; name: string } },
+ ): number | undefined => {
const key = buildGatewayKey(obj.metadata.namespace, obj.metadata.name, metricsWorkloadSuffix);
const total = totalRequestsByGateway[key]?.total;
- return Number.isFinite(total) ? Math.round(total) : 0;
+ return Number.isFinite(total) ? Math.round(total) : undefined;
};
const getSuccessfulRequests = (obj: {
metadata: { namespace: string; name: string };
- }): number => {
+ }): number | undefined => {
const key = buildGatewayKey(obj.metadata.namespace, obj.metadata.name, metricsWorkloadSuffix);
const success = totalRequestsByGateway[key]?.total - totalRequestsByGateway[key]?.errors;
- return Number.isFinite(success) ? Math.round(success) : 0;
+ return Number.isFinite(success) ? Math.round(success) : undefined;
};
- const getErrorRate = (obj: { metadata: { namespace: string; name: string } }): string => {
+ const getErrorRate = (
+ obj: { metadata: { namespace: string; name: string } },
+ ): string | undefined => {
const key = buildGatewayKey(obj.metadata.namespace, obj.metadata.name, metricsWorkloadSuffix);
const rate = (totalRequestsByGateway[key]?.errors / totalRequestsByGateway[key]?.total) * 100;
- return Number.isFinite(rate) ? rate.toFixed(1) : '-';
+ return Number.isFinite(rate) ? rate.toFixed(1) : undefined;
};- {getTotalRequests(obj) || '-'}
+ {getTotalRequests(obj) ?? '-'}
...
- {getSuccessfulRequests(obj) || '-'}
+ {getSuccessfulRequests(obj) ?? '-'}
...
- {getErrorRate(obj) || '-'}%
+ {(() => {
+ const errorRate = getErrorRate(obj);
+ return errorRate == null ? '-' : `${errorRate}%`;
+ })()}Also applies to: 688-707
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/KuadrantOverviewPage.tsx` around lines 551 - 566, The helpers
getTotalRequests, getSuccessfulRequests and getErrorRate treat 0 as missing
because they rely on falsy checks; change them to use explicit nullish handling:
for getTotalRequests use const total = totalRequestsByGateway[key]?.total ?? 0
and return Math.round(total); for getSuccessfulRequests read const total =
totalRequestsByGateway[key]?.total; const errors =
totalRequestsByGateway[key]?.errors ?? 0 and if total == null return 0 (or
preserve existing missing-vs-zero behavior by returning 0 when total is null?),
otherwise compute success = Math.round((total ?? 0) - errors); for getErrorRate
read const total = totalRequestsByGateway[key]?.total; const errors =
totalRequestsByGateway[key]?.errors; if total == null return '-'; otherwise
compute rate = total === 0 ? 0 : (errors ?? 0) / total * 100 and return
rate.toFixed(1); apply the same nullish-based fixes to the corresponding
helpers/cells referenced around lines 688-707.
| {!resourceRBAC['Gateway']?.create ? ( | ||
| <Tooltip content="You do not have permission to create a Gateway"> | ||
| <Button className="kuadrant-overview-create-button" isAriaDisabled> |
There was a problem hiding this comment.
Localise the new permission tooltips.
These messages are new user-facing strings but bypass t(...), so they will remain hard-coded in English while the rest of the page is translated.
🌐 Possible fix
- <Tooltip content="You do not have permission to create a Gateway">
+ <Tooltip content={t('You do not have permission to create a Gateway')}>
...
- <Tooltip content="You do not have permission to create a HTTPRoute">
+ <Tooltip content={t('You do not have permission to create a HTTPRoute')}>Also applies to: 1122-1124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/KuadrantOverviewPage.tsx` around lines 973 - 975, The tooltip
strings in KuadrantOverviewPage are hard-coded (e.g. the Tooltip content "You do
not have permission to create a Gateway" used alongside
resourceRBAC['Gateway']?.create and the similar strings around the create Button
with class kuadrant-overview-create-button); update the component to use the
useTranslation hook and replace those literal strings with t('...') calls using
new i18n keys (e.g. gateway.permission.create.tooltip), then add the
corresponding keys/translations to your i18n resource files so the tooltips are
localisable.
That’s a valid point, however, I’m not sure I fully agree. Minor versions, by semantic versioning, never break backward compatibility. Kuadrant patch releases should pick the latest minor version of the dependencies. This way, dependencies do not need to maintain stable minor release branches. I see Kuadrant dependencies as libraries, always moving forward instead of maintaining long-lived stable minor release branches. It obviously makes total sense to keep long-lived major release branches like release-v1.x. Anyway, as I said, not a big deal. It's already been approved |
6 participants
Summary
Patch release
v0.3.7— cherry-picked fixes frommainontorelease-v0.3(branched fromv0.3.6).Once merged, tag
v0.3.7fromrelease-v0.3.Bug fixes
useHistorywithuseNavigate)Security
Platform support
Cherry-pick conflict resolutions
kuadrant-dev-setup/files (from kuadrant dev setup #330) don't exist on the release branch — removed from cherry-pickKuadrantOverviewPage.tsxhunks — took incomingwatchNamespace+emptyResourceNametypo fix over#ALL_NS#+emtpyResourceNameNot included (deferred to v0.4.0)
Test plan
useHistorydeprecation warnings🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Chores