Skip to content

Commit

Permalink
fix(assets): permissions granted on shared prefix (#510)
Browse files Browse the repository at this point in the history
We need to give asset consumers permissions on all versions of an asset,
not just the latest version. Otherwise, as soon as we deploy a new
version, the Lambda (or other construct) will immediately lose
permissions to old versions and we won't be able to do rolling
updates.

OTHER CHANGES

- Toolkit: add caching on credential providers, to reduce network
  roundtrips when multiple AWS clients are constructed.

This fixes #484.
  • Loading branch information
rix0rrr authored Aug 20, 2018
1 parent 2c9674f commit 2a5240c
Show file tree
Hide file tree
Showing 15 changed files with 512 additions and 300 deletions.
23 changes: 16 additions & 7 deletions packages/@aws-cdk/assets/lib/asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ export class Asset extends cdk.Construct {

private readonly bucket: s3.BucketRef;

/**
* The S3 prefix where all different versions of this asset are stored
*/
private readonly s3Prefix: cdk.Token;

constructor(parent: cdk.Construct, id: string, props: GenericAssetProps) {
super(parent, id);

Expand All @@ -84,15 +89,15 @@ export class Asset extends cdk.Construct {
description: `S3 bucket for asset "${this.path}"`,
});

const keyParam = new cdk.Parameter(this, 'S3ObjectKey', {
const keyParam = new cdk.Parameter(this, 'S3VersionKey', {
type: 'String',
description: `S3 object for asset "${this.path}"`
description: `S3 key for asset version "${this.path}"`
});

this.s3BucketName = bucketParam.value;
this.s3ObjectKey = keyParam.value;

// grant the lambda's role read permissions on the code s3 object
this.s3Prefix = new cdk.FnSelect(0, new cdk.FnSplit(cxapi.ASSET_PREFIX_SEPARATOR, keyParam.value));
const s3Filename = new cdk.FnSelect(1, new cdk.FnSplit(cxapi.ASSET_PREFIX_SEPARATOR, keyParam.value));
this.s3ObjectKey = new cdk.FnConcat(this.s3Prefix, s3Filename);

this.bucket = s3.BucketRef.import(parent, 'AssetBucket', {
bucketName: this.s3BucketName
Expand All @@ -105,9 +110,9 @@ export class Asset extends cdk.Construct {
// for tooling to be able to package and upload a directory to the
// s3 bucket and plug in the bucket name and key in the correct
// parameters.

const asset: cxapi.AssetMetadataEntry = {
path: this.assetPath,
id: this.uniqueId,
packaging: props.packaging,
s3BucketParameter: bucketParam.logicalId,
s3KeyParameter: keyParam.logicalId,
Expand All @@ -124,7 +129,11 @@ export class Asset extends cdk.Construct {
* Grants read permissions to the principal on the asset's S3 object.
*/
public grantRead(principal?: iam.IPrincipal) {
this.bucket.grantRead(principal, this.s3ObjectKey);
// We give permissions on all files with the same prefix. Presumably
// different versions of the same file will have the same prefix
// and we don't want to accidentally revoke permission on old versions
// when deploying a new version.
this.bucket.grantRead(principal, `${this.s3Prefix}*`);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
"Type": "String",
"Description": "S3 bucket for asset \"aws-cdk-asset-test/SampleAsset\""
},
"SampleAssetS3ObjectKey6F5D200B": {
"SampleAssetS3VersionKey3E106D34": {
"Type": "String",
"Description": "S3 object for asset \"aws-cdk-asset-test/SampleAsset\""
"Description": "S3 key for asset version \"aws-cdk-asset-test/SampleAsset\""
}
},
"Resources": {
Expand Down Expand Up @@ -76,7 +76,25 @@
},
"/",
{
"Ref": "SampleAssetS3ObjectKey6F5D200B"
"Fn::Join": [
"",
[
{
"Fn::Select": [
0,
{
"Fn::Split": [
"||",
{
"Ref": "SampleAssetS3VersionKey3E106D34"
}
]
}
]
},
"*"
]
]
}
]
]
Expand Down
24 changes: 21 additions & 3 deletions packages/@aws-cdk/assets/test/integ.assets.file.lit.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
"Type": "String",
"Description": "S3 bucket for asset \"aws-cdk-asset-file-test/SampleAsset\""
},
"SampleAssetS3ObjectKey6F5D200B": {
"SampleAssetS3VersionKey3E106D34": {
"Type": "String",
"Description": "S3 object for asset \"aws-cdk-asset-file-test/SampleAsset\""
"Description": "S3 key for asset version \"aws-cdk-asset-file-test/SampleAsset\""
}
},
"Resources": {
Expand Down Expand Up @@ -76,7 +76,25 @@
},
"/",
{
"Ref": "SampleAssetS3ObjectKey6F5D200B"
"Fn::Join": [
"",
[
{
"Fn::Select": [
0,
{
"Fn::Split": [
"||",
{
"Ref": "SampleAssetS3VersionKey3E106D34"
}
]
}
]
},
"*"
]
]
}
]
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
"Type": "String",
"Description": "S3 bucket for asset \"aws-cdk-asset-refs/MyFile\""
},
"MyFileS3ObjectKey4641930D": {
"MyFileS3VersionKey568C3C9F": {
"Type": "String",
"Description": "S3 object for asset \"aws-cdk-asset-refs/MyFile\""
"Description": "S3 key for asset version \"aws-cdk-asset-refs/MyFile\""
}
},
"Resources": {
Expand Down Expand Up @@ -76,7 +76,25 @@
},
"/",
{
"Ref": "MyFileS3ObjectKey4641930D"
"Fn::Join": [
"",
[
{
"Fn::Select": [
0,
{
"Fn::Split": [
"||",
{
"Ref": "MyFileS3VersionKey568C3C9F"
}
]
}
]
},
"*"
]
]
}
]
]
Expand Down
81 changes: 76 additions & 5 deletions packages/@aws-cdk/assets/test/integ.assets.refs.lit.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
"Type": "String",
"Description": "S3 bucket for asset \"aws-cdk-asset-refs/SampleAsset\""
},
"SampleAssetS3ObjectKey6F5D200B": {
"SampleAssetS3VersionKey3E106D34": {
"Type": "String",
"Description": "S3 object for asset \"aws-cdk-asset-refs/SampleAsset\""
"Description": "S3 key for asset version \"aws-cdk-asset-refs/SampleAsset\""
}
},
"Outputs": {
Expand All @@ -20,7 +20,37 @@
},
"S3ObjectKey": {
"Value": {
"Ref": "SampleAssetS3ObjectKey6F5D200B"
"Fn::Join": [
"",
[
{
"Fn::Select": [
0,
{
"Fn::Split": [
"||",
{
"Ref": "SampleAssetS3VersionKey3E106D34"
}
]
}
]
},
{
"Fn::Select": [
1,
{
"Fn::Split": [
"||",
{
"Ref": "SampleAssetS3VersionKey3E106D34"
}
]
}
]
}
]
]
},
"Export": {
"Name": "aws-cdk-asset-refs:S3ObjectKey"
Expand All @@ -46,7 +76,30 @@
},
"/",
{
"Ref": "SampleAssetS3ObjectKey6F5D200B"
"Fn::Select": [
0,
{
"Fn::Split": [
"||",
{
"Ref": "SampleAssetS3VersionKey3E106D34"
}
]
}
]
},
{
"Fn::Select": [
1,
{
"Fn::Split": [
"||",
{
"Ref": "SampleAssetS3VersionKey3E106D34"
}
]
}
]
}
]
]
Expand Down Expand Up @@ -123,7 +176,25 @@
},
"/",
{
"Ref": "SampleAssetS3ObjectKey6F5D200B"
"Fn::Join": [
"",
[
{
"Fn::Select": [
0,
{
"Fn::Split": [
"||",
{
"Ref": "SampleAssetS3VersionKey3E106D34"
}
]
}
]
},
"*"
]
]
}
]
]
Expand Down
Loading

0 comments on commit 2a5240c

Please sign in to comment.