aws / aws/aws-cdk

(ecs): Allow specifying that an imported TaskDefinition does not have a revision specified

Open
#32,485 4 comments 0 reactions 0 assignees View on GitHub
@aws-cdk/aws-ecs effort/small feature-request p2
Dominant language
TypeScript
Stars
12.9k
Forks
4.6k
Avg merge
2d 3h
Merged PRs (30d)
83

Description

### Describe the feature

Today, we cannot specify whether or not an imported TaskDefinition is "fully qualified" (e.g., `MyTaskDef:123`) or not (e.g., `MyTaskDef`).

There are certain cases, like granting IAM permissions - see https://github.com/aws/aws-cdk/issues/30390 / https://github.com/aws/aws-cdk/pull/31615, where different behaviors need to happen depending on if the task def is fully qualified with a revision or not.

### Use Case

I expose various `pgdump` containers to assist with dumping Aurora Postgres databases to S3. I create a single, shared Task Definition that I expose via CloudFormation outputs. Then, I use `Fn.importValue` to import these in shared logic.

Here's the some sample code to show the issue

```ts
import { Fn, Stack, type StackProps } from 'aws-cdk-lib';
import { SubnetType } from 'aws-cdk-lib/aws-ec2';
import { Cluster, NetworkMode, type ICluster } from 'aws-cdk-lib/aws-ecs';
import { FargateTaskDefinition } from 'aws-cdk-lib/aws-ecs';
import { Rule, Schedule } from 'aws-cdk-lib/aws-events';
import { Role } from 'aws-cdk-lib/aws-iam';
import { Construct } from 'constructs';
import { EcsTask as EcsTaskTarget, EcsTaskProps } from "aws-cdk-lib/aws-events-targets";

export class ExampleStack extends Stack {
constructor(scope: Construct, id: string, props?: StackProps) {
super(scope, id, props);

const ecsCluster = new Cluster(this, "EcsCluster", {});
const clusterMajorVersion = "16";

// IMPORTANT - this is a task definition _without_ a revision number (e.g., arn:aws:ecs:us-west-2:12345678910:task-definition/PgDump-16)
const pgDumpTaskDefArn = Fn.importValue(`PgDump-${clusterMajorVersion}-TaskDefArn`);
const pgDumpContainerName = Fn.importValue(`PgDump-${clusterMajorVersion}-ContainerName`);
const pgDumpTaskRole = Role.fromRoleArn(
this,
"PgDumpTaskRoleImport",
Fn.importValue(`PgDump-${clusterMajorVersion}-TaskDefRoleArn`)
);
const pgDumpExecutionRole = Role.fromRoleArn(
this,
"PgDumpExecutionRoleImport",
Fn.importValue(`PgDump-${clusterMajorVersion}-TaskDefExecutionRoleArn`)
);

const pgDumpEcsTask = FargateTaskDefinition.fromFargateTaskDefinitionAttributes(this, "PgDumpTaskDefImport", {
taskDefinitionArn: pgDumpTaskDefArn,
executionRole: pgDumpExecutionRole,
taskRole: pgDumpTaskRole,
networkMode: NetworkMode.AWS_VPC,
});

const commonEcsTaskTargetProps: EcsTaskProps = {
cluster: ecsCluster,
taskDefinition: pgDumpEcsTask,
taskCount: 1,
subnetSelection: { subnetType: SubnetType.PRIVATE_WITH_EGRESS },
// Other values are not important
};

const scheduleTarget = new EcsTaskTarget(commonEcsTaskTargetProps);
new Rule(this, "PgDumpScheduleRule", {
targets: [scheduleTarget],
schedule: Schedule.cron({
minute: "0",
hour: "23",
}),
});
}
}
```

If you synth this code, you'll see that `ecs:RunTask` specifies the Task Definition _exactly_ as imported. This assumes that it has a revision attached:

