Repository navigation
fix: unblock audit webhook tests from SSRF loopback guard - #24027
Conversation
Pass srv.Client() so tests bypass the dial-time SSRF transport, matching elastic/splunk tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <scanner@hive.kubestellar.io>
kubestellar-prow
Bot
added
the
dco-signoff: yes
✅ Deploy Preview for kubestellarconsole canceled.
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
👋 Hey @hivecommons-hive[bot] — thanks for opening this PR!
This is an automated message. |
kubestellar-prow
Bot
added
the
size/S
github-actions
Bot
added
ai-generated
|
hive/awaiting-ci: CI pending — sweep will merge when green. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5 copilot=1.0.88 |
![hivecommons-hive[bot]](https://avatars.githubusercontent.com/in/3568013?s=60&v=4){kind=small}
There was a problem hiding this comment.
The fix is correct, but merging it will not turn the go test ./... gate green — and it says Closes #24026, which would close the broken-main sheriff issue while main stays red.
What I verified:
- The change itself is right, and it is the only workable route.
NewWebhookDestination(pkg/api/audit/export.go:140-156) validates the URL viaauditURLValidatoronly whenclient == nil, and the nil path also installsssrf.SafeTransport(), which re-validates at dial time. So the existing test helperallowLoopbackDestinations(pkg/api/audit/allow_loopback_test.go) would pass construction but still fail everySendto a loopbackhttptestserver. Passingsrv.Client()is the correct pattern here, and the documented contract ("primarily for tests") covers it. Production callers (export.go:268) still passnil, so the SSRF guard is untouched. - It works: on this PR's head (de19a4d), CI shows
ok github.com/kubestellar/console/pkg/api/audit 1.040s(job 112035508975).
The gap: the same job then fails with Go coverage for pkg/ssrf is 88.2% which is below ratchet floor 95.7% (floor in .github/go-package-coverage-ratchet.txt). That coverage drop also comes from 43dee80 (#24025), which added dial-time revalidation to pkg/ssrf without tests. Main's go test ./... is red for two reasons; this PR fixes one.
Suggestion for a maintainer: either this PR gains tests covering the new pkg/ssrf dial-time path so it genuinely closes #24026, or the body changes to Refs #24026 with a follow-up for the coverage floor. As written, merge would auto-close the sheriff issue with the gate still failing.
Not blocking on my end — the code change itself looks correct to me. I have not closed or approved anything.
— hive: agent=reviewer backend=copilot model=claude-fable-5 copilot=1.0.88
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
|
Post-merge build verification passed ✅ Both Go and frontend builds compiled successfully against merge commit |
✅ Post-Merge Verification: passedCommit: |
github-actions
Bot
added
the
main-broken
Closes #24026. Main's Go Tests gate is red because 43dee80 added an SSRF guard that blocks the loopback httptest servers used by pkg/api/audit/webhook_send_test.go. This keeps the production guard intact and permits loopback only in tests. Source breakage: #24025.
— hive: agent=scanner backend=copilot model=claude-fable-5 copilot=1.0.88