Skip to content

fix(e2e): reap stale OAuth2 credential providers alongside API key providers#1679

Merged
aidandaly24 merged 2 commits into
mainfrom
fix/e2e-oauth2-cleanup-reaper
Jul 1, 2026
Merged

fix(e2e): reap stale OAuth2 credential providers alongside API key providers#1679
aidandaly24 merged 2 commits into
mainfrom
fix/e2e-oauth2-cleanup-reaper

Conversation

@aidandaly24

@aidandaly24 aidandaly24 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Description

The E2E suite's globalSetup reaps stale credential providers before each run via cleanupStaleCredentialProviders, but it only walked the API key list. OAuth2 credential providers are a separate resource type with their own List/Delete APIs — they do not appear in ListApiKeyCredentialProviders — so they were never reaped.

The CUSTOM_JWT harness test (harness-custom-jwt.test.ts) registers a managed OAuth2 provider named <harness>-oauth, created imperatively outside the CloudFormation stack. Stack teardown doesn't remove it and nothing else reaped it, so every run leaked one E2eHrnsJwt<ts>-oauth provider. These accumulate against the account's 50-provider OAuth2 quota (L-431051DC). Once full, every CUSTOM_JWT deploy fails with:

The number of agent identity Oauth2 credential providers in this account has reached its limit

which fails the E2E Tests (Full Suite) CUSTOM_JWT shard (the account was at exactly 50/50, all E2eHrnsJwt*-oauth).

This is a test-infra fix. (The underlying CLI-side leak — the CLI orphaning providers it creates on teardown, which affects real users too — is addressed separately in #1675.)

Changes

Single-file change in e2e-tests/utils/credential-provider-cleanup.ts:

  • cleanupStaleCredentialProviders now reaps both provider types in one call — it walks the API key list and the OAuth2 list (ListOauth2CredentialProvidersCommand / DeleteOauth2CredentialProviderCommand). Splitting cleanup by resource type is exactly what let OAuth2 providers go unreaped; from the caller's view "reap stale credential providers" is one job, so it's one function.
  • Adds a deleteOAuth2CredentialProvider helper (mirrors the existing deleteCredentialProvider).

globalSetup is unchanged — it already calls cleanupStaleCredentialProviders, which now covers OAuth2 too.

Related Issue

Closes #1674

Documentation PR

N/A — internal E2E test infrastructure only.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Other (please describe):

Testing

How have you tested the change?

  • I ran npm run test:unit and npm run test:integ
  • I ran npm run typecheck
  • I ran npm run lint
  • If I modified src/assets/, I ran npm run test:update-snapshots and committed the updated snapshots

Additional verification (live, against the E2E account):

  • Confirmed OAuth2 providers do not appear in ListApiKeyCredentialProviders (only in ListOauth2CredentialProviders), proving the old single-list cleanup could never reap them.
  • Seeded one E2e-prefixed API key provider and one OAuth2 provider, ran the unified cleanupStaleCredentialProviders, and confirmed it deleted both (Deleted credential provider: ... + Deleted OAuth2 credential provider: ...), leaving none behind.
  • Manually reaped the 5 leaked E2eHrnsJwt*-oauth providers to unblock the suite immediately.

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.

The e2e globalSetup already reaps stale API key credential providers, but
OAuth2 providers are a separate resource type with their own List/Delete
APIs — they do NOT appear in ListApiKeyCredentialProviders, so the existing
cleanup can never see them. The CUSTOM_JWT harness test registers a managed
OAuth2 provider (`<name>-oauth`) created outside the CloudFormation stack, so
teardown leaves it behind. These accumulate against the account's 50-provider
OAuth2 quota (L-431051DC) until every CUSTOM_JWT deploy fails with "The number
of agent identity Oauth2 credential providers in this account has reached its
limit", failing the E2E suite.

Add cleanupStaleOAuth2CredentialProviders (mirrors the API key reaper, using
ListOauth2CredentialProviders / DeleteOauth2CredentialProvider) and wire it
into globalSetup alongside the existing stack / API key / recommendation
cleanups.
@aidandaly24 aidandaly24 requested a review from a team July 1, 2026 15:44
@github-actions github-actions Bot added the size/s PR size: S label Jul 1, 2026
@github-actions github-actions Bot added the agentcore-harness-reviewing AgentCore Harness review in progress label Jul 1, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jul 1, 2026
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.21.1.tgz

How to install

gh release download pr-1679-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.21.1.tgz

@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jul 1, 2026

@agentcore-cli-automation agentcore-cli-automation left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed the changes. This is a clean, low-risk test-infra fix that mirrors the existing cleanupStaleCredentialProviders pattern almost exactly, only against the OAuth2 List/Delete APIs. The rationale is well-documented in both the code comment and PR description, and the author reports live-verifying against the E2E account that (a) OAuth2 providers don't appear in ListApiKeyCredentialProviders and (b) the new reaper successfully deletes a seeded E2e-prefixed OAuth2 provider.

The 30-minute minAgeMs and E2e prefix match the existing reapers, so no in-flight run will get its provider yanked. Pagination and filter logic are identical to the API-key path.

Nothing blocking. LGTM.

@github-actions github-actions Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Jul 1, 2026
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 38.03% 14093 / 37055
🔵 Statements 37.32% 15009 / 40215
🔵 Functions 32.66% 2423 / 7417
🔵 Branches 31.92% 9381 / 29385
Generated in workflow #3954 for commit 738ce44 by the Vitest Coverage Report Action

…Providers

Fold the OAuth2 reaping into cleanupStaleCredentialProviders rather than a
separate function + second globalSetup call. From the caller's view "reap
stale credential providers" is one job; splitting it by resource type is what
let OAuth2 providers go unreaped in the first place. The function now walks
both the API key and OAuth2 list APIs. globalSetup reverts to a single call
(no longer changed vs main).
@github-actions github-actions Bot added size/s PR size: S and removed size/s PR size: S labels Jul 1, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jul 1, 2026
@aidandaly24 aidandaly24 changed the title fix(e2e): reap stale OAuth2 credential providers in globalSetup fix(e2e): reap stale OAuth2 credential providers alongside API key providers Jul 1, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jul 1, 2026

@Hweinstock Hweinstock 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.

nice fix!

}
): Promise<void> {
const cutoff = new Date(Date.now() - options.minAgeMs);
const isStale = (p: { name?: string; createdTime?: Date }): boolean =>

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.

nit: the helper function is called isStale but includes prefix matching logic which feels unrelated.

@aidandaly24 aidandaly24 merged commit 5e9f6d0 into main Jul 1, 2026
33 of 34 checks passed
@aidandaly24 aidandaly24 deleted the fix/e2e-oauth2-cleanup-reaper branch July 1, 2026 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/s PR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

E2E: harness-custom-jwt shard fails — leaked OAuth2 credential providers exhaust account quota

3 participants