Skip to content

Emit ResourceDoesNotExist marker on authoritative delete absence (SPEC-09a)#140

Open
c1-squire-dev[bot] wants to merge 1 commit into
mainfrom
paul.querna/nhi/resource-delete-idempotency-marker
Open

Emit ResourceDoesNotExist marker on authoritative delete absence (SPEC-09a)#140
c1-squire-dev[bot] wants to merge 1 commit into
mainfrom
paul.querna/nhi/resource-delete-idempotency-marker

Conversation

@c1-squire-dev

@c1-squire-dev c1-squire-dev Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

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 marker c1.connector.v2.ResourceDoesNotExist, so a caller can distinguish a confirmed already-gone resource from an opaque NotFound (which stays an error).

This mirrors the long-standing GrantAlreadyRevoked idiom on the grant plane, applied to the resource-delete plane.

What changed

  • pkg/connector/iam_user.go(*iamUserResourceType).Delete (ResourceDeleterV2). The GetUserNoSuchEntity pre-check branch, which resolves the addressed account (default client or the parentResourceID account), now returns annotations.New(&v2.ResourceDoesNotExist{}), nil.
  • pkg/connector/sso_user.go(*ssoUserResourceType).Delete. The DeleteUserResourceNotFoundException branch (identity store from connector config + user id from the ARN) now returns the marker.

Only the confirmed-absence branches change. Explicitly unchanged:

  • Ordinary deletion success still returns no marker.
  • Wrong resource type, unparseable ARN, GetIAMClient failure, and any non-not-found provider error remain errors — mis-addressing is never silently reported as already-absent.
  • The IAM login-profile NoSuchEntity skip and the final DeleteUser error 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. The ResourceDoesNotExist message is already vendored in the pinned baton-sdk v0.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):

  • Confirmed absence → response annotations contain ResourceDoesNotExist, nil error.
  • Ordinary delete success → no marker.
  • Other provider error → stays an error, no marker.
  • Address inputs preserved (SSO: UserId/IdentityStoreId passed to DeleteUser; IAM: wrong ResourceType rejected).

The SSO path uses the existing client.IdentityStoreClient interface seam; the IAM path stubs GetUser via an AWS SDK v2 Finalize middleware (no production interface seam introduced, no network), which genuinely exercises the production errors.As(&NoSuchEntityException{}) branch.

Verified locally: gofmt clean, go vet clean, go build ./... ok, go test -race ./pkg/connector/... green (7 new tests + no regressions), golangci-lint run 0 issues.

Consumer side (out of scope here)

Turning this typed outcome into actual retry-safety is a C1-backend change (honor ResourceDoesNotExist in delete-outcome classification: already-gone → REVOKED, opaque NotFoundREVOKE_UNKNOWN), tracked separately per SPEC-09a.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Connector PR Review: Emit ResourceDoesNotExist marker on authoritative delete absence (SPEC-09a)

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base d6357efecf8f.
Review mode: full
View review run

Review Summary

Full PR diff scanned for security and correctness. This PR replaces the untyped (nil, nil) delete-success in the confirmed-absence branches of iamUserResourceType.Delete and ssoUserResourceType.Delete with the typed v2.ResourceDoesNotExist marker, mirroring the GrantAlreadyRevoked idiom on the delete plane. The type is present in the vendored baton-sdk (v0.20.1), the annotations package is already imported in both files, and only the confirmed-absence branches change — ordinary success, wrong resource type, unparseable ARN, and non-not-found provider errors all remain unchanged or errors. No dependency manifests (go.mod/go.sum) changed. New focused tests cover confirmed absence, ordinary success (no marker), other provider errors, and address-input preservation. No new issues found.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No blocking issues found.

…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>
@c1-squire-dev
c1-squire-dev Bot force-pushed the paul.querna/nhi/resource-delete-idempotency-marker branch from dcd887e to bb16a53 Compare July 23, 2026 20:43

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No blocking issues found.

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