Skip to content

Commit 596ed1b

Browse files
mergify[bot]mergify[bot]
authored
Merge branch 'master' into merge-back/1.136.0
2 parents ae13a75 + 77144f5 commit 596ed1b

30 files changed

+837
-1012
lines changed

‎packages/@aws-cdk/aws-codebuild/lib/linux-gpu-build-image.ts

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as ecr from '@aws-cdk/aws-ecr';
22
import * as core from '@aws-cdk/core';
3-
import { FactName, RegionInfo } from '@aws-cdk/region-info';
3+
import { FactName } from '@aws-cdk/region-info';
44
import { BuildSpec } from './build-spec';
55
import { runScriptLinuxBuildSpec } from './private/run-script-linux-build-spec';
66
import {
@@ -12,8 +12,6 @@ import {
1212
// eslint-disable-next-line no-duplicate-imports, import/order
1313
import { Construct } from '@aws-cdk/core';
1414

15-
const mappingName = 'AwsDeepLearningContainersRepositoriesAccounts';
16-
1715
/**
1816
* A CodeBuild GPU image running Linux.
1917
*
@@ -109,38 +107,38 @@ export class LinuxGpuBuildImage implements IBindableBuildImage {
109107

110108
public readonly type = 'LINUX_GPU_CONTAINER';
111109
public readonly defaultComputeType = ComputeType.LARGE;
112-
public readonly imageId: string;
113110
public readonly imagePullPrincipalType?: ImagePullPrincipalType = ImagePullPrincipalType.SERVICE_ROLE;
111+
public readonly imageId: string;
114112

115-
private readonly accountExpression: string;
113+
private _imageAccount?: string;
116114

117115
private constructor(private readonly repositoryName: string, tag: string, private readonly account: string | undefined) {
118-
this.accountExpression = account ?? core.Fn.findInMap(mappingName, core.Aws.REGION, 'repositoryAccount');
119-
this.imageId = `${this.accountExpression}.dkr.ecr.${core.Aws.REGION}.${core.Aws.URL_SUFFIX}/${repositoryName}:${tag}`;
116+
const imageAccount = account ?? core.Lazy.string({
117+
produce: () => {
118+
if (this._imageAccount === undefined) {
119+
throw new Error('Make sure this \'LinuxGpuBuildImage\' is used in a CodeBuild Project construct');
120+
}
121+
return this._imageAccount;
122+
},
123+
});
124+
125+
// The value of imageId below *should* have been `Lazy.stringValue(() => repository.repositoryUriForTag(this.tag))`,
126+
// but we can't change that anymore because someone somewhere might at this point have written code
127+
// to do `image.imageId.includes('pytorch')` and changing this to a full-on token would break them.
128+
this.imageId = `${imageAccount}.dkr.ecr.${core.Aws.REGION}.${core.Aws.URL_SUFFIX}/${repositoryName}:${tag}`;
120129
}
121130

122131
public bind(scope: Construct, project: IProject, _options: BuildImageBindOptions): BuildImageConfig {
123-
if (!this.account) {
124-
const scopeStack = core.Stack.of(scope);
125-
// Unfortunately, the account IDs of the DLC repositories are not the same in all regions.
126-
// Because of that, use a (singleton) Mapping to find the correct account
127-
if (!scopeStack.node.tryFindChild(mappingName)) {
128-
const mapping: { [k1: string]: { [k2: string]: any } } = {};
129-
// get the accounts from the region-info module
130-
const region2Accounts = RegionInfo.regionMap(FactName.DLC_REPOSITORY_ACCOUNT);
131-
for (const [region, account] of Object.entries(region2Accounts)) {
132-
mapping[region] = { repositoryAccount: account };
133-
}
134-
new core.CfnMapping(scopeStack, mappingName, { mapping });
135-
}
136-
}
137-
132+
const account = this.account ?? core.Stack.of(scope).regionalFact(FactName.DLC_REPOSITORY_ACCOUNT);
138133
const repository = ecr.Repository.fromRepositoryAttributes(scope, 'AwsDlcRepositoryCodeBuild', {
139134
repositoryName: this.repositoryName,
140-
repositoryArn: ecr.Repository.arnForLocalRepository(this.repositoryName, scope, this.accountExpression),
135+
repositoryArn: ecr.Repository.arnForLocalRepository(this.repositoryName, scope, account),
141136
});
137+
142138
repository.grantPull(project);
143139

140+
this._imageAccount = account;
141+
144142
return {
145143
};
146144
}

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1138,9 +1138,8 @@ export class Project extends ProjectBase {
11381138
}
11391139

11401140
// bind
1141-
const bindFunction = (this.buildImage as any).bind;
1142-
if (bindFunction) {
1143-
bindFunction.call(this.buildImage, this, this, {});
1141+
if (isBindableBuildImage(this.buildImage)) {
1142+
this.buildImage.bind(this, this, {});
11441143
}
11451144
}
11461145

@@ -2123,3 +2122,7 @@ export enum ProjectNotificationEvents {
21232122
*/
21242123
BUILD_PHASE_SUCCEEDED = 'codebuild-project-build-phase-success',
21252124
}
2125+
2126+
function isBindableBuildImage(x: unknown): x is IBindableBuildImage {
2127+
return typeof x === 'object' && !!x && !!(x as any).bind;
2128+
}

‎packages/@aws-cdk/aws-codebuild/test/integ.aws-deep-learning-container-build-image.expected.json

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,11 @@
135135
":",
136136
{
137137
"Fn::FindInMap": [
138-
"AwsDeepLearningContainersRepositoriesAccounts",
138+
"DlcRepositoryAccountMap",
139139
{
140140
"Ref": "AWS::Region"
141141
},
142-
"repositoryAccount"
142+
"value"
143143
]
144144
},
145145
":repository/mxnet-training"
@@ -177,11 +177,11 @@
177177
[
178178
{
179179
"Fn::FindInMap": [
180-
"AwsDeepLearningContainersRepositoriesAccounts",
180+
"DlcRepositoryAccountMap",
181181
{
182182
"Ref": "AWS::Region"
183183
},
184-
"repositoryAccount"
184+
"value"
185185
]
186186
},
187187
".dkr.ecr.",
@@ -215,66 +215,66 @@
215215
}
216216
},
217217
"Mappings": {
218-
"AwsDeepLearningContainersRepositoriesAccounts": {
218+
"DlcRepositoryAccountMap": {
219219
"ap-east-1": {
220-
"repositoryAccount": "871362719292"
220+
"value": "871362719292"
221221
},
222222
"ap-northeast-1": {
223-
"repositoryAccount": "763104351884"
223+
"value": "763104351884"
224224
},
225225
"ap-northeast-2": {
226-
"repositoryAccount": "763104351884"
226+
"value": "763104351884"
227227
},
228228
"ap-south-1": {
229-
"repositoryAccount": "763104351884"
229+
"value": "763104351884"
230230
},
231231
"ap-southeast-1": {
232-
"repositoryAccount": "763104351884"
232+
"value": "763104351884"
233233
},
234234
"ap-southeast-2": {
235-
"repositoryAccount": "763104351884"
235+
"value": "763104351884"
236236
},
237237
"ca-central-1": {
238-
"repositoryAccount": "763104351884"
238+
"value": "763104351884"
239239
},
240240
"cn-north-1": {
241-
"repositoryAccount": "727897471807"
241+
"value": "727897471807"
242242
},
243243
"cn-northwest-1": {
244-
"repositoryAccount": "727897471807"
244+
"value": "727897471807"
245245
},
246246
"eu-central-1": {
247-
"repositoryAccount": "763104351884"
247+
"value": "763104351884"
248248
},
249249
"eu-north-1": {
250-
"repositoryAccount": "763104351884"
250+
"value": "763104351884"
251251
},
252252
"eu-west-1": {
253-
"repositoryAccount": "763104351884"
253+
"value": "763104351884"
254254
},
255255
"eu-west-2": {
256-
"repositoryAccount": "763104351884"
256+
"value": "763104351884"
257257
},
258258
"eu-west-3": {
259-
"repositoryAccount": "763104351884"
259+
"value": "763104351884"
260260
},
261261
"me-south-1": {
262-
"repositoryAccount": "217643126080"
262+
"value": "217643126080"
263263
},
264264
"sa-east-1": {
265-
"repositoryAccount": "763104351884"
265+
"value": "763104351884"
266266
},
267267
"us-east-1": {
268-
"repositoryAccount": "763104351884"
268+
"value": "763104351884"
269269
},
270270
"us-east-2": {
271-
"repositoryAccount": "763104351884"
271+
"value": "763104351884"
272272
},
273273
"us-west-1": {
274-
"repositoryAccount": "763104351884"
274+
"value": "763104351884"
275275
},
276276
"us-west-2": {
277-
"repositoryAccount": "763104351884"
277+
"value": "763104351884"
278278
}
279279
}
280280
}

‎packages/@aws-cdk/aws-iam/lib/principals.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as cdk from '@aws-cdk/core';
2-
import { Default, RegionInfo } from '@aws-cdk/region-info';
2+
import { Default, FactName, RegionInfo } from '@aws-cdk/region-info';
33
import { IOpenIdConnectProvider } from './oidc-provider';
44
import { Condition, Conditions, PolicyStatement } from './policy-statement';
55
import { ISamlProvider } from './saml-provider';
@@ -331,6 +331,7 @@ export interface ServicePrincipalOpts {
331331
* The region in which the service is operating.
332332
*
333333
* @default the current Stack's region.
334+
* @deprecated You should not need to set this. The stack's region is always correct.
334335
*/
335336
readonly region?: string;
336337

@@ -694,9 +695,23 @@ class ServicePrincipalToken implements cdk.IResolvable {
694695
}
695696

696697
public resolve(ctx: cdk.IResolveContext) {
697-
const region = this.opts.region || cdk.Stack.of(ctx.scope).region;
698-
const fact = RegionInfo.get(region).servicePrincipal(this.service);
699-
return fact || Default.servicePrincipal(this.service, region, cdk.Aws.URL_SUFFIX);
698+
if (this.opts.region) {
699+
// Special case, handle it separately to not break legacy behavior.
700+
return (
701+
RegionInfo.get(this.opts.region).servicePrincipal(this.service) ??
702+
Default.servicePrincipal(
703+
this.service,
704+
this.opts.region,
705+
cdk.Aws.URL_SUFFIX,
706+
)
707+
);
708+
}
709+
710+
const stack = cdk.Stack.of(ctx.scope);
711+
return stack.regionalFact(
712+
FactName.servicePrincipal(this.service),
713+
Default.servicePrincipal(this.service, stack.region, cdk.Aws.URL_SUFFIX),
714+
);
700715
}
701716

702717
public toString() {

‎packages/@aws-cdk/aws-iam/lib/util.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,4 +137,4 @@ export class UniqueStringSet implements IResolvable, IPostProcessor {
137137

138138
function isEmptyObject(x: { [key: string]: any }): boolean {
139139
return Object.keys(x).length === 0;
140-
}
140+
}

‎packages/@aws-cdk/aws-iam/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@
8080
"license": "Apache-2.0",
8181
"devDependencies": {
8282
"@aws-cdk/assert-internal": "0.0.0",
83+
"@aws-cdk/assertions": "0.0.0",
8384
"@aws-cdk/cdk-build-tools": "0.0.0",
8485
"@aws-cdk/cdk-integ-tools": "0.0.0",
8586
"@aws-cdk/cfn2ts": "0.0.0",

‎packages/@aws-cdk/aws-iam/test/policy-document.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import '@aws-cdk/assert-internal/jest';
2+
import { testDeprecated } from '@aws-cdk/cdk-build-tools';
23
import { Lazy, Stack, Token } from '@aws-cdk/core';
34
import {
45
AccountPrincipal, Anyone, AnyPrincipal, ArnPrincipal, CanonicalUserPrincipal, CompositePrincipal,
@@ -444,7 +445,8 @@ describe('IAM policy document', () => {
444445
});
445446
});
446447

447-
test('regional service principals resolve appropriately (with user-set region)', () => {
448+
// Deprecated: 'region' parameter to ServicePrincipal shouldn't be used.
449+
testDeprecated('regional service principals resolve appropriately (with user-set region)', () => {
448450
const stack = new Stack(undefined, undefined, { env: { region: 'cn-northeast-1' } });
449451
const s = new PolicyStatement();
450452
s.addActions('test:Action');

‎packages/@aws-cdk/aws-iam/test/principals.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import '@aws-cdk/assert-internal/jest';
2+
import { Template } from '@aws-cdk/assertions';
23
import { App, CfnOutput, Stack } from '@aws-cdk/core';
34
import * as iam from '../lib';
45

@@ -243,4 +244,20 @@ test('PrincipalWithConditions inherits principalAccount from AccountPrincipal ',
243244
// THEN
244245
expect(accountPrincipal.principalAccount).toStrictEqual('123456789012');
245246
expect(principalWithConditions.principalAccount).toStrictEqual('123456789012');
247+
});
248+
249+
test('ServicePrincipal in agnostic stack generates lookup table', () => {
250+
// GIVEN
251+
const stack = new Stack();
252+
253+
// WHEN
254+
new iam.Role(stack, 'Role', {
255+
assumedBy: new iam.ServicePrincipal('ssm.amazonaws.com'),
256+
});
257+
258+
// THEN
259+
const template = Template.fromStack(stack);
260+
const mappings = template.findMappings('ServiceprincipalMap');
261+
expect(mappings.ServiceprincipalMap['af-south-1']?.ssm).toEqual('ssm.af-south-1.amazonaws.com');
262+
expect(mappings.ServiceprincipalMap['us-east-1']?.ssm).toEqual('ssm.amazonaws.com');
246263
});

0 commit comments

Comments
 (0)