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

Accessing a resource's computed list or set attribute #1573

Closed

Conversation

DustinChaloupka
Copy link

This is a very much work in progress PR right now. I have added a failing test for accessing a resource attribute that is either a set or a list to grab all of those attributes for another resource.

Sample config:

resource "aws_route53_zone" "dev" {
  name = "dev.example.com"
}

resource "aws_route53_record" "ns_in_dev" {
  zone_id = "${aws_route53_zone.dev.zone_id}"
  name = "local.dev.example.com"
  type = "NS"
  records = [ "${aws_route53_zone.dev.name_servers.*}" ]
}

(Name servers for route53 would be something like in this PR: #1525, just a computed Set)

Was tracking why, with a config like above, the returned value of ${aws_route53_zone.dev.name_servers.*} was just a number which turned out to be the size of the Set which lead to https://github.com/hashicorp/terraform/blob/master/terraform/interpolate.go#L345.

Should this be possible to do? Or is there a simpler way to achieve this? This is my first time really diving into terraform code so I'm not sure if I'm missing something or if there is a different place that something like this should be handled.

I have started working on a fix but wanted to get this somewhere eyes can be on it and for a discussion. Any help or suggestions would be wonderful.

/cc @catsby @mitchellh

@knuckolls
Copy link
Contributor

Both @rubbish and I also reviewed this and tried to get it to work. If there's a way to pass a computed set into another resource, we sure couldn't figure it out ourselves. I'm reasonably convinced that this is a bug.

@DustinChaloupka
Copy link
Author

Accidentally closed it.

So I committed "fix", though there is still a couple of questions I have and potentially some cleaning up to do as well as a few more test parts.

My first question is how should arbitrarily deeply nested attributes be handled? I'm not sure if there is even an example of something like that but this does not handle, for example, something like resource.name.attribute.*.nested_attribute.* or even resource.name.attribute.*.nest_attribute.keeps.going.down.

Another question is the fact that I am doing this sort: https://github.com/hashicorp/terraform/pull/1573/files#diff-6baa6e1e203f1aabf89e2169ec8c89b1R406. I'm not sure how someone is supposed to know which one they want out of the set/list or if that is even possible and how things should be sorted so they do know what they will be getting.

My last question (for now) is how to lay out the tests. Right now it is just set up as one test that is handling all the different scenarios. Did not know if it would be better to split them all out even though they are using the same state/module, or if having multiple testInterpolate(...) in the test was fine.

I'm going to add to the test a list as well, though in the attributes it seems to be the same as a set anyways (correct me if I'm wrong). Any formatting/go code idiosyncrasies suggestions are also definitely welcome.

@DustinChaloupka DustinChaloupka changed the title [WIP] Resource Multi Attribute Resource Multi Attribute Apr 20, 2015
@DustinChaloupka DustinChaloupka changed the title Resource Multi Attribute Resource Computed Multi Attribute Apr 20, 2015
@DustinChaloupka DustinChaloupka changed the title Resource Computed Multi Attribute Accessing a resource's computed list or set attribute Apr 20, 2015
@mitchellh
Copy link
Contributor

Okayyyyy so I agree with @knuckolls that this feature should be supported. But I don't want to try and tackle supporting this with the current internals of how this data is stored (as a flat string map). We have plans in the future to actually retain the full type information in the state file (arrays are arrays, maps are maps!) and this will make this mountains easier to implement.

Trying to implement it now will likely be subpar. I'm sorry there isn't a good solution to this.

@DustinChaloupka If you can get this working 100% with a good set of test cases then I'd be open to taking a look, but for the above reasons I'm not going to WIP this for now. Sorry!

@ghost
Copy link

ghost commented May 3, 2020

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.

@ghost ghost locked and limited conversation to collaborators May 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants