r/aws_route, r/aws_route_table, r/aws_default_route_table: Validate CIDR blocks #13778
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
added
size/XL
ewbankkit
commented
Jun 16, 2020
| @@ -233,17 +232,3 @@ func validatePorts(to int64, from int64, expected expectedPortPair) bool { | |||
|
|
|||
| return true | |||
| } | |||
|
|
|||
| // validateCIDRBlock ensures the passed CIDR block represents an implied | |||
There was a problem hiding this comment.
Moved to validators.go.
ewbankkit
commented
Jun 16, 2020
| @@ -98,23 +98,3 @@ func Test_validatePorts(t *testing.T) { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| func Test_validateCIDRBlock(t *testing.T) { | |||
There was a problem hiding this comment.
Moved to validators_test.go.
ewbankkit
commented
Jun 16, 2020
| Optional: true, | ||
| ForceNew: true, | ||
| ValidateFunc: validation.Any( | ||
| validation.StringIsEmpty, |
There was a problem hiding this comment.
Allow "", often used with conditionals.
ewbankkit
commented
Jun 16, 2020
|
|
||
| return nil | ||
| return resourceAwsRouteRead(d, meta) |
There was a problem hiding this comment.
Read after create.
ewbankkit
commented
Jun 16, 2020
| @@ -374,7 +375,7 @@ resource "aws_wafv2_ip_set" "ip_set" { | |||
| scope = "REGIONAL" | |||
| ip_address_version = "IPV6" | |||
| addresses = [ | |||
| "0:0:0:0:0:ffff:7f00:1/64", | |||
There was a problem hiding this comment.
If you enter 0:0:0:0:0:ffff:7f00:1/64 in the console it reports as 0000:0000:0000:0000:0000:0000:0000:0000/64 when you refresh. Replace with a more usual IPv6 CIDR.
ewbankkit
commented
Jun 16, 2020
| @@ -5459,13 +5458,3 @@ func flattenRoute53ResolverRuleTargetIps(targetAddresses []*route53resolver.Targ | |||
|
|
|||
| return vTargetIps | |||
| } | |||
|
|
|||
| func isIpv6CidrsEquals(first, second string) bool { | |||
There was a problem hiding this comment.
Replaced by cidrBlocksEqual.
ewbankkit
changed the title
|
In theory the UpdateVerified that for routes embedded in the route table resource you see a perpetual diff with "misaligned" CIDRs: $ terraform plan
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.
aws_vpc.test: Refreshing state... (ID: vpc-0b787780783cb384a)
data.aws_region.current: Refreshing state...
aws_subnet.test: Refreshing state... (ID: subnet-0d6d246d8f63068f1)
aws_internet_gateway.test: Refreshing state... (ID: igw-0e882d0cea86da7b2)
aws_route_table.test: Refreshing state... (ID: rtb-04ca8b2adc497c27d)
------------------------------------------------------------------------
An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
~ update in-place
Terraform will perform the following actions:
~ aws_route_table.test
route.3386248778.cidr_block: "10.4.0.0/16" => ""
route.3386248778.egress_only_gateway_id: "" => ""
route.3386248778.gateway_id: "igw-0e882d0cea86da7b2" => ""
route.3386248778.instance_id: "" => ""
route.3386248778.ipv6_cidr_block: "" => ""
route.3386248778.nat_gateway_id: "" => ""
route.3386248778.network_interface_id: "" => ""
route.3386248778.transit_gateway_id: "" => ""
route.3386248778.vpc_peering_connection_id: "" => ""
route.4280590009.cidr_block: "" => "10.4.0.1/16"
route.4280590009.egress_only_gateway_id: "" => ""
route.4280590009.gateway_id: "" => "igw-0e882d0cea86da7b2"
route.4280590009.instance_id: "" => ""
route.4280590009.ipv6_cidr_block: "" => ""
route.4280590009.nat_gateway_id: "" => ""
route.4280590009.network_interface_id: "" => ""
route.4280590009.transit_gateway_id: "" => ""
route.4280590009.vpc_peering_connection_id: "" => ""
Plan: 0 to add, 1 to change, 0 to destroy.
------------------------------------------------------------------------
Note: You didn't specify an "-out" parameter to save this plan, so Terraform
can't guarantee that exactly these actions will be performed if
"terraform apply" is subsequently run. |
ewbankkit
changed the title
…lock' must be specified * Use 'validateIpv4CIDRNetworkAddress', 'validateIpv6CIDRNetworkAddress' and 'suppressEqualCIDRBlockDiffs' Acceptance test output: $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSRoute_' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws/ -v -count 1 -parallel 1 -run=TestAccAWSRoute_ -timeout 120m === RUN TestAccAWSRoute_basic === PAUSE TestAccAWSRoute_basic === RUN TestAccAWSRoute_disappears === PAUSE TestAccAWSRoute_disappears === RUN TestAccAWSRoute_ipv6Support === PAUSE TestAccAWSRoute_ipv6Support === RUN TestAccAWSRoute_ipv6ToInternetGateway === PAUSE TestAccAWSRoute_ipv6ToInternetGateway === RUN TestAccAWSRoute_ipv6ToInstance === PAUSE TestAccAWSRoute_ipv6ToInstance === RUN TestAccAWSRoute_ipv6ToNetworkInterface === PAUSE TestAccAWSRoute_ipv6ToNetworkInterface === RUN TestAccAWSRoute_ipv6ToPeeringConnection === PAUSE TestAccAWSRoute_ipv6ToPeeringConnection === RUN TestAccAWSRoute_changeRouteTable === PAUSE TestAccAWSRoute_changeRouteTable === RUN TestAccAWSRoute_changeCidr === PAUSE TestAccAWSRoute_changeCidr === RUN TestAccAWSRoute_noopdiff === PAUSE TestAccAWSRoute_noopdiff === RUN TestAccAWSRoute_doesNotCrashWithVPCEndpoint === PAUSE TestAccAWSRoute_doesNotCrashWithVPCEndpoint === RUN TestAccAWSRoute_TransitGatewayID_DestinationCidrBlock === PAUSE TestAccAWSRoute_TransitGatewayID_DestinationCidrBlock === CONT TestAccAWSRoute_basic --- PASS: TestAccAWSRoute_basic (51.53s) === CONT TestAccAWSRoute_changeRouteTable --- PASS: TestAccAWSRoute_changeRouteTable (81.91s) === CONT TestAccAWSRoute_TransitGatewayID_DestinationCidrBlock --- PASS: TestAccAWSRoute_TransitGatewayID_DestinationCidrBlock (356.43s) === CONT TestAccAWSRoute_doesNotCrashWithVPCEndpoint --- PASS: TestAccAWSRoute_doesNotCrashWithVPCEndpoint (58.07s) === CONT TestAccAWSRoute_noopdiff --- PASS: TestAccAWSRoute_noopdiff (115.41s) === CONT TestAccAWSRoute_changeCidr --- PASS: TestAccAWSRoute_changeCidr (79.86s) === CONT TestAccAWSRoute_ipv6ToInstance --- PASS: TestAccAWSRoute_ipv6ToInstance (147.08s) === CONT TestAccAWSRoute_ipv6ToPeeringConnection --- PASS: TestAccAWSRoute_ipv6ToPeeringConnection (41.01s) === CONT TestAccAWSRoute_ipv6ToNetworkInterface --- PASS: TestAccAWSRoute_ipv6ToNetworkInterface (136.78s) === CONT TestAccAWSRoute_ipv6Support --- PASS: TestAccAWSRoute_ipv6Support (58.36s) === CONT TestAccAWSRoute_ipv6ToInternetGateway --- PASS: TestAccAWSRoute_ipv6ToInternetGateway (50.74s) === CONT TestAccAWSRoute_disappears --- PASS: TestAccAWSRoute_disappears (46.79s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 1224.034s
…stination_ipv6_cidr_block' (TF 0.11 compatibility).
…ngs for 'cidr_block' and 'ipv6_cidr_block' (TF 0.11 compatibility).
…pty strings for 'cidr_block' and 'ipv6_cidr_block' (TF 0.11 compatibility).
bflad
approved these changes
Jun 24, 2020
There was a problem hiding this comment.
LGTM 🚀 Hopefully these validation functions can get hoisted into the Terraform Plugin SDK.
Output from acceptance testing:
--- PASS: TestAccAWSDefaultNetworkAcl_basic (31.81s)
--- PASS: TestAccAWSDefaultNetworkAcl_basicIpv6Vpc (33.74s)
--- PASS: TestAccAWSDefaultNetworkAcl_deny_ingress (44.85s)
--- PASS: TestAccAWSDefaultNetworkAcl_SubnetReassign (96.04s)
--- PASS: TestAccAWSDefaultNetworkAcl_SubnetRemoval (229.81s)
--- PASS: TestAccAWSDefaultNetworkAcl_withIpv6Ingress (48.25s)
--- PASS: TestAccAWSDefaultRouteTable_basic (48.92s)
--- PASS: TestAccAWSDefaultRouteTable_ConditionalCidrBlock (73.28s)
--- PASS: TestAccAWSDefaultRouteTable_disappears_Vpc (19.28s)
--- PASS: TestAccAWSDefaultRouteTable_Route (132.34s)
--- PASS: TestAccAWSDefaultRouteTable_Route_TransitGatewayID (351.03s)
--- PASS: TestAccAWSDefaultRouteTable_swap (64.92s)
--- PASS: TestAccAWSDefaultRouteTable_tags (86.65s)
--- PASS: TestAccAWSDefaultRouteTable_vpc_endpoint (65.16s)
--- PASS: TestAccAWSNetworkAcl_basic (32.26s)
--- PASS: TestAccAWSNetworkAcl_CaseSensitivityNoChanges (35.00s)
--- PASS: TestAccAWSNetworkAcl_disappears (72.93s)
--- PASS: TestAccAWSNetworkAcl_Egress_ConfigMode (94.25s)
--- PASS: TestAccAWSNetworkAcl_EgressAndIngressRules (26.59s)
--- PASS: TestAccAWSNetworkAcl_espProtocol (25.26s)
--- PASS: TestAccAWSNetworkAcl_Ingress_ConfigMode (83.73s)
--- PASS: TestAccAWSNetworkAcl_ipv6ICMPRules (43.27s)
--- PASS: TestAccAWSNetworkAcl_ipv6Rules (33.96s)
--- PASS: TestAccAWSNetworkAcl_ipv6VpcRules (51.15s)
--- PASS: TestAccAWSNetworkAcl_OnlyEgressRules (33.49s)
--- PASS: TestAccAWSNetworkAcl_OnlyIngressRules_basic (52.21s)
--- PASS: TestAccAWSNetworkAcl_OnlyIngressRules_update (67.18s)
--- PASS: TestAccAWSNetworkAcl_SubnetChange (98.79s)
--- PASS: TestAccAWSNetworkAcl_Subnets (94.70s)
--- PASS: TestAccAWSNetworkAcl_SubnetsDelete (59.72s)
--- PASS: TestAccAWSRoute_basic (42.87s)
--- PASS: TestAccAWSRoute_changeCidr (59.56s)
--- PASS: TestAccAWSRoute_changeRouteTable (51.10s)
--- PASS: TestAccAWSRoute_ConditionalCidrBlock (46.05s)
--- PASS: TestAccAWSRoute_disappears (44.72s)
--- PASS: TestAccAWSRoute_doesNotCrashWithVPCEndpoint (54.54s)
--- PASS: TestAccAWSRoute_ipv6Support (48.70s)
--- PASS: TestAccAWSRoute_ipv6ToInstance (125.79s)
--- PASS: TestAccAWSRoute_ipv6ToInternetGateway (69.53s)
--- PASS: TestAccAWSRoute_ipv6ToNetworkInterface (148.39s)
--- PASS: TestAccAWSRoute_ipv6ToPeeringConnection (51.11s)
--- PASS: TestAccAWSRoute_noopdiff (89.35s)
--- PASS: TestAccAWSRoute_TransitGatewayID_DestinationCidrBlock (332.00s)
--- PASS: TestAccAWSRouteTable_basic (78.10s)
--- PASS: TestAccAWSRouteTable_ConditionalCidrBlock (68.64s)
--- PASS: TestAccAWSRouteTable_instance (150.39s)
--- PASS: TestAccAWSRouteTable_ipv6 (26.20s)
--- PASS: TestAccAWSRouteTable_panicEmptyRoute (27.82s)
--- PASS: TestAccAWSRouteTable_Route_ConfigMode (110.18s)
--- PASS: TestAccAWSRouteTable_Route_TransitGatewayID (323.70s)
--- PASS: TestAccAWSRouteTable_tags (95.64s)
--- PASS: TestAccAWSRouteTable_vgwRoutePropagation (69.82s)
--- PASS: TestAccAWSRouteTable_vpcPeering (54.12s)
--- PASS: TestAccAwsWafv2IPSet_Basic (16.89s)
--- PASS: TestAccAwsWafv2IPSet_ChangeNameForceNew (19.97s)
--- PASS: TestAccAwsWafv2IPSet_Disappears (9.42s)
--- PASS: TestAccAwsWafv2IPSet_IPv6 (9.15s)
--- PASS: TestAccAwsWafv2IPSet_Large (23.08s)
--- PASS: TestAccAwsWafv2IPSet_Minimal (19.65s)
--- PASS: TestAccAwsWafv2IPSet_Tags (37.93s)
|
This has been released in version 2.68.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks! |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
ghost
locked and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Community Note
Closes #338.
Closes #10666.
Closes #13003.
"Misaligned" CIDR blocks (where the IP address portion of the block is more specific than the netmask specifies) are accepted by the AWS API, but the DescribeRouteTable call returns the "aligned" CIDR and therefore routes are being reported as not found.
Also, re-allows empty strings for destination CIDR blocks.
Release note for CHANGELOG:
Output from acceptance testing: