Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(stepfunctions-tasks): stateMachine construct doesn't generate a valid policy for default StateMachineRole #31801

Merged
merged 5 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as ecs from 'aws-cdk-lib/aws-ecs';
import * as sfn from 'aws-cdk-lib/aws-stepfunctions';
import * as cdk from 'aws-cdk-lib';
import * as tasks from 'aws-cdk-lib/aws-stepfunctions-tasks';
import { EC2_RESTRICT_DEFAULT_SECURITY_GROUP } from 'aws-cdk-lib/cx-api';
import { EC2_RESTRICT_DEFAULT_SECURITY_GROUP, STEPFUNCTIONS_TASKS_FIX_RUN_ECS_TASK_POLICY } from 'aws-cdk-lib/cx-api';
import { IntegTest } from '@aws-cdk/integ-tests-alpha';

/*
Expand All @@ -20,6 +20,7 @@ import { IntegTest } from '@aws-cdk/integ-tests-alpha';
const app = new cdk.App();
const stack = new cdk.Stack(app, 'aws-sfn-tasks-ecs-run-task');
stack.node.setContext(EC2_RESTRICT_DEFAULT_SECURITY_GROUP, false);
stack.node.setContext(STEPFUNCTIONS_TASKS_FIX_RUN_ECS_TASK_POLICY, false);

const cluster = new ecs.Cluster(stack, 'Ec2Cluster');
cluster.addCapacity('DefaultAutoScalingGroup', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as ecs from 'aws-cdk-lib/aws-ecs';
import * as sfn from 'aws-cdk-lib/aws-stepfunctions';
import * as cdk from 'aws-cdk-lib';
import * as tasks from 'aws-cdk-lib/aws-stepfunctions-tasks';
import { EC2_RESTRICT_DEFAULT_SECURITY_GROUP } from 'aws-cdk-lib/cx-api';
import { EC2_RESTRICT_DEFAULT_SECURITY_GROUP, STEPFUNCTIONS_TASKS_FIX_RUN_ECS_TASK_POLICY } from 'aws-cdk-lib/cx-api';
import { IntegTest } from '@aws-cdk/integ-tests-alpha';

/*
Expand All @@ -19,6 +19,7 @@ import { IntegTest } from '@aws-cdk/integ-tests-alpha';
const app = new cdk.App();
const stack = new cdk.Stack(app, 'aws-sfn-tasks-ecs-fargate-run-task');
stack.node.setContext(EC2_RESTRICT_DEFAULT_SECURITY_GROUP, false);
stack.node.setContext(STEPFUNCTIONS_TASKS_FIX_RUN_ECS_TASK_POLICY, false);

const cluster = new ecs.Cluster(stack, 'FargateCluster');

Expand Down
16 changes: 15 additions & 1 deletion packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/ecs/run-task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import * as ecs from '../../../aws-ecs';
import * as iam from '../../../aws-iam';
import * as sfn from '../../../aws-stepfunctions';
import * as cdk from '../../../core';
import { STEPFUNCTIONS_TASKS_FIX_RUN_ECS_TASK_POLICY } from '../../../cx-api';
import { integrationResourceArn, validatePatternSupported } from '../private/task-utils';

/**
Expand Down Expand Up @@ -368,7 +369,7 @@ export class EcsRunTask extends sfn.TaskStateBase implements ec2.IConnectable {
const policyStatements = [
new iam.PolicyStatement({
actions: ['ecs:RunTask'],
resources: [`${this.getTaskDefinitionFamilyArn()}:*`],
resources: [cdk.FeatureFlags.of(this).isEnabled(STEPFUNCTIONS_TASKS_FIX_RUN_ECS_TASK_POLICY) ? this.getTaskDefinitionArn() : this.getTaskDefinitionFamilyArn() + ':*'],
}),
new iam.PolicyStatement({
actions: ['ecs:StopTask', 'ecs:DescribeTasks'],
Expand Down Expand Up @@ -398,6 +399,19 @@ export class EcsRunTask extends sfn.TaskStateBase implements ec2.IConnectable {
return policyStatements;
}

private getTaskDefinitionArn(): string {
const tastDefinitionArn = this.props.taskDefinition.taskDefinitionArn;
let needsRevisionWildcard = false;
// Check if there is a taskdefinition revision
// (arn will end with : followed by digits) included in the arn already
if (!cdk.Token.isUnresolved(tastDefinitionArn)) {
xazhao marked this conversation as resolved.
Show resolved Hide resolved
const revisionAtEndPattern = /:[0-9]+$/;
const hasRevision = revisionAtEndPattern.test(tastDefinitionArn);
needsRevisionWildcard = !hasRevision;
}
return tastDefinitionArn + needsRevisionWildcard ? ':*' : '';
}

/**
* Returns the ARN of the task definition family by removing the
* revision from the task definition ARN
Expand Down
20 changes: 19 additions & 1 deletion packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ Flags come in three types:
| [@aws-cdk/aws-lambda-nodejs:sdkV3ExcludeSmithyPackages](#aws-cdkaws-lambda-nodejssdkv3excludesmithypackages) | When enabled, both `@aws-sdk` and `@smithy` packages will be excluded from the Lambda Node.js 18.x runtime to prevent version mismatches in bundled applications. | 2.161.0 | (fix) |
| [@aws-cdk/aws-rds:setCorrectValueForDatabaseInstanceReadReplicaInstanceResourceId](#aws-cdkaws-rdssetcorrectvaluefordatabaseinstancereadreplicainstanceresourceid) | When enabled, the value of property `instanceResourceId` in construct `DatabaseInstanceReadReplica` will be set to the correct value which is `DbiResourceId` instead of currently `DbInstanceArn` | 2.161.0 | (fix) |
| [@aws-cdk/core:cfnIncludeRejectComplexResourceUpdateCreatePolicyIntrinsics](#aws-cdkcorecfnincluderejectcomplexresourceupdatecreatepolicyintrinsics) | When enabled, CFN templates added with `cfn-include` will error if the template contains Resource Update or Create policies with CFN Intrinsics that include non-primitive values. | 2.161.0 | (fix) |
| [@aws-cdk/aws-stepfunctions-tasks:fixRunEcsTaskPolicy](#aws-cdkaws-stepfunctions-tasksfixrunecstaskpolicy) | When enabled, the resource of IAM Run Ecs policy generated by SFN EcsRunTask will reference the definition, instead of constructing ARN. | V2NEXT | (fix) |

<!-- END table -->

Expand Down Expand Up @@ -146,7 +147,8 @@ The following json shows the current recommended set of flags, as `cdk init` wou
"@aws-cdk/aws-appsync:appSyncGraphQLAPIScopeLambdaPermission": true,
"@aws-cdk/aws-rds:setCorrectValueForDatabaseInstanceReadReplicaInstanceResourceId": true,
"@aws-cdk/core:cfnIncludeRejectComplexResourceUpdateCreatePolicyIntrinsics": true,
"@aws-cdk/aws-lambda-nodejs:sdkV3ExcludeSmithyPackages": true
"@aws-cdk/aws-lambda-nodejs:sdkV3ExcludeSmithyPackages": true,
"@aws-cdk/aws-stepfunctions-tasks:fixRunEcsTaskPolicy": true
}
}
```
Expand Down Expand Up @@ -1491,4 +1493,20 @@ Enabling this feature flag will make `cfn-include` throw on these templates, unl
| 2.161.0 | `false` | `true` |


### @aws-cdk/aws-stepfunctions-tasks:fixRunEcsTaskPolicy

*When enabled, the resource of IAM Run Ecs policy generated by SFN EcsRunTask will reference the definition, instead of constructing ARN.* (fix)

Currently, in the IAM Run Ecs policy generated by SFN EcsRunTask(), CDK will construct the ARN with wildcard attached at the end.
The revision number at the end will be replaced with a wildcard which it shouldn't.

When this feature flag is enabled, if the task definition is created in the stack, the 'Resource' section will 'Ref' the taskDefinition.


| Since | Default | Recommended |
| ----- | ----- | ----- |
| (not in v1) | | |
| V2NEXT | `false` | `true` |


<!-- END details -->
15 changes: 15 additions & 0 deletions packages/aws-cdk-lib/cx-api/lib/features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ export const APPSYNC_GRAPHQLAPI_SCOPE_LAMBDA_FUNCTION_PERMISSION = '@aws-cdk/aws
export const USE_CORRECT_VALUE_FOR_INSTANCE_RESOURCE_ID_PROPERTY = '@aws-cdk/aws-rds:setCorrectValueForDatabaseInstanceReadReplicaInstanceResourceId';
export const CFN_INCLUDE_REJECT_COMPLEX_RESOURCE_UPDATE_CREATE_POLICY_INTRINSICS = '@aws-cdk/core:cfnIncludeRejectComplexResourceUpdateCreatePolicyIntrinsics';
export const LAMBDA_NODEJS_SDK_V3_EXCLUDE_SMITHY_PACKAGES = '@aws-cdk/aws-lambda-nodejs:sdkV3ExcludeSmithyPackages';
export const STEPFUNCTIONS_TASKS_FIX_RUN_ECS_TASK_POLICY = '@aws-cdk/aws-stepfunctions-tasks:fixRunEcsTaskPolicy';

export const FLAGS: Record<string, FlagInfo> = {
//////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -1219,6 +1220,20 @@ export const FLAGS: Record<string, FlagInfo> = {
introducedIn: { v2: '2.161.0' },
recommendedValue: true,
},

//////////////////////////////////////////////////////////////////////
[STEPFUNCTIONS_TASKS_FIX_RUN_ECS_TASK_POLICY]: {
type: FlagType.BugFix,
summary: 'When enabled, the resource of IAM Run Ecs policy generated by SFN EcsRunTask will reference the definition, instead of constructing ARN.',
detailsMd: `
Currently, in the IAM Run Ecs policy generated by SFN EcsRunTask(), CDK will construct the ARN with wildcard attached at the end.
The revision number at the end will be replaced with a wildcard which it shouldn't.

When this feature flag is enabled, if the task definition is created in the stack, the 'Resource' section will 'Ref' the taskDefinition.
`,
introducedIn: { v2: 'V2NEXT' },
recommendedValue: true,
},
};

const CURRENT_MV = 'v2';
Expand Down
Loading