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

CDK Deploy: Deploy with hotswap now failing #26871

Closed
jackkitley opened this issue Aug 24, 2023 · 4 comments · Fixed by #26876
Closed

CDK Deploy: Deploy with hotswap now failing #26871

jackkitley opened this issue Aug 24, 2023 · 4 comments · Fixed by #26876
Labels
@aws-cdk/aws-logs Related to Amazon CloudWatch Logs bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@jackkitley
Copy link

jackkitley commented Aug 24, 2023

Describe the bug

When using the following command:

cdk deploy dealeros-uat-web --hotswap -c config=uat --require-approval=never

--hotswap is the culprit here. Without it, deploy can happen.

It was working fine this morning and nothing has changed with the way we deploy.

The following error when rolling out to ECS:

failed: ClientException: Log driver awslogs requires options: awslogs-region, awslogs-group
    at Request.extractError (/usr/local/lib/node_modules/aws-cdk/lib/index.js:338:33170)
    at Request.callListeners (/usr/local/lib/node_modules/aws-cdk/lib/index.js:338:89985)
    at Request.emit (/usr/local/lib/node_modules/aws-cdk/lib/index.js:338:89433)
    at Request.emit (/usr/local/lib/node_modules/aws-cdk/lib/index.js:338:196191)
    at Request.transition (/usr/local/lib/node_modules/aws-cdk/lib/index.js:338:189743)
    at AcceptorStateMachine.runTo (/usr/local/lib/node_modules/aws-cdk/lib/index.js:338:154615)
    at /usr/local/lib/node_modules/aws-cdk/lib/index.js:338:154945
    at Request.<anonymous> (/usr/local/lib/node_modules/aws-cdk/lib/index.js:338:190035)
    at Request.<anonymous> (/usr/local/lib/node_modules/aws-cdk/lib/index.js:338:196266)
    at Request.callListeners (/usr/local/lib/node_modules/aws-cdk/lib/index.js:338:90153) {
  code: 'ClientException',
  '[__type]': 'See error.__type for details.',
  time: 2023-08-24T16:44:22.046Z,
  requestId: 'fb2f5f67-5f28-4a0e-8f36-8ecd89cf01dd',
  statusCode: 400,
  retryable: false,
  retryDelay: 39.6604227853812

Expected Behavior

--hotswap is acting up here.

I expect the code to deploy to ECS without issues.

Current Behavior

Failing the build process with --hotswap in the cdk deploy.

giving error:

failed: ClientException: Log driver awslogs requires options: awslogs-region, awslogs-group
    at Request.extractError (/usr/local/lib/node_modules/aws-cdk/lib/index.js:338:33170)
    at Request.callListeners (/usr/local/lib/node_modules/aws-cdk/lib/index.js:338:89985)
    at Request.emit (/usr/local/lib/node_modules/aws-cdk/lib/index.js:338:89433)
    at Request.emit (/usr/local/lib/node_modules/aws-cdk/lib/index.js:338:196191)
    at Request.transition (/usr/local/lib/node_modules/aws-cdk/lib/index.js:338:189743)
    at AcceptorStateMachine.runTo (/usr/local/lib/node_modules/aws-cdk/lib/index.js:338:154615)
    at /usr/local/lib/node_modules/aws-cdk/lib/index.js:338:154945
    at Request.<anonymous> (/usr/local/lib/node_modules/aws-cdk/lib/index.js:338:190035)
    at Request.<anonymous> (/usr/local/lib/node_modules/aws-cdk/lib/index.js:338:196266)
    at Request.callListeners (/usr/local/lib/node_modules/aws-cdk/lib/index.js:338:90153) {
  code: 'ClientException',
  '[__type]': 'See error.__type for details.',
  time: 2023-08-24T16:44:22.046Z,
  requestId: 'fb2f5f67-5f28-4a0e-8f36-8ecd89cf01dd',
  statusCode: 400,
  retryable: false,
  retryDelay: 39.6604227853812
}

Reproduction Steps

CDK version:
2.93.0 (build 724bd01)

Run this for a deploy:

cdk deploy dealeros-uat-web -c config=uat --require-approval=never

Possible Solution

This was a recent change. im not sure.

Additional Information/Context

No response

CDK CLI Version

2.93.0 (build 724bd01)

Framework Version

No response

Node.js Version

16

OS

ubuntu

Language

Typescript

Language Version

No response

Other information

No response

@jackkitley jackkitley added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 24, 2023
@github-actions github-actions bot added the @aws-cdk/aws-logs Related to Amazon CloudWatch Logs label Aug 24, 2023
@jackkitley jackkitley changed the title CDK Deplot: Hotswap for uat failing CDK Deploy: Deploy with hotswap now failing Aug 24, 2023
@tmokmss
Copy link
Contributor

tmokmss commented Aug 24, 2023

This should be related with #26404. I'm looking at it 👀

@peterwoodworth
Copy link
Contributor

Thanks for being on top of it @tmokmss

@peterwoodworth peterwoodworth added p1 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Aug 24, 2023
@tmokmss
Copy link
Contributor

tmokmss commented Aug 24, 2023

@jackkitley I just submitted a pr to fix it, sorry for inconvenience! In the mean time you can use cdk 2.92.0 🙏

@mergify mergify bot closed this as completed in #26876 Aug 25, 2023
mergify bot pushed a commit that referenced this issue Aug 25, 2023
## Problem
ECS hotswap fails on a task definition containing log configuration like the following.

```ts
const taskDefinition = new ecs.FargateTaskDefinition(stack, 'Task', {});

taskDefinition.addContainer('EcsApp', {
  image: ecs.ContainerImage.fromRegistry('nginx:stable'),
  logging: ecs.LogDriver.awsLogs({ streamPrefix: 'log' }),
});
```

## Root cause
When we transform object keys in a task definition, we pass `excludeFromTransform` to avoid from transforming keys with arbitrary string, such as [`logDriver.options`](https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_LogConfiguration.html#ECS-Type-LogConfiguration-options).

However, it was not working when we transform keys with upperCaseFirstCharacter, because the keys in `excludeFromTransform` was uppercased, where the source object was lowercased.

```
// source task definition
{
  "logConfiguration": {
      "logDriver": "awslogs",
      "options": {
          "awslogs-group": "my-test-stack-TaskEcsAppLogGroupD5C9C1DD-BPB6zgX4S0wU",
          "awslogs-region": "ap-northeast-1",
      },
  },
}

// excludeFromTransform 
{ 
  LogConfiguration: {
      Options: true,
    },
  },
}

// where it should be
{ 
  logConfiguration: {
      options: true,
    },
  },
}
```

This misconfiguration resulted in the output task definition uppercased in an unexpected way:

```json
{
  "logConfiguration": {
      "logDriver": "awslogs",
      "options": {
          "Awslogs-group": "my-test-stack-TaskEcsAppLogGroupD5C9C1DD-BPB6zgX4S0wU",
          "Awslogs-region": "ap-northeast-1",
      },
  },
}

```

The problem was not detected by unit tests because they only contained cases with uppercase keys in a source task definition.

## Fix
Use lowercased `excludeFromTransform` when we use it with `upperCaseFirstCharacter`, also adding a test case with lowercased keys in a source task definition.


Closes #26871.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-logs Related to Amazon CloudWatch Logs bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants