Skip to content

Commit 3fb8f16

Browse files
notgitikagitikavj
andauthored
fix(migrate): backfill credential secret ARNs v0 didn't record (#2600)
* fix(migrate): backfill credential secret ARNs v0 didn't record v0 never records clientSecretArn for API-key credential providers, and v1's CDK app needs it to let a Gateway read the secret, so migrate failed at v1 synth for any v0 project with an authenticated Gateway target. Migrate now looks up each missing secret ARN with the selected target's credentials and records it in the synth workspace and the migrated deployed-state.json. Closes #2599 * fix(migrate): backfill secret ARNs for every deployed target The vended CDK app synthesizes a stack for every target, so backfilling only the selected target still failed v1 synth (and build/deploy after migrating) for a multi-target project with an authenticated Gateway. Look up missing secret ARNs for every deployed target the active credentials' account owns, resolving credentials once per region, and fail the check with recovery steps for a target in another account whose Gateway credentials lack one. --------- Co-authored-by: gitikavj <gitikavj@amazon.com>
1 parent 165656d commit 3fb8f16

8 files changed

Lines changed: 950 additions & 9 deletions

File tree

‎src/core/index.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ export class CoreClient implements AwsClients {
155155
new FsProjectMigrator({
156156
logger: this.logger.child({ module: "projectMigrator" }),
157157
createCloudFormationClient: config.createCloudFormationClient,
158+
identity: this.identity,
158159
});
159160
}
160161

