Skip to content

Commit 8af1257

Browse files
committed
fix: address review comments — add clarifying comments for vendor config, race condition, and ARN inconsistency
1 parent a717414 commit 8af1257

1 file changed

Lines changed: 18 additions & 1 deletion

File tree

src/cli/operations/identity/oauth2-credential-provider.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,14 @@ export interface OAuth2ProviderParams {
3232
* Extract credential provider ARN from API response, handling field name inconsistency.
3333
* The API may return the ARN as `credentialProviderArn` or `oAuth2CredentialProviderArn`.
3434
*/
35+
/**
36+
* Extract credential provider ARN from API response, handling field name inconsistency.
37+
* The API may return the ARN as `credentialProviderArn` or `oAuth2CredentialProviderArn`.
38+
*
39+
* Note: This casts to Record<string, unknown> to handle the inconsistency. The typed SDK
40+
* response only declares `credentialProviderArn`, but older API versions may return
41+
* `oAuth2CredentialProviderArn`. Remove the fallback once the API stabilizes.
42+
*/
3543
function extractArn(response: Record<string, unknown>): string | undefined {
3644
return (
3745
(response.credentialProviderArn as string | undefined) ??
@@ -77,6 +85,12 @@ export async function oAuth2ProviderExists(
7785
}
7886
}
7987

88+
/**
89+
* Build the OAuth2 provider config for Create/Update commands.
90+
* Always uses customOauth2ProviderConfig regardless of vendor — the vendor field
91+
* controls server-side behavior (token endpoints, scopes), but the config shape
92+
* is the same for all vendors in the current API.
93+
*/
8094
function buildOAuth2Config(params: OAuth2ProviderParams) {
8195
return {
8296
name: params.name,
@@ -111,7 +125,10 @@ export async function createOAuth2Provider(
111125
} catch (error) {
112126
const errorName = (error as { name?: string }).name;
113127
if (errorName === 'ConflictException' || errorName === 'ResourceAlreadyExistsException') {
114-
// Unlike API key providers, OAuth needs the ARN back for deployed-state.json
128+
// Unlike API key providers, OAuth needs the ARN back for deployed-state.json.
129+
// This only triggers in a race condition (another process created between exists-check
130+
// and create). The caller already routes to update for known-existing providers, so
131+
// falling back to GET here is safe — the next deploy will update with fresh credentials.
115132
return getOAuth2Provider(client, params.name);
116133
}
117134
return {

0 commit comments

Comments
 (0)