feat(ec2): ipv6AddressCount property for an instance
#31076
Conversation
github-actions
bot
added
feature-request
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
aws-cdk-automation
added
the
pr/needs-community-review
There was a problem hiding this comment.
It seems that the ipv6AddressCount and network interfaces should not be specified together.
You cannot specify this option and the network interfaces option in the same request.
Also, when the associatePublicIpAddress is specified (can be true or false), network interfaces array is automatically set.
In other words, specifying the ipv6AddressCount and the associatePublicIpAddress should result in an error for CFn. Therefore, how about putting in a validation for this? (It would also be good to add that description to the JSDoc.)
P.S.) I actually tried to run the integ test with those properties specified and CFn got an error.
Resource handler returned message: "Network interfaces and an instance-level IPv6 address count may not be specified on the same request (Service: Ec2, Status Code: 400, Request ID: ...
| } | ||
| } | ||
|
|
||
| const testCase = new TestStack(app, 'TestStack'); |
There was a problem hiding this comment.
How about making the stack name specific to this integ test? Because there are often other integ tests with stacks of the same name, which will fail if we try to run them in parallel.
There was a problem hiding this comment.
I've updated it!
|
@go-to-k Thank you for your review and great suggestion!! I've addressed your comments. |
aws-cdk-automation
added
the
pr/needs-maintainer-review
|
Thanks always! |
| if ( | ||
| props.ipv6AddressCount !== undefined && | ||
| !Token.isUnresolved(props.ipv6AddressCount) && | ||
| (props.ipv6AddressCount < 0 || !Number.isInteger(props.ipv6AddressCount)) | ||
| ) { | ||
| throw new Error(`\'ipv6AddressCount\' must be a non-negative integer, got: ${props.ipv6AddressCount}`); | ||
| } | ||
|
|
There was a problem hiding this comment.
Curious that if there's a upper bound value for this property?
There was a problem hiding this comment.
I tried to create an instance by specifying ipv6AddressCount as 2000.
Result:
Failed resources:
InstanceIpv6AddressCountTestStack | 8:04:58 AM | CREATE_FAILED | AWS::EC2::Instance | Instance (InstanceC1063A87) Resource handler returned message: "Address count 2000 per interface exceeds the limit for m5.large (Service: Ec2, Status Code: 400, Request ID: 6fcb0c3e-a029-4b62-9e31-4452642bede8)" (RequestToken: 498ff895-1532-848d-2a6b-2e2af377efb0, HandlerErrorCode: InvalidRequest)It seems that the upper limit varies depending on the instance type. However, I couldn't find any documentation on this...
There was a problem hiding this comment.
Thanks for giving it a try. To be clear I don't want us to do the validation, but just curious about this.
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
aws-cdk-automation
removed
the
pr/needs-maintainer-review
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
Closes #31075.
Reason for this change
AWS CDK cannot configure
ipv6AddressCountproperty but cloudformation can do that.Description of changes
ipv6AddressCounttoInstancePropsipv6AddressCountis a positive integerDescription of how you validated changes
Add both unit and integ tests
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license