Lines changed: 310 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,310 @@
1+
import { afterEach, describe, expect, test } from "bun:test";
2+
import { readFile } from "node:fs/promises";
3+
import { join } from "node:path";
4+
import { ResourceNotFoundException } from "@aws-sdk/client-bedrock-agentcore-control";
5+
import {
6+
backfillCredentialSecrets,
7+
gatewayCredentialNames,
8+
type CredentialSecretLookups,
9+
type TargetAccess,
10+
} from "./credentialSecrets";
11+
import { F1_DEPLOYED_STATE, F1_SPEC, writeV0Project } from "./migrate-test-support";
12+
13+
const cleanups: Array<() => Promise<void>> = [];
14+
afterEach(() => Promise.all(cleanups.splice(0).map((cleanup) => cleanup())));
15+
16+
const ARN = "arn:aws:bedrock-agentcore:us-east-1:998846730471";
17+
const KEY_SECRET = "arn:aws:secretsmanager:us-east-1:998846730471:secret:apikey/compatA-key-AbCdEf";
18+
const OAUTH_SECRET =
19+
"arn:aws:secretsmanager:us-east-1:998846730471:secret:oauth2/compatA-oauth-GhIjKl";
20+
const DENIED = "User is not authorized to perform this action";
21+
22+
/** Identity reads answered from a map of provider name to secret ARN (or the error to throw). */
23+
function fakeIdentity(secrets: Record<string, string | Error | undefined>) {
24+
const calls: string[] = [];
25+
const answer = (kind: string, name: string) => {
26+
calls.push(`${kind}:${name}`);
27+
const secret = secrets[name];
28+
if (secret instanceof Error) throw secret;
29+
return secret;
30+
};
31+
const identity: CredentialSecretLookups = {
32+
getApiKeyCredentialProvider: async (name) => {
33+
const secretArn = answer("apiKey", name);
34+
return { ...(secretArn && { apiKeySecretArn: { secretArn } }) } as never;
35+
},
36+
getOauth2CredentialProvider: async (name) => {
37+
const secretArn = answer("oauth2", name);
38+
return { ...(secretArn && { clientSecretArn: { secretArn } }) } as never;
39+
},
40+
};
41+
return Object.assign(identity, { calls });
42+
}
43+
44+
async function project(deployedState: unknown = F1_DEPLOYED_STATE): Promise<string> {
45+
const { root, cleanup } = await writeV0Project({ deployedState });
46+
cleanups.push(cleanup);
47+
return root;
48+
}
49+
50+
const DEFAULT = { name: "default", account: "998846730471", region: "us-east-1" };
51+
52+
function backfill(
53+
root: string,
54+
identity: CredentialSecretLookups,
55+
gatewayCredentials: string[] = [],
56+
credentials: { name: string; authorizerType: string }[] = F1_SPEC.credentials,
57+
targets: { name: string; account: string; region: string }[] = [DEFAULT],
58+
access: (target: { region: string }) => Promise<TargetAccess> = async (target) => ({
59+
options: { region: target.region },
60+
}),
61+
) {
62+
return backfillCredentialSecrets({
63+
identity,
64+
rootPath: root,
65+
credentials,
66+
gatewayCredentials: new Set(gatewayCredentials),
67+
targets,
68+
access,
69+
});
70+
}
71+
72+
const recorded = (deployedState: string | undefined) =>
73+
JSON.parse(deployedState!).targets.default.resources.credentials;
74+
75+
const notFound = () => new ResourceNotFoundException({ message: "not found", $metadata: {} });
76+
77+
describe("backfillCredentialSecrets", () => {
78+
test("fills in the API key and OAuth secret ARNs v0 didn't record", async () => {
79+
const root = await project();
80+
const identity = fakeIdentity({ "compatA-key": KEY_SECRET, "compatA-oauth": OAUTH_SECRET });
81+
82+
const result = await backfill(root, identity);
83+
84+
expect(identity.calls).toEqual(["apiKey:compatA-key", "oauth2:compatA-oauth"]);
85+
expect(recorded(result.deployedState)).toEqual({
86+
"compatA-key": {
87+
credentialProviderArn: `${ARN}:token-vault/default/apikeycredentialprovider/compatA-key`,
88+
clientSecretArn: KEY_SECRET,
89+
},
90+
"compatA-oauth": {
91+
credentialProviderArn: `${ARN}:token-vault/default/oauth2credentialprovider/compatA-oauth`,
92+
clientSecretArn: OAUTH_SECRET,
93+
},
94+
});
95+
// Everything else in the file is kept as it was.
96+
expect(JSON.parse(result.deployedState!).targets.default.resources.memories).toEqual(
97+
F1_DEPLOYED_STATE.targets.default.resources.memories,
98+
);
99+
expect(result.check).toEqual({
100+
name: "credential secrets",
101+
ok: true,
102+
message:
103+
"found the secret ARNs v0 didn't record for compatA-key, compatA-oauth; migrating " +
104+
"records them in agentcore/.cli/deployed-state.json",
105+
});
106+
expect(result.warnings).toEqual([]);
107+
// The project itself isn't written.
108+
expect(
109+
JSON.parse(await readFile(join(root, "agentcore", ".cli", "deployed-state.json"), "utf8")),
110+
).toEqual(F1_DEPLOYED_STATE);
111+
});
112+
113+
test("makes no lookups when every secret ARN is recorded, and skips payment providers", async () => {
114+
const credentials = {
115+
"compatA-key": {
116+
credentialProviderArn: `${ARN}:token-vault/default/apikeycredentialprovider/compatA-key`,
117+
clientSecretArn: KEY_SECRET,
118+
},
119+
"compatA-pay": {
120+
credentialProviderArn: `${ARN}:token-vault/default/paymentcredentialprovider/compatA-pay`,
121+
},
122+
};
123+
const root = await project({
124+
targets: {
125+
default: { resources: { ...F1_DEPLOYED_STATE.targets.default.resources, credentials } },
126+
},
127+
});
128+
const identity = fakeIdentity({});
129+
130+
const result = await backfill(
131+
root,
132+
identity,
133+
[],
134+
[
135+
{ name: "compatA-key", authorizerType: "ApiKeyCredentialProvider" },
136+
{ name: "compatA-pay", authorizerType: "PaymentCredentialProvider" },
137+
],
138+
);
139+
140+
expect(identity.calls).toEqual([]);
141+
expect(result).toEqual({ warnings: [] });
142+
});
143+
144+
test("fails the check for a Gateway's credential whose secret ARN can't be read", async () => {
145+
const root = await project();
146+
const identity = fakeIdentity({
147+
"compatA-key": new Error(DENIED),
148+
"compatA-oauth": OAUTH_SECRET,
149+
});
150+
151+
const result = await backfill(root, identity, ["compatA-key"]);
152+
153+
expect(result.check?.ok).toBe(false);
154+
expect(result.check?.message).toContain(
155+
`compatA-key: GetApiKeyCredentialProvider failed (${DENIED}); check that these ` +
156+
"credentials can call bedrock-agentcore:GetApiKeyCredentialProvider in us-east-1",
157+
);
158+
// Only the API actually called is named.
159+
expect(result.check?.message).not.toContain("GetOauth2CredentialProvider");
160+
expect(result.check?.message).toContain("run `agentcore migrate` again");
161+
expect(result.check?.message).toContain("The secret ARNs of compatA-oauth were found.");
162+
expect(result.warnings).toEqual([]);
163+
expect(recorded(result.deployedState)["compatA-key"].clientSecretArn).toBeUndefined();
164+
expect(recorded(result.deployedState)["compatA-oauth"].clientSecretArn).toBe(OAUTH_SECRET);
165+
});
166+
167+
test("only warns for credentials no Gateway target uses, with advice for each cause", async () => {
168+
const root = await project();
169+
const identity = fakeIdentity({ "compatA-key": notFound() });
170+
171+
const result = await backfill(root, identity);
172+
173+
expect(result.check).toBeUndefined();
174+
expect(result.deployedState).toBeUndefined();
175+
expect(result.warnings).toEqual([
176+
"v0 didn't record the secret ARN of credential compatA-key, and it couldn't be found: its " +
177+
"provider 'compatA-key' no longer exists in AgentCore Identity; recreate it, or remove " +
178+
"the credential from the project. No Gateway target uses it, so migrating doesn't need it.",
179+
"v0 didn't record the secret ARN of credential compatA-oauth, and it couldn't be found: its " +
180+
"provider 'compatA-oauth' has no stored secret. No Gateway target uses it, so migrating " +
181+
"doesn't need it.",
182+
]);
183+
});
184+
185+
test("names the OAuth API when an OAuth provider's lookup fails", async () => {
186+
const root = await project();
187+
const identity = fakeIdentity({
188+
"compatA-key": KEY_SECRET,
189+
"compatA-oauth": new Error(DENIED),
190+
});
191+
192+
const result = await backfill(root, identity, ["compatA-oauth"]);
193+
194+
expect(result.check?.ok).toBe(false);
195+
expect(result.check?.message).toContain(
196+
`compatA-oauth: GetOauth2CredentialProvider failed (${DENIED})`,
197+
);
198+
expect(result.check?.message).not.toContain("GetApiKeyCredentialProvider");
199+
});
200+
});
201+
202+
describe("backfillCredentialSecrets across targets", () => {
203+
const PROD = { name: "prod", account: "111111111111", region: "eu-west-1" };
204+
const twoTargets = {
205+
targets: {
206+
default: F1_DEPLOYED_STATE.targets.default,
207+
prod: F1_DEPLOYED_STATE.targets.default,
208+
},
209+
};
210+
const otherAccount = async (target: { region: string }): Promise<TargetAccess> =>
211+
target.region === "eu-west-1" ? { account: "998846730471" } : { options: target };
212+
213+
test("fails the check for another account's Gateway credential, saying how to record it", async () => {
214+
const root = await project(twoTargets);
215+
const identity = fakeIdentity({ "compatA-key": KEY_SECRET, "compatA-oauth": OAUTH_SECRET });
216+
217+
const result = await backfill(
218+
root,
219+
identity,
220+
["compatA-key"],
221+
F1_SPEC.credentials,
222+
[DEFAULT, PROD],
223+
otherAccount,
224+
);
225+
226+
// Only the reachable target is looked up.
227+
expect(identity.calls).toEqual(["apiKey:compatA-key", "oauth2:compatA-oauth"]);
228+
expect(result.check?.ok).toBe(false);
229+
expect(result.check?.message).toBe(
230+
"Target prod is in account 111111111111 (eu-west-1), but the active credentials are for " +
231+
"account 998846730471. v1's CDK app synthesizes every target's stack, so it needs the " +
232+
"secret ARNs of compatA-key there too, and migrate reads every target with the one set " +
233+
"of credentials it runs with. Record them by hand: with credentials for that account, " +
234+
"run `aws bedrock-agentcore-control get-api-key-credential-provider` (or " +
235+
"get-oauth2-credential-provider) --name <provider> --region eu-west-1, add the secret " +
236+
"ARN as clientSecretArn under targets.prod.resources.credentials.<name> in " +
237+
"agentcore/.cli/deployed-state.json, commit it, then run `agentcore migrate` again. " +
238+
"The secret ARNs of compatA-key (target default), compatA-oauth (target default) were found.",
239+
);
240+
expect(result.warnings).toEqual([
241+
"v0 didn't record the secret ARN of credential compatA-oauth (target prod), and it " +
242+
"couldn't be found: target prod is in account 111111111111, not the active account " +
243+
"998846730471. No Gateway target uses it, so migrating doesn't need it.",
244+
]);
245+
const state = JSON.parse(result.deployedState!);
246+
expect(state.targets.default.resources.credentials["compatA-key"].clientSecretArn).toBe(
247+
KEY_SECRET,
248+
);
249+
expect(state.targets.prod.resources.credentials["compatA-key"].clientSecretArn).toBeUndefined();
250+
});
251+
252+
test("only warns for another account's credentials no Gateway target uses", async () => {
253+
const root = await project(twoTargets);
254+
const identity = fakeIdentity({ "compatA-key": KEY_SECRET, "compatA-oauth": OAUTH_SECRET });
255+
256+
const result = await backfill(
257+
root,
258+
identity,
259+
[],
260+
F1_SPEC.credentials,
261+
[DEFAULT, PROD],
262+
otherAccount,
263+
);
264+
265+
expect(result.check?.ok).toBe(true);
266+
expect(result.warnings).toHaveLength(2);
267+
});
268+
269+
test("reports a region whose credentials can't be resolved", async () => {
270+
const root = await project(twoTargets);
271+
const identity = fakeIdentity({ "compatA-key": KEY_SECRET, "compatA-oauth": OAUTH_SECRET });
272+
273+
const result = await backfill(
274+
root,
275+
identity,
276+
["compatA-key"],
277+
F1_SPEC.credentials,
278+
[DEFAULT, { ...PROD, account: DEFAULT.account }],
279+
async (target) =>
280+
target.region === "eu-west-1" ? { error: "no profile" } : { options: target },
281+
);
282+
283+
expect(result.check?.ok).toBe(false);
284+
expect(result.check?.message).toContain(
285+
"compatA-key (target prod): no credentials could be resolved for eu-west-1 (no profile)",
286+
);
287+
});
288+
});
289+
290+
describe("gatewayCredentialNames", () => {
291+
test("names the credentials Gateway targets authenticate with by API key or OAuth", () => {
292+
const spec = {
293+
agentCoreGateways: [
294+
{
295+
targets: [
296+
{ outboundAuth: { type: "API_KEY", credentialName: "orders" } },
297+
{ outboundAuth: { type: "OAUTH", credentialName: "crm" } },
298+
{ outboundAuth: { type: "NONE" } },
299+
{ outboundAuth: { type: "GATEWAY_IAM_ROLE", credentialName: "ignored" } },
300+
{},
301+
],
302+
},
303+
{},
304+
],
305+
};
306+
307+
expect([...gatewayCredentialNames(spec)]).toEqual(["orders", "crm"]);
308+
expect(gatewayCredentialNames({}).size).toBe(0);
309+
});
310+
});

0 commit comments

Comments
 (0)