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_route53: cannot use CfnParameter.valueAsNumber for L2 RecordSet weight #31810

Closed
1 task
nicholaschiasson opened this issue Oct 18, 2024 · 6 comments · Fixed by #31823
Closed
1 task

aws_route53: cannot use CfnParameter.valueAsNumber for L2 RecordSet weight #31810

nicholaschiasson opened this issue Oct 18, 2024 · 6 comments · Fixed by #31823
Labels
@aws-cdk/aws-route53 Related to Amazon Route 53 bug This issue is a bug. effort/small Small work item – less than a day of effort p2

Comments

@nicholaschiasson
Copy link
Contributor

Describe the bug

Cannot construct ARecord (and presumably any RecordSet) with weight resolved by CfnParameter value.

Regression Issue

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

Last Known Working CDK Version

No response

Expected Behavior

RecordSet should successfully be synthesized with the parameter's token value for the weight.

Current Behavior

Synthesis fails, token value representing a number outside the bounds of a valid weight (0-255).

Error: weight must be between 0 and 255 inclusive, got: -1.888154589708757e+289
    at new RecordSet (/.../node_modules/aws-cdk-lib/aws-route53/lib/record-set.js:1:3296)
    at new ARecord (/.../node_modules/aws-cdk-lib/aws-route53/lib/record-set.js:1:7665)
    ...

Reproduction Steps

import { aws_route53 as r53, CfnParameter } from 'aws-cdk-lib';

const weightParameter: CfnParameter = new CfnParameter(this, 'Weight', {
  default: 0,
  maxValue: 255,
  minValue: 0,
  type: 'Number'
});

const zone = r53.HostedZone.fromLookup(this, 'HostedZone', {
  domainName: 'example.com'
});

const aRecord = new r53.ARecord(this, 'ARecord', {
  weight: weightParameter.valueAsNumber,
  zone
});

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.151.0 (build b8289e2)

Framework Version

No response

Node.js Version

v20.13.1

OS

Linux 5.15.153.1-microsoft-standard-WSL2 #1 SMP Fri Mar 29 23:14:13 UTC 2024 x86_64 x86_64 GNU/Linux

Language

TypeScript

Language Version

No response

Other information

Currently using this workaround:

...

const aRecord = new r53.ARecord(this, 'ARecord', {
  weight: 0,
  zone
});

(aRecord.node.defaultChild as r53.CfnRecordSet).addPropertyOverride('Weight', weightParameter.valueAsNumber);
@nicholaschiasson nicholaschiasson added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 18, 2024
@github-actions github-actions bot added the @aws-cdk/aws-route53 Related to Amazon Route 53 label Oct 18, 2024
@ashishdhingra ashishdhingra self-assigned this Oct 18, 2024
@ashishdhingra ashishdhingra added p2 investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels Oct 18, 2024
@ashishdhingra
Copy link
Contributor

@nicholaschiasson Good afternoon. Thanks for opening the issue. Issue is reproducible using below code:

import * as cdk from 'aws-cdk-lib';
import * as ec2 from 'aws-cdk-lib/aws-ec2';
import * as r53 from 'aws-cdk-lib/aws-route53';
import * as r53targets from 'aws-cdk-lib/aws-route53-targets';
import { ApplicationLoadBalancer } from 'aws-cdk-lib/aws-elasticloadbalancingv2';


export class CdktestStack extends cdk.Stack {
  constructor(scope: cdk.App, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    const domainName = 'example.com';
    const vpc = ec2.Vpc.fromLookup(this, 'DefaultVpc', { isDefault: true });
    const weightParameter = new cdk.CfnParameter(this, 'Weight', {
      default: 0,
      maxValue: 255,
      minValue: 0,
      type: 'Number'
    });
    
    const zone = r53.HostedZone.fromLookup(this, 'HostedZone', {
      domainName: 'example.com'
    });
    
    const loadBalancer = new ApplicationLoadBalancer(this, 'ServiceLB', {
      vpc,
      internetFacing: true
    });
    const serviceSG = new ec2.SecurityGroup(this, 'ServiceSecurityGroup', { vpc });
    serviceSG.connections.allowFrom(loadBalancer, ec2.Port.tcp(80));
    console.log(`${weightParameter}`);
    const aRecord = new r53.ARecord(this, 'ARecord', {
      target: r53.RecordTarget.fromAlias(new r53targets.LoadBalancerTarget(loadBalancer)),
      weight: weightParameter.valueAsNumber,
      zone
    });
  }
}

However, although AWS CloudFormation parameters can be defined in the AWS CDK, they are generally discouraged because AWS CloudFormation parameters are resolved only during deployment. This means that you cannot determine their value in your code. Because the AWS CDK takes an approach where concrete templates are resolved at synthesis time and so is the validation in this case, the alternative approach is to use context values either from the command line or via the cdk.json. Refer Parameters and the AWS CDK for details.

So in your use case, you could modify your code as below:

...
const weight = this.node.tryGetContext('Weight');
    
const aRecord = new r53.ARecord(this, 'ARecord', {
  ...,
  weight: Number.parseInt(weight),
  zone
});

and run cdk synth -c Weight=20 as an example.

Thanks,
Ashish

@ashishdhingra ashishdhingra added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Oct 18, 2024
@nicholaschiasson
Copy link
Contributor Author

Hi @ashishdhingra , thanks for your quick response.

I'm aware of parameters being discouraged in favour of context arguments. Still, I consider this an unexpected behaviour and for my use case, I strongly prefer the use of a CFN parameter for this.

Here are some similar issues I was able to find which have been stated this behaviour is a bug, and which have been resolved: #7126 #9038

I imagine the fix should be similar to the fixes for those issues as well: #8252 #9176

Indeed, in the current HEAD on main branch, I see the weight property is not being validated as a Token.

I will try to open a PR to resolve this since it looks to be a simple fix.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Oct 19, 2024
@badmintoncryer
Copy link
Contributor

@nicholaschiasson I've implemented this validation and this is my fault for not checking whether weight is token or not. I would greatly appreciate it if you could create a PR.

@ashishdhingra
Copy link
Contributor

Hi @ashishdhingra , thanks for your quick response.

I'm aware of parameters being discouraged in favour of context arguments. Still, I consider this an unexpected behaviour and for my use case, I strongly prefer the use of a CFN parameter for this.

Here are some similar issues I was able to find which have been stated this behaviour is a bug, and which have been resolved: #7126 #9038

I imagine the fix should be similar to the fixes for those issues as well: #8252 #9176

Indeed, in the current HEAD on main branch, I see the weight property is not being validated as a Token.

I will try to open a PR to resolve this since it looks to be a simple fix.

@nicholaschiasson Thanks for pointing to related issues and PR contribution.

@ashishdhingra ashishdhingra removed their assignment Oct 21, 2024
@ashishdhingra ashishdhingra added the effort/small Small work item – less than a day of effort label Oct 21, 2024
@mergify mergify bot closed this as completed in #31823 Oct 31, 2024
@mergify mergify bot closed this as completed in 14561ac Oct 31, 2024
Copy link

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.

1 similar comment
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
@aws-cdk/aws-route53 Related to Amazon Route 53 bug This issue is a bug. effort/small Small work item – less than a day of effort p2
Projects
None yet
3 participants