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): We don't support attributes of the 'AWS::KMS::Key' resource #25418

Closed
Assignees
Labels
@aws-cdk/aws-lambda Related to AWS Lambda bug This issue is a bug. effort/medium Medium work item – several days of effort hotswap p1 package/tools Related to AWS CDK Tools or CLI

Comments

@jmataway
Copy link

jmataway commented May 3, 2023

Describe the bug

--hotswap deploy is not working on a Lambda function that has a KMS Key ARN obtained from key.keyArn set as an environment for that function. The KMS key is created in a parent stack while the Lambda function is created in a nested stack. The Lambda function simply references the KMS key ARN in its environment variables and the KMS key has not been changed at all when attempting to do the hotswap deploy.

Expected Behavior

Hotswap deploy of the modified Lambda function source code, which was the only thing modified before attempting to do the cdk deploy --hotswap TempProjectStack command.

Current Behavior

Hotswap deploy fails and I get the following error:

Could not perform a hotswap deployment, because the CloudFormation template could not be resolved: We don't support attributes of the 'AWS::KMS::Key' resource. This is a CDK limitation. Please report it at https://github.com/aws/aws-cdk/issues/new/choose

Reproduction Steps

TypeScript files to reproduce on a new project

bin/cdk.ts

#!/usr/bin/env node
import 'source-map-support/register';
import * as cdk from 'aws-cdk-lib';
import { TempProjectStack } from '../lib/temp-project-stack';

const app = new cdk.App();
new TempProjectStack(app, 'TempProjectStack', {});

lib/temp-project-stack.ts

import {
  Stack,
  StackProps,
  NestedStack,
  NestedStackProps,
  RemovalPolicy,
} from 'aws-cdk-lib';
import { Construct } from 'constructs';
import {
  Alias,
  AssetCode,
  Function,
  FunctionProps,
  Runtime,
  Version,
} from 'aws-cdk-lib/aws-lambda';
import { Key, KeySpec, KeyUsage } from 'aws-cdk-lib/aws-kms';

// Properties for NestedStacks
interface LambdaStackProps extends NestedStackProps {
  codeDirectory: string;
  lambdaProps: Omit<FunctionProps, 'code'>;
}

// Example NestedStack used for Lambdas with some defaults like versioning and an alias
export class NestedLambdaStack extends NestedStack {
  public lambda: Function;

  public alias: Alias;

  public version: Version;

  constructor(
    scope: Construct,
    id: string,
    { lambdaProps, codeDirectory, ...props }: LambdaStackProps
  ) {
    super(scope, id, props);

    const lambdaCode = AssetCode.fromAsset(codeDirectory);

    this.lambda = new Function(this, `${id}-lambda`, {
      code: lambdaCode,
      ...lambdaProps,
    });

    this.version = this.lambda.currentVersion;

    this.alias = new Alias(this, `${id}-alias`, {
      aliasName: 'live',
      version: this.version,
    });
  }
}

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

    const key = new Key(this, 'key', {
      keySpec: KeySpec.RSA_2048,
      keyUsage: KeyUsage.SIGN_VERIFY,
      alias: 'jwt',
      removalPolicy: RemovalPolicy.DESTROY,
      enabled: true,
    });

    // Using the NestedStack to create the Lambda function with its defaults and a few set properties
    const lambdaStack = new NestedLambdaStack(this, 'example', {
      codeDirectory: 'src',
      lambdaProps: {
        environment: {
          KEY: key.keyArn, // This environment variable value being set via key.keyArn causes the error to appear
        },
        handler: 'index.handler',
        runtime: Runtime.NODEJS_18_X,
      },
    });
  }
}

src/index.js

exports.handler = function (event, context) {
  // Modify the number after hello world to trigger a simple source code change for the hotswap deploy
  context.succeed('hello world 1');
};
The above files have triggered the error for me on a fresh CDK project created using: `cdk init app --language typescript`

Possible Solution

Allow the KMS Key ARN string returned from key.keyArn to be used in an environment variable for a Lambda function. Using something like key.keyId did seem to work in some quick testing I did, but we would prefer to use the full ARN string returned from key.keyArn rather than have to build our own ARN or store the ARN in something like ParameterStore. We aren't trying to deploy modified KMS keys with the --hotswap deploy, just looking to update the Lambda source code that has the key ARN set as an environment variable.

