Carry the credential type discriminator, and gate org API key deletion - #41
Merged
highb merged 9 commits intoAug 28, 2026
Conversation
baton-sdk#1109 splits credential shape from credential kind: the closed CredentialIssueOptionDescriptor.option enum keeps naming the shape, and the open secret_resource_type_id names which kind of that shape comes back. It also adds CredentialIssueOptions.secret_resource_type_id, which issuance requests must now set, and CredentialIssueOptionDescriptor .preferred, which names the default descriptor when several share a shape. This pins a pseudo-version of that PR's head rather than a released tag, so it must be re-pointed at a released baton-sdk tag before merge. Until then the check-versions CI job fails: .versions.yaml on main pins v0.24.4 and is managed by baton-admin, so it cannot be updated here. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
The organization API key deleter was registered whenever sync-secrets was on. Any install already syncing secrets would therefore acquire org-wide Datadog API key deletion the moment it upgraded the connector, without anyone choosing it. Reading a credential inventory is not consent to destroy what is in it. Deletion now needs allow-org-api-key-deletion, a separate flag that defaults to off. Sync behaviour is unchanged either way. The capability has to be absent, not merely refused: C1 resolves what it may do from the advertisement, and the SDK derives CAPABILITY_RESOURCE_DELETE from a type assertion on the registered syncer. So Delete moves off apiTokenBuilder onto deletableAPITokenBuilder and only that variant is registered when the grant is set. The advertised Datadog permissions follow the same split: api_keys_delete and api_keys_write are advertised only by the variant that can reach them. Also updates credential_lifecycle_test.go for the moved constructor. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Datadog has two kinds of API key and they are not interchangeable. A service account application key is scoped to and owned by one identity; an organization API key is owned by the whole organization, has no owner inside it, and cannot be scoped at all. Both are the API_KEY shape, so the shape enum alone cannot tell a caller which one it is getting. IssueCapabilityDetails now advertises one descriptor per kind, separated by secret_resource_type_id, with the service account application key marked preferred. Issue dispatches on the requested type rather than always minting an application key, and refuses an unrecognised type instead of falling back to a default arm. Organization API key issuance follows allow-org-api-key-deletion. The SDK will not register an issuance descriptor whose secret resource type has no ResourceDeleterV2, and that is the right constraint: a credential this connector cannot revoke is one it should not mint. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Regenerated from the built connector: ./connector config > config_schema.json ./connector capabilities > baton_capabilities.json README and docs/connector.mdx describe allow-org-api-key-deletion and the two issuance kinds. The capabilities document is static and has no conditional form, so main.go forces every optional surface on when generating it, as it already did for sync-secrets and sync-schedules. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
TestCredentialIssueLifecycle covers the service account application key arm only. The organization API key arm is the new one and had no live coverage, so a caller could not tell from the suite whether the dispatch actually reaches a different Datadog API. The new test mirrors the existing opt-in guard and always revokes what it mints. It asserts the issued resource comes back as the kind that was requested rather than the preferred one, that it carries no parent resource id, and that the key authenticates before revocation and stops afterwards. The probe is GET /api/v1/validate, which authenticates on the API key alone. An organization API key has no application key to pair with, so the connector's own ValidateCredentials path would not isolate the credential under test. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Datadog answers 404, not an empty list, when asked for a disabled service account's application keys. listApplicationKeyPage treats 404 as fatal, so one disabled service account failed the entire application-key walk — and Datadog never deletes users, it only disables them. Any organization that has ever disabled a service account could therefore never sync a single application key, and the resource type never appeared in C1 at all. The walk now skips service accounts Datadog reports as disabled. They cannot authenticate, so no live credential is dropped. A 404 on an account this walk did choose to visit still fails closed: that one was enabled when the users page was read, so not being readable now is a real anomaly. Found against a live Datadog organization, where it kept the service-account application key from ever reaching C1. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
…lient The organization API key smoke test hand-rolled an http.Client call to /api/v1/validate. gosec's taint analysis flagged it (G704), and it was the wrong shape anyway: every other probe in this package goes through client.DatadogClient. ValidateCredentials calls the same endpoint. It authenticates on the API key alone, which is what makes it the right probe for an organization key — that kind has no application key to pair with, so a probe requiring one would not isolate the credential under test. The empty application key is deliberate and now documented. Datadog refusing the credential is now returned as "not valid" rather than as a probe error, which is what the revocation check was already treating it as. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
gosec flags G101 on a struct literal that assigns string constants to fields named apiKey and appKey, even in a test against a fake provider. These tests read advertised capabilities, which never touch the connector's own credentials, so the fields were doing nothing. Leaving them unset removes the finding rather than suppressing it. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
The credential type discriminator is in a released tag now, so the pre-release pseudo-version this pinned can go. It pointed at the pull request head, which the squash merge has since made unreachable. The vendored diff is smaller than the version jump suggests: the discriminator's generated sources are byte-identical, because the pseudo-version already carried them. What changes is everything else that landed on main while the pull request was open. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
highb
merged commit Aug 28, 2026
d69f124
into
santhosh.kumar/credential-issuance
7 of 10 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Targets
santhosh.kumar/credential-issuance(the branch behind #40) rather thanmain, so the diff shows only the delta on top of that work.Why
Two changes that belong together, because the SDK invariant ties them.
1. Carry the credential type discriminator. baton-sdk#1109 splits credential shape from credential kind: the closed
CredentialIssueOptionDescriptor.optionenum keeps naming the shape, and the opensecret_resource_type_idnames which kind of that shape comes back. Descriptors are now identified by the pair,CredentialIssueOptions.secret_resource_type_idis required on every issue request, and a newpreferredflag names the default descriptor when several share a shape.Datadog has two kinds of API key and they are not interchangeable. A service account application key is scoped to and owned by one identity. An organization API key is owned by the whole organization, has no owner inside it, and cannot be scoped at all. Both are the
API_KEYshape, so the shape enum alone cannot tell a caller which one it is getting.This connector now advertises one descriptor per kind and dispatches
Issueon the requested type instead of always minting an application key. An unrecognised type is refused rather than falling back to a default arm.2. Gate organization API key deletion on its own grant. The organization API key deleter was registered whenever
sync-secretswas on. Any install already syncing secrets would acquire org-wide Datadog API key deletion the moment it upgraded, without anyone choosing it. Reading a credential inventory is not consent to destroy what is in it.Deletion now requires
allow-org-api-key-deletion, a separate flag defaulting to off. Sync behaviour is unchanged either way.The capability has to be absent, not merely refused at call time: C1 resolves what it may do from the advertisement, and the SDK derives
CAPABILITY_RESOURCE_DELETEfrom a type assertion on the registered syncer. SoDeletemoved offapiTokenBuilderontodeletableAPITokenBuilder, and only that variant is registered when the grant is set. Advertised Datadog permissions follow the same split:api_keys_deleteandapi_keys_writeappear only on the variant that can reach them.How the two interact
The SDK refuses to register an issuance descriptor whose
secret_resource_type_idhas noResourceDeleterV2. So organization API key issuance followsallow-org-api-key-deletionas well. That constraint is the right one: a credential this connector cannot revoke is one it should not mint.Verification
Against a live disposable Datadog organization, both arms issue and revoke real credentials:
Each test issues, confirms the provider lists the key, confirms it authenticates, revokes it through the connector's own deleter, confirms it stops authenticating, and confirms it is delisted.
TestOrganizationAPIKeyIssueLifecycleis new; the existing smoke test covered only the application key arm, so the new dispatch arm had no live coverage. It asserts the issued resource comes back asapi-keyrather than the preferredservice-account-application-key, which is the dispatch claim.Seven new unit tests cover the gate through
NewConnector/GetMetadata(the advertisement C1 actually reads), the permission split, both-kinds advertisement, and refusal of scoped, unknown, and ungranted requests.go build,go build -tags baton_lambda_support,go vet,go test ./...andgolangci-lint runare all clean.Before merge
go.modpins a pre-release baton-sdk revision (v0.25.2-0.20260827221151-1ff7ce2d6fda, the head of baton-sdk#1109). It must be re-pointed at a released baton-sdk tag before this merges. A directrequireis used rather than areplacebecause this repo'sgolangci-lintconfig enablesgomoddirectives, which rejectsreplaceoutright.Until that release lands,
check-versions / verify-versions-matchwill fail: it comparesgo.modagainst.versions.yamlonmain(pinned tov0.24.4), and a sibling job blocks any PR that edits.versions.yamlbecause baton-admin owns that file.Noticed while testing, not fixed here
Datadog returns 404 from the service account application keys endpoint for a disabled service account.
applicationKeyBuilder.listApplicationKeyPagetreats that 404 as fatal and reports it as "the service account was not found, and may have been deleted mid-sync", so any Datadog organization containing a disabled service account fails async-secretssync. Reproduced live. That code is untouched by this PR and the behaviour predates it, but it looks worth its own issue.