Skip to content

Commit

Permalink
Refactoring where "allowCrossAccountAssetPublishing" lives
Browse files Browse the repository at this point in the history
  • Loading branch information
rix0rrr committed Nov 5, 2024
1 parent 8b6f902 commit 78be3c6
Show file tree
Hide file tree
Showing 9 changed files with 85 additions and 177 deletions.
2 changes: 1 addition & 1 deletion packages/aws-cdk/lib/api/deployments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ export class Deployments {
const env = await this.envs.accessStackForReadOnlyStackOperations(stackArtifact);
const cfn = env.sdk.cloudFormation();

const proof = await uploadStackTemplateAssets(stackArtifact, this.assetSdkProvider, env.resolvedEnvironment);
const proof = await uploadStackTemplateAssets(stackArtifact, this.assetSdkProvider, env);

// Upload the template, if necessary, before passing it to CFN
let cfnParam;
Expand Down
4 changes: 4 additions & 0 deletions packages/aws-cdk/lib/api/environment-resources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,10 @@ export class NoBootstrapStackEnvironmentResources extends EnvironmentResources {
public async lookupToolkit(): Promise<ToolkitInfo> {
throw new Error('Trying to perform an operation that requires a bootstrap stack; you should not see this error, this is a bug in the CDK CLI.');
}

public async allowCrossAccountAssetPublishing(): Promise<boolean> {
return true;
}
}

/**
Expand Down
12 changes: 8 additions & 4 deletions packages/aws-cdk/lib/api/util/cloudformation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { AssetManifestBuilder } from '../../util/asset-manifest-builder';
import { AssetsPublishedProof, multipleAssetPublishedProof, publishAssets } from '../../util/asset-publishing';
import { SdkProvider } from '../aws-auth';
import { Deployments } from '../deployments';
import { TargetEnvironment } from '../environment-access';

export type Template = {
Parameters?: Record<string, TemplateParameter>;
Expand Down Expand Up @@ -365,7 +366,7 @@ function templatesFromAssetManifestArtifact(artifact: cxapi.AssetManifestArtifac
async function uploadBodyParameterAndCreateChangeSet(options: PrepareChangeSetOptions): Promise<DescribeChangeSetOutput | undefined> {
try {
const env = (await options.deployments.envs.accessStackForMutableStackOperations(options.stack));
const proof = await uploadStackTemplateAssets(options.stack, options.sdkProvider, env.resolvedEnvironment);
const proof = await uploadStackTemplateAssets(options.stack, options.sdkProvider, env);

let bodyParameter;
const bodyAction = await makeBodyParameter(options.stack, env, proof);
Expand All @@ -377,7 +378,9 @@ async function uploadBodyParameterAndCreateChangeSet(options: PrepareChangeSetOp
case 'upload':
const builder = new AssetManifestBuilder();
bodyParameter = bodyAction.addToManifest(builder);
await publishAssets(builder.toManifest(options.stack.assembly.directory), options.sdkProvider, env.resolvedEnvironment);
await publishAssets(builder.toManifest(options.stack.assembly.directory), options.sdkProvider, env.resolvedEnvironment, {
allowCrossAccount: await env.resources.allowCrossAccountAssetPublishing(),
});
break;
}

Expand Down Expand Up @@ -418,13 +421,14 @@ async function uploadBodyParameterAndCreateChangeSet(options: PrepareChangeSetOp
export async function uploadStackTemplateAssets(
stack: cxapi.CloudFormationStackArtifact,
sdkProvider: SdkProvider,
environment: cxapi.Environment,
env: TargetEnvironment,
): Promise<AssetsPublishedProof> {
const assetDependencies = stack.dependencies.filter(cxapi.AssetManifestArtifact.isAssetManifestArtifact);
const allowCrossAccount = await env.resources.allowCrossAccountAssetPublishing();

return multipleAssetPublishedProof(assetDependencies, (artifact) => {
const templates = templatesFromAssetManifestArtifact(artifact);
return publishAssets(templates, sdkProvider, environment);
return publishAssets(templates, sdkProvider, env.resolvedEnvironment, { allowCrossAccount });
});
}

Expand Down
6 changes: 2 additions & 4 deletions packages/aws-cdk/lib/util/type-brands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ export type Branded<T, B> = T & Brand<B>;
* values which are branded by construction (really just an elaborate
* way to write 'as').
*/
export function createBranded<A extends Branded<any, any>>(value: TypeUnderlyingBrand<A>): A {
return value as A;
export function createBranded<A, B>(value: A): Branded<A, B> {
return value as Branded<A, B>;
}

type TypeUnderlyingBrand<A> = A extends Branded<infer T, any> ? T : never;
3 changes: 0 additions & 3 deletions packages/aws-cdk/test/api/bootstrap.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@ const env = {
name: 'mock',
};

jest.mock('../../lib/api/util/checks', () => ({
determineAllowCrossAccountAssetPublishing: jest.fn().mockResolvedValue(true),
}));
let sdk: MockSdkProvider;
let executed: boolean;
let protectedTermination: boolean;
Expand Down
23 changes: 13 additions & 10 deletions packages/aws-cdk/test/api/deploy-stack.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@ import { setCI } from '../../lib/logging';
import { DEFAULT_FAKE_TEMPLATE, testStack } from '../util';
import { MockedObject, mockResolvedEnvironment, MockSdk, MockSdkProvider, SyncHandlerSubsetOf } from '../util/mock-sdk';
import { NoBootstrapStackEnvironmentResources } from '../../lib/api/environment-resources';
import { createBranded } from '../../lib/util/type-brands';
import { StringWithoutPlaceholders } from '../../lib/api/util/placeholders';

jest.mock('../../lib/api/hotswap-deployments');
jest.mock('../../lib/api/util/checks', () => ({
determineAllowCrossAccountAssetPublishing: jest.fn().mockResolvedValue(true),
}));

const FAKE_STACK = testStack({
stackName: 'withouterrors',
Expand Down Expand Up @@ -83,10 +82,17 @@ function standardDeployStackArguments(): DeployStackOptions {
const resolvedEnvironment = mockResolvedEnvironment();
return {
stack: FAKE_STACK,
sdk,
env: {
didAssumeRole: true,
isFallbackCredentials: false,
resolvedEnvironment,
sdk,
resources: new NoBootstrapStackEnvironmentResources(resolvedEnvironment, sdk),
replacePlaceholders(x: string | undefined): Promise<StringWithoutPlaceholders | undefined> {
return Promise.resolve(x ? createBranded(x) : undefined);
},
},
sdkProvider,
resolvedEnvironment,
envResources: new NoBootstrapStackEnvironmentResources(resolvedEnvironment, sdk),
};
}

Expand Down Expand Up @@ -590,19 +596,16 @@ test('deploy not skipped if template did not change but tags changed', async ()
});

// WHEN
const resolvedEnvironment = mockResolvedEnvironment();
await deployStack({
...standardDeployStackArguments(),
stack: FAKE_STACK,
sdk,
sdkProvider,
resolvedEnvironment,
tags: [
{
Key: 'Key',
Value: 'NewValue',
},
],
envResources: new NoBootstrapStackEnvironmentResources(resolvedEnvironment, sdk),
});

// THEN
Expand Down
61 changes: 56 additions & 5 deletions packages/aws-cdk/test/api/environment-resources.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ beforeEach(() => {

afterEach(() => {
toolkitMock.dispose();
jest.clearAllMocks();
});

function mockToolkitInfo(ti: ToolkitInfo) {
Expand Down Expand Up @@ -69,10 +70,6 @@ describe('validateversion without bootstrap stack', () => {
mockToolkitInfo(ToolkitInfo.bootstrapStackNotFoundInfo('TestBootstrapStack'));
});

afterEach(() => {
jest.clearAllMocks();
});

test('validating version with explicit SSM parameter succeeds', async () => {
// GIVEN
mockSdk.stubSsm({
Expand Down Expand Up @@ -140,4 +137,58 @@ describe('validateversion without bootstrap stack', () => {
// WHEN
await expect(envResources().validateVersion(8, '/abc')).rejects.toThrow(/Has the environment been bootstrapped?/);
});
});
});

describe('determineAllowCrossAccountAssetPublishing', () => {
it('should return true when hasStagingBucket is false', async () => {
mockToolkitInfo(ToolkitInfo.fromStack(mockBootstrapStack(mockSdk, {
Outputs: [{ OutputKey: 'BootstrapVersion', OutputValue: '1' }],
})));

const result = await envResources().allowCrossAccountAssetPublishing();
expect(result).toBe(true);
});

it.each(['', '-', '*', '---'])('should return true when the bucket output does not look like a real bucket', async (notABucketName) => {
mockToolkitInfo(ToolkitInfo.fromStack(mockBootstrapStack(mockSdk, {
Outputs: [
{ OutputKey: 'BootstrapVersion', OutputValue: '1' },
{ OutputKey: 'BucketName', OutputValue: notABucketName },
],
})));

const result = await envResources().allowCrossAccountAssetPublishing();
expect(result).toBe(true);
});

it('should return true when bootstrap version is >= 21', async () => {
mockToolkitInfo(ToolkitInfo.fromStack(mockBootstrapStack(mockSdk, {
Outputs: [
{ OutputKey: 'BootstrapVersion', OutputValue: '21' },
{ OutputKey: 'BucketName', OutputValue: 'some-bucket' },
],
})));

const result = await envResources().allowCrossAccountAssetPublishing();
expect(result).toBe(true);
});

it('should return true if looking up the bootstrap stack fails', async () => {
mockToolkitInfo(ToolkitInfo.bootstrapStackNotFoundInfo('TestBootstrapStack'));

const result = await envResources().allowCrossAccountAssetPublishing();
expect(result).toBe(true);
});

it('should return false for other scenarios', async () => {
mockToolkitInfo(ToolkitInfo.fromStack(mockBootstrapStack(mockSdk, {
Outputs: [
{ OutputKey: 'BootstrapVersion', OutputValue: '20' },
{ OutputKey: 'BucketName', OutputValue: 'some-bucket' },
],
})));

const result = await envResources().allowCrossAccountAssetPublishing();
expect(result).toBe(false);
});
});
150 changes: 0 additions & 150 deletions packages/aws-cdk/test/api/util/checks.test.ts

This file was deleted.

1 change: 1 addition & 0 deletions packages/aws-cdk/test/util/mock-toolkitinfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export class MockToolkitInfo extends ToolkitInfo {
public readonly version: number;
public readonly variant: string;
public readonly stackName = 'MockBootstrapStack';
public readonly allowCrossAccountAssetPublishing = false;

private readonly _bootstrapStack?: CloudFormationStack;

Expand Down

0 comments on commit 78be3c6

Please sign in to comment.