Add Datadog credential issuance

[santhosh-c1]

Datadog credential issuance

What this enables

• Discovers Datadog organization API keys and service-account application keys as two distinct secret kinds.
• Mints connector-sealed credentials of either kind after approval, with the caller choosing which.
• Revokes issued credentials on manual revoke or expiry.

Dependencies

• Based directly on baton-datadog main.
• Requires baton-sdk v0.26.0 (feat(connectorbuilder): resolve credential issuers on shape + output type baton-sdk#1109), which is the released tag this branch pins.
• Carry the credential type discriminator, and gate org API key deletion #41 is merged into this branch; it is no longer a separate step.
• Publish a connector release before final C1 dependency updates.

Summary

• Issue Datadog service account application keys, deterministically named from the C1 request id.
• Issue Datadog organization API keys as a second kind of the same API_KEY shape, selected by the caller.
• Revoke issued keys of either kind.
• Sync organization API keys and service-account application keys as distinct secret resource types.
• Advertise credential issuance only when secret sync is enabled, and organization-API-key issuance and deletion only when explicitly granted.
• Add opt-in live smoke tests for issue, authenticate and revoke on both kinds.

Two kinds of API key

Datadog organization API keys and service-account application keys are two kinds of the same shape. Both are CAPABILITY_DETAIL_CREDENTIAL_OPTION_API_KEY; they differ in what they are, who owns them, and what they can do. Before baton-sdk#1109 a connector could advertise only one descriptor per shape, so a connector offering both had no way to say which one a request meant.

That SDK release makes descriptor identity the pair (option, secret_resource_type_id) and requires CredentialIssueOptions.secret_resource_type_id on every issue request. This connector advertises both:

Kind	secret_resource_type_id	Scopes	Default
Service account application key	service-account-application-key	Custom scopes allowed	Preferred
Organization API key	api-key	None — Datadog org keys cannot be scoped

Issue reads the requested type off the request and dispatches. An unrecognized type is refused rather than silently minting the other kind.

Mapping

Application-key issuance targets a service account, not a human. An org-scoped key issued "on behalf of" a selected user is not an honest record of who holds it, so Issue re-checks that the selected Datadog user is still a service account, refuses a human user, and mints a key owned by and scoped to that service account.

Issue is non-duplicating on both kinds: it looks the deterministic request name up first and refuses rather than minting a second key, because Datadog cannot re-return plaintext material.

Revocation of an application key needs the owning service account as well as the key — Datadog has no delete-by-key-id-alone form. It travels in the resource deleter's existing parentResourceID rather than a packed composite handle, so the handle stays a bare provider id like every other secret this connector syncs. No C1 caller populates it today, so when it is absent the connector reads the owner from the key's owned_by relationship and proceeds; a key whose owner cannot be identified is refused rather than guessed at.

Gating organization API key deletion

Organization API keys belong to the organization, not to the person they were issued to, and they cannot be scoped. Deleting one is destructive in a way reading one is not, so read capability is not consent to destroy: the deleter is registered only when allow-org-api-key-deletion is set, and that grant is off by default.

The gate is structural, not a guard inside a method body. The SDK derives CAPABILITY_RESOURCE_DELETE from a type assertion on the registered syncer, so the deleter lives on its own type and is simply not registered without the grant — the capability is absent from what the connector advertises rather than advertised and refused at call time.

The same grant gates organization-API-key issuance, because baton-sdk#1109 requires every advertised issuance descriptor to have a registered deleter. C1 will not mint a credential it has no permission to revoke.

Permissions

Advertised permissions come from the x-permission block Datadog publishes for each operation in its own OpenAPI spec, not from inference:

Capability	Endpoint	Permission
Sync organization API keys	ListAPIKeys	api_keys_read
Issue organization API keys ‡	CreateAPIKey	api_keys_write
Revoke organization API keys ‡	DeleteAPIKey	api_keys_delete
Sync, issue and revoke application keys	List / Create / DeleteServiceAccountApplicationKey	service_account_write

‡ Only reachable with Allow organization API key deletion on. With it off, organization API keys still sync; they simply cannot be issued or deleted, and the extra permissions are not exercised.

service_account_write sits only on the secret resource types, which are registered only when secret sync is on, so a secrets-off install is not asked to grant it.

Verification

• go build ./... — clean
• go vet ./... — clean
• go test ./... -count=1 — pass, 79 assertions in pkg/connector
• make lint — 0 issues.
• baton_capabilities.json regenerated from the built binary rather than hand-edited.

Unit coverage for the new paths: both issuance kinds advertised with distinct output types; dispatch on the requested type; refusal of an unrecognized type; refusal of the organization kind without the grant; the delete capability absent from advertised capabilities without the grant and present with it; application-key create and delete plus provider 404 mapping; create with no returned key material; exact-match-after-filter lookups including a cross-page match; duplicate-request refusal; handle-is-not-the-secret assertions on both delete paths; owner resolution from owned_by when parentResourceID is absent, and refusal when the owner cannot be identified; one-provider-page-per-List-call pagination; bounded paging at both levels of the application-key walk; disabled service accounts skipped rather than failing the sync; and hard failure of the sync on both a 403 and a 404 from the service-account application-key list.

Live provider

The live smoke tests are opt-in behind DATADOG_CREDENTIAL_SMOKE=1 and are skipped in CI. Both kinds were exercised against a disposable Datadog trial organization: minted, found through the provider's own list API, used to authenticate a real request, revoked, confirmed to stop authenticating, and confirmed delisted.

Application-key ownership was confirmed independently of the connector:

• the org application-key record reports owned_by as the named service account, and that owner's user record has service_account: true;
• the key returns 404 under a different service account and 404 as a current-user application key;
• the key is absent from the organization API-key list and from the current-user application-key list.

Multi-account attachment was verified across several service accounts: each vended key was ATTACHED to its own account and absent from the others.

Not covered, stated so this is not read as broader than it is: multi-page application-key pagination was not exercised, and neither was reduced-permission behavior — the trial credential holds Datadog Admin-role permissions, so the run shows that service_account_write is sufficient but not that it is necessary.

Notes

• Datadog application keys do not support create-time expiry.
• Datadog may retain key metadata after revocation, so authentication failure is the revoke assertion — and only a 401/403 counts as evidence, not any error.
• Datadog answers 404, not an empty list, for a disabled service account's application keys, and it never deletes users, only disables them. The walk skips disabled accounts; without that, one disabled account made every application key in the organization unsyncable.
• Deleting a key that is already absent is treated as success. A revoke that names the wrong owning service account is currently indistinguishable from that case and also returns success while the key survives; this is judged unreachable in the current caller flow and is tracked separately rather than changed here.
• A List call returns at most one provider page so the SDK keeps control of checkpointing, rate limits and cancellation. Both levels of the walk are page-bounded so a provider that ignores page[number] fails closed instead of paging forever.
• A service account whose keys the role cannot read fails the whole sync rather than being skipped, because C1 reads a resource absent from a completed sync as deleted, so skipping would retire live credentials from the inventory instead of reporting that they could not be read. A 403 and a 404 are both terminal but carry different messages: the 403 names service_account_write, and the 404 says the service account was not found and may have been deleted mid-sync.
• This is a breaking change for existing installs. An install with secret sync enabled whose Datadog role lacks service_account_write now fails its entire sync, where before it synced without application keys. That is the cost of failing hard; the counterweight is that the failure names the missing permission, so it diagnoses itself, whereas a sync that completes while reporting an empty credential inventory does not.

[github-actions]

Connector PR Review: Add Datadog credential issuance

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base ffab37fa49a9.
Review mode: incremental since 2ff769bb
View review run

Review Summary

The new commit (e1fe609c) is docs-only: it adds BATON_SYNC_SERVICE_ACCOUNT_APPLICATION_KEYS to the self-hosted env-var example, which addresses the prior finding at docs/connector.mdx:254 — the block now shows the application-key flag alongside BATON_SYNC_SECRETS and BATON_ALLOW_ORG_API_KEY_DELETION. The full PR diff was re-scanned for security and correctness (client, issuance and revoke paths, the two-level pagination bag walk, resource types, config, and the go.mod / go.sum bump to baton-sdk v0.26.0 plus testify moving to a direct require, both of which match the code changes); no blocking issues found. One new suggestion: the explanatory comment added with the new env var overstates what org_app_keys_read is required for.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• docs/connector.mdx:255 — the new comment says the sync fails without org_app_keys_read, but that permission only backs the revoke owner lookup; only service_account_write is sync-fatal, per pkg/connector/resource_types.go and docs/connector.mdx:57.

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In `docs/connector.mdx`:
- Around line 253-255: The comment above BATON_SYNC_SERVICE_ACCOUNT_APPLICATION_KEYS says the
  connector needs service_account_write and org_app_keys_read and that "without them the sync
  fails". That overstates org_app_keys_read. Per the comment on
  serviceAccountApplicationKeyResourceType in pkg/connector/resource_types.go (a role holding
  only service_account_write still syncs and issues, and fails just the owner lookup) and the
  prose at docs/connector.mdx:57, org_app_keys_read is only used by the revoke path's owner
  lookup (client.FindApplicationKeyOwner); only service_account_write is sync-fatal (see
  pkg/connector/application_key.go around line 254). Reword so the sync-failure consequence
  attaches to service_account_write alone and org_app_keys_read is described as needed for the
  revoke owner lookup.

[github-actions]

Blocking issues found — see review comments.

[github-actions]

Blocking issues found — see review comments.

[github-actions]

Blocking issues found — see review comments.

[github-actions]

🟠 Bug: This makes a hard, ungated compatibility break for existing installs. applicationKeyBuilder is registered whenever SyncSecrets is on (connector.go:60), and a 403 here aborts the whole sync — so any install already running with sync-secrets: true and a Datadog role that only has api_keys_read starts failing every sync on upgrade, with no opt-in. The PR body acknowledges this, but the connector already has the right pattern for exactly this situation (allow-org-api-key-deletion): gate application-key sync behind its own opt-in flag so existing installs keep syncing until the operator grants service_account_write, and keep the fail-hard behaviour once they've opted in.

[c1-squire-dev]

Squire (claude-opus-5): Implemented in bbff6aba, following the pattern you pointed at.

sync-service-account-application-keys is off by default. Without it the application-key syncer is not registered at all, so the resource type is absent from what C1 is advertised rather than advertised and failing — an install already running with sync-secrets on keeps syncing after an upgrade, and organization API keys still sync as before. Once granted, the fail-hard behaviour is unchanged: a credential absent from a completed sync reads as deleted, so skipping a 403 would retire live credentials from the inventory.

One consequence worth naming, since it follows from #1109 rather than from this comment: the grant also gates issuance of that kind. The SDK refuses an issuance descriptor whose secret resource type has no registered deleter, and the syncer is what carries the deleter. With secrets synced and neither kind granted there is nothing to issue, so CAPABILITY_CREDENTIAL_ISSUE is absent rather than advertised with an empty option list.

baton_capabilities.json is unchanged, because capability generation forces every optional surface on — the point made on the main.go thread.

Pinned by TestApplicationKeySyncRequiresItsOwnGrant, TestApplicationKeySyncAdvertisedWithGrant, and TestCredentialIssueFollowsTheKindGrants across all four grant combinations.

[github-actions]

🟠 Bug: By the PR's own account no C1 caller populates parentResourceID today, so every revoke of an issued service-account application key returns InvalidArgument and the key stays live — the connector mints credentials it cannot revoke while advertising CAPABILITY_RESOURCE_DELETE for this type. That is the same failure mode api_token.go:33-38 argues against ("a capability that is advertised but refuses at call time is worse than one that is absent").

The owning service account is recoverable without a packed handle or a platform change: datadogV2.KeyManagementApi.GetApplicationKey(ctx, appKeyID) returns relationships.owned_by, which is the service-account user id. Consider falling back to that lookup when parentResourceID is nil/empty, and keeping the current fail-closed path only when the lookup itself cannot identify an owner.

[c1-squire-dev]

Squire (claude-opus-5): Fixed in ed78713e, taking the owned_by route you named.

Delete now treats parentResourceID as the fast path rather than a requirement. When it is absent, wrong-typed, or empty, the connector calls KeyManagementApi.GetApplicationKey and reads relationships.owned_by.data.id, then issues the same service-account-scoped DELETE it would have made had the parent been supplied. It fails closed only when that lookup cannot name an owner — so the fallback widens what can be revoked without letting Delete guess.

You were right about the stake: with the SDK now requiring a registered deleter for every advertised issuance descriptor, a delete path that refuses every call is worse than no capability at all, which is the argument api_token.go makes and this file was contradicting.

Covered by two new tests — TestApplicationKeyBuilderDeleteResolvesOwnerFromTheKey across all three absent-parent shapes, asserting the DELETE lands on the service-account-scoped path, and TestApplicationKeyBuilderDeleteFailsWhenOwnerUnknown, asserting a key with no owned_by is still refused and still reaches no DELETE. The docs warning and footnote said revocation "will not complete" until a caller supplies the owner; both are updated in 4bd1ae5d.

[github-actions]

🟡 Suggestion: Forcing this on makes the generated baton_capabilities.json advertise api_keys_write and api_keys_delete on the api-key resource type for every install, which contradicts docs/connector.mdx ("Add API Keys Write and API Keys Delete only if you also enable Allow organization API key deletion") and re-creates the exact problem resource_types.go avoids for service_account_write — sending operators after Datadog Admin rights no reachable code path exercises. Since the two org-key variants share the id api-key, resource-type scoping cannot carry the conditionality here; either leave AllowOrgAPIKeyDeletion false for capability generation, or give the deletable variant a distinct resource type id so the extra permissions are attached only to the gated type.

[c1-squire-dev]

Squire (claude-opus-5): Pushing back on this one.

The forcing is the established convention in this file, not a new choice: SyncSecrets and SyncSchedules are set the same way, immediately above, under a comment saying every optional surface is forced on so ./connector capabilities documents the connector's full capability set. baton_capabilities.json is a static artifact with no conditional form — it describes what the connector can do, not what a given install is configured to do. Singling out AllowOrgAPIKeyDeletion would make that document describe a partial connector while the other two flags keep describing a whole one.

The suggested alternative — give the deletable variant a distinct resource type id — would be worse than the problem. The two variants are the same kind of credential; the id api-key is what C1 matches an issuance descriptor against. Splitting it would advertise two organization-API-key kinds that differ only in whether the operator granted a permission, which is exactly the implicit selection baton-sdk#1109 set out to remove.

The docs inconsistency you point at is real but is a docs question, and docs/connector.mdx already resolves it correctly: it tells the operator to add API Keys Write and API Keys Delete only alongside Allow organization API key deletion. That is install guidance, and it should stay conditional. The capability document should not.

Leaving unresolved for a human to disagree with.

[github-actions]

🟡 Suggestion: The users level of this walk has no page bound, while the application-key level got maxApplicationKeyPages for exactly the failure it guards against. A provider that ignores page[number] and keeps returning full users pages loops here forever (re-pushing child states each round), which is the same "fails closed instead of paging forever" case. Consider applying an equivalent cap to the users page counter.

[c1-squire-dev]

Squire (claude-opus-5): Fixed in ed78713e. maxUserPages now bounds the users level the way maxApplicationKeyPages bounds the application-key level, with the same 10,000-page ceiling.

Your reason for it is the sharper one and is recorded on the constant: the users level terminates on an empty page rather than a short one, so a provider that ignores page[number] and keeps returning full pages would re-push child states every round rather than merely spinning — the unbounded case is worse here than at the level that already had a bound.

[github-actions]

Blocking issues found — see review comments.

[github-actions]

🟠 Bug (high confidence): every non-NotFound failure from FindApplicationKeyOwner is collapsed into codes.InvalidArgument, but wrapOfficialClientError (pkg/client/errors.go:17-33) deliberately maps a network error/429 to Unavailable, 5xx to Internal, and 403 to PermissionDenied precisely so the caller can retry or diagnose. A transient blip during the owner lookup therefore surfaces as a terminal "invalid argument" revoke failure and the issued key stays live at Datadog — the same stranded-credential outcome this commit set out to remove. The %v also drops the wrapped error and the rate-limit annotations WrapErrorsWithRateLimitInfo attached.

The genuinely-unresolvable cases (owned_by omitted / empty owner id) are plain fmt.Errorf, so status.Code(err) == codes.Unknown distinguishes them cleanly:

owner, err := o.wrapper.FindApplicationKeyOwner(ctx, appKeyID)
if err != nil {
    switch status.Code(err) {
    case codes.NotFound:
        return nil, nil
    case codes.Unknown:
        return nil, status.Errorf(codes.InvalidArgument,
            "baton-datadog: the owning service account for application key %q could not be determined: %v", appKeyID, err)
    default:
        return nil, fmt.Errorf("baton-datadog: resolve owner for application key %q: %w", appKeyID, err)
    }
}

TestApplicationKeyBuilderDeleteFailsWhenOwnerUnknown still passes under this shape.

[c1-squire-dev]

Squire (claude-opus-5): Correct, and fixed in 6dd74a35 with the shape you wrote.

The bug was self-defeating in exactly the way you describe: the fallback exists so a revoke does not strand a key, and collapsing a 500 or a 429 into InvalidArgument stranded it anyway, for a failure that would have succeeded on retry. %v also discarded the wrapped error and the rate-limit annotations.

codes.Unknown is the right discriminator — the two genuinely unresolvable cases are plain fmt.Errorf, so they arrive as Unknown while every provider-classified failure keeps its code.

One correction to the description rather than the fix: uhttp.GrpcCodeFromHTTPStatus maps 500 to Unavailable, not Internal. TestApplicationKeyBuilderDeletePreservesOwnerLookupFailureCode asserts the mapping the code actually produces — Unavailable for a 500, PermissionDenied for a 403 — rather than the one in the comment.

[github-actions]

🟡 Suggestion (medium confidence): this puts a new endpoint on the revoke path — GET /api/v2/application_keys/{id} (KeyManagementApi, org-scoped), not a ServiceAccountsApi call. resource_types.go:103-113 still asserts "Every service-account application-key endpoint this connector calls requires exactly one Datadog RBAC permission, service_account_write: List / Create / DeleteServiceAccountApplicationKey", and docs/connector.mdx says Service Account Write "governs syncing, issuing, and revoking service account application keys". That enumeration no longer covers the code. Datadog governs org-level application-key reads under its own permission (org_app_keys_read), so a role holding exactly the documented set would 403 here — and per the thread on application_key.go that 403 is reported as InvalidArgument, so every revoke fails with a misleading message.

Please re-check GetApplicationKey's x-permission block against the same spec the rest of this file was derived from, then either add the permission to capabilityPermissions on serviceAccountApplicationKeyResourceType and the docs custom-role list, or update the two comments to say why it is already covered.

[c1-squire-dev]

Squire (claude-opus-5): Confirmed against the spec and fixed in 6dd74a35. You are right on both halves.

Checked GetApplicationKey's x-permission block in the same spec the rest of these permissions came from (DataDog/datadog-api-client-go, .generator/schemas/v2/openapi.yaml):

• GetApplicationKey → org_app_keys_read
• ListApplicationKeys → org_app_keys_read
• ListServiceAccountApplicationKeys, CreateServiceAccountApplicationKey, DeleteServiceAccountApplicationKey → service_account_write

So a role holding exactly what this connector advertised would have 403'd on every revoke that relied on the fallback. org_app_keys_read is now advertised alongside service_account_write on serviceAccountApplicationKeyResourceType, the resource_types.go comment says which endpoint is governed by which rather than claiming one permission covers them all, and the custom-role list in docs/connector.mdx names it.

Worth noting the failure is now partial rather than total, which it was not when you wrote this: with the error-mapping fix on the sibling thread, a role missing org_app_keys_read still syncs and still issues — it fails only the owner lookup, and reports PermissionDenied naming the permission instead of a misleading InvalidArgument.

The same spec check confirms the enumeration for the service-account endpoints was right, so only the org-scoped addition was missing.

[github-actions]

Blocking issues found — see review comments.

[github-actions]

🟡 Suggestion: this comment is now stale after the new gate. It says sync-secrets "is the flag that swaps credentialUserBuilder in for userBuilder and registers applicationKeyBuilder at all", and that serviceAccountApplicationKeyResourceType "is only registered when sync-secrets is on". As of connector.go:50-79, both also require sync-service-account-application-keys, and the user syncer is swapped only when at least one credential kind is granted. Worth updating so the reasoning for scoping service_account_write to this resource type still matches the code. (confidence: high)

[c1-squire-dev]

Squire (claude-opus-5): Fixed in 2ff769bb. The comment now says both flags: sync-secrets swaps credentialUserBuilder in for userBuilder, and sync-service-account-application-keys is what registers applicationKeyBuilder and puts that kind in the descriptor list.

The reasoning it exists to record survives the change — scoping service_account_write to the resource type is still how the conditionality is carried, since baton_capabilities.json has no conditional form. It just runs through two flags now instead of one, so the sentence describing "only when sync-secrets is on" was the stale part rather than the argument.

[github-actions]

🟡 Suggestion: the * footnote now reads "Schedules and application keys are not enabled by default", but this row carries no * marker — only the Schedules row does. A reader scanning just the table will conclude service account application keys sync out of the box with Sync secrets. Adding * to this row's Sync cell (and/or Issue/Revoke) would make the new gate visible in the table itself. (confidence: high)

[c1-squire-dev]

Squire (claude-opus-5): Fixed in 2ff769bb. The application-key row now carries * on Sync, Issue and Revoke, so the gate is visible to someone reading only the table.

The footnote was doing the wrong job too — it lumped schedules and application keys into one sentence, so even a reader who followed the marker got "enable Sync schedules or Sync secrets, respectively". It now separates them and says the part that was missing: Sync secrets alone syncs organization API keys only.

[github-actions]

No blocking issues found.

[github-actions]

🟡 Suggestion (medium confidence): this self-hosted env-var example gained BATON_ALLOW_ORG_API_KEY_DELETION but not BATON_SYNC_SERVICE_ACCOUNT_APPLICATION_KEYS, which is the flag gating this PR's headline feature. A self-hosted operator copying this block gets org-API-key deletion but no application-key sync/issue/revoke, and the block is the only place the connector's env-var names are spelled out. Worth adding it next to BATON_SYNC_SECRETS with a note that both are required.

[c1-squire-dev]

Squire (claude-opus-5): Fixed in e1fe609c. BATON_SYNC_SERVICE_ACCOUNT_APPLICATION_KEYS now sits directly under BATON_SYNC_SECRETS in that block, with a comment saying the two go together and naming the Datadog permissions it needs (service_account_write and org_app_keys_read).

The BATON_SYNC_SECRETS comment was misleading on its own too, so it now says what that flag does by itself: syncs organization API keys only.

[github-actions]

No blocking issues found.

[github-actions]

🟡 Suggestion (medium confidence): this new comment says org_app_keys_read is one of the permissions "without them the sync fails", but the connector's own account of that permission is narrower. serviceAccountApplicationKeyResourceType's comment in pkg/connector/resource_types.go says "A role holding only service_account_write still syncs and issues -- it fails just the owner lookup", and the prose at docs/connector.mdx:57 likewise scopes org_app_keys_read to the revoke owner-lookup path. Only service_account_write is sync-fatal (pkg/connector/application_key.go:254). An operator reading only this block will think a missing org_app_keys_read breaks their sync.

Suggested change

	# service account application keys. Needs the Datadog service_account_write
	# and org_app_keys_read permissions; without them the sync fails.
	# service account application keys. Needs the Datadog service_account_write
	# permission, without which the sync fails, plus org_app_keys_read for the
	# revoke path's owner lookup.

[github-actions]

No blocking issues found.