Additional Information/Context

cdk diff of the project after I modify the Lambda function's source code before trying the cdk deploy --hotswap TempProjectStack command:

Stack TempProjectStack
Resources
[~] AWS::CloudFormation::Stack example.NestedStack/example.NestedStackResource exampleNestedStackexampleNestedStackResource5A5A6965 
 ├─ [~] NestedTemplate
 │   └─ [~] .Resources:
 │       ├─ [~] .examplealiasBA70626E:
 │       │   └─ [~] .Properties:
 │       │       └─ [~] .FunctionVersion:
 │       │           └─ [~] .Fn::GetAtt:
 │       │               └─ @@ -1,4 +1,4 @@
 │       │                  [ ] [
 │       │                  [-]   "examplelambdaCurrentVersion05CA5AA8470043849f73f8155b8852de9b0850f4",
 │       │                  [+]   "examplelambdaCurrentVersion05CA5AA88b74736583e6b20723c32744b83d2a79",
 │       │                  [ ]   "Version"
 │       │                  [ ] ]
 │       ├─ [~] .examplelambda0C208F3A:
 │       │   ├─ [~] .Metadata:
 │       │   │   └─ [~] .aws:asset:path:
 │       │   │       ├─ [-] asset.3d81e02fc44b68609772c5c05e0cb64b97f1ee44c43e4d528f15e21ff0065ca4
 │       │   │       └─ [+] asset.25b5a9e4aa62b199e107ab1387251fb18b45739653a1be7a05e2c466828f57c0
 │       │   └─ [~] .Properties:
 │       │       └─ [~] .Code:
 │       │           └─ [~] .S3Key:
 │       │               ├─ [-] 3d81e02fc44b68609772c5c05e0cb64b97f1ee44c43e4d528f15e21ff0065ca4.zip
 │       │               └─ [+] 25b5a9e4aa62b199e107ab1387251fb18b45739653a1be7a05e2c466828f57c0.zip
 │       ├─ [-] Removed: .examplelambdaCurrentVersion05CA5AA8470043849f73f8155b8852de9b0850f4
 │       └─ [+] Added: .examplelambdaCurrentVersion05CA5AA88b74736583e6b20723c32744b83d2a79
 └─ [~] TemplateURL
     └─ [~] .Fn::Join:
         └─ @@ -13,6 +13,6 @@
            [ ]     {
            [ ]       "Fn::Sub": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}"
            [ ]     },
            [-]     "/5436631a386bbbc256f5ec3d5665122848bbf643905cc81ad8c2762d2ca82b4a.json"
            [+]     "/558d9b9201959a9601fb929be493271d9edc35001bdbc7e20e747d93831c072c.json"
            [ ]   ]
            [ ] ]

CDK CLI Version

2.73.0

Framework Version

No response

Node.js Version

v18.15.0

OS

Ubuntu 20.04 LTS

Language

Typescript

Language Version

No response

Other information

No response

@jmataway jmataway added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels May 3, 2023
@github-actions github-actions bot added the @aws-cdk/aws-lambda Related to AWS Lambda label May 3, 2023
@jmataway jmataway changed the title (module name): (short issue description) cdk deploy --hotswap: We don't support attributes of the 'AWS::KMS::Key' resource. May 3, 2023
@peterwoodworth peterwoodworth added p1 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels May 3, 2023
@peterwoodworth
Copy link
Contributor

Thanks for reporting this and #25417, and for providing code that's easy to reproduce this with.

It looks to me like this is only occurring within a Nested Stack. Eliminating the nested stack will allow hotswapping to succeed. There's likely some bug related to how we're processing the changeset when nested stacks are involved

const nestedHotswappableResources = await findNestedHotswappableChanges(logicalId, change, nestedStackNames, evaluateCfnTemplate, sdk);

@peterwoodworth
Copy link
Contributor

Both of these errors are likely caused from cross stack references. For some reason these are getting picked up even though they're not changing. Maybe we haven't implemented the logic to resolve cfn parameter values when attempting to hotswap, but I'm not sure that would explain why the streamArn is getting caught when it's only used on the EventSourceMapping

