Skip to content

Commit 136c523

Browse files
mergify[bot]mergify[bot]
authored
Merge branch 'master' into njlynch/rds-cluster-attributes
2 parents 2b3e14c + 54dfe83 commit 136c523

15 files changed

Lines changed: 134 additions & 52 deletions

File tree

‎packages/@aws-cdk/aws-cloudfront-origins/lib/s3-origin.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ class S3BucketOrigin extends cloudfront.OriginBase {
5959
public bind(scope: cdk.Construct, options: cloudfront.OriginBindOptions): cloudfront.OriginBindConfig {
6060
if (!this.originAccessIdentity) {
6161
this.originAccessIdentity = new cloudfront.OriginAccessIdentity(scope, 'S3Origin', {
62-
comment: `Access identity for ${options.originId}`,
62+
comment: `Identity for ${options.originId}`,
6363
});
6464
this.bucket.grantRead(this.originAccessIdentity);
6565
}

‎packages/@aws-cdk/aws-cloudfront-origins/test/integ.origin-group.expected.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@
6060
"Type": "AWS::CloudFront::CloudFrontOriginAccessIdentity",
6161
"Properties": {
6262
"CloudFrontOriginAccessIdentityConfig": {
63-
"Comment": "Access identity for cloudfrontorigingroupDistributionOrigin137659A54"
63+
"Comment": "Identity for cloudfrontorigingroupDistributionOrigin137659A54"
6464
}
6565
}
6666
},
@@ -153,4 +153,4 @@
153153
}
154154
}
155155
}
156-
}
156+
}

‎packages/@aws-cdk/aws-cloudfront-origins/test/integ.s3-origin.expected.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@
6060
"Type": "AWS::CloudFront::CloudFrontOriginAccessIdentity",
6161
"Properties": {
6262
"CloudFrontOriginAccessIdentityConfig": {
63-
"Comment": "Access identity for cloudfronts3originDistributionOrigin1741C4E95"
63+
"Comment": "Identity for cloudfronts3originDistributionOrigin1741C4E95"
6464
}
6565
}
6666
},
@@ -106,4 +106,4 @@
106106
}
107107
}
108108
}
109-
}
109+
}

