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

Faulty Read of Client VPN Network associations break state #11586

Closed
fgrelak opened this issue Jan 13, 2020 · 6 comments · Fixed by #18199
Closed

Faulty Read of Client VPN Network associations break state #11586

fgrelak opened this issue Jan 13, 2020 · 6 comments · Fixed by #18199
Labels
service/ec2 Issues and PRs that pertain to the ec2 service.
Milestone

Comments

@fgrelak
Copy link

fgrelak commented Jan 13, 2020

In aws/resource_aws_ec2_client_vpn_network_association.go line 112 commit/acc6a21a3bd6159f4f8c10a86bcd08e70edce4f2
d.Set("subnet_id", result.ClientVpnTargetNetworks[0].TargetNetworkId)
leads to wrong subnet-ids if more than one Subnet is associated with the VPN Endpoint (as only the first entry of the map is read. Minimal example breaking the state:

resource "aws_ec2_client_vpn_network_association" "subnet-cvpn-associations" {
  for_each               = toset(var.subnets)
  client_vpn_endpoint_id = aws_ec2_client_vpn_endpoint.main-cvpn-endpoint.id
  subnet_id              = each.value
}

variable "subnets" {
  default = [
    "subnet-123",
    "subnet-456",
  ]
}

will lead to replacement on every terraform plan.

This may have teh reason that the API call here result, err := conn.DescribeClientVpnTargetNetworks(&ec2.DescribeClientVpnTargetNetworksInput{
ClientVpnEndpointId: aws.String(d.Get("client_vpn_endpoint_id").(string)),
AssociationIds: []*string{aws.String(d.Id())},
actually does NOT return a list with only one entry, but all associations. One can test this by making this call with a wrong AssociationId.

@ghost ghost added the service/ec2 Issues and PRs that pertain to the ec2 service. label Jan 13, 2020
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Jan 13, 2020
@nijave
Copy link
Contributor

nijave commented Jul 29, 2020

This appears to be an Amazon API bug

doesn't work

'body': {
  'Action': 'DescribeClientVpnTargetNetworks',
  'Version': '2016-11-15',
  'ClientVpnEndpointId': 'cvpn-endpoint-123',
  'AssociationIds.1': 'cvpn-assoc-123'
}

works

'body': {
  'Action': 'DescribeClientVpnTargetNetworks', 
  'Version': '2016-11-15',
  'ClientVpnEndpointId': 'cvpn-endpoint-123', 
  'Filter.1.Name': 'association-id', 
  'Filter.1.Value.1': 'cvpn-assoc-123'
}

@cosmin-oprea
Copy link

I wonder when the pull request will be approved.
It's holding up my deployment in AWS as I make heavily use of this module and really don't like to start adding vpn client endpoint associations manually.

@breathingdust
Copy link
Member

Hi all! 👋 Just wanted to direct you to our public roadmap for this quarter (Nov-Jan) in which this item has been mentioned.

Due to the significant community interest in resolving this issue, we will be looking at merging existing contributions soon.

We appreciate all the contributions and feedback thus far.

@bill-rich
Copy link
Contributor

The ability to use AssociationIds was fixed earlier this year. Rather than switching to using filters, I've left the original method, but added the acceptance test from #14380.

@ghost
Copy link

ghost commented Mar 18, 2021

This has been released in version 3.33.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!

@ghost
Copy link

ghost commented Apr 18, 2021

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 ghost locked as resolved and limited conversation to collaborators Apr 18, 2021
@breathingdust breathingdust removed the needs-triage Waiting for first response or review from a maintainer. label Sep 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
service/ec2 Issues and PRs that pertain to the ec2 service.
Projects
None yet
5 participants