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

(aws-cdk-lib/aws-eks): nodegroup instanceTypes does not accept tokens #20914

Closed
rshad opened this issue Jun 29, 2022 · 4 comments
Closed

(aws-cdk-lib/aws-eks): nodegroup instanceTypes does not accept tokens #20914

rshad opened this issue Jun 29, 2022 · 4 comments
Assignees
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort p1 package/tools Related to AWS CDK Tools or CLI

Comments

@rshad
Copy link

rshad commented Jun 29, 2022

Describe the bug

CDK Version: 2.20.0 (build 738ef49)

CfnParameter is not interpreted as expected in CDK synth or deploy

Expected Behavior

The parameter should be interpreted as string.

Current Behavior

<local-project-path>/cdk/node_modules/aws-cdk-lib/aws-eks/lib/managed-nodegroup.ts:476
    throw new Error('Malformed instance type identifier');
          ^
Error: Malformed instance type identifier
    at isGpuInstanceType (<local-project-path>/cdk/node_modules/aws-cdk-lib/aws-eks/lib/managed-nodegroup.ts:476:11)
    at typeToArch (<local-project-path>/cdk/node_modules/aws-cdk-lib/aws-eks/lib/managed-nodegroup.ts:492:12)
    at Array.map (<anonymous>)
    at getPossibleAmiTypes (<local-project-path>/cdk/node_modules/aws-cdk-lib/aws-eks/lib/managed-nodegroup.ts:499:69)
    at new Nodegroup (<local-project-path>/cdk/node_modules/aws-cdk-lib/aws-eks/lib/managed-nodegroup.ts:370:26)
    at <local-project-path>/cdk/lib/stacks/k8s-nodegroup.ts:63:12
    at new K8snodegroups (<local-project-path>/cdk/lib/stacks/k8s-nodegroup.ts:84:8)
    at Object.<anonymous> (<local-project-path>/cdk/bin/cdk.ts:39:20)
    at Module._compile (node:internal/modules/cjs/loader:1105:14)
    at Module.m._compile (<local-project-path>/cdk/node_modules/ts-node/src/index.ts:1056:23)

When printing the value of the parameter I get:

${Token[TOKEN.423]}

Reproduction Steps

I am trying to use a CfnParameter representing the value of an EC2 instance type but I get an error with Malformed instance type, as the CfnParameter is returning a none interpreted Token.

const nodeInstanceType = new CfnParameter(this, 'nodegroupInstanceType', {
    type: 'String',
    description: 'Instance Type to be used with nodegroup ng-1',
    default: 't3.small',
});

new ec2.InstanceType(${nodeInstanceType.valueAsString});

Possible Solution

No response

Additional Information/Context

CDK CLI Version

2.20.0 (build 738ef49)

Framework Version

No response

Node.js Version

v18.3.0

OS

MacOs Monterey 12.4

Language

Typescript

Language Version

No response

Other information

No response

@rshad rshad added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 29, 2022
@github-actions github-actions bot added the package/tools Related to AWS CDK Tools or CLI label Jun 29, 2022
@peterwoodworth
Copy link
Contributor

The value of your parameter will not be known until deploy time - see our documentation on parameters

What this means is that you will be able to get use out of the parameter only once its found in the template. If you depend on the value of the parameter for any synth time logic, synth will fail from trying to parse the token as actual data.

In this case, it seems we don't support the case where the machine image passed to a nodegroup is a token. Due to the line here, where it attempts to parse the instance type

const instanceTypeComponents = instanceType.toString().match(/^([a-z]+)(\d{1,2})([a-z]*)\.([a-z0-9]+)$/);

Since this code is put in place to prevent users from mistakes and it depends on knowing the value at synth time, I think we should be able to skip this code if it detects that the instance type supplied is a token

@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 Jul 6, 2022
@peterwoodworth peterwoodworth changed the title (aws-cdk-lib/aws-eks): (CfnParameter is not interpreted as expected in cdk synth or deploy) (aws-cdk-lib/aws-eks): nodegroup instanceTypes does not accept tokens Jul 6, 2022
@rshad
Copy link
Author

rshad commented Jul 7, 2022

Actually, this is not working when deploying not only when running synth.

@peterwoodworth
Copy link
Contributor

That wouldn't make sense to me - what sort of deploy time failure are you getting? I would expect that that would be a separate error

@rshad rshad closed this as completed Aug 25, 2022
@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
bug This issue is a bug. effort/small Small work item – less than a day of effort p1 package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Development

No branches or pull requests

3 participants