TRT-2756: Test-details links in on-demand staging environment point to localhost#3704
Conversation
Use GetBaseFrontendURL instead of GetBaseURL in the test details handler so the generated "latest" links use the Origin header (the frontend's host) rather than the backend's request Host, which resolves to localhost in proxied environments like on-demand staging. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
openshift-ci
Bot
added
the
ready-for-human-review
openshift-ci-robot
added
the
jira/valid-reference
|
@openshift-trt-agent[bot]: This pull request references TRT-2756 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 bug to target the "5.0.0" version, but no target version was set. DetailsIn 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
Bot
added
the
needs-ok-to-test
|
Hi @openshift-trt-agent[bot]. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
|
/test staging |
|
/test agentic-staging |
|
/ok-to-test |
openshift-ci
Bot
added
ok-to-test
Sippy Staging EnvironmentURL: https://weather-emerald-organizer-violation.trycloudflare.com This environment is built from this PR and will remain available for approximately 60 minutes (until ~13:13 UTC). |
|
This doesn't work, and the staging environment confirms it. Attempting to navigate to a test_details report from the RegressedTestModal is non-functional. |
|
Scheduling required tests: |
|
/hold Claude was working on fixing my comment, but there is a 15 minute timeout for followups, and it was killed in the middle. I need to increase that |
openshift-ci
Bot
added
the
do-not-merge/hold
convertApiUrlToUiUrl was preserving the full URL including scheme+host from HATEOAS links. In staging (same-origin), browsers omit the Origin header on GET requests so GetBaseFrontendURL falls back to req.Host (e.g. localhost:8080), producing unreachable links. Strip the origin and emit relative paths (/sippy-ng/...) so the browser resolves them against whatever host the user is actually on. Also removes debug console.log statements and the compareUrlQueryParams helper that was only used for debugging. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@smg247 Fixed. The root cause was in the frontend's The problem: When the backend generates HATEOAS The fix: Also cleaned up debug All e2e tests pass (107/108, the one failure is |
|
Scheduling required tests: |
|
/test agentic-staging |
Sippy Staging EnvironmentURL: https://few-useful-tales-turtle.trycloudflare.com This environment is built from this PR and will remain available for approximately 60 minutes (until ~19:52 UTC). |
|
This works nicely on staging now. Since the change is larger in scope, I will check this out locally and confirm. |
|
Confirmed this works locally. No real way to test prod without merging, but it should be safe. /lgtm |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
WalkthroughThe server now uses the frontend base URL when building component readiness test-details links, and the frontend converts server-provided API links into UI links before returning them. ChangesComponent readiness test-details links
Estimated review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 21✅ Passed checks (21 passed)
✨ Finishing Touches🧪 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 |
openshift-ci
Bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@sippy-ng/src/component_readiness/CompReadyUtils.js`:
- Around line 803-815: The URL rewrite logic in convertApiUrlToUiUrl is not
covered by tests, including the special /api/component_readiness/ rewrite and
the non-/api/ direct-return branch. Add regression cases in
CompReadyUtils.test.js for convertApiUrlToUiUrl and
generateTestDetailsReportLink that verify an absolute
/api/component_readiness/test_details URL becomes the expected relative
/sippy-ng/component_readiness/test_details path, and that inputs without /api/
are returned unchanged.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 1476a717-1395-4fe6-990e-920fccf22c59
📒 Files selected for processing (2)
pkg/sippyserver/server.gosippy-ng/src/component_readiness/CompReadyUtils.js
…ReportLink Addresses CodeRabbit review feedback requesting test coverage for the URL rewrite logic that converts API URLs to UI-relative paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/lgtm |
openshift-ci
Bot
added
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: openshift-trt-agent[bot], smg247 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Scheduling required tests: |
2 participants
Summary
Fixes test-details "latest" links pointing to
localhostin staging and production environments.The
jsonComponentReportTestDetailsFromBigQueryhandler was usingapi.GetBaseURL(req)to construct the base URL for generated links.GetBaseURLderives the host fromreq.Host, which resolves tolocalhostin proxied environments (like on-demand staging). The sibling handlerjsonComponentReportFromBigQueryalready usesapi.GetBaseFrontendURL(req), which prefers theOriginheader (containing the actual frontend host) and falls back toGetBaseURLwhen no origin is present. This fix applies the same pattern to the test-details handler.Fixes: TRT-2756
Test plan
go vetpasses on affected packagesmake lintpasses (0 issues)pkg/sippyserverandpkg/api/...TestRegressionCacheLoader)GetBaseFrontendURLis already used correctly in the component report handler (line 1027) and that the link injector middleware receives the correct base URL through that flowGenerated with Claude Code
Summary by CodeRabbit