diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deployments.ts b/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deployments.ts index 2329e6e50..d27ed7130 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deployments.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/deployments/deployments.ts @@ -321,7 +321,12 @@ export class Deployments { private readonly publisherCache = new Map>(); - private _allowCrossAccountAssetPublishing: boolean | undefined; + // Cache by resolved environment: this Deployments instance is reused across every + // stack in a single `cdk deploy` invocation, and stacks can target different + // accounts/regions. determineAllowCrossAccountAssetPublishing() reads that specific + // environment's bootstrap stack, so caching a single un-keyed value would silently + // reuse the first stack's environment's answer for every other stack's environment. + private readonly allowCrossAccountAssetPublishingCache = new Map(); private readonly ioHelper: IoHelper; @@ -744,11 +749,18 @@ export class Deployments { } private async allowCrossAccountAssetPublishingForEnv(stack: cxapi.CloudFormationStackArtifact): Promise { - if (this._allowCrossAccountAssetPublishing === undefined) { - const env = await this.envs.accessStackForReadOnlyStackOperations(stack); - this._allowCrossAccountAssetPublishing = await determineAllowCrossAccountAssetPublishing(env.sdk, this.ioHelper, this.props.toolkitStackName); + const resolvedEnvironment = await this.envs.resolveStackEnvironment(stack); + const envKey = `${resolvedEnvironment.account}:${resolvedEnvironment.region}`; + + const cached = this.allowCrossAccountAssetPublishingCache.get(envKey); + if (cached !== undefined) { + return cached; } - return this._allowCrossAccountAssetPublishing; + + const env = await this.envs.accessStackForReadOnlyStackOperations(stack); + const allowed = await determineAllowCrossAccountAssetPublishing(env.sdk, this.ioHelper, this.props.toolkitStackName); + this.allowCrossAccountAssetPublishingCache.set(envKey, allowed); + return allowed; } /** diff --git a/packages/@aws-cdk/toolkit-lib/test/api/deployments/cloudformation-deployments.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/deployments/cloudformation-deployments.test.ts index 7e06b25b5..d92a66437 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/deployments/cloudformation-deployments.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/deployments/cloudformation-deployments.test.ts @@ -12,6 +12,7 @@ import { GetParameterCommand } from '@aws-sdk/client-ssm'; import { CloudFormationStack } from '../../../lib/api/cloudformation'; import { Deployments } from '../../../lib/api/deployments'; import * as cfnApi from '../../../lib/api/deployments/cfn-api'; +import { determineAllowCrossAccountAssetPublishing } from '../../../lib/api/deployments/checks'; import { deployStack, destroyStack } from '../../../lib/api/deployments/deploy-stack'; import { ToolkitInfo } from '../../../lib/api/toolkit-info'; import { testStack } from '../../_helpers/assembly'; @@ -29,6 +30,7 @@ import { FakeCloudformationStack } from '../_helpers/fake-cloudformation-stack'; jest.mock('../../../lib/api/deployments/deploy-stack'); jest.mock('../../../lib/api/deployments/asset-publishing'); +jest.mock('../../../lib/api/deployments/checks'); let sdkProvider: MockSdkProvider; let sdk: MockSdk; @@ -1344,6 +1346,44 @@ describe('cachedPublisher', () => { }); }); +describe('allowCrossAccountAssetPublishingForEnv', () => { + // Regression test: the cross-account-asset-publishing answer used to be cached in a + // single un-keyed instance field, so the first stack's environment's answer was + // silently reused for every other stack's environment on the same Deployments + // instance (which is reused for every stack in one `cdk deploy` invocation). + test('does not reuse the answer across different environments', async () => { + sdkProvider.forEnvironment = jest.fn().mockImplementation(() => ({ sdk: new MockSdk() })); + const mockDetermine = determineAllowCrossAccountAssetPublishing as jest.Mock; + mockDetermine.mockResolvedValueOnce(false).mockResolvedValueOnce(true); + + const stackA = testStack({ stackName: 'StackA', env: 'aws://111111111111/us-east-1' }); + const stackB = testStack({ stackName: 'StackB', env: 'aws://222222222222/eu-west-1' }); + + const allowedForA = await (deployments as any).allowCrossAccountAssetPublishingForEnv(stackA); + const allowedForB = await (deployments as any).allowCrossAccountAssetPublishingForEnv(stackB); + + expect(allowedForA).toBe(false); + expect(allowedForB).toBe(true); + expect(mockDetermine).toHaveBeenCalledTimes(2); + }); + + test('reuses the cached answer for repeat calls with the same environment', async () => { + sdkProvider.forEnvironment = jest.fn().mockImplementation(() => ({ sdk: new MockSdk() })); + const mockDetermine = determineAllowCrossAccountAssetPublishing as jest.Mock; + mockDetermine.mockResolvedValueOnce(true); + + const stackA = testStack({ stackName: 'StackA', env: 'aws://111111111111/us-east-1' }); + const stackAAgain = testStack({ stackName: 'StackA', env: 'aws://111111111111/us-east-1' }); + + const first = await (deployments as any).allowCrossAccountAssetPublishingForEnv(stackA); + const second = await (deployments as any).allowCrossAccountAssetPublishingForEnv(stackAAgain); + + expect(first).toBe(true); + expect(second).toBe(true); + expect(mockDetermine).toHaveBeenCalledTimes(1); + }); +}); + function pushStackResourceSummaries(stackName: string, ...items: StackResourceSummary[]) { if (!currentCfnStackResources[stackName]) { currentCfnStackResources[stackName] = [];