Terraform-provider-aws: Proposal: Consolidate `aws_ec2_client_vpn_network_association` functionality into `aws_ec2_client_vpn_endpoint` resource

[slapula]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

Proposal: When I took on the task of adding support for EC2 Client VPN endpoints to this provider, I started with a plan to make the resources modular. aws_ec2_client_vpn_network_association was the first resource I wrote following this pattern. I also had resources for routes and authorization rules in the pipeline too. However, when writing the resource for authorization rules, I found that this functionality made more sense as a feature within the aws_ec2_client_vpn_endpoint resource. This got me to rethink my overall approach to these VPN endpoints. The better approach would be to consolidate these features into functionality within the aws_ec2_client_vpn_endpoint resource specifically as opposed to them existing as separate resources.

Currently, one of these "sub"-resources is live in the codebase (aws_ec2_client_vpn_network_association). I would like to propose we depreciate that resource in favor of adding its functionality to aws_ec2_client_vpn_endpoint. I already have a PR up for review that shows how this would look: #7564.

cc: @bflad

New or Affected Resource(s)

• aws_ec2_client_vpn_endpoint
• aws_ec2_client_vpn_network_association

Potential Terraform Configuration

This is how Client VPN endpoint configurations look now:

resource "aws_ec2_client_vpn_endpoint" "example" {
  description = "terraform-clientvpn-example"
  server_certificate_arn = "${aws_acm_certificate.cert.arn}"
  client_cidr_block = "10.0.0.0/16"

  authentication_options {
    type = "certificate-authentication"
    root_certificate_chain_arn = "${aws_acm_certificate.root_cert.arn}"
  }

  connection_log_options {
    enabled = true
    cloudwatch_log_group = "${aws_cloudwatch_log_group.lg.name}"
    cloudwatch_log_stream = "${aws_cloudwatch_log_stream.ls.name}"
  }
}

resource aws_ec2_client_vpn_network_authorization "example" {
    ...
}

This is how it would look post-change:

resource "aws_ec2_client_vpn_endpoint" "example" {
  description = "terraform-clientvpn-example"
  server_certificate_arn = "${aws_acm_certificate.cert.arn}"
  client_cidr_block = "10.0.0.0/16"

  authentication_options {
    type = "certificate-authentication"
    root_certificate_chain_arn = "${aws_acm_certificate.root_cert.arn}"
  }

  connection_log_options {
    enabled = true
    cloudwatch_log_group = "${aws_cloudwatch_log_group.lg.name}"
    cloudwatch_log_stream = "${aws_cloudwatch_log_stream.ls.name}"
  }

  network_authorization {
    ...
  }

  authorization_rule {
    ...
  }

  route {
   ...
  }
}

References

• #7564

[ewbankkit]

I see value in leaving aws_ec2_client_vpn_network_association as a separate resource with maybe adding the association as an embedded attribute in the aws_ec2_client_vpn_endpoint resource for simpler scenarios:

• The association(s) can be managed in a state file separate from the VPN endpoint
• More complex scenarios can be enabled (e.g. # of subnets not known and need to use count on the association)
• The association seems to be a distinct entity in the AWS API (it has its own ID)

[slapula]

@ewbankkit That's a good point. It should also be noted that routes and authorization rules do not have their own IDs.

[bflad]

More important than whether something has its own bespoke ID is whether being able to manage the lifecycle of a piece of infrastructure is possible. If some piece of infrastructure has CR(U)D-like functionality, then having a separate Terraform resource allows for better compose-ability of infrastructure across Terraform modules and states.

[slapula]

Are there examples of resources here in this provider that do not return an associated ID from AWS yet are still managed as an independent resource? For my work on these resources (in general), I've been using that as a cutoff point in terms of whether something should be a resource or an attribute of some other parent resource. Like Client VPN endpoint routes for example, CRUD operations are all centered around the endpoint itself. You are not performing the actions on the route, you are performing the actions on the endpoint. That's the way I see it but I'm open to other perspectives.

[bflad]

Some recent examples are the aws_ram_principal_association and aws_ram_resource_association resources. The API actions involve a RAM Resource Share (the aws_ram_resource_share Terraform resource) and fit the lifecycle model by mapping AssociateResourceShare to create and DisassociateResourceShare to delete and there is no direct association ID in AWS (in Terraform its thought of as a "complex" ID of the Resource Share and Principal/Resource). The associations could theoretically be part of the aws_ram_resource_share resource, but they are split so a shared RAM Resource Share can be defined while associations can be elsewhere closer to their appropriate counterpart if necessary.

There is no real silver bullet logic to what functionality should constitute belonging to a single resource versus split out (in fact one could easily argue it might make sense in the RAM scenario above to have principal associations _not_ split out for almost all use cases 😄 ), but many times splitting it out where possible allows larger teams/infrastructures to each handle separate pieces. A common place where splitting usually makes sense is with networking related functionality since networking topologies can be provisioned in many various ways. Some places might prefer a central team to manage everything while others would allow other teams to associate with the shared resource but not necessarily manage the shared resource itself.

[holyjak]

Hi @slapula , we are eagerly awaiting this capability, thank you for the hard work! What is the current status? Are you reconsidering whether to deprecate the stand-alone resource or just waiting for someone's approval? Thank you!

[slapula]

@holyjak I think we are just waiting on a review of https://github.com/terraform-providers/terraform-provider-aws/pull/7564.

[mbenabda]

Thank you @slapula for this !
I agree with @ewbankkit about the value of having aws_ec2_client_vpn_network_association as a separate resource. Separating it would make the point valid for route as well.
authorization_rule would also benefit from being a separate resource (eg: dynamic # of Active Directory groups)