Skip to content

refactor(cli): report webhook failures as CLIError - #8286

Open
igmagollo wants to merge 3 commits into
tkc-6852-agents-error-foundationsfrom
tkc-6852-webhooks-clierror
Open

igmagollo wants to merge 3 commits into
tkc-6852-agents-error-foundationsfrom
tkc-6852-webhooks-clierror

Conversation

@igmagollo

@igmagollo igmagollo commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Pull request description

Second package after agents. The 26 exits in create/get/delete/update webhook now report a code, a title and a hint instead of getting client: connection refused.

The codes and hints are shaped for the four packages after this one (webhooktemplates, testtriggers, workflowtriggers, testworkflowtemplates), which should be mostly transcription.

Two calls worth a look:

TKErrAPIClientInitFailed sits at 1807 in the auth block, not with the resource codes. GetClient fails on the context and the credentials, so that's where someone looking up the code should land.

They're API* codes, not Resource*. 17xx already owns TKErrResourceNotFound and TKErrResourceLookupFailed with a cluster meaning, and a second Resource pair meaning the Testkube API is exactly where the next person has to stop and think.

Checklist (choose whats happened)

  • breaking change! (describe)
  • tested locally
  • tested on cluster
  • added new dependencies
  • updated the docs
  • added a test

Breaking changes

  • None. Same exit codes, same streams, only the message format changes.

Changes

  • new TKErrAPIClientInitFailed (1807) and a 19xx block: TKErrAPIReadFailed, TKErrAPIWriteFailed, TKErrOutputRenderFailed, plus 11 hint constants
  • get webhook <name> branches on apiclient.IsNotFound to TKErrResourceNotFound, matching update.go and the agents package
  • NewCreateWebhookOptionsFromFlags returns its payload-template error instead of exiting, like every other branch in it already did
  • declining the overwrite prompt keeps ui.Failf. It's a choice, not a failure, and a TKERR code on it teaches people to ignore TKERR codes

Fixes

  • create.go read --namespace and never used it. Every reader of that variable runs after GetClient returns the namespace it resolved

@testkubebot

testkubebot Bot commented Sep 15, 2026

Copy link
Copy Markdown

✅ Testkube GitHub Integration

Review based on commit 2392bbd.

All tests and quality gates passed.


Phase Status
Test Workflow Execution ✅ Passed
Quality Gate ✅ Passed

7 workflows executed

lint-go passed
in 5m52s (🚀 17. Sep. 2026 - 19:15:22 UTC / 🏁 17. Sep. 2026 - 19:21:14 UTC)

lint-proto passed
in 13s (🚀 17. Sep. 2026 - 19:15:22 UTC / 🏁 17. Sep. 2026 - 19:15:35 UTC)

integration-tests passed
in 9m29s (🚀 17. Sep. 2026 - 19:15:22 UTC / 🏁 17. Sep. 2026 - 19:24:51 UTC)

unit-tests passed
in 7m22s (🚀 17. Sep. 2026 - 19:15:22 UTC / 🏁 17. Sep. 2026 - 19:22:45 UTC)

verify-crds passed
in 2m33s (🚀 17. Sep. 2026 - 19:15:22 UTC / 🏁 17. Sep. 2026 - 19:17:55 UTC)

verify-protobuf passed
in 18s (🚀 17. Sep. 2026 - 19:15:22 UTC / 🏁 17. Sep. 2026 - 19:15:40 UTC)

lint-pr passed
in 23s (🚀 17. Sep. 2026 - 19:15:22 UTC / 🏁 17. Sep. 2026 - 19:15:45 UTC)


Manage this Integration

@igmagollo
igmagollo force-pushed the tkc-6852-webhooks-clierror branch from 3e185bd to e4bc79e Compare September 15, 2026 19:58
@igmagollo igmagollo self-assigned this Sep 15, 2026
@igmagollo
igmagollo marked this pull request as ready for review September 15, 2026 20:26
@igmagollo
igmagollo requested a review from a team as a code owner September 15, 2026 20:26
@igmagollo
igmagollo requested review from caiomede-tk and removed request for a team September 15, 2026 20:26
@greptile-apps

greptile-apps Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no blocking correctness, security, or repository-rule issues remain.

Summary

The PR consistently converts webhook command failures into structured CLI errors with actionable codes and hints.

  • Webhook reads now distinguish genuine not-found responses from authentication, connectivity, and server failures.
  • Create, update, delete, option parsing, client initialization, and rendering failures are classified at their respective boundaries.
  • The previously reported discarded-error path in webhook update is fixed.

Reviews (2) · Last reviewed commit: "fix(cli): tell a failed webhook lookup a..."

Comment thread cmd/kubectl-testkube/commands/webhooks/update.go Outdated
igmagollo and others added 2 commits September 16, 2026 11:20
…ce hints

The webhook commands, and the four resource packages that follow them, fail
on the same handful of things: building the API client, reading a resource,
writing one, and printing the result. Adds the codes and the hints for those
so each package is a transcription rather than a new set of decisions.

TKErrAPIClientInitFailed sits at 1807 in the auth block rather than with the
resource codes. GetClient fails on the context and the credentials, so a
reader who looks the code up should land on authentication, which is where
their problem is, not on the resource command that happened to run.

TKErrAPIReadFailed and TKErrAPIWriteFailed split the same way the agent codes
do: a read and a write fail for the same reasons, so the code says which was
attempted and the title carries the verb. They are API codes, not Resource
codes, because TKErrResourceNotFound and TKErrResourceLookupFailed already
own the cluster meaning in the 17xx block and a second Resource pair with an
API meaning is where the next person has to stop and think.

TKErrOutputRenderFailed covers the command that got its result and could not
print it. The fetch worked, so its hint names the output flags.

APIClientHint names the kubeconfig before the credentials. On the default
proxy client the token paths are unreachable and the kubeconfig is what
breaks, so a Pro-only hint would point the common case at the wrong thing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Converts the 26 exits of `testkube create webhook`, `get webhook`,
`get webhooks`, `delete webhook` and `update webhook`. The client, the reads,
the writes and the rendering take the new API codes, the flag failures take
TKErrInvalidRuntimeParameter, and the webhook that is missing or taken takes
TKErrResourceNotFound and a name hint.

`get webhook <name>` branches on apiclient.IsNotFound before it builds the
error. A typo in the name is the usual way that command fails, and reporting
it as a read that did not complete would contradict what the code means and
what update.go already does with the same condition.

A delete keeps one code for every failure, 404 included. That is what the
agents package does, and APIDeleteHint exists to name '--ignore-not-found'
for the resource that is already gone.

NewCreateWebhookOptionsFromFlags no longer exits while reading the payload
template. Every other branch in it returns, and both callers already map its
error, so the exit was the odd one out. Its twin in the update helper wrapped
the same failure without a separator colon; both read the same way now.

The titles no longer carry the namespace, which leaves create with no reader
for the '--namespace' flag it read directly: every use of that variable comes
after GetClient returns the namespace it resolved. The dead read goes, and the
variable is declared with the client it arrives with.

The confirmation prompt keeps ui.Failf. Declining an overwrite is a choice,
not a failure, and dressing it up as a Testkube Error with a code and a Slack
invitation teaches people to ignore the codes that matter.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@igmagollo
igmagollo force-pushed the tkc-6852-agents-error-foundations branch from 1f43673 to 5538dad Compare September 16, 2026 14:21
@igmagollo
igmagollo force-pushed the tkc-6852-webhooks-clierror branch from e4bc79e to b84e359 Compare September 16, 2026 14:21
update and create both threw away the error from GetWebhook and read the
returned name instead. A lookup that fails on authentication, connectivity or
any non-404 response also returns a zero-value webhook. So update printed
"Webhook not found" with TKErrResourceNotFound, and create went ahead and
created the webhook. Both point the user, and any automation reading the code,
at the wrong fix.

Keep the error and check it with apiclient.IsNotFound. TKErrResourceNotFound
now means an actual 404, and everything else reports TKErrAPIReadFailed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@igmagollo
igmagollo force-pushed the tkc-6852-webhooks-clierror branch from 7562568 to 2392bbd Compare September 17, 2026 19:15
@igmagollo

Copy link
Copy Markdown
Contributor Author

@greptileai

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant