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

Fix tagging collision for VPC network peering resources #1215

Merged
merged 13 commits into from
Mar 17, 2023
Merged

Conversation

markdboyd
Copy link
Contributor

@markdboyd markdboyd commented Jan 20, 2023

Changes proposed in this pull request:

Due to a collision in the tags applied on the aws_vpc_peering_connection and aws_vpc_peering_connection_accepter resources, Terraform always thinks there are changes to apply for one resource, even when nothing has changed. The output from terraform plan looks something like:

  # module.stack.module.vpc_peering_parentbosh.aws_vpc_peering_connection_accepter.peer will be updated in-place
  ~ resource "aws_vpc_peering_connection_accepter" "peer" {
        id                        = <redacted>
      ~ tags                      = {
          - "deployment" = "cf-<deployment>" -> null
            # (1 unchanged element hidden)
        }
        # (8 unchanged attributes hidden)

        # (2 unchanged blocks hidden)
    }

This behavior is problematic because it causes our CI system to send us a message in Slack telling us to review the terraform plan output, when in reality nothing has changed or needs to change.

The problem turned out to be caused by the fact that the Terraform aws_vpc_peering_connection and aws_vpc_peering_connection_accepter resources actually managed a single AWS VPC peering connection resource. So defining tags on both Terraform resources and having default tags (tags_all) from different providers apply to each resource was causing Terraform to get persistently confused about which tags to apply.

The changes in this PR update the aws_vpc_peering_connection_accepter Terraform resource to ignore changes to tags and tags_all (default tags), which is fine, because tags (including Name) are set on the aws_vpc_peering_connection resource. And this change resolves the persistent confusion and detection of changes by Terraform.

security considerations

None

@markdboyd markdboyd requested a review from a team January 20, 2023 15:29
Copy link
Contributor

@jameshochadel jameshochadel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want these resources to be tagged though, right? Do we need to introduce another method of tagging them?

@markdboyd
Copy link
Contributor Author

@jameshochadel Yeah, we want them to be tagged, but as is, this code just produces confusion because it makes the terraform plan always produce a diff, even though there are actually no changes to apply, which looks like this:

  # module.stack.module.vpc_peering_parentbosh.aws_vpc_peering_connection_accepter.peer will be updated in-place
  ~ resource "aws_vpc_peering_connection_accepter" "peer" {
        id                        = "<redacted>"
      ~ tags                      = {
          - "deployment" = "cf-staging" -> null
            # (1 unchanged element hidden)
        }
        # (8 unchanged attributes hidden)

        # (2 unchanged blocks hidden)
    }

I've tried to fix it and keep the default_tags to no avail.

It's some permutation of this bug caused by us having multiple provider blocks with default_tags: hashicorp/terraform-provider-aws#18311

@markdboyd markdboyd changed the title remove default tagging from parent and tooling bosh providers Fix tagging collision for VPC network peering resources Mar 17, 2023
@jameshochadel jameshochadel self-requested a review March 17, 2023 19:59
Copy link
Contributor

@jameshochadel jameshochadel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh you figured it out! Fantastic news!

@markdboyd markdboyd merged commit 11e611a into main Mar 17, 2023
@markdboyd markdboyd deleted the fix-tagging branch March 17, 2023 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants