Skip to content

fix(security): surface XSUAA error bodies via shared helper#736

Open
maximilianbraun wants to merge 5 commits into
mainfrom
fix/xsuaa-surface-btp-error-bodies
Open

fix(security): surface XSUAA error bodies via shared helper#736
maximilianbraun wants to merge 5 commits into
mainfrom
fix/xsuaa-surface-btp-error-bodies

Conversation

@maximilianbraun

Copy link
Copy Markdown
Member

Summary

Mirrors PR #716 (subaccount) for the three XSUAA security clients. Adds a single SpecifyAPIError helper at internal/clients/security/errors.go and routes every .Execute() error in:

  • internal/clients/security/rolecollection/xsuaa_rolecollection.go
  • internal/clients/security/rolecollectiongroupassignment/xsuaa_group_assigner.go
  • internal/clients/security/rolecollectionuserassignment/xsuaa_user_assigner.go

through it. The helper type-asserts to *xsuaa.GenericOpenAPIError and surfaces the parsed model (or, failing that, the raw response body) so users see the actual XSUAA error payload in Synced conditions instead of a bare 500 Internal Server Error.

Why a separate helper

The XSUAA OpenAPI client lives in its own module (internal/openapi_clients/btp-xsuaa-service-api-go/pkg) with its own GenericOpenAPIError type, so the existing helper in internal/controller/account/subaccount/subaccount.go cannot be reused as-is. The new helper has the same shape but targets the XSUAA error type, with map[string]any as the model.

The existing 404 short-circuits in rolecollection.Delete and userassignment.HasRole are deliberately preserved.

Test plan

  • go build ./... && go vet ./... clean
  • go test -race -count=1 ./internal/clients/security/... green (helper unit tests + one per-file integration test asserting the response body is surfaced)
  • gofmt -l internal/clients/security/ clean

release-notes/bugfix

Adds internal/clients/security/SpecifyAPIError so the rolecollection,
group-assigner and user-assigner clients all unwrap *xsuaa.GenericOpenAPIError
into a message that carries the response body instead of the HTTP status only.
Existing 404 short-circuits are preserved.
@maximilianbraun maximilianbraun added the release-notes/bugfix Marks PR as bugfix for release note generation label Jun 20, 2026
@maximilianbraun maximilianbraun marked this pull request as draft June 20, 2026 08:19
XSUAA models are map[string]any which prints as map[k:v] — uglier than the
JSON Body() they were parsed from. The model branch was mimicry of the
subaccount helper; for XSUAA, Body() alone is the cleaner surface.

@maximilianbraun maximilianbraun left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

internal/clients/security/{rolecollection,rolecollectiongroupassignment,rolecollectionuserassignment}/*_test.go — The four-line reflect.NewAt(...UnsafeAddr()).Elem().Set(reflect.ValueOf(body)) dance to set the unexported body field on *xsuaa.GenericOpenAPIError is now inlined 3× across the new sub-package tests, plus a 4th copy as newGenericErr in errors_test.go. Same shape PR #731 just consolidated for the account-side: drop these copies, add internal/testutils/openapi_errors.go::NewXsuaaAPIError(body []byte) error, call it from all four sites. Each test shrinks to one line and the regeneration-rot risk lives in one place.

Reviewer flagged 4 inline reflect+unsafe copies that all set the unexported
`body` field on a *xsuaa.GenericOpenAPIError. Extract to
internal/testutils/openapi_errors_xsuaa.go::NewXsuaaAPIError so each test
site collapses to one line.

Filed under a separate filename from PR #731's openapi_errors.go to avoid
a rebase collision; the two helpers can be unified once both branches land.

@maximilianbraun maximilianbraun left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Helper is minimal — single body []byte param, nil short-circuit matches the empty-returns-self case, the IsValid() guard handles SDK regeneration. All 4 inline copies collapsed to one-liners, no scope creep. Resolved.

@maximilianbraun maximilianbraun marked this pull request as ready for review June 20, 2026 08:47
@jn-av jn-av temporarily deployed to pr-e2e-approval June 23, 2026 15:48 — with GitHub Actions Inactive
@jn-av jn-av temporarily deployed to pr-e2e-no-approval June 23, 2026 15:53 — with GitHub Actions Inactive
@jn-av jn-av temporarily deployed to pr-e2e-no-approval June 23, 2026 15:53 — with GitHub Actions Inactive
@jn-av jn-av temporarily deployed to pr-e2e-no-approval June 23, 2026 15:53 — with GitHub Actions Inactive
@jn-av jn-av temporarily deployed to pr-e2e-no-approval June 23, 2026 15:53 — with GitHub Actions Inactive
@jn-av jn-av temporarily deployed to pr-e2e-no-approval June 23, 2026 15:53 — with GitHub Actions Inactive
@jn-av jn-av temporarily deployed to pr-e2e-no-approval June 23, 2026 15:53 — with GitHub Actions Inactive
@jn-av jn-av temporarily deployed to pr-e2e-no-approval June 23, 2026 15:53 — with GitHub Actions Inactive
@jn-av jn-av temporarily deployed to pr-e2e-no-approval June 23, 2026 15:53 — with GitHub Actions Inactive
@jn-av jn-av temporarily deployed to pr-e2e-no-approval June 23, 2026 15:53 — with GitHub Actions Inactive
@jn-av jn-av temporarily deployed to pr-e2e-no-approval June 23, 2026 15:53 — with GitHub Actions Inactive
@jn-av jn-av temporarily deployed to pr-e2e-no-approval June 23, 2026 15:53 — with GitHub Actions Inactive
@jn-av jn-av temporarily deployed to pr-e2e-no-approval June 23, 2026 15:53 — with GitHub Actions Inactive
@jn-av jn-av temporarily deployed to pr-e2e-no-approval June 23, 2026 15:53 — with GitHub Actions Inactive
@jn-av jn-av temporarily deployed to pr-e2e-no-approval June 23, 2026 15:53 — with GitHub Actions Inactive
@jn-av jn-av temporarily deployed to pr-e2e-no-approval June 23, 2026 15:53 — with GitHub Actions Inactive
@jn-av jn-av temporarily deployed to pr-e2e-no-approval June 23, 2026 15:53 — with GitHub Actions Inactive
@jn-av jn-av temporarily deployed to pr-e2e-no-approval June 23, 2026 15:53 — with GitHub Actions Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes/bugfix Marks PR as bugfix for release note generation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants