Skip to content

Commit bc47b29

Browse files
authored
fix(pipelines): self-mutate always adds analytics (#19010)
When we moved analytics generation from the CLI to the framework, we kept a fallback in the CLI for backwards compatibility: if there were no analytics in the templates but analytics were requested, they would still be added in the CLI. This breaks in the case of CDK Pipelines self-mutate step, if analytics have been disabled: there are no analytics in the template, and the default setting for analytics is `true`, since there is no `cdk.json` to opt out, so they will always be added back in. A better solution to check whether the CLI needs to do the fallback is to go off of the cx schema version number: the CLI can know that it never needs to do anything for cloud assemblies generated starting at a certain version. Fixes #18933. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent b5ae377 commit bc47b29

File tree

2 files changed

+72
-39
lines changed

2 files changed

+72
-39
lines changed

packages/aws-cdk/lib/api/cxapp/cloud-executable.ts

+12-7
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,20 @@ import { debug, warning } from '../../logging';
66
import { Configuration } from '../../settings';
77
import { SdkProvider } from '../aws-auth';
88
import { CloudAssembly } from './cloud-assembly';
9+
import * as semver from 'semver';
910

1011
/**
1112
* @returns output directory
1213
*/
1314
type Synthesizer = (aws: SdkProvider, config: Configuration) => Promise<cxapi.CloudAssembly>;
1415

16+
/**
17+
* The Cloud Assembly schema version where the framework started to generate analytics itself
18+
*
19+
* See https://github.com/aws/aws-cdk/pull/10306
20+
*/
21+
const TEMPLATE_INCLUDES_ANALYTICS_SCHEMA_VERSION = '6.0.0';
22+
1523
export interface CloudExecutableProps {
1624
/**
1725
* Application configuration (settings and context)
@@ -104,13 +112,10 @@ export class CloudExecutable {
104112
}
105113
}
106114

107-
if (trackVersions) {
108-
// @deprecated(v2): remove this 'if' block and all code referenced by it.
109-
// This should honestly not be done here. The framework
110-
// should (and will, shortly) synthesize this information directly into
111-
// the template. However, in order to support old framework versions
112-
// that don't synthesize this info yet, we can only remove this code
113-
// once we break backwards compatibility.
115+
if (trackVersions && !semver.gte(assembly.version, TEMPLATE_INCLUDES_ANALYTICS_SCHEMA_VERSION)) {
116+
// @deprecate(v2): the framework now manages its own analytics. For
117+
// Cloud Assemblies *older* than when we introduced this feature, have
118+
// the CLI add it. Otherwise, do nothing.
114119
await this.addMetadataResource(assembly);
115120
}
116121

packages/aws-cdk/test/api/cloud-executable.test.ts

+60-32
Original file line numberDiff line numberDiff line change
@@ -5,46 +5,63 @@ import { registerContextProvider } from '../../lib/context-providers';
55
import { MockCloudExecutable } from '../util';
66

77
describe('AWS::CDK::Metadata', () => {
8-
test('is generated for relocatable stacks', async () => {
9-
const cx = await testCloudExecutable({ env: `aws://${cxapi.UNKNOWN_ACCOUNT}/${cxapi.UNKNOWN_REGION}`, versionReporting: true });
10-
const cxasm = await cx.synthesize();
11-
12-
const result = cxasm.stackById('withouterrors').firstStack;
13-
const metadata = result.template.Resources && result.template.Resources.CDKMetadata;
14-
expect(metadata).toEqual({
15-
Type: 'AWS::CDK::Metadata',
16-
Properties: {
17-
// eslint-disable-next-line @typescript-eslint/no-require-imports
18-
Modules: `${require('../../package.json').name}=${require('../../package.json').version}`,
19-
},
20-
Condition: 'CDKMetadataAvailable',
8+
test('is generated for relocatable stacks from old frameworks', async () => {
9+
await withFakeCurrentCxVersion('2.0.0', async () => {
10+
const cx = await testCloudExecutable({ env: `aws://${cxapi.UNKNOWN_ACCOUNT}/${cxapi.UNKNOWN_REGION}`, versionReporting: true });
11+
const cxasm = await cx.synthesize();
12+
13+
const result = cxasm.stackById('withouterrors').firstStack;
14+
const metadata = result.template.Resources && result.template.Resources.CDKMetadata;
15+
expect(metadata).toEqual({
16+
Type: 'AWS::CDK::Metadata',
17+
Properties: {
18+
// eslint-disable-next-line @typescript-eslint/no-require-imports
19+
Modules: `${require('../../package.json').name}=${require('../../package.json').version}`,
20+
},
21+
Condition: 'CDKMetadataAvailable',
22+
});
23+
24+
expect(result.template.Conditions?.CDKMetadataAvailable).toBeDefined();
2125
});
26+
});
2227

23-
expect(result.template.Conditions?.CDKMetadataAvailable).toBeDefined();
28+
test('is generated for stacks in supported regions from old frameworks', async () => {
29+
await withFakeCurrentCxVersion('2.0.0', async () => {
30+
const cx = await testCloudExecutable({ env: 'aws://012345678912/us-east-1', versionReporting: true });
31+
const cxasm = await cx.synthesize();
32+
33+
const result = cxasm.stackById('withouterrors').firstStack;
34+
const metadata = result.template.Resources && result.template.Resources.CDKMetadata;
35+
expect(metadata).toEqual({
36+
Type: 'AWS::CDK::Metadata',
37+
Properties: {
38+
// eslint-disable-next-line @typescript-eslint/no-require-imports
39+
Modules: `${require('../../package.json').name}=${require('../../package.json').version}`,
40+
},
41+
});
42+
});
2443
});
2544

26-
test('is generated for stacks in supported regions', async () => {
27-
const cx = await testCloudExecutable({ env: 'aws://012345678912/us-east-1', versionReporting: true });
28-
const cxasm = await cx.synthesize();
29-
30-
const result = cxasm.stackById('withouterrors').firstStack;
31-
const metadata = result.template.Resources && result.template.Resources.CDKMetadata;
32-
expect(metadata).toEqual({
33-
Type: 'AWS::CDK::Metadata',
34-
Properties: {
35-
// eslint-disable-next-line @typescript-eslint/no-require-imports
36-
Modules: `${require('../../package.json').name}=${require('../../package.json').version}`,
37-
},
45+
test('is not generated for stacks in unsupported regions from old frameworks', async () => {
46+
await withFakeCurrentCxVersion('2.0.0', async () => {
47+
const cx = await testCloudExecutable({ env: 'aws://012345678912/bermuda-triangle-1337', versionReporting: true });
48+
const cxasm = await cx.synthesize();
49+
50+
const result = cxasm.stackById('withouterrors').firstStack;
51+
const metadata = result.template.Resources && result.template.Resources.CDKMetadata;
52+
expect(metadata).toBeUndefined();
3853
});
3954
});
4055

41-
test('is not generated for stacks in unsupported regions', async () => {
42-
const cx = await testCloudExecutable({ env: 'aws://012345678912/bermuda-triangle-1337', versionReporting: true });
43-
const cxasm = await cx.synthesize();
56+
test('is not generated for new frameworks', async () => {
57+
await withFakeCurrentCxVersion('8.0.0', async () => {
58+
const cx = await testCloudExecutable({ env: 'aws://012345678912/us-east-1', versionReporting: true });
59+
const cxasm = await cx.synthesize();
4460

45-
const result = cxasm.stackById('withouterrors').firstStack;
46-
const metadata = result.template.Resources && result.template.Resources.CDKMetadata;
47-
expect(metadata).toBeUndefined();
61+
const result = cxasm.stackById('withouterrors').firstStack;
62+
const metadata = result.template.Resources && result.template.Resources.CDKMetadata;
63+
expect(metadata).toBeUndefined();
64+
});
4865
});
4966
});
5067

@@ -117,3 +134,14 @@ async function testCloudExecutable({ env, versionReporting = true }: { env?: str
117134

118135
return cloudExec;
119136
}
137+
138+
139+
async function withFakeCurrentCxVersion<A>(version: string, block: () => Promise<A>): Promise<A> {
140+
const currentVersionFn = cxschema.Manifest.version;
141+
cxschema.Manifest.version = () => version;
142+
try {
143+
return await block();
144+
} finally {
145+
cxschema.Manifest.version = currentVersionFn;
146+
}
147+
}

0 commit comments

Comments
 (0)