Emit ResourceDoesNotExist marker on authoritative delete absence (SPEC-09a)#140
Emit ResourceDoesNotExist marker on authoritative delete absence (SPEC-09a)#140c1-squire-dev[bot] wants to merge 1 commit into
Conversation
Connector PR Review: Emit ResourceDoesNotExist marker on authoritative delete absence (SPEC-09a)Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0 Review SummaryFull PR diff scanned for security and correctness. This PR replaces the untyped (nil, nil) delete-success in the confirmed-absence branches of Security IssuesNone found. Correctness IssuesNone found. SuggestionsNone. |
…C-09a)
The IAM-user and SSO-user resource deleters already normalize an
authoritatively-absent provider resource to a successful delete. Replace
their untyped (nil, nil) success with the typed already-absent marker
c1.connector.v2.ResourceDoesNotExist, so callers can distinguish a
confirmed already-gone resource from an opaque NotFound (per SPEC-09a).
- iam_user.go: GetUser NoSuchEntity (ResourceDeleterV2; addressed account
via parentResourceID) now returns annotations.New(&v2.ResourceDoesNotExist{}), nil.
- sso_user.go: DeleteUser ResourceNotFoundException now returns the marker.
Only the confirmed-absence branches change: ordinary deletion success still
carries no marker, and wrong-type/unparseable/other-provider errors remain
errors. Constructs the existing vendored marker directly (baton-sdk v0.19.1);
a later SDK bump can swap in resource.ResourceDoesNotExistAnnotations() once
baton-sdk#1033 is released.
Adds focused tests: ordinary success has no marker, confirmed absence has the
marker, provider errors stay errors, and address inputs are preserved.
Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
dcd887e to
bb16a53
Compare
Summary
First conforming connector producer of the typed already-absent delete outcome defined by SPEC-09a (portable resource-deleter idempotency contract). The IAM-user and SSO-user resource deleters in baton-aws already normalize an authoritatively-absent provider resource to a successful delete; this PR replaces their untyped
(nil, nil)success with the typed markerc1.connector.v2.ResourceDoesNotExist, so a caller can distinguish a confirmed already-gone resource from an opaqueNotFound(which stays an error).This mirrors the long-standing
GrantAlreadyRevokedidiom on the grant plane, applied to the resource-delete plane.What changed
pkg/connector/iam_user.go—(*iamUserResourceType).Delete(ResourceDeleterV2). TheGetUser→NoSuchEntitypre-check branch, which resolves the addressed account (default client or theparentResourceIDaccount), now returnsannotations.New(&v2.ResourceDoesNotExist{}), nil.pkg/connector/sso_user.go—(*ssoUserResourceType).Delete. TheDeleteUser→ResourceNotFoundExceptionbranch (identity store from connector config + user id from the ARN) now returns the marker.Only the confirmed-absence branches change. Explicitly unchanged:
GetIAMClientfailure, and any non-not-found provider error remain errors — mis-addressing is never silently reported as already-absent.NoSuchEntityskip and the finalDeleteUsererror path are untouched.SDK dependency
Conceptually depends on ConductorOne/baton-sdk#1033, which makes the contract normative and adds the
resource.ResourceDoesNotExistAnnotations()helper — but there is no code dependency here. TheResourceDoesNotExistmessage is already vendored in the pinned baton-sdkv0.19.1, so this PR constructs the marker directly. Once #1033 is released and baton-aws bumps its SDK, these two call sites can be swapped to the helper (noted in code comments). This PR is therefore independently mergeable and does not require an unreleased/pseudo-version dependency.Tests
New focused tests (
pkg/connector/sso_user_test.go,pkg/connector/iam_user_delete_test.go):ResourceDoesNotExist, nil error.UserId/IdentityStoreIdpassed toDeleteUser; IAM: wrongResourceTyperejected).The SSO path uses the existing
client.IdentityStoreClientinterface seam; the IAM path stubsGetUservia an AWS SDK v2 Finalize middleware (no production interface seam introduced, no network), which genuinely exercises the productionerrors.As(&NoSuchEntityException{})branch.Verified locally:
gofmtclean,go vetclean,go build ./...ok,go test -race ./pkg/connector/...green (7 new tests + no regressions),golangci-lint run0 issues.Consumer side (out of scope here)
Turning this typed outcome into actual retry-safety is a C1-backend change (honor
ResourceDoesNotExistin delete-outcome classification: already-gone →REVOKED, opaqueNotFound→REVOKE_UNKNOWN), tracked separately per SPEC-09a.🤖 Generated with Claude Code