Skip to content

Commit

Permalink
fix(ec2): cfn-init user data hash not updated if file asset changes (#…
Browse files Browse the repository at this point in the history
…10216)

We fingerprint the cfn-init configuration to insert into the user data so
changes to the cfn-init config can trigger an instance replacement; however,
the fingerprint was being calculated on the raw config, including tokens,
so the asset hash was not being considered in the fingerprint.

This fix resolves the tokens so the fingerprint takes asset hashes into
consideration.

fixes #10206

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
njlynch authored Sep 7, 2020
1 parent 20f5535 commit 0d7ca63
Show file tree
Hide file tree
Showing 5 changed files with 122 additions and 16 deletions.
4 changes: 4 additions & 0 deletions packages/@aws-cdk/aws-ec2/lib/cfn-init-elements.ts
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ export abstract class InitFile extends InitElement {
source: asset.httpUrl,
}),
authentication: standardS3Auth(bindOptions.instanceRole, asset.s3BucketName),
assetHash: asset.assetHash,
};
}
}(targetFileName, options);
Expand All @@ -449,6 +450,7 @@ export abstract class InitFile extends InitElement {
source: asset.httpUrl,
}),
authentication: standardS3Auth(bindOptions.instanceRole, asset.s3BucketName),
assetHash: asset.assetHash,
};
}
}(targetFileName, options);
Expand Down Expand Up @@ -899,6 +901,7 @@ export abstract class InitSource extends InitElement {
return {
config: { [this.targetDirectory]: asset.httpUrl },
authentication: standardS3Auth(bindOptions.instanceRole, asset.s3BucketName),
assetHash: asset.assetHash,
};
}
}(targetDirectory, options.serviceRestartHandles);
Expand All @@ -915,6 +918,7 @@ export abstract class InitSource extends InitElement {
return {
config: { [this.targetDirectory]: asset.httpUrl },
authentication: standardS3Auth(bindOptions.instanceRole, asset.s3BucketName),
assetHash: asset.assetHash,
};
}
}(targetDirectory, options.serviceRestartHandles);
Expand Down
24 changes: 19 additions & 5 deletions packages/@aws-cdk/aws-ec2/lib/cfn-init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,12 @@ export class CloudFormationInit {
// Note: This will not reflect mutations made after attaching.
const bindResult = this.bind(attachedResource.stack, attachOptions);
attachedResource.addMetadata('AWS::CloudFormation::Init', bindResult.configData);
const fingerprint = contentHash(JSON.stringify(bindResult.configData)).substr(0, 16);

// Need to resolve the various tokens from assets in the config,
// as well as include any asset hashes provided so the fingerprint is accurate.
const resolvedConfig = attachedResource.stack.resolve(bindResult.configData);
const fingerprintInput = { config: resolvedConfig, assetHash: bindResult.assetHash };
const fingerprint = contentHash(JSON.stringify(fingerprintInput)).substr(0, 16);

attachOptions.instanceRole.addToPolicy(new iam.PolicyStatement({
actions: ['cloudformation:DescribeStackResource', 'cloudformation:SignalResource'],
Expand Down Expand Up @@ -140,7 +145,7 @@ export class CloudFormationInit {
}
}

private bind(scope: Construct, options: AttachInitOptions): { configData: any, authData: any } {
private bind(scope: Construct, options: AttachInitOptions): { configData: any, authData: any, assetHash?: any } {
const nonEmptyConfigs = mapValues(this._configs, c => c.isEmpty() ? undefined : c);

const configNameToBindResult = mapValues(nonEmptyConfigs, c => c._bind(scope, options));
Expand All @@ -151,6 +156,7 @@ export class CloudFormationInit {
...mapValues(configNameToBindResult, c => c.config),
},
authData: Object.values(configNameToBindResult).map(c => c.authentication).reduce(deepMerge, undefined),
assetHash: combineAssetHashesOrUndefined(Object.values(configNameToBindResult).map(c => c.assetHash)),
};
}

Expand Down Expand Up @@ -201,9 +207,9 @@ export class InitConfig {
// Must be last!
const servicesConfig = this.bindForType(InitElementType.SERVICE, bindOptions);

const authentication = [packageConfig, groupsConfig, usersConfig, sourcesConfig, filesConfig, commandsConfig, servicesConfig]
.map(c => c?.authentication)
.reduce(deepMerge, undefined);
const allConfig = [packageConfig, groupsConfig, usersConfig, sourcesConfig, filesConfig, commandsConfig, servicesConfig];
const authentication = allConfig.map(c => c?.authentication).reduce(deepMerge, undefined);
const assetHash = combineAssetHashesOrUndefined(allConfig.map(c => c?.assetHash));

return {
config: {
Expand All @@ -216,6 +222,7 @@ export class InitConfig {
services: servicesConfig?.config,
},
authentication,
assetHash,
};
}

Expand All @@ -228,6 +235,7 @@ export class InitConfig {
return {
config: bindResults.map(r => r.config).reduce(deepMerge, undefined) ?? {},
authentication: bindResults.map(r => r.authentication).reduce(deepMerge, undefined),
assetHash: combineAssetHashesOrUndefined(bindResults.map(r => r.assetHash)),
};
}

Expand Down Expand Up @@ -310,6 +318,12 @@ function mapValues<A, B>(xs: Record<string, A>, fn: (x: A) => B | undefined): Re
return ret;
}

// Combines all input asset hashes into one, or if no hashes are present, returns undefined.
function combineAssetHashesOrUndefined(hashes: (string | undefined)[]): string | undefined {
const hashArray = hashes.filter((x): x is string => x !== undefined);
return hashArray.length > 0 ? hashArray.join('') : undefined;
}

function contentHash(content: string) {
return crypto.createHash('sha256').update(content).digest('hex');
}
7 changes: 7 additions & 0 deletions packages/@aws-cdk/aws-ec2/lib/private/cfn-init-internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,13 @@ export interface InitElementConfig {
* @default - No authentication associated with the config
*/
readonly authentication?: Record<string, any>;

/**
* Optional string representing a hash of the asset associated with this element (if any).
*
* @default - No hash is provided
*/
readonly assetHash?: string;
}

/**
Expand Down
93 changes: 87 additions & 6 deletions packages/@aws-cdk/aws-ec2/test/cfn-init.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import { arrayWith, ResourcePart, stringLike } from '@aws-cdk/assert';
import '@aws-cdk/assert/jest';
import * as iam from '@aws-cdk/aws-iam';
import * as s3 from '@aws-cdk/aws-s3';
import * as s3_assets from '@aws-cdk/aws-s3-assets';
import { App, Aws, CfnResource, Stack } from '@aws-cdk/core';
import { Asset } from '@aws-cdk/aws-s3-assets';
import { App, Aws, CfnResource, Stack, DefaultStackSynthesizer, IStackSynthesizer, FileAssetSource, FileAssetLocation } from '@aws-cdk/core';
import * as ec2 from '../lib';

let app: App;
Expand All @@ -13,10 +15,15 @@ let instanceRole: iam.Role;
let resource: CfnResource;
let linuxUserData: ec2.UserData;

beforeEach(() => {
function resetState() {
resetStateWithSynthesizer();
}

function resetStateWithSynthesizer(customSynthesizer?: IStackSynthesizer) {
app = new App();
stack = new Stack(app, 'Stack', {
env: { account: '1234', region: 'testregion' },
synthesizer: customSynthesizer,
});
instanceRole = new iam.Role(stack, 'InstanceRole', {
assumedBy: new iam.ServicePrincipal('ec2.amazonaws.com'),
Expand All @@ -25,7 +32,9 @@ beforeEach(() => {
type: 'CDK::Test::Resource',
});
linuxUserData = ec2.UserData.forLinux();
});
};

beforeEach(resetState);

test('whole config with restart handles', () => {
// WHEN
Expand Down Expand Up @@ -238,7 +247,7 @@ describe('assets n buckets', () => {
[''],
])('InitFile.from%sAsset', (existing: string) => {
// GIVEN
const asset = new s3_assets.Asset(stack, 'Asset', { path: __filename });
const asset = new Asset(stack, 'Asset', { path: __filename });
const init = ec2.CloudFormationInit.fromElements(
existing
? ec2.InitFile.fromExistingAsset('/etc/fun.js', asset)
Expand Down Expand Up @@ -292,7 +301,7 @@ describe('assets n buckets', () => {
[''],
])('InitSource.from%sAsset', (existing: string) => {
// GIVEN
const asset = new s3_assets.Asset(stack, 'Asset', { path: path.join(__dirname, 'asset-fixture') });
const asset = new Asset(stack, 'Asset', { path: path.join(__dirname, 'asset-fixture') });
const init = ec2.CloudFormationInit.fromElements(
existing
? ec2.InitSource.fromExistingAsset('/etc/fun', asset)
Expand Down Expand Up @@ -472,6 +481,64 @@ describe('assets n buckets', () => {
},
});
});

test('fingerprint data changes on asset hash update', () => {
function calculateFingerprint(assetFilePath: string): string | undefined {
resetState(); // Needed so the same resources/assets/filenames can be used.
const init = ec2.CloudFormationInit.fromElements(
ec2.InitFile.fromAsset('/etc/myFile', assetFilePath),
);
init._attach(resource, linuxOptions());

return linuxUserData.render().split('\n').find(line => line.match(/# fingerprint:/));
}

// Setup initial asset file
const assetFileDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cfn-init-test'));
const assetFilePath = path.join(assetFileDir, 'fingerprint-test');
fs.writeFileSync(assetFilePath, 'hello');

const fingerprintOne = calculateFingerprint(assetFilePath);
const fingerprintOneAgain = calculateFingerprint(assetFilePath);
// Consistent without changes.
expect(fingerprintOneAgain).toEqual(fingerprintOne);

// Change asset file content/hash
fs.writeFileSync(assetFilePath, ' world');

const fingerprintTwo = calculateFingerprint(assetFilePath);

expect(fingerprintTwo).not.toEqual(fingerprintOne);
});

test('fingerprint data changes on existing asset update, even for assets with unchanging URLs', () => {
function calculateFingerprint(assetFilePath: string): string | undefined {
resetStateWithSynthesizer(new SingletonLocationSythesizer());
const init = ec2.CloudFormationInit.fromElements(
ec2.InitFile.fromExistingAsset('/etc/myFile', new Asset(stack, 'FileAsset', { path: assetFilePath })),
);
init._attach(resource, linuxOptions());

return linuxUserData.render().split('\n').find(line => line.match(/# fingerprint:/));
}

// Setup initial asset file
const assetFileDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cfn-init-test'));
const assetFilePath = path.join(assetFileDir, 'fingerprint-test');
fs.writeFileSync(assetFilePath, 'hello');

const fingerprintOne = calculateFingerprint(assetFilePath);
const fingerprintOneAgain = calculateFingerprint(assetFilePath);
// Consistent without changes.
expect(fingerprintOneAgain).toEqual(fingerprintOne);

// Change asset file content/hash
fs.writeFileSync(assetFilePath, ' world');

const fingerprintTwo = calculateFingerprint(assetFilePath);

expect(fingerprintTwo).not.toEqual(fingerprintOne);
});
});

function linuxOptions() {
Expand Down Expand Up @@ -512,3 +579,17 @@ function cmdArg(command: string, argument: string) {
function escapeRegex(s: string) {
return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); // $& means the whole matched string
}

/** Creates file assets that have a hard-coded asset url, rather than the default based on asset hash */
class SingletonLocationSythesizer extends DefaultStackSynthesizer {
public addFileAsset(_asset: FileAssetSource): FileAssetLocation {
const httpUrl = 'https://MyBucket.s3.amazonaws.com/MyAsset';
return {
bucketName: 'MyAssetBucket',
objectKey: 'MyAssetFile',
httpUrl,
s3ObjectUrl: httpUrl,
s3Url: httpUrl,
};
}
}
10 changes: 5 additions & 5 deletions packages/@aws-cdk/aws-ec2/test/integ.instance-init.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@
]
}
},
"Instance255F3526589c13387332ee3de": {
"Instance255F35265813bd3c1f652ed5b": {
"Type": "AWS::EC2::Instance",
"Properties": {
"AvailabilityZone": "us-east-1a",
Expand Down Expand Up @@ -161,23 +161,23 @@
"Fn::Join": [
"",
[
"#!/bin/bash\n# fingerprint: 061ec8b06d437783\n(\n set +e\n /opt/aws/bin/cfn-init -v --region ",
"#!/bin/bash\n# fingerprint: 336ad3625c000098\n(\n set +e\n /opt/aws/bin/cfn-init -v --region ",
{
"Ref": "AWS::Region"
},
" --stack ",
{
"Ref": "AWS::StackName"
},
" --resource Instance255F3526589c13387332ee3de -c default\n /opt/aws/bin/cfn-signal -e $? --region ",
" --resource Instance255F35265813bd3c1f652ed5b -c default\n /opt/aws/bin/cfn-signal -e $? --region ",
{
"Ref": "AWS::Region"
},
" --stack ",
{
"Ref": "AWS::StackName"
},
" --resource Instance255F3526589c13387332ee3de\n cat /var/log/cfn-init.log >&2\n)"
" --resource Instance255F35265813bd3c1f652ed5b\n cat /var/log/cfn-init.log >&2\n)"
]
]
}
Expand Down Expand Up @@ -319,4 +319,4 @@
"Description": "Artifact hash for asset \"f8a1af398dac2fad92eeea4fb7620be1c4f504e23e3bfcd859fbb5744187930b\""
}
}
}
}

0 comments on commit 0d7ca63

Please sign in to comment.