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

[Bug]: aws_network_acls doesnt return tags #38511

Open
matthewmedway opened this issue Jul 24, 2024 · 6 comments
Open

[Bug]: aws_network_acls doesnt return tags #38511

matthewmedway opened this issue Jul 24, 2024 · 6 comments
Labels
bug Addresses a defect in current functionality. needs-triage Waiting for first response or review from a maintainer. service/vpc Issues and PRs that pertain to the vpc service. waiting-response Maintainers are waiting on response from community or contributor.

Comments

@matthewmedway
Copy link

matthewmedway commented Jul 24, 2024

Terraform Core Version

v1.5.0

AWS Provider Version

1.5.59

Affected Resource(s)

aws_network_acls

Expected Behavior

tags should be returned

Actual Behavior

Tags is null

Relevant Error/Panic Output Snippet

no panic or error

Terraform Configuration Files

data "aws_network_acls" "all" {
  vpc_id = "vpc-123456"
  filter {
    name = "network-acl-id"
    values = ["ac-123456678"]
  }
}

output "acltest" {
  value = data.aws_network_acls.all.tags # is null
}

Steps to Reproduce

Use the supplied tf config files with a valid acl ID, vpc ID, that has tags in the aws console. No tags are returned

Debug Output

No response

Panic Output

No response

Important Factoids

No response

References

Similar issue where no tags were being returned: #37886

Would you like to implement a fix?

No

@matthewmedway matthewmedway added the bug Addresses a defect in current functionality. label Jul 24, 2024
Copy link

Community Note

Voting for Prioritization

  • Please vote on this issue by adding a 👍 reaction to the original post to help the community and maintainers prioritize this request.
  • Please see our prioritization guide for information on how we prioritize.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

Volunteering to Work on This Issue

  • If you are interested in working on this issue, please leave a comment.
  • If this would be your first contribution, please review the contribution guide.

@github-actions github-actions bot added the service/vpc Issues and PRs that pertain to the vpc service. label Jul 24, 2024
@terraform-aws-provider terraform-aws-provider bot added the needs-triage Waiting for first response or review from a maintainer. label Jul 24, 2024
@justinretzolk
Copy link
Member

Hey @matthewmedway 👋 Thank you for taking the time to raise this! Are you able to supply debug logs (redacted as needed)? That information will likely be key for whoever picks this issue up to look into it. Additionally, can you clarify the version of the AWS Provider that you're using? You can verify this by running terraform --version in the initialized directory.

@justinretzolk justinretzolk added the waiting-response Maintainers are waiting on response from community or contributor. label Jul 24, 2024
@acwwat
Copy link
Contributor

acwwat commented Jul 25, 2024

AFAIK the aws_network_acls data source only returns a list of NACL IDs that are found, not the actual details of the NACLs. The tags argument is only for search/filter purposes.

@stefanfreitag
Copy link
Contributor

stefanfreitag commented Jul 25, 2024

Hi @acwwat, I agree with you on the filter part.
But there is also a sentence in the "Attribute Reference" section that states

This data source exports the following attributes in addition to the arguments above.

With tags listed under the "Argument Reference" section, I would also assume that tags show up as attributes.
On code level I saw what you explained, in addition Id is set.

d.SetId(meta.(*conns.AWSClient).Region)
d.Set(names.AttrIDs, naclIDs)

Would an update of the documentation help here? e.g. removing "This data source exports the following attributes in addition to the arguments above" as this seems to be not true.

@matthewmedway
Copy link
Author

matthewmedway commented Jul 31, 2024

Re: aws provider version @justinretzolk (i updated since my initial post)

terraform --version
Terraform v1.9.3
on linux_arm64
+ provider registry.terraform.io/hashicorp/aws v5.59.0

re @acwwat and @stefanfreitag
Yep, the lack of full tag details is whats missing. I was planning looping over some attributes, like tags, for the nacls in our account using the pluralized aws_network_acls but there isn't an aws_network_acl data source. Similar to how its done for aws_subnets. As a workaround I was going to settle for just the tags but they're always null too.

The data resource for aws_network_acls doesnt have much beyond just the IDs, and the data doesnt contain the tags for each ID that matched the filter either, even if only 1 result is returned.

ex: with no filter, no tags are returned, but i get all the acls in my account

Changes to Outputs:
  + acltest = {
      + filter   = null
      + id       = "ca-central-1"
      + ids      = [
          + "acl-0d5d2c556bxxx",
          + "acl-0bcd886aafbxxx",
          + "acl-05d44639842xxx",
          + "acl-07dc347f2axxx",
        ]
      + tags     = null
      + timeouts = null
      + vpc_id   = "vpc-074d0699xxxxx"
    }

ex2: with a filter that returns only 1 id, still no tags

    Changes to Outputs:
  + acltest = {
      + filter   = [
          + {
              + name   = "network-acl-id"
              + values = [
                  + "acl-0d5d2c556bxxxx",
                ]
            },
        ]
      + id       = "ca-central-1"
      + ids      = [
          + "acl-0d5d2c556bxxxx",
        ]
      + tags     = null
      + timeouts = null
      + vpc_id   = "vpc-074d069xxxxx"
    }

In both examples I am expecting ~7 tags for this acl but none are returned.

@github-actions github-actions bot removed the waiting-response Maintainers are waiting on response from community or contributor. label Jul 31, 2024
@justinretzolk
Copy link
Member

justinretzolk commented Aug 27, 2024

Would an update of the documentation help here? e.g. removing "This data source exports the following attributes in addition to the arguments above" as this seems to be not true.

The "...in addition to the arguments above" is used in the documentation across all of our resources and data sources, and I would make the case that it's still true in this case (at least as far as Terraform is concerned). That you're able to reference the tags via interpolation without Terraform throwing an error means that the argument is "exported", regardless of what value is you're presented with.

Looking at this more, I'm seeing a few different converging things that are leading to the current behavior. The first worth calling out is the description of the data source, which, as far as I can tell, does not currently appear on the Registry:

Provides a list of network ACL ids for a VPC

In my opinion, this definition is key in interpreting whether this is a bug or working as expected. My interpretation of the following is that this is working as is currently expected, but there's potentially room for enhancements.

As of right now, the tags argument is only used for filtering the NACLs. If the tags were returned for each of the NACLs IDs that were returned in ids, it wouldn't make sense to put that at the top level tags argument since there's no guarantee that the same tags exist on each of the NACLs that are returned. The way to guarantee that would be to filter by tags, which is what the current tags implementation does.

Updating the schema to accommodate the tags for each NACL that's returned would be a breaking change, but would also alter the purpose of the data source as currently described.

If only a single NACL ID was returned, it may seem intuitive to include any tags in the tags attribute, but changing how that attribute is computed based on other factors at runtime feels wrong to me (if even possible). The correct resolution may be to introduce a singular version of the data source that could be used to get more detailed information about a specific NACL. Thankfully, like @matthewmedway mentioned, there seems to be a precedent for this already with aws_subnet and aws_subnets.

@justinretzolk justinretzolk added the waiting-response Maintainers are waiting on response from community or contributor. label Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Addresses a defect in current functionality. needs-triage Waiting for first response or review from a maintainer. service/vpc Issues and PRs that pertain to the vpc service. waiting-response Maintainers are waiting on response from community or contributor.
Projects
None yet
Development

No branches or pull requests

4 participants