Skip to content

Commit

Permalink
fix(ecs): only works in 'aws' partition (#18496)
Browse files Browse the repository at this point in the history 
instead of assuming `aws` partition, use the stack to determine partition (which will result in a reference to `AWS::Partition`)

fixes #18429 
----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
@makrsmark
makrsmark authored Jan 21, 2022
1 parent a408382 commit 525ac07
Show file tree
Hide file tree
Showing 10 changed files with 1,874 additions and 1,760 deletions.
10 changes: 5 additions & 5 deletions packages/@aws-cdk/aws-ecs/lib/base/base-service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ export abstract class BaseService extends Resource
resources: ['*'],
}));

const logGroupArn = logConfiguration?.cloudWatchLogGroup ? `arn:aws:logs:${this.stack.region}:${this.stack.account}:log-group:${logConfiguration.cloudWatchLogGroup.logGroupName}:*` : '*';
const logGroupArn = logConfiguration?.cloudWatchLogGroup ? `arn:${this.stack.partition}:logs:${this.stack.region}:${this.stack.account}:log-group:${logConfiguration.cloudWatchLogGroup.logGroupName}:*` : '*';
this.taskDefinition.addToTaskRolePolicy(new iam.PolicyStatement({
actions: [
'logs:CreateLogStream',
Expand All @@ -491,14 +491,14 @@ export abstract class BaseService extends Resource
actions: [
's3:PutObject',
],
resources: [`arn:aws:s3:::${logConfiguration.s3Bucket.bucketName}/*`],
resources: [`arn:${this.stack.partition}:s3:::${logConfiguration.s3Bucket.bucketName}/*`],
}));
if (logConfiguration.s3EncryptionEnabled) {
this.taskDefinition.addToTaskRolePolicy(new iam.PolicyStatement({
actions: [
's3:GetEncryptionConfiguration',
],
resources: [`arn:aws:s3:::${logConfiguration.s3Bucket.bucketName}`],
resources: [`arn:${this.stack.partition}:s3:::${logConfiguration.s3Bucket.bucketName}`],
}));
}
}
Expand All @@ -518,7 +518,7 @@ export abstract class BaseService extends Resource
'kms:*',
],
resources: ['*'],
principals: [new iam.ArnPrincipal(`arn:aws:iam::${this.stack.account}:root`)],
principals: [new iam.ArnPrincipal(`arn:${this.stack.partition}:iam::${this.stack.account}:root`)],
}));

