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

cli: cdk migrate wrongly converts casing in ecs task definition props #29027

Closed
tmokmss opened this issue Feb 8, 2024 · 4 comments
Closed

cli: cdk migrate wrongly converts casing in ecs task definition props #29027

tmokmss opened this issue Feb 8, 2024 · 4 comments
Assignees
Labels
bug This issue is a bug. p1 package/tools Related to AWS CDK Tools or CLI toolkit/migrate Related to cdk migrate

Comments

@tmokmss
Copy link
Contributor

tmokmss commented Feb 8, 2024

Describe the bug

cdk migrate fails in handling the casing of ECS task definition properties. See the reproduction steps below.

Expected Behavior

casing is kept from the original template.

          logConfiguration: {
            options: {
              awslogs-group: '/aws/ecs/test/main',
              awslogs-region: 'ap-northeast-1',
              awslogs-stream-prefix: 'ecs',
            },
            logDriver: 'awslogs',
          },

Current Behavior

they're converted to camel case.

          logConfiguration: {
            secretOptions: [
            ],
            options: {
              awslogsGroup: '/aws/ecs/test/main',
              awslogsRegion: 'ap-northeast-1',
              awslogsStreamPrefix: 'ecs',
            },
            logDriver: 'awslogs',
          },

It results in the error below when you try to deploy the template:

Resource handler returned message: "Invalid request provided: Create TaskDefinition: Log driver awslogs requires options: awslogs-region, awslogs-group (Service: AmazonECS; Status Code: 400
; Error Code: ClientException; ...

Reproduction Steps

Deploy this template from CFn management console:

AWSTemplateFormatVersion: '2010-09-09'

Resources:
  BackendECSTaskRole:
    Type: AWS::IAM::Role
    Properties:
      Path: /
      AssumeRolePolicyDocument:
        Statement:
          - Action: sts:AssumeRole
            Effect: Allow
            Principal:
              Service: ecs-tasks.amazonaws.com

  ECSTaskExecutionRole:
    Type: AWS::IAM::Role
    Properties:
      Path: /
      AssumeRolePolicyDocument:
        Statement:
          - Action: sts:AssumeRole
            Effect: Allow
            Principal:
              Service: ecs-tasks.amazonaws.com
      ManagedPolicyArns:
        - arn:aws:iam::aws:policy/service-role/AmazonECSTaskExecutionRolePolicy
        - arn:aws:iam::aws:policy/AmazonSSMReadOnlyAccess
        - arn:aws:iam::aws:policy/SecretsManagerReadWrite

  BackendServiceECSTaskDefinition:
    Type: AWS::ECS::TaskDefinition
    Properties:
      Family: test
      RequiresCompatibilities:
        - FARGATE
      Memory: "1024"
      Cpu: "256"
      NetworkMode: awsvpc
      ExecutionRoleArn: !GetAtt ECSTaskExecutionRole.Arn
      TaskRoleArn: !GetAtt BackendECSTaskRole.Arn
      ContainerDefinitions:
        - Name: "main"
          Image: "nginx"
          LogConfiguration:
            Options:
              awslogs-group: '/aws/ecs/test/main'
              awslogs-region: ap-northeast-1
              awslogs-stream-prefix: ecs
            LogDriver: awslogs

And then use cdk migrate:

npx cdk@latest migrate --from-stack --stack-name xxx

Possible Solution

No response

Additional Information/Context

There are some props in CFn that casing should remain unchanged. One of them is an object of string that takes any string as a key: example

CDK CLI Version

2.126.0

Framework Version

No response

Node.js Version

v20.10.0

OS

macOS

Language

TypeScript

Language Version

No response

Other information

No response

@tmokmss tmokmss added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 8, 2024
@github-actions github-actions bot added the package/tools Related to AWS CDK Tools or CLI label Feb 8, 2024
@tim-finnigan tim-finnigan self-assigned this Feb 8, 2024
@tim-finnigan tim-finnigan added investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels Feb 8, 2024
@tim-finnigan
Copy link

Thanks for reporting, I could reproduce the issue. It looks like the options need to remain in that format to deploy properly:

logConfiguration: {
  options: {
    'awslogs-group': '/aws/ecs/test/main',
    'awslogs-region': 'ap-northeast-1',
    'awslogs-stream-prefix': 'ecs',
  },
  logDriver: 'awslogs',
},

@tim-finnigan tim-finnigan added p1 and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Feb 8, 2024
@scanlonp scanlonp self-assigned this Feb 10, 2024
@TheRealAmazonKendra TheRealAmazonKendra added the toolkit/migrate Related to cdk migrate label Feb 21, 2024
@TheRealAmazonKendra
Copy link
Contributor

Duplicate-ish of #28997. Same issue in the code, different manifestation of the bug.

@TheRealAmazonKendra
Copy link
Contributor

This fix for this was released in v2.132.

Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@colifran colifran self-assigned this Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p1 package/tools Related to AWS CDK Tools or CLI toolkit/migrate Related to cdk migrate
Projects
None yet
Development

No branches or pull requests

5 participants