```yaml
PgDumpTaskDefImportEventsRoleDefaultPolicy3690671F:
Type: AWS::IAM::Policy
Properties:
PolicyDocument:
Statement:
- Action: ecs:RunTask
Condition:
ArnEquals:
ecs:cluster:
Fn::GetAtt:
- EcsCluster97242B84
- Arn
Effect: Allow
Resource:
Fn::ImportValue: PgDump-16-TaskDefArn
- Action: ecs:TagResource
Effect: Allow
Resource:
Fn::Join:
- ""
- - "arn:"
- Ref: AWS::Partition
- ":ecs:"
- Ref: AWS::Region
- :*:task/
- Ref: EcsCluster97242B84
- /*
- Action: iam:PassRole
Effect: Allow
Resource:
- Fn::ImportValue: PgDump-16-TaskDefExecutionRoleArn
- Fn::ImportValue: PgDump-16-TaskDefRoleArn
Version: "2012-10-17"
PolicyName: PgDumpTaskDefImportEventsRoleDefaultPolicy3690671F
Roles:
- Ref: PgDumpTaskDefImportEventsRoleF7E90D4B
Metadata:
aws:cdk:path: ExampleStack/PgDumpTaskDefImport/EventsRole/DefaultPolicy/Resource
```

Specifically, this action:

```
- Action: ecs:RunTask
Condition:
ArnEquals:
ecs:cluster:
Fn::GetAtt:
- EcsCluster97242B84
- Arn
Effect: Allow
Resource:
Fn::ImportValue: PgDump-16-TaskDefArn
```

Instead, I want to grant access to the imported value + `:*` (see https://github.com/aws/aws-cdk/issues/30390 for why). If I could specify that in my `FargateTaskDefinition.fromFargateTaskDefinitionAttributes` call, then the downstream logic (implemented here: https://github.com/aws/aws-cdk/pull/31615) could check that.

### Proposed Solution

I suggest adding a field on `IFargateTaskDefinition` like `arnIncludesRevision` or similar. Then, when we need to know about this (e.g., in https://github.com/aws/aws-cdk/pull/31615), we can use that value instead of checking the string, like @samson-keung added in that PR.

### Other Information

For a workaround to the sample code posted above, you can do something like this:

```ts
const role = (scheduleTarget as any).role as Role;
const statements = (role as any).defaultPolicy.document.statements as PolicyStatement[];
const withoutBadStatements = statements.filter((s) => {
const isBadStatement = s.actions.length === 1 && s.actions[0] === "ecs:RunTask";
return !isBadStatement;
});
(role as any).defaultPolicy.document.statements = withoutBadStatements;

role.addToPrincipalPolicy(
new PolicyStatement({
actions: ["ecs:RunTask"],
resources: [pgDumpTaskDefArn + ":*"], // this is the fix for the bug
conditions: {
ArnEquals: {
"ecs:cluster": ecsCluster.clusterArn,
},
},
})
);
```

This forces the `:*` on the imported task def, as you can see in this synthed template:

```yaml
- Action: ecs:RunTask
Condition:
ArnEquals:
ecs:cluster:
Fn::GetAtt:
- EcsCluster97242B84
- Arn
Effect: Allow
Resource:
Fn::Join:
- ""
- - Fn::ImportValue: PgDump-16-TaskDefArn
- :*
```

### Acknowledgements

- [ ] I may be able to implement this feature request
- [ ] This feature might incur a breaking change

### CDK version used

2.172.0

### Environment details (OS name and version, etc.)

macOS

Contributor guide

Open the contributing guide

Research direction

Start with IFargateTaskDefinition and FargateTaskDefinition.fromFargateTaskDefinitionAttributes, then trace the downstream IAM logic described in pull request #31615. Compare synthesized policies for imported task definition ARNs with and without revisions; done means an explicitly unrevisioned import produces a RunTask resource using the imported ARN plus :*, while a revision-qualified import remains exact.

Written by the indexing model from the issue text.

Assessment

Tech stack
aws, typescript
Domain
cloud, infrastructure
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.