├─ [-] Parameters
 │   └─ {"referencetoHotswapStackdynamodbtable407947B9StreamArn":{"Fn::GetAtt":["dynamodbtable650E77A7","StreamArn"]},"referencetoHotswapStackkeyB46E41B6Arn":{"Fn::GetAtt":["keyFEDD6EC0","Arn"]}}

@peterwoodworth peterwoodworth changed the title cdk deploy --hotswap: We don't support attributes of the 'AWS::KMS::Key' resource. (hotswap): Hotswap fails when Lambda in a nested stack uses cross stack CfnParameter references May 3, 2023
@peterwoodworth peterwoodworth added package/tools Related to AWS CDK Tools or CLI hotswap and removed needs-review labels Aug 30, 2023
mergify bot pushed a commit that referenced this issue Sep 28, 2023
…7195)

This PR solves two problems when doing hotswap deployments with nested stacks.

1. Adding capabilities to evaluate CFN parameters of `AWS::CloudFormation::Stack` type (i.e. a nested stack). In this PR, we are only resolving `Outputs` section of `AWS::CloudFormation::Stack` and it can be expanded to other fields in the future. See https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/quickref-cloudformation.html#w2ab1c17c23c19b5 for CFN documentation around using Outputs ref parameters
2. If a template has parameters with default values and they are not provided (a value) by the parent stack when invoking, then resolve these parameters using the default values in the template.

Partially helps #23533 and #25418

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@comcalvi comcalvi self-assigned this May 8, 2024
@comcalvi
Copy link
Contributor

comcalvi commented May 8, 2024

The reason why this works if there's no nested stacks involved is that the function's environment hasn't changed. If you remove the nested stacks and modify the environment like so:

    const lambdaFunc = new Function(this, 'example', {
      code: lambda.Code.fromAsset('src'),
      environment: {
        KEY: key.keyArn,
        FOO: 'bar', // new property
      },
      handler: 'index.handler',
      runtime: Runtime.NODEJS_18_X,
    });
  }

Then the EvaluateCfnTemplate will attempt to evaluate both KEY and FOO. It fails on KEY with the same error reported here for nested stacks.

@peterwoodworth is correct that the reason we're only seeing this for nested stacks is cross-stack references. Because it's a nested stack, the value of the KEY parameter is passed as a CFN Parameter to the nested stack. Nested stack parameters are always evaluated on hotswap. This is technically the same behavior that we have for top-level stacks, where parameters are also always evaluated. This results in different behavior for two cases that one would expect to be the same, since this is a cross-stack reference.

This is expected behavior, as seemingly unexpected as it is. The actual error is that the KMS::Key resource is not supported.

@comcalvi
Copy link
Contributor

comcalvi commented May 8, 2024

One could argue that nested stack parameters should not always be evaluated, since they may be used in non-hotswappable resources. This is fair, but it wouldn't fix this particular case; the Key is being passed to a lambda, which is hotswappable. The lambda function is using that environment variable, since that's the only reason to include the key arn in the environment, so we have to evaluate the parameters at least if they're used in a hotswappable resource.

@comcalvi comcalvi changed the title (hotswap): Hotswap fails when Lambda in a nested stack uses cross stack CfnParameter references (CLI): We don't support attributes of the 'AWS::KMS::Key' resource May 8, 2024
@mergify mergify bot closed this as completed in #30112 May 9, 2024
mergify bot pushed a commit that referenced this issue May 9, 2024
### Issue # (if applicable)

Closes #25418.

### Reason for this change

KMS Keys cannot be referenced in hotswappable resources. CDK complains that this is a limitation: 

```
Could not perform a hotswap deployment, because the CloudFormation template could not be resolved: We don't support attributes of the 'AWS::KMS::Key' resource. This is a CDK limitation. Please report it at https://github.com/aws/aws-cdk/issues/new/choose.
```
### Description of changes

Add KMS keys to the supported list of resource attributes for hotswapping. 

### Description of how you validated changes
Tests

----

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

github-actions bot commented May 9, 2024

⚠️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