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

Better (any?) documentation for retrieving values from set data type #25717

Closed
kevinburke1 opened this issue Jul 31, 2020 · 9 comments
Closed
Labels
documentation new new issue not yet triaged

Comments

@kevinburke1
Copy link
Contributor

kevinburke1 commented Jul 31, 2020

The terraform-aws provider started using sets as output values in version 3, instead of lists or maps, so last night was my first experience using them. I have a set that has a single value in it. I would expect to be able to access this value using a key, or the [0] index, or something. Instead, the only documentation resources I can see reference accessing the values in sets by iterating over all of them using the for_each operator.

This is awkward in some places, like here (just as an example, don't worry about what this is actually doing)

resource "aws_route53_record" "root_validation" {
  count   = (var.https == "" || var.has_lb == false) ? 0 : 1
  name    = aws_acm_certificate.root[0].domain_validation_options.0.resource_record_name
  type    = aws_acm_certificate.root[0].domain_validation_options.0.resource_record_type
  zone_id = var.route53_zone_id == "" ? aws_route53_zone.domain[0].zone_id : var.route53_zone_id
  records = [aws_acm_certificate.root[0].domain_validation_options.0.resource_record_value]
  ttl     = 60
}

I tried adding a for_each field but this interacts poorly with the count field - you can't have both - so I ended up doing this:

resource "aws_route53_record" "root_validation" {
  count = (var.https == "" || var.has_lb == false) ? 0 : 1

  name    = tolist(aws_acm_certificate.root[0].domain_validation_options)[0].resource_record_name
  type    = tolist(aws_acm_certificate.root[0].domain_validation_options)[0].resource_record_type
  zone_id = var.route53_zone_id == "" ? aws_route53_zone.domain[0].zone_id : var.route53_zone_id
  records = [tolist(aws_acm_certificate.root[0].domain_validation_options)[0].resource_record_value]
  ttl     = 60
}

Which also feels awkward.

I tried searching the Terraform documentation for information about the set datatype but found it lacking, particularly for how to get values out. There is this page, which explains that sets are unordered lists with no duplicates, but does not explain how to get values out of a set. https://www.terraform.io/docs/configuration/types.html

Contrast this with the list documentation - if I search Google for "terraform retrieve value from list" the answer is in both the first and third paragraphs of the top search result.

It would be useful to explain (if this is true? I don't know) that the only way to get values out of a set is to use the for_each operator and explain how to convert existing resources that might have a count field like the one I have above.

@kevinburke1
Copy link
Contributor Author

Maybe another way to look at my question is, it seems like the set abstraction makes it difficult to do two things that used to be easy - condition a resource existence on a variable (in the count above), and to get the one item in a list out of that list, and I'm struggling to figure out the right ways to do that now with the new abstraction.

@danieldreier danieldreier added new new issue not yet triaged documentation labels Aug 5, 2020
@simonc-613
Copy link

I second this, Sets feel awkward to work with.
Currently I can't use the for_each example from the AWS docs with the output from ACM Cert as we use the output of Route53 resources to generate the certificate which are computed and can't be used in for_each
Trying to index the set can't be done and as per the docs converting it to a list will put the contained maps in an arbitrary order which defeats the point of it being changed to a set in the first place

@apparentlymart
Copy link
Contributor

Hi @kevinburkemeter,

This change of type was a design tradeoff made by the AWS provider team, but based on what they wrote in the upgrade guide it seems like they were intending this as a response to the problem that there can potentially be more than one "validation option" and the remote API doesn't consider them to be ordered, and so referring to the "zeroth" one isn't a stable/meaningful idea.

The upgrade guide also includes an example pattern for interacting with the new interface, which matches with some initial ideas I had after first reviewing your issue here. Here's an attempt at implementing the guidance from the upgrade guide in conjunction with what you showed in your comment (I'm not really familiar with this particular resource, so I'm working completely from the AWS provider docs here):

resource "aws_route53_record" "root_validation" {
  for_each = (var.https == "" || var.has_lb == false) ? {} : {
    for dvo in aws_acm_certificate.root[0].domain_validation_options : dvo.domain_name => dvo
  }

  allow_overwrite = true
  name            = each.value.resource_record_name
  records         = [each.value.resource_record_value]
  ttl             = 60
  type            = each.value.resource_record_type
  zone_id         = data.aws_route53_zone.public_root_domain.zone_id
}

resource "aws_acm_certificate_validation" "root_validation" {
  certificate_arn         = aws_acm_certificate.root[0].arn
  validation_record_fqdns = [for record in aws_route53_record.root_validation : record.fqdn]
}

This would, assuming I'm understanding the aws_acm_certificate docs correctly, cause there to be one instance of aws_route53_record per element of the domain_validation_options set, which seems to follow how ACM certificate validation is intended to work.

As far as I can tell, the provider team made this design decision so that the provider's abstraction would match better with the remote system's abstraction, and so I'm not really sure how to generalize the above advice to all uses of sets: any time a provider team decides to use a set they are presumably doing so because that's the best fit for the design of the remote system, part of which includes there being no meaningful concept of "the first element" (because the items aren't ordered). When a provider uses a set to represent an unordered collection then, the typical intent is to do something with the entire set, rather than individual items from the set, but exactly what is intended will depend on the situation.

It does seems like there is an opportunity here to factor out this particular combination of aws_acm_certificate+aws_route53_record+aws_acm_certificate_validation into a shared module in the Terraform Registry, since I assume that it's a pretty common situation to use Route53 to host your ACM validation records but the configuration for achieving that seems to be almost entirely boilerplate, aside from the domain name(s) and the zone id:

module "certificate" {
  source = "some-organization/acm-certificate-with-route53/aws" # just a hypothetical; doesn't actually exist
  count  = (var.https == "" || var.has_lb == false) ? 0 : 1

  route53_zone_id = aws_route53_record.www.zone_id
  domain_name     = aws_route53_record.www.name
  subject_alternative_names = [
    aws_route53_record.www.name,
    aws_route53_zone.main.name,
  ]
}

@loganpowell

This comment was marked as off-topic.

@apparentlymart
Copy link
Contributor

Hi @loganpowell,

If what you are asking is how to write something like the example I showed above using the JSON version of the Terraform language, that seems like something better suited to the Terraform community forum. Thanks!

@loganpowell
Copy link

@apparentlymart will do. Thank you!

loganpowell added a commit to subs0/micro that referenced this issue Oct 26, 2023
@loganpowell

This comment was marked as off-topic.

loganpowell added a commit to subs0/micro that referenced this issue Oct 26, 2023
@crw
Copy link
Collaborator

crw commented Jul 29, 2024

There does not appear to be a documentation change being requested here after #25717 (comment) -- closing this issue, but will re-open if I missed the outstanding request.

@crw crw closed this as completed Jul 29, 2024
Copy link

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation new new issue not yet triaged
Projects
None yet
Development

No branches or pull requests

6 participants