kms.Key: kms.Key.isLookupDummy incorrectly throws error when no-pre-existing-key / dummy-key is found

[neoakris]

Describe the bug

@go-to-k I just not around to testing the feature you added in #31574, and discovered:
There was a problem in logic of original feature request (which has now been auto-closed.)

The intent of the new feature was to allow:

1. check for existence of pre-existing kms key with given alias
   ^--kms.Key.isLookupDummy() method was intended to do this, before the feature keyid would return a token which didn't allow an if statement check to occur, the new method returns true when key not found.
   (The bug is that it needlessly throws an error, which stops execution / doesn't make sense for an existence check. This is a relatively new feature and I suspect it never worked.)
2. if (pre-existing kms with alias exists) use it
3. if (pre-existing kms with alias doesn't exist) create it then use it.

Regression Issue

• Select this option if this issue appears to be a regression.

Last Known Working CDK Version

Not Applicable

Expected Behavior

Note: the value of this.config.kmsKeyAlias = alias/eks/dev1-eks (it'll show in error message)

if (kms.Key.isLookupDummy(kms.Key.fromLookup(this.stack, "pre-existing-kms-key", { aliasName: this.config.kmsKeyAlias, returnDummyKeyOnMissing: true,  }))){
  console.log("pre-existing kms not found, will create, then use");
  eksBlueprint.resourceProvider(blueprints.GlobalResources.KmsKey, new blueprints.CreateKmsKeyProvider(
    this.config.kmsKeyAlias, {description: "Easy EKS generated kms key, used to encrypt etcd and ebs-csi-driver provisioned volumes"}
  ));
}
else {
  console.log("pre-existing kms key found, will use");
  eksBlueprint.resourceProvider(blueprints.GlobalResources.KmsKey, new blueprints.LookupKmsKeyProvider(this.config.kmsKeyAlias));
}

^--I expect this logic (described in laymans terms above) to work.

Current Behavior

When I use cdk deploy dev1-eks
to run the above logic snippet

It throws an error that stops everything, error shouldn't be thrown for an existance check.

pre-existing kms not found, will create, then use
pre-existing kms not found, will create, then use
pre-existing kms not found, will create, then use
pre-existing kms not found, will create, then use
[Error at /dev1-eks] Could not find any key with alias named alias/eks/dev1-eks
Found errors

Reproduction Steps

I'm going to ask if I can make some code public, which will make reproduction easier. (With luck that could happen around Jan/Feb)

Possible Solution

Make the method not throw error when the isLookupDummy() is used

Additional Information/Context

Below is the workaround I'm currently using today:
I'd like to replace it with a upstream cdk fix to feature of checking for existing kms, because the workaround has 3 negligible, yet undesirable side effects:

• ensure's existance of aliased kms key prior to cdk's y/n prompt, so a key could be created even if you just want to dry run or cancel
• jq and aws become dependencies
• negligibly low risk of command injection.

function ensure_existance_of_aliased_kms_key(kmsKeyAlias: string){
    let kms_key:kms.Key;   
    const cmd = `aws kms list-aliases | jq '.Aliases[] | select(.AliasName == "${kmsKeyAlias}") | .TargetKeyId'`
    const cmd_results = execSync(cmd).toString();
    if(cmd_results===""){ //if alias not found, then make a kms key with the alias
        const create_key_cmd = `aws kms create-key --description="Generated kms key, used to encrypt etcd and ebs-csi-driver provisioned volumes"`
        const results = JSON.parse( execSync(create_key_cmd).toString() );
        const key_id = results.KeyMetadata.KeyId;
        const add_alias_cmd = `aws kms create-alias --alias-name ${kmsKeyAlias} --target-key-id ${key_id}`;
        execSync(add_alias_cmd);
    }
}

...(main logic)...
ensure_existance_of_aliased_kms_key(this.config.kmsKeyAlias)
eksBlueprint.resourceProvider(blueprints.GlobalResources.KmsKey, new blueprints.LookupKmsKeyProvider(this.config.kmsKeyAlias));

CDK CLI Version

2.170.0

Framework Version

No response

Node.js Version

v20.18.1

OS

Mac 15.1.1

Language

TypeScript

Language Version

5.4.5

Other information

No response

[neoakris]

https://github.com/aws/aws-cdk/blob/v2.170.0/packages/aws-cdk-lib/aws-ssm/lib/parameter.ts#L587
^-- There's a variable named "ignoreErrorOnMissingContext"

so I think there's agreement in regards to my proposed solution to make the isLookupDummy() not throw an error, but the logic involved in that variable doesn't seem to prevent an error from being thrown.

[khushail]

Hi @neoakris , I tested the feature and here is my observation-

1. The key is returned when alias exists -

2. Dummy key is returned when it does not exist -

so as proposed in this #31676, dummy value is returned in my case.

As mentioned, the returned dummy key - arn:aws:kms:us-east-1:3*******:key/1234abcd-12ab-34cd-56ef-1234567890ab matches with the one mentioned in the merged PR -1234abcd-12ab-34cd-56ef-1234567890ab

is this what you are talking about ?

[neoakris]

Hi khushail, thank you for taking some time to look into and test this.

Summary:
There's a good chance this is a PEBCAK problem. If possible I'd like to wait until the next release of eks blueprints to confirm no integration issue exists (between cdk and eks-blueprints) before closing this, and that it's just because I messed up my packages.json

---

Additional Details:

I tried to create a short (relatively) self-contained example of the bug based on "aws-cdk": "2.170.0"
https://github.com/neoakris/aws-cdk_32368/blob/works_without_eks_blueprints/bin/cdk-main.ts
but I noticed it worked (since you tested pre-exist and non-exist, I tested both cases and verified it worked.)

This lead me to believe it's some kind of integration issue with eks-blueprints, so I tried to integrate eks-blueprints into the example to reproduce the error.

Hum...
per https://github.com/aws-quickstart/cdk-eks-blueprints/tree/blueprints-1.16.2?tab=readme-ov-file#getting-started-1
The current latest eks-blueprints is based on aws-cdk-lib@2.162.1 (b4 your fix came out), I wounder if that could be a problem. (regardless of any answers/responses) When they next release I'll re-test to see if it goes away.

When I first encountered the issue, I had managed to combine cdk-eks-blueprints 1.16.2, with a newer version of cdk 2.170.0, but things mostly seemed to work, but maybe this issue arose from me mixing cdk-eks-blueprint with a newer version of AWS CDK? instead of the version their project pins to.

I forget the specifics of how I managed that so I'll replace the package.json with the one from the eks-blueprints project I was working on when I 1st found the issue to see if I can reproduce.

setting my package.json to be like the other, ended up with npm peer dependencies error.

I'm now woundering how this managed to work on my other setup, after clearing node_modules and trying to install fresh I noticed breakage there too. so this might be that I did something to my package.json file and it's getting stuck trying to use the old and new code at the same time, since the latest eks-blueprints uses an older version of cdk that predates your fix, and I somehow managed to do some tinkering to get it to temporarily partially work on my machine possibly in a wya with broken peer dependencies without realizing it.

That probably means when I originally found the error it may have been due to a faulty local dev environment, that somehow allowed me to mix old and new version. So my bug report is likely invalid. Sorry for the noise and the trouble I've caused.

[khushail]

No problem @neoakris. Happy to help!

[github-actions]

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.