if (logging === ExecuteCommandLogging.DEFAULT || this.cluster.executeCommandConfiguration?.logConfiguration?.cloudWatchEncryptionEnabled) {
Expand All @@ -533,7 +533,7 @@ export abstract class BaseService extends Resource
resources: ['*'],
principals: [new iam.ServicePrincipal(`logs.${this.stack.region}.amazonaws.com`)],
conditions: {
ArnLike: { 'kms:EncryptionContext:aws:logs:arn': `arn:aws:logs:${this.stack.region}:${this.stack.account}:*` },
ArnLike: { 'kms:EncryptionContext:aws:logs:arn': `arn:${this.stack.partition}:logs:${this.stack.region}:${this.stack.account}:*` },
},
}));
}
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-ecs/lib/container-definition.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,7 @@ export class ContainerDefinition extends CoreConstruct {
workingDirectory: this.props.workingDirectory,
logConfiguration: this.logDriverConfig,
environment: this.environment && Object.keys(this.environment).length ? renderKV(this.environment, 'name', 'value') : undefined,
environmentFiles: this.environmentFiles && renderEnvironmentFiles(this.environmentFiles),
environmentFiles: this.environmentFiles && renderEnvironmentFiles(cdk.Stack.of(this).partition, this.environmentFiles),
secrets: this.secrets,
extraHosts: this.props.extraHosts && renderKV(this.props.extraHosts, 'hostname', 'ipAddress'),
healthCheck: this.props.healthCheck && renderHealthCheck(this.props.healthCheck),
Expand Down Expand Up @@ -757,7 +757,7 @@ function renderKV(env: { [key: string]: string }, keyName: string, valueName: st
return ret;
}

function renderEnvironmentFiles(environmentFiles: EnvironmentFileConfig[]): any[] {
function renderEnvironmentFiles(partition: string, environmentFiles: EnvironmentFileConfig[]): any[] {
const ret = [];
for (const environmentFile of environmentFiles) {
const s3Location = environmentFile.s3Location;
Expand All @@ -768,7 +768,7 @@ function renderEnvironmentFiles(environmentFiles: EnvironmentFileConfig[]): any[

ret.push({
type: environmentFile.fileType,
value: `arn:aws:s3:::${s3Location.bucketName}/${s3Location.objectKey}`,
value: `arn:${partition}:s3:::${s3Location.bucketName}/${s3Location.objectKey}`,
});
}
return ret;
Expand Down
30 changes: 25 additions & 5 deletions packages/@aws-cdk/aws-ecs/test/container-definition.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,11 @@ describe('container definition', () => {
'Fn::Join': [
'',
[
'arn:aws:s3:::',
'arn:',
{
Ref: 'AWS::Partition',
},
':s3:::',
{
Ref: 'AssetParameters872561bf078edd1685d50c9ff821cdd60d2b2ddfb0013c4087e79bf2bb50724dS3Bucket7B2069B7',
},
Expand Down Expand Up @@ -840,7 +844,11 @@ describe('container definition', () => {
'Fn::Join': [
'',
[
'arn:aws:s3:::',
'arn:',
{
Ref: 'AWS::Partition',
},
':s3:::',
{
Ref: 'AssetParameters872561bf078edd1685d50c9ff821cdd60d2b2ddfb0013c4087e79bf2bb50724dS3Bucket7B2069B7',
},
Expand Down Expand Up @@ -905,7 +913,11 @@ describe('container definition', () => {
'Fn::Join': [
'',
[
'arn:aws:s3:::',
'arn:',
{
Ref: 'AWS::Partition',
},
':s3:::',
{
Ref: 'Bucket83908E77',
},
Expand Down Expand Up @@ -943,7 +955,11 @@ describe('container definition', () => {
'Fn::Join': [
'',
[
'arn:aws:s3:::',
'arn:',
{
Ref: 'AWS::Partition',
},
':s3:::',
{
Ref: 'AssetParameters872561bf078edd1685d50c9ff821cdd60d2b2ddfb0013c4087e79bf2bb50724dS3Bucket7B2069B7',
},
Expand Down Expand Up @@ -1008,7 +1024,11 @@ describe('container definition', () => {
'Fn::Join': [
'',
[
'arn:aws:s3:::',
'arn:',
{
Ref: 'AWS::Partition',
},
':s3:::',
{
Ref: 'Bucket83908E77',
},
Expand Down
60 changes: 50 additions & 10 deletions packages/@aws-cdk/aws-ecs/test/ec2/ec2-service.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,11 @@ describe('ec2 service', () => {
'Fn::Join': [
'',
[
'arn:aws:logs:',
'arn:',
{
Ref: 'AWS::Partition',
},
':logs:',
{
Ref: 'AWS::Region',
},
Expand Down Expand Up @@ -288,7 +292,11 @@ describe('ec2 service', () => {
'Fn::Join': [
'',
[
'arn:aws:s3:::',
'arn:',
{
Ref: 'AWS::Partition',
},
':s3:::',
{
Ref: 'ExecBucket29559356',
},
Expand Down Expand Up @@ -392,7 +400,11 @@ describe('ec2 service', () => {
'Fn::Join': [
'',
[
'arn:aws:logs:',
'arn:',
{
Ref: 'AWS::Partition',
},
':logs:',
{
Ref: 'AWS::Region',
},
Expand Down Expand Up @@ -421,7 +433,11 @@ describe('ec2 service', () => {
'Fn::Join': [
'',
[
'arn:aws:s3:::',
'arn:',
{
Ref: 'AWS::Partition',
},
':s3:::',
{
Ref: 'EcsExecBucket4F468651',
},
Expand Down Expand Up @@ -491,7 +507,11 @@ describe('ec2 service', () => {
'Fn::Join': [
'',
[
'arn:aws:iam::',
'arn:',
{
Ref: 'AWS::Partition',
},
':iam::',
{
Ref: 'AWS::AccountId',
},
Expand Down Expand Up @@ -598,7 +618,11 @@ describe('ec2 service', () => {
'Fn::Join': [
'',
[
'arn:aws:logs:',
'arn:',
{
Ref: 'AWS::Partition',
},
':logs:',
{
Ref: 'AWS::Region',
},
Expand Down Expand Up @@ -627,7 +651,11 @@ describe('ec2 service', () => {
'Fn::Join': [
'',
[
'arn:aws:s3:::',
'arn:',
{
Ref: 'AWS::Partition',
},
':s3:::',
{
Ref: 'EcsExecBucket4F468651',
},
Expand All @@ -643,7 +671,11 @@ describe('ec2 service', () => {
'Fn::Join': [
'',
[
'arn:aws:s3:::',
'arn:',
{
Ref: 'AWS::Partition',
},
':s3:::',
{
Ref: 'EcsExecBucket4F468651',
},
Expand Down Expand Up @@ -712,7 +744,11 @@ describe('ec2 service', () => {
'Fn::Join': [
'',
[
'arn:aws:iam::',
'arn:',
{
Ref: 'AWS::Partition',
},
':iam::',
{
Ref: 'AWS::AccountId',
},
Expand All @@ -737,7 +773,11 @@ describe('ec2 service', () => {
'Fn::Join': [
'',
[
'arn:aws:logs:',
'arn:',
{
Ref: 'AWS::Partition',
},
':logs:',
{
Ref: 'AWS::Region',
},
Expand Down
6 changes: 5 additions & 1 deletion packages/@aws-cdk/aws-ecs/test/ec2/ec2-task-definition.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,11 @@ describe('ec2 task definition', () => {
'Fn::Join': [
'',
[
'arn:aws:s3:::',
'arn:',
{
Ref: 'AWS::Partition',
},
':s3:::',
{
Ref: 'AssetParameters872561bf078edd1685d50c9ff821cdd60d2b2ddfb0013c4087e79bf2bb50724dS3Bucket7B2069B7',
},
Expand Down
Loading

0 comments on commit 525ac07

Please sign in to comment.