Skip to content

Commit

Permalink
fix(cx-api): bootstrap stack is validated even if the custom synthesi…
Browse files Browse the repository at this point in the history
…zer does not require it (aws#21518)

This fixes aws#21324

The property `requiresBootstrapStackVersion` of the class `AssetManifestArtifact` is a [required property of type number](https://github.com/aws/aws-cdk/blob/34e31b90c8cdd51b8af61f352aa0ab7a0332ed4c/packages/%40aws-cdk/cx-api/lib/artifacts/asset-manifest-artifact.ts#L38-L41) that is [defaulted to `1`]((https://github.com/aws/aws-cdk/blob/34e31b90c8cdd51b8af61f352aa0ab7a0332ed4c/packages/%40aws-cdk/cx-api/lib/artifacts/asset-manifest-artifact.ts#L58)). This behavior prevents the custom synthesizers from disabling the validation of the bootstrap stack when it knows that it does not need this stack. 

This change allows the property to be `undefined`, removes the behaviour of defaulting it to `1`, and adds a breaking unit test. 

----

### All Submissions:

* [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
huyphan authored Aug 9, 2022
1 parent 50ce07f commit afb1c2d
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 6 deletions.
5 changes: 5 additions & 0 deletions allowed-breaking-changes.txt
Original file line number Diff line number Diff line change
Expand Up @@ -139,3 +139,8 @@ removed:@aws-cdk/aws-lambda-event-sources.SelfManagedKafkaEventSourceProps.onFai

# removed kubernetes version from EKS
removed:@aws-cdk/aws-eks.KubernetesVersion.V1_22

# changed the type of requiresBootstrapStackVersion to Optional<number> (formerly number)
# to allow the CLI to skip validating the bootstrap stack when the stack is not needed
changed-type:@aws-cdk/cx-api.AssetManifestArtifact.requiresBootstrapStackVersion

Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import * as fs from 'fs';
import * as cxschema from '@aws-cdk/cloud-assembly-schema';
import { ArtifactType } from '@aws-cdk/cloud-assembly-schema';
import * as cxapi from '@aws-cdk/cx-api';
import { App, Aws, CfnResource, ContextProvider, DefaultStackSynthesizer, FileAssetPackaging, Stack } from '../../lib';
import { ISynthesisSession } from '../../lib/stack-synthesizers/types';
import { evaluateCFN } from '../evaluate-cfn';

const CFN_CONTEXT = {
Expand Down Expand Up @@ -60,14 +62,18 @@ describe('new style synthesis', () => {

});

test('version check is added to template', () => {
test('version check is added to both template and manifest artifact', () => {
// GIVEN
new CfnResource(stack, 'Resource', {
type: 'Some::Resource',
});

// THEN
const template = app.synth().getStackByName('Stack').template;
const asm = app.synth();
const manifestArtifact = getAssetManifest(asm);
expect(manifestArtifact.requiresBootstrapStackVersion).toEqual(6);

const template = asm.getStackByName('Stack').template;
expect(template?.Parameters?.BootstrapVersion?.Type).toEqual('AWS::SSM::Parameter::Value<String>');
expect(template?.Parameters?.BootstrapVersion?.Default).toEqual('/cdk-bootstrap/hnb659fds/version');
expect(template?.Parameters?.BootstrapVersion?.Description).toContain(cxapi.SSMPARAM_NO_INVALIDATE);
Expand All @@ -79,8 +85,6 @@ describe('new style synthesis', () => {
{ 'Fn::Contains': [['1', '2', '3', '4', '5'], { Ref: 'BootstrapVersion' }] },
],
});


});

test('version check is not added to template if disabled', () => {
Expand Down Expand Up @@ -124,8 +128,46 @@ describe('new style synthesis', () => {

// THEN - the asset manifest has an SSM parameter entry
expect(manifestArtifact.bootstrapStackVersionSsmParameter).toEqual('stack-version-parameter');
});

test('contains asset but not requiring a specific version parameter', () => {
// GIVEN
class BootstraplessStackSynthesizer extends DefaultStackSynthesizer {


/**
* Synthesize the associated bootstrap stack to the session.
*/
public synthesize(session: ISynthesisSession): void {
if (!this.stack) {
throw new Error('You must call bind() with a stack instance first');
}
this.synthesizeStackTemplate(this.stack, session);
session.assembly.addArtifact('FAKE_ARTIFACT_ID', {
type: ArtifactType.ASSET_MANIFEST,
properties: {
file: 'FAKE_ARTIFACT_ID.json',
},
});
this.emitStackArtifact(this.stack, session, {
additionalDependencies: ['FAKE_ARTIFACT_ID'],
});
}
}

const myapp = new App();

// WHEN
new Stack(myapp, 'mystack', {
synthesizer: new BootstraplessStackSynthesizer(),
});

// THEN
const asm = myapp.synth();
const manifestArtifact = getAssetManifest(asm);

// THEN - the asset manifest should not define a required bootstrap stack version
expect(manifestArtifact.requiresBootstrapStackVersion).toEqual(undefined);
});

test('generates missing context with the lookup role ARN as one of the missing context properties', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export class AssetManifestArtifact extends CloudArtifact {
/**
* Version of bootstrap stack required to deploy this stack
*/
public readonly requiresBootstrapStackVersion: number;
public readonly requiresBootstrapStackVersion: number | undefined;

/**
* Name of SSM parameter with bootstrap stack version
Expand All @@ -55,7 +55,7 @@ export class AssetManifestArtifact extends CloudArtifact {
throw new Error('Invalid AssetManifestArtifact. Missing "file" property');
}
this.file = path.resolve(this.assembly.directory, properties.file);
this.requiresBootstrapStackVersion = properties.requiresBootstrapStackVersion ?? 1;
this.requiresBootstrapStackVersion = properties.requiresBootstrapStackVersion;
this.bootstrapStackVersionSsmParameter = properties.bootstrapStackVersionSsmParameter;
}
}
Expand Down

0 comments on commit afb1c2d

Please sign in to comment.