Skip to content

fix(kube): clarify server-side apply patch errors#31981

Merged
TerryHowe merged 1 commit intohelm:mainfrom
abhay1999:fix/ssa-error-context
Apr 20, 2026
Merged

fix(kube): clarify server-side apply patch errors#31981
TerryHowe merged 1 commit intohelm:mainfrom
abhay1999:fix/ssa-error-context

Conversation

@abhay1999
Copy link
Copy Markdown
Contributor

What this PR does / why we need it

Refs #31529

When a generic server-side apply patch fails, Helm currently returns the raw API error without clearly indicating that the failure happened on the server-side apply path.

This change wraps generic server-side apply patch errors with resource context and an explicit server-side apply failed prefix, while leaving the existing incompatible-server and conflict-specific messages unchanged.

What changed

  • wrap generic errors returned from patchResourceServerSide() with server-side apply context
  • add regression coverage for a duplicate-key style typed patch error

Validation

  • env GOCACHE=/tmp/go-cache GOMODCACHE=/tmp/go-mod-cache go test ./pkg/kube -run 'Test(Create|PatchResourceServerSide)' -count=1

Copilot AI review requested due to automatic review settings March 30, 2026 13:47
@pull-request-size pull-request-size Bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 30, 2026
Signed-off-by: abhay1999 <abhaychaurasiya19@gmail.com>
@abhay1999 abhay1999 force-pushed the fix/ssa-error-context branch from d1e4473 to f257c95 Compare March 30, 2026 13:50
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves Helm’s Kubernetes server-side apply (SSA) patch error reporting by adding explicit SSA context and resource identifiers to generic patch failures, addressing confusion where raw API errors didn’t indicate they came from the SSA path.

Changes:

  • Wrap generic errors returned from patchResourceServerSide() with a server-side apply failed prefix plus namespace/name and GVK.
  • Add a regression test covering a “duplicate key” typed patch error returned by the API.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
pkg/kube/client.go Wraps non-conflict, non-incompatible-server SSA patch errors with clear SSA + resource context.
pkg/kube/client_test.go Adds coverage ensuring generic SSA patch errors surface the new contextual message.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@abhay1999
Copy link
Copy Markdown
Contributor Author

Hi maintainers! 👋 Just following up on this PR — it's been 3 days with no human review yet.

This fixes a confusing error message when server-side apply patch operations fail, giving users clearer context on why the patch was rejected. Happy to address any feedback or rebase if needed. Thanks for your time!

@gjenkins8 gjenkins8 added the Has One Approval This PR has one approval. It still needs a second approval to be merged. label Apr 20, 2026
@gjenkins8
Copy link
Copy Markdown
Member

Thanks for the PR. Agree this is an improvement

Copy link
Copy Markdown
Contributor

@TerryHowe TerryHowe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@TerryHowe TerryHowe merged commit 29d309e into helm:main Apr 20, 2026
5 checks passed
@TerryHowe TerryHowe removed the Has One Approval This PR has one approval. It still needs a second approval to be merged. label Apr 20, 2026
abhay1999 added a commit to abhay1999/abhay_portfolio that referenced this pull request Apr 21, 2026
- OpenSource: add PR #31981 'fix(kube): clarify server-side apply patch errors'
  to STATIC_MERGED and OTHER_FEATURED (alongside existing helm #31931)
- Skills: bump CNCF PRs stat from 9 → 10

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@abhay1999 abhay1999 deleted the fix/ssa-error-context branch April 21, 2026 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants