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): --hotswap does not detect function env variables changes #16444

Closed
misterjoshua opened this issue Sep 9, 2021 · 7 comments · Fixed by #16591
Closed

(cli): --hotswap does not detect function env variables changes #16444

misterjoshua opened this issue Sep 9, 2021 · 7 comments · Fixed by #16591
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort in-progress This issue is being actively worked on. package/tools Related to AWS CDK Tools or CLI

Comments

@misterjoshua
Copy link
Contributor

misterjoshua commented Sep 9, 2021

The new hot-swap feature doesn't detect function prop changes like environment variables.

Reproduction Steps

app.ts

export class BugStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props: cdk.StackProps = {}) {
    super(scope, id, props);

    const dateString = new Date().toISOString();

    new lambda.Function(this, 'Function', {
      runtime: lambda.Runtime.NODEJS_14_X,
      code: lambda.Code.fromAsset(__dirname),
      handler: 'bug-handler.handler',
      environment: {
        ENV_VARS_LAST_UPDATE: dateString,
      },
    });
  }
}

const app = new cdk.App();
new BugStack(app, 'BugStack');

bug-handler.js

exports.handler = async () => process.env.ENV_VARS_LAST_UPDATE || 'unset';

What did you expect to happen?

I expected cdk deploy BugStack --hotswap to deploy the stack every time I ran it.

What actually happened?

It said (no changes) on the second deployment, failing to update the environment variable.

Environment

  • CDK CLI Version : 1.122.0
  • Framework Version: 1.122.0
  • Node.js Version: v14.15.4
  • OS : Windows
  • Language (Version): TypeScript (4.4.2)

Other


This is 🐛 Bug Report

@misterjoshua misterjoshua added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 9, 2021
@github-actions github-actions bot added the package/tools Related to AWS CDK Tools or CLI label Sep 9, 2021
@skinny85
Copy link
Contributor

Hey @misterjoshua,

thanks for opening the issue!

Can you describe your full flow? I was unable to reproduce the issue on my end.

Here's what I did:

  1. I deployed my Lambda stack without the environment property (full deployment, no hotswap). It succeeded.
  2. I changed the environment property to add an env variable, and ran cdk deploy --hotswap. Here's what I saw:
...
Could not perform a hotswap deployment, as the stack HackathonExampleStack contains non-Asset changes
Falling back to doing a full deployment
...

And the CloudFormation deployment went through, updating the env variable (I verified it was updated by looking at the function in the AWS Console).

Let me know what was your flow that led to the env variable not being updated - I'm very curious where is the discrepancy!

Thanks,
Adam

@skinny85 skinny85 assigned skinny85 and unassigned rix0rrr Sep 10, 2021
@skinny85 skinny85 added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Sep 10, 2021
@misterjoshua
Copy link
Contributor Author

misterjoshua commented Sep 10, 2021

Let me know what was your flow that led to the env variable not being updated - I'm very curious where is the discrepancy!

@skinny85 It's strange - I've tried this again in a fresh repository just now and my example above does not reproduce the problem either. However, I've made some adjustments in such a way that I can reproduce the problem here:

https://github.com/misterjoshua/cdk-hotswap-bug

To reproduce in this repository, install the dependencies and deploy the first time:

npm install
cdk deploy

Deploy it again with hotswap (no changes to the code are necessary):

cdk deploy --hotswap

Here's what I get:

PS E:\p\cdk-hotswap-bug> cdk deploy --hotswap
⚠️ The --hotswap flag deliberately introduces CloudFormation drift to speed up deployments
⚠️ It should only be used for development - never use it for your production Stacks!
BugStack: deploying...
[0%] start: Publishing 440a8aadc96e32d24c62a5ecdb3a12ea2e494161bb49760bc64a67f7b41a8424:current
[100%] success: Published 440a8aadc96e32d24c62a5ecdb3a12ea2e494161bb49760bc64a67f7b41a8424:current

 ✅  BugStack (no changes)

Stack ARN:
arn:aws:cloudformation:ca-central-1:111111111111:stack/BugStack/807f7e90-11d8-11ec-abba-06d051b5b7e2

But, I expected that ENV_VARS_LAST_UPDATE should have changed, causing a non-asset change. (It changes every second.)

image

@skinny85
Copy link
Contributor

Thanks @misterjoshua!

If I could ask for one more thing 🙂. Before running cdk deploy --hotswap, can you run cdk diff, and show me the output of that command?

@misterjoshua
Copy link
Contributor Author

@skinny85 Here you go:

PS E:\p\cdk-hotswap-bug> cdk deploy --require-approval never
BugStack: deploying...
[0%] start: Publishing 440a8aadc96e32d24c62a5ecdb3a12ea2e494161bb49760bc64a67f7b41a8424:current
[100%] success: Published 440a8aadc96e32d24c62a5ecdb3a12ea2e494161bb49760bc64a67f7b41a8424:current
BugStack: creating CloudFormation changeset...
  0/4 |12:21:41 p.m. | REVIEW_IN_PROGRESS   | AWS::CloudFormation::Stack | BugStack User Initiated
  0/4 |12:21:46 p.m. | CREATE_IN_PROGRESS   | AWS::CloudFormation::Stack | BugStack User Initiated
  0/4 |12:22:19 p.m. | CREATE_IN_PROGRESS   | AWS::CDK::Metadata    | CDKMetadata/Default (CDKMetadata)
  0/4 |12:22:19 p.m. | CREATE_IN_PROGRESS   | AWS::IAM::Role        | Function/ServiceRole (FunctionServiceRole675BB04A)
  0/4 |12:22:19 p.m. | CREATE_IN_PROGRESS   | AWS::IAM::Role        | Function/ServiceRole (FunctionServiceRole675BB04A) Resource creation Initiated     
  0/4 |12:22:21 p.m. | CREATE_IN_PROGRESS   | AWS::CDK::Metadata    | CDKMetadata/Default (CDKMetadata) Resource creation Initiated
  1/4 |12:22:21 p.m. | CREATE_COMPLETE      | AWS::CDK::Metadata    | CDKMetadata/Default (CDKMetadata)
  2/4 |12:22:33 p.m. | CREATE_COMPLETE      | AWS::IAM::Role        | Function/ServiceRole (FunctionServiceRole675BB04A)
  2/4 |12:22:35 p.m. | CREATE_IN_PROGRESS   | AWS::Lambda::Function | Function (Function76856677)
  2/4 |12:22:38 p.m. | CREATE_IN_PROGRESS   | AWS::Lambda::Function | Function (Function76856677) Resource creation Initiated
  3/4 |12:22:39 p.m. | CREATE_COMPLETE      | AWS::Lambda::Function | Function (Function76856677)
  4/4 |12:22:40 p.m. | CREATE_COMPLETE      | AWS::CloudFormation::Stack | BugStack

 ✅  BugStack

Stack ARN:
arn:aws:cloudformation:ca-central-1:111111111111111:stack/BugStack/f1e483b0-1263-11ec-98b9-06f81ab62864
PS E:\p\cdk-hotswap-bug> cdk diff
Stack BugStack
There were no differences
PS E:\p\cdk-hotswap-bug> cdk deploy --hotswap
⚠️ The --hotswap flag deliberately introduces CloudFormation drift to speed up deployments
⚠️ It should only be used for development - never use it for your production Stacks!
BugStack: deploying...
[0%] start: Publishing 440a8aadc96e32d24c62a5ecdb3a12ea2e494161bb49760bc64a67f7b41a8424:current
[100%] success: Published 440a8aadc96e32d24c62a5ecdb3a12ea2e494161bb49760bc64a67f7b41a8424:current

 ✅  BugStack (no changes)

Stack ARN:
arn:aws:cloudformation:ca-central-1:111111111111111:stack/BugStack/f1e483b0-1263-11ec-98b9-06f81ab62864

So it doesn't look like cdk diff catches the changing environment variable. Perhaps that's why --hotswap isn't picking up on the change. But interestingly, when I cdk deploy without hot-swapping, CloudFormation picks up on this change:

PS E:\p\cdk-hotswap-bug> cdk deploy --hotswap
⚠️ The --hotswap flag deliberately introduces CloudFormation drift to speed up deployments
⚠️ It should only be used for development - never use it for your production Stacks!
BugStack: deploying...
[0%] start: Publishing 440a8aadc96e32d24c62a5ecdb3a12ea2e494161bb49760bc64a67f7b41a8424:current
[100%] success: Published 440a8aadc96e32d24c62a5ecdb3a12ea2e494161bb49760bc64a67f7b41a8424:current

 ✅  BugStack (no changes)

Stack ARN:                                                                                                                                               
arn:aws:cloudformation:ca-central-1:111111111111111:stack/BugStack/f1e483b0-1263-11ec-98b9-06f81ab62864
PS E:\p\cdk-hotswap-bug> cdk deploy
BugStack: deploying...
[0%] start: Publishing 440a8aadc96e32d24c62a5ecdb3a12ea2e494161bb49760bc64a67f7b41a8424:current
[100%] success: Published 440a8aadc96e32d24c62a5ecdb3a12ea2e494161bb49760bc64a67f7b41a8424:current
BugStack: creating CloudFormation changeset...
  0/2 |12:29:35 p.m. | UPDATE_IN_PROGRESS   | AWS::CloudFormation::Stack | BugStack User Initiated
  0/2 |12:30:01 p.m. | UPDATE_IN_PROGRESS   | AWS::Lambda::Function | Function (Function76856677) 
  1/2 |12:30:04 p.m. | UPDATE_COMPLETE      | AWS::Lambda::Function | Function (Function76856677) 
  2/2 |12:30:06 p.m. | UPDATE_COMPLETE_CLEA | AWS::CloudFormation::Stack | BugStack
  3/2 |12:30:06 p.m. | UPDATE_COMPLETE      | AWS::CloudFormation::Stack | BugStack 

 ✅  BugStack

Stack ARN:
arn:aws:cloudformation:ca-central-1:111111111111111:stack/BugStack/f1e483b0-1263-11ec-98b9-06f81ab62864

Shouldn't the diff be catching this? Given that environment variables can change the behaviour of the lambda, I would expect that that would be a meaningful diff from the customer's perspective.

@skinny85
Copy link
Contributor

Yeah, this definitely looks like a bug in the diff command (which hotswap uses, like you correctly guessed).

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Sep 21, 2021
skinny85 added a commit to skinny85/aws-cdk that referenced this issue Sep 22, 2021
Turns out, `parseFloat()` in JavaScript is even crazier than we thought,
and returns nonsense like `2021` for a string containing a Date like `'2021-10-25'`.
For that reason, add an explicit check that the string parsed looks like a number before calling `parseFloat()`.

Fixes aws#16444
@skinny85 skinny85 added the in-progress This issue is being actively worked on. label Sep 22, 2021
skinny85 added a commit to skinny85/aws-cdk that referenced this issue Oct 6, 2021
Turns out, `parseFloat()` in JavaScript is even crazier than we thought,
and returns nonsense like `2021` for a string containing a Date like `'2021-10-25'`.
For that reason, add an explicit check that the string parsed looks like a number before calling `parseFloat()`.

Fixes aws#16444
@peterwoodworth peterwoodworth removed the needs-triage This issue or PR still needs to be triaged. label Oct 12, 2021
@skinny85 skinny85 changed the title (cli): --hotswap does not detect function prop changes (cli): --hotswap does not detect function env variables changes Oct 12, 2021
@skinny85 skinny85 added the effort/small Small work item – less than a day of effort label Oct 12, 2021
@mergify mergify bot closed this as completed in #16591 Oct 19, 2021
mergify bot pushed a commit that referenced this issue Oct 19, 2021
Turns out, `parseFloat()` in JavaScript is even crazier than we thought,
and returns nonsense like `2021` for a string containing a Date like `'2021-10-25'`.
For that reason, add an explicit check that the string parsed looks like a number before calling `parseFloat()`.

Fixes #16444

----

*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.

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
Turns out, `parseFloat()` in JavaScript is even crazier than we thought,
and returns nonsense like `2021` for a string containing a Date like `'2021-10-25'`.
For that reason, add an explicit check that the string parsed looks like a number before calling `parseFloat()`.

Fixes aws#16444

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@AllanOricil
Copy link

@skinny85 #30101

My issue is similar, but I'm not using this hotswap flag. Can you help me.

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. effort/small Small work item – less than a day of effort in-progress This issue is being actively worked on. package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants