Skip to content

Commit

Permalink
Apply PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
RomainMuller committed Apr 23, 2019
1 parent e98da5e commit 127847a
Show file tree
Hide file tree
Showing 42 changed files with 149 additions and 321 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,8 @@ exports.handler = async function(event, context, _callback, respond) {
}
const isRepoUri = repo.match(/^(\d+\.dkr\.ecr\.[^.]+\.[^/]+\/)(.+)$/i);
if (isRepoUri) {
console.log(`"${repo}" like a repository URI: dropping "${isRepoUri[1]}"`);
repo = isRepoUri[2];
}
console.log('RepositoryName', repo);

const adopter = await getAdopter(repo);
if (event.RequestType === 'Delete') {
Expand Down
4 changes: 4 additions & 0 deletions packages/@aws-cdk/assets-docker/lib/adopted-repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ export class AdoptedRepository extends ecr.RepositoryBase {
PolicyDocument: this.policyDocument
}
});
if (fn.role) {
// Need to explicitly depend on the role's policies, so they are applied before we try to use them
adopter.node.addDependency(fn.role);
}

// we use the Fn::GetAtt with the RepositoryName returned by the custom
// resource in order to implicitly create a dependency between consumers
Expand Down
12 changes: 6 additions & 6 deletions packages/@aws-cdk/assets-docker/lib/image-asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export class DockerImageAsset extends cdk.Construct implements assets.IAsset {
public repository: ecr.IRepository;

public readonly sourceHash: string;
public readonly bundleHash: string;
public readonly artifactHash: string;

/**
* Directory where the source files are stored
Expand Down Expand Up @@ -84,10 +84,10 @@ export class DockerImageAsset extends cdk.Construct implements assets.IAsset {

this.node.addMetadata(cxapi.ASSET_METADATA, asset);

// parse repository name and tag from the parameter (<REPO_NAME>:<TAG>)
const components = cdk.Fn.split(':', imageNameParameter.stringValue);
// parse repository name and tag from the parameter (<REPO_NAME>@sha256:<TAG>)
const components = cdk.Fn.split('@sha256:', imageNameParameter.stringValue);
const repositoryName = cdk.Fn.select(0, components).toString();
const imageTag = cdk.Fn.select(1, components).toString();
const imageSha = cdk.Fn.select(1, components).toString();

// Require that repository adoption happens first, so we route the
// input ARN into the Custom Resource and then get the URI which we use to
Expand All @@ -96,7 +96,7 @@ export class DockerImageAsset extends cdk.Construct implements assets.IAsset {
// If adoption fails (because the repository might be twice-adopted), we
// haven't already started using the image.
this.repository = new AdoptedRepository(this, 'AdoptRepository', { repositoryName });
this.imageUri = this.repository.repositoryUriForTag(imageTag);
this.bundleHash = imageTag;
this.imageUri = `${this.repository.repositoryUri}@sha256:${imageSha}`;
this.artifactHash = imageSha;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
"Type": "String",
"Description": "S3 key for asset version \"integ-assets-docker/AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62c/Code\""
},
"AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cCodeBundleHash7C06FC82": {
"AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cCodeArtifactHash8BCBAA49": {
"Type": "String",
"Description": "Bundle hash for asset \"integ-assets-docker/AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62c/Code\""
"Description": "Artifact hash for asset \"integ-assets-docker/AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62c/Code\""
}
},
"Resources": {
Expand All @@ -32,15 +32,19 @@
0,
{
"Fn::Split": [
":",
"@sha256:",
{
"Ref": "DockerImageImageName266E5998"
}
]
}
]
}
}
},
"DependsOn": [
"AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cServiceRoleDefaultPolicy6BC8737C",
"AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cServiceRoleD788AA17"
]
},
"AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cServiceRoleD788AA17": {
"Type": "AWS::IAM::Role",
Expand Down Expand Up @@ -119,7 +123,7 @@
0,
{
"Fn::Split": [
":",
"@sha256:",
{
"Ref": "DockerImageImageName266E5998"
}
Expand Down Expand Up @@ -200,13 +204,13 @@
}
},
"Outputs": {
"BundleHash": {
"ArtifactHash": {
"Value": {
"Fn::Select": [
1,
{
"Fn::Split": [
":",
"@sha256:",
{
"Ref": "DockerImageImageName266E5998"
}
Expand Down Expand Up @@ -300,13 +304,13 @@
"RepositoryName"
]
},
":",
"@sha256:",
{
"Fn::Select": [
1,
{
"Fn::Split": [
":",
"@sha256:",
{
"Ref": "DockerImageImageName266E5998"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const asset = new assets.DockerImageAsset(stack, 'DockerImage', {
directory: path.join(__dirname, 'demo-image'),
});

new cdk.CfnOutput(stack, 'BundleHash', { value: asset.bundleHash });
new cdk.CfnOutput(stack, 'ArtifactHash', { value: asset.artifactHash });
new cdk.CfnOutput(stack, 'ImageUri', { value: asset.imageUri });

app.run();
2 changes: 1 addition & 1 deletion packages/@aws-cdk/assets-docker/test/test.image-asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export = {
// THEN
expect(stack).to(haveResource('Custom::ECRAdoptedRepository', {
"RepositoryName": {
"Fn::Select": [ 0, { "Fn::Split": [ ":", { "Ref": "ImageImageName5E684353" } ] } ]
"Fn::Select": [ 0, { "Fn::Split": [ "@sha256:", { "Ref": "ImageImageName5E684353" } ] } ]
},
"PolicyDocument": {
"Statement": [
Expand Down
18 changes: 9 additions & 9 deletions packages/@aws-cdk/assets/lib/asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,19 @@ export interface AssetProps {
export interface IAsset extends cdk.IConstruct {
/**
* A hash of the source of this asset, which is available at construction time. As this is a plain
* string, it can be used in `Construct` ids in order to enforce creation of a new resource when
* string, it can be used in construct IDs in order to enforce creation of a new resource when
* the content hash has changed.
*/
readonly sourceHash: string;

/**
* A hash of the bundled for of this asset, which is only available at deployment time. As this is
* a late-bound token, it may not be used in `Construct` ids, but can be passed as a resource
* A hash of the bundle for of this asset, which is only available at deployment time. As this is
* a late-bound token, it may not be used in construct IDs, but can be passed as a resource
* property in order to force a change on a resource when an asset is effectively updated. This is
* more reliable than `sourceHash` in particular for assets which bundling phase involve external
* resources that can change over time (such as Docker image builds).
*/
readonly bundleHash: string;
readonly artifactHash: string;
}

/**
Expand Down Expand Up @@ -99,7 +99,7 @@ export class Asset extends cdk.Construct implements IAsset {
public readonly isZipArchive: boolean;

public readonly sourceHash: string;
public readonly bundleHash: string;
public readonly artifactHash: string;

/**
* The S3 prefix where all different versions of this asset are stored
Expand Down Expand Up @@ -139,16 +139,16 @@ export class Asset extends cdk.Construct implements IAsset {
description: `S3 key for asset version "${this.node.path}"`
});

const hashParam = new cdk.CfnParameter(this, 'BundleHash', {
description: `Bundle hash for asset "${this.node.path}"`,
const hashParam = new cdk.CfnParameter(this, 'ArtifactHash', {
description: `Artifact hash for asset "${this.node.path}"`,
type: 'String',
});

this.s3BucketName = bucketParam.stringValue;
this.s3Prefix = cdk.Fn.select(0, cdk.Fn.split(cxapi.ASSET_PREFIX_SEPARATOR, keyParam.stringValue)).toString();
const s3Filename = cdk.Fn.select(1, cdk.Fn.split(cxapi.ASSET_PREFIX_SEPARATOR, keyParam.stringValue)).toString();
this.s3ObjectKey = `${this.s3Prefix}${s3Filename}`;
this.bundleHash = hashParam.stringValue;
this.artifactHash = hashParam.stringValue;

this.bucket = s3.Bucket.import(this, 'AssetBucket', {
bucketName: this.s3BucketName
Expand All @@ -169,7 +169,7 @@ export class Asset extends cdk.Construct implements IAsset {

s3BucketParameter: bucketParam.logicalId,
s3KeyParameter: keyParam.logicalId,
bundleHashParameter: hashParam.logicalId,
artifactHashParameter: hashParam.logicalId,
};

this.node.addMetadata(cxapi.ASSET_METADATA, asset);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
"Type": "String",
"Description": "S3 key for asset version \"aws-cdk-asset-test/SampleAsset\""
},
"SampleAssetBundleHashDC778178": {
"SampleAssetArtifactHashE80944C9": {
"Type": "String",
"Description": "Bundle hash for asset \"aws-cdk-asset-test/SampleAsset\""
"Description": "Artifact hash for asset \"aws-cdk-asset-test/SampleAsset\""
}
},
"Resources": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
"Type": "String",
"Description": "S3 key for asset version \"aws-cdk-asset-file-test/SampleAsset\""
},
"SampleAssetBundleHashDC778178": {
"SampleAssetArtifactHashE80944C9": {
"Type": "String",
"Description": "Bundle hash for asset \"aws-cdk-asset-file-test/SampleAsset\""
"Description": "Artifact hash for asset \"aws-cdk-asset-file-test/SampleAsset\""
}
},
"Resources": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
"Type": "String",
"Description": "S3 key for asset version \"aws-cdk-asset-refs/MyFile\""
},
"MyFileBundleHashFD0DD32E": {
"MyFileArtifactHashAB5F44E1": {
"Type": "String",
"Description": "Bundle hash for asset \"aws-cdk-asset-refs/MyFile\""
"Description": "Artifact hash for asset \"aws-cdk-asset-refs/MyFile\""
}
},
"Resources": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
"Type": "String",
"Description": "S3 key for asset version \"aws-cdk-asset-refs/SampleAsset\""
},
"SampleAssetBundleHashDC778178": {
"SampleAssetArtifactHashE80944C9": {
"Type": "String",
"Description": "Bundle hash for asset \"aws-cdk-asset-refs/SampleAsset\""
"Description": "Artifact hash for asset \"aws-cdk-asset-refs/SampleAsset\""
}
},
"Outputs": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
"Type": "String",
"Description": "S3 key for asset version \"aws-cdk-multi-assets/SampleAsset1\""
},
"SampleAsset1BundleHashD0F1B38D": {
"SampleAsset1ArtifactHash9E24B5F0": {
"Type": "String",
"Description": "Bundle hash for asset \"aws-cdk-multi-assets/SampleAsset1\""
"Description": "Artifact hash for asset \"aws-cdk-multi-assets/SampleAsset1\""
},
"SampleAsset2S3BucketC94C651A": {
"Type": "String",
Expand All @@ -25,9 +25,9 @@
"Type": "String",
"Description": "S3 key for asset version \"aws-cdk-multi-assets/SampleAsset2\""
},
"SampleAsset2BundleHashD956324F": {
"SampleAsset2ArtifactHash62F55C83": {
"Type": "String",
"Description": "Bundle hash for asset \"aws-cdk-multi-assets/SampleAsset2\""
"Description": "Artifact hash for asset \"aws-cdk-multi-assets/SampleAsset2\""
}
}
}
6 changes: 3 additions & 3 deletions packages/@aws-cdk/assets/test/test.asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export = {
sourceHash: '6b84b87243a4a01c592d78e1fd3855c4bfef39328cd0a450cc97e81717fea2a2',
s3BucketParameter: 'MyAssetS3Bucket68C9B344',
s3KeyParameter: 'MyAssetS3VersionKey68E1A45D',
bundleHashParameter: 'MyAssetBundleHashB259CF93',
artifactHashParameter: 'MyAssetArtifactHashF518BDDE',
});

test.equal(template.Parameters.MyAssetS3Bucket68C9B344.Type, 'String');
Expand All @@ -60,7 +60,7 @@ export = {
sourceHash: '6b84b87243a4a01c592d78e1fd3855c4bfef39328cd0a450cc97e81717fea2a2',
s3BucketParameter: "MyAssetS3Bucket68C9B344",
s3KeyParameter: "MyAssetS3VersionKey68E1A45D",
bundleHashParameter: 'MyAssetBundleHashB259CF93',
artifactHashParameter: 'MyAssetArtifactHashF518BDDE',
});

test.done();
Expand All @@ -83,7 +83,7 @@ export = {
sourceHash: '78add9eaf468dfa2191da44a7da92a21baba4c686cf6053d772556768ef21197',
s3BucketParameter: 'MyAssetS3Bucket68C9B344',
s3KeyParameter: 'MyAssetS3VersionKey68E1A45D',
bundleHashParameter: 'MyAssetBundleHashB259CF93',
artifactHashParameter: 'MyAssetArtifactHashF518BDDE',
});

// verify that now the template contains parameters for this asset
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
"Type": "String",
"Description": "S3 key for asset version \"test-codebuild-docker-asset/AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62c/Code\""
},
"AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cCodeBundleHash7C06FC82": {
"AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cCodeArtifactHash8BCBAA49": {
"Type": "String",
"Description": "Bundle hash for asset \"test-codebuild-docker-asset/AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62c/Code\""
"Description": "Artifact hash for asset \"test-codebuild-docker-asset/AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62c/Code\""
}
},
"Resources": {
Expand All @@ -32,7 +32,7 @@
0,
{
"Fn::Split": [
":",
"@sha256:",
{
"Ref": "MyImageImageName953AD232"
}
Expand Down Expand Up @@ -67,7 +67,11 @@
],
"Version": "2012-10-17"
}
}
},
"DependsOn": [
"AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cServiceRoleDefaultPolicy6BC8737C",
"AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cServiceRoleD788AA17"
]
},
"AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cServiceRoleD788AA17": {
"Type": "AWS::IAM::Role",
Expand Down Expand Up @@ -146,7 +150,7 @@
0,
{
"Fn::Split": [
":",
"@sha256:",
{
"Ref": "MyImageImageName953AD232"
}
Expand Down Expand Up @@ -417,13 +421,13 @@
"RepositoryName"
]
},
":",
"@sha256:",
{
"Fn::Select": [
1,
{
"Fn::Split": [
":",
"@sha256:",
{
"Ref": "MyImageImageName953AD232"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
"Type": "String",
"Description": "S3 key for asset version \"aws-cdk-codebuild-project-shell/Bundle\""
},
"BundleBundleHash2DCEE924": {
"BundleArtifactHashEA214C27": {
"Type": "String",
"Description": "Bundle hash for asset \"aws-cdk-codebuild-project-shell/Bundle\""
"Description": "Artifact hash for asset \"aws-cdk-codebuild-project-shell/Bundle\""
}
},
"Resources": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -533,9 +533,9 @@
"Type": "String",
"Description": "S3 key for asset version \"aws-cdk-codebuild-project-vpc/Bundle\""
},
"BundleBundleHash2DCEE924": {
"BundleArtifactHashEA214C27": {
"Type": "String",
"Description": "Bundle hash for asset \"aws-cdk-codebuild-project-vpc/Bundle\""
"Description": "Artifact hash for asset \"aws-cdk-codebuild-project-vpc/Bundle\""
}
}
}
Loading

0 comments on commit 127847a

Please sign in to comment.