‎packages/@aws-cdk/aws-cloudfront-origins/test/s3-origin.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ describe('With bucket', () => {
5454

5555
expect(stack).toHaveResourceLike('AWS::CloudFront::CloudFrontOriginAccessIdentity', {
5656
CloudFrontOriginAccessIdentityConfig: {
57-
Comment: 'Access identity for StackDistOrigin15754CE84',
57+
Comment: 'Identity for StackDistOrigin15754CE84',
5858
},
5959
});
6060
expect(stack).toHaveResourceLike('AWS::S3::BucketPolicy', {

‎packages/@aws-cdk/aws-cloudfront/lib/origin_access_identity.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,10 @@ export class OriginAccessIdentity extends OriginAccessIdentityBase implements IO
106106
constructor(scope: cdk.Construct, id: string, props?: OriginAccessIdentityProps) {
107107
super(scope, id);
108108

109+
// Comment has a max length of 128.
110+
const comment = (props?.comment ?? 'Allows CloudFront to reach the bucket').substr(0, 128);
109111
this.resource = new CfnCloudFrontOriginAccessIdentity(this, 'Resource', {
110-
cloudFrontOriginAccessIdentityConfig: {
111-
comment: (props && props.comment) || 'Allows CloudFront to reach the bucket',
112-
},
112+
cloudFrontOriginAccessIdentityConfig: { comment },
113113
});
114114
// physical id - OAI name
115115
this.originAccessIdentityName = this.getResourceNameAttribute(this.resource.ref);

‎packages/@aws-cdk/aws-cloudfront/test/oai.test.ts‎

Lines changed: 36 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,69 +1,71 @@
1-
import { expect } from '@aws-cdk/assert';
1+
import '@aws-cdk/assert/jest';
22
import * as cdk from '@aws-cdk/core';
3-
import { nodeunitShim, Test } from 'nodeunit-shim';
43
import { OriginAccessIdentity } from '../lib';
54

6-
/* eslint-disable quote-props */
7-
8-
nodeunitShim({
9-
'Origin Access Identity with automatic comment'(test: Test) {
5+
describe('Origin Access Identity', () => {
6+
test('With automatic comment', () => {
107
const stack = new cdk.Stack();
118

129
new OriginAccessIdentity(stack, 'OAI');
1310

14-
expect(stack).toMatch(
11+
expect(stack).toMatchTemplate(
1512
{
16-
'Resources': {
17-
'OAIE1EFC67F': {
18-
'Type': 'AWS::CloudFront::CloudFrontOriginAccessIdentity',
19-
'Properties': {
20-
'CloudFrontOriginAccessIdentityConfig': {
21-
'Comment': 'Allows CloudFront to reach the bucket',
13+
Resources: {
14+
OAIE1EFC67F: {
15+
Type: 'AWS::CloudFront::CloudFrontOriginAccessIdentity',
16+
Properties: {
17+
CloudFrontOriginAccessIdentityConfig: {
18+
Comment: 'Allows CloudFront to reach the bucket',
2219
},
2320
},
2421
},
2522
},
2623
},
2724
);
25+
});
2826

29-
test.done();
30-
},
31-
'Origin Access Identity with comment'(test: Test) {
27+
test('With provided comment', () => {
3228
const stack = new cdk.Stack();
3329

3430
new OriginAccessIdentity(stack, 'OAI', {
3531
comment: 'test comment',
3632
});
3733

38-
expect(stack).toMatch(
34+
expect(stack).toMatchTemplate(
3935
{
40-
'Resources': {
41-
'OAIE1EFC67F': {
42-
'Type': 'AWS::CloudFront::CloudFrontOriginAccessIdentity',
43-
'Properties': {
44-
'CloudFrontOriginAccessIdentityConfig': {
45-
'Comment': 'test comment',
36+
Resources: {
37+
OAIE1EFC67F: {
38+
Type: 'AWS::CloudFront::CloudFrontOriginAccessIdentity',
39+
Properties: {
40+
CloudFrontOriginAccessIdentityConfig: {
41+
Comment: 'test comment',
4642
},
4743
},
4844
},
4945
},
5046
},
5147
);
48+
});
49+
50+
test('Truncates long comments', () => {
51+
const stack = new cdk.Stack();
52+
53+
new OriginAccessIdentity(stack, 'OAI', {
54+
comment: 'This is a really long comment. Auto-generated comments based on ids of origins might sometimes be this long or even longer and that will break',
55+
});
5256

53-
test.done();
54-
},
57+
expect(stack).toHaveResourceLike('AWS::CloudFront::CloudFrontOriginAccessIdentity', {
58+
CloudFrontOriginAccessIdentityConfig: {
59+
Comment: 'This is a really long comment. Auto-generated comments based on ids of origins might sometimes be this long or even longer and t',
60+
},
61+
});
62+
});
5563

56-
'Builds ARN of CloudFront user'(test: Test) {
64+
test('Builds ARN of CloudFront user', () => {
5765
const stack = new cdk.Stack();
5866

5967
const oai = OriginAccessIdentity.fromOriginAccessIdentityName(stack, 'OAI', 'OAITest');
6068

61-
test.ok(
62-
oai.grantPrincipal.policyFragment.principalJson.AWS[0].endsWith(
63-
':iam::cloudfront:user/CloudFront Origin Access Identity OAITest',
64-
),
65-
);
66-
67-
test.done();
68-
},
69+
expect(oai.grantPrincipal.policyFragment.principalJson.AWS[0]).toMatch(/:iam::cloudfront:user\/CloudFront Origin Access Identity OAITest$/);
70+
});
6971
});

‎packages/@aws-cdk/aws-codebuild/README.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,18 @@ const bbSource = codebuild.Source.bitBucket({
115115
});
116116
```
117117

118+
### For all Git sources
119+
120+
For all Git sources, you can fetch submodules while cloing git repo.
121+
122+
```typescript
123+
const gitHubSource = codebuild.Source.gitHub({
124+
owner: 'awslabs',
125+
repo: 'aws-cdk',
126+
fetchSubmodules: true,
127+
});
128+
```
129+
118130
## Artifacts
119131

120132
CodeBuild Projects can produce Artifacts and upload them to S3. For example:

‎packages/@aws-cdk/aws-codebuild/lib/source.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,13 @@ interface GitSourceProps extends SourceProps {
119119
* @default the default branch's HEAD commit ID is used
120120
*/
121121
readonly branchOrRef?: string;
122+
123+
/**
124+
* Whether to fetch submodules while cloning git repo.
125+
*
126+
* @default false
127+
*/
128+
readonly fetchSubmodules?: boolean;
122129
}
123130

124131
/**
@@ -127,12 +134,14 @@ interface GitSourceProps extends SourceProps {
127134
abstract class GitSource extends Source {
128135
private readonly cloneDepth?: number;
129136
private readonly branchOrRef?: string;
137+
private readonly fetchSubmodules?: boolean;
130138

131139
protected constructor(props: GitSourceProps) {
132140
super(props);
133141

134142
this.cloneDepth = props.cloneDepth;
135143
this.branchOrRef = props.branchOrRef;
144+
this.fetchSubmodules = props.fetchSubmodules;
136145
}
137146

138147
public bind(_scope: Construct, _project: IProject): SourceConfig {
@@ -142,6 +151,9 @@ abstract class GitSource extends Source {
142151
sourceProperty: {
143152
...superConfig.sourceProperty,
144153
gitCloneDepth: this.cloneDepth,
154+
gitSubmodulesConfig: this.fetchSubmodules ? {
155+
fetchSubmodules: this.fetchSubmodules,
156+
} : undefined,
145157
},
146158
};
147159
}

‎packages/@aws-cdk/aws-codebuild/test/test.codebuild.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,7 @@ export = {
546546
owner: 'testowner',
547547
repo: 'testrepo',
548548
cloneDepth: 3,
549+
fetchSubmodules: true,
549550
webhook: true,
550551
reportBuildStatus: false,
551552
webhookFilters: [
@@ -561,6 +562,9 @@ export = {
561562
Location: 'https://github.com/testowner/testrepo.git',
562563
ReportBuildStatus: false,
563564
GitCloneDepth: 3,
565+
GitSubmodulesConfig: {
566+
FetchSubmodules: true,
567+
},
564568
},
565569
}));
566570

‎packages/@aws-cdk/aws-kms/lib/key.ts‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as iam from '@aws-cdk/aws-iam';
2-
import { Construct, IResource, RemovalPolicy, Resource, Stack } from '@aws-cdk/core';
2+
import { Construct, IConstruct, IResource, RemovalPolicy, Resource, Stack } from '@aws-cdk/core';
33
import { Alias } from './alias';
44
import { CfnKey } from './kms.generated';
55

@@ -207,11 +207,18 @@ abstract class KeyBase extends Resource implements IKey {
207207
* undefined otherwise
208208
*/
209209
private granteeStackDependsOnKeyStack(grantee: iam.IGrantable): string | undefined {
210-
if (!(Construct.isConstruct(grantee))) {
210+
const grantPrincipal = grantee.grantPrincipal;
211+
if (!(Construct.isConstruct(grantPrincipal))) {
212+
return undefined;
213+
}
214+
// this logic should only apply to newly created
215+
// (= not imported) resources
216+
if (!this.principalIsANewlyCreatedResource(grantPrincipal)) {
211217
return undefined;
212218
}
219+
// return undefined;
213220
const keyStack = Stack.of(this);
214-
const granteeStack = Stack.of(grantee);
221+
const granteeStack = Stack.of(grantPrincipal);
215222
if (keyStack === granteeStack) {
216223
return undefined;
217224
}
@@ -220,6 +227,14 @@ abstract class KeyBase extends Resource implements IKey {
220227
: undefined;
221228
}
222229

230+
private principalIsANewlyCreatedResource(principal: IConstruct): boolean {
231+
// yes, this sucks
232+
// this is just a temporary stopgap to stem the bleeding while we work on a proper fix
233+
return principal instanceof iam.Role ||
234+
principal instanceof iam.User ||
235+
principal instanceof iam.Group;
236+
}
237+
223238
private isGranteeFromAnotherRegion(grantee: iam.IGrantable): boolean {
224239
if (!(Construct.isConstruct(grantee))) {
225240
return false;

0 commit comments

Comments
 (0)