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

Google Compute Packet Mirroring Subnet order causes state drift #20637

Open
jmgreger opened this issue Dec 9, 2024 · 2 comments · May be fixed by GoogleCloudPlatform/magic-modules#12534
Open

Comments

@jmgreger
Copy link

jmgreger commented Dec 9, 2024

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.
  • If an issue is assigned to a user, that user is claiming responsibility for the issue.
  • Customers working with a Google Technical Account Manager or Customer Engineer can ask them to reach out internally to expedite investigation and resolution of this issue.

Terraform Version & Provider Version(s)

Terraform v1.10.1
on darwin_arm64

  • provider registry.terraform.io/hashicorp/archive v2.7.0
  • provider registry.terraform.io/hashicorp/google v6.12.0
  • provider registry.terraform.io/hashicorp/google-beta v6.12.0
  • provider registry.terraform.io/hashicorp/null v3.2.3
  • provider registry.terraform.io/hashicorp/random v3.6.3
  • provider registry.terraform.io/hashicorp/time v0.12.1

Affected Resource(s)

google_compute_packet_mirroring

Terraform Configuration

data "google_compute_forwarding_rule" "mirror-ilb" {
  name    = "mirror-ilb-dest"
  project = var.project
  region  = var.region
}

module "vpc" {
  source  = "terraform-google-modules/network/google"
  version = "~> 9.0"

  project_id      = var.project_id
  network_name    = "vpc"
  routing_mode    = "GLOBAL"
  shared_vpc_host = true

  subnets = [
    {
      subnet_name               = "a-subnet"
      subnet_ip                 = "10.0.0.0/24"
      subnet_region             = var.region
      description               = "Example Subnet A"
      subnet_flow_logs          = "true"
      subnet_private_access     = "true"
      subnet_flow_logs_interval = "INTERVAL_10_MIN"
      subnet_flow_logs_sampling = 0.7
      subnet_flow_logs_metadata = "INCLUDE_ALL_METADATA"
    },
    {
      subnet_name               = "b-subnet"
      subnet_ip                 = "10.0.0.1/24"
      subnet_region             = var.region
      description               = "Example Subnet B"
      subnet_flow_logs          = "true"
      subnet_private_access     = "true"
      subnet_flow_logs_interval = "INTERVAL_10_MIN"
      subnet_flow_logs_sampling = 0.7
      subnet_flow_logs_metadata = "INCLUDE_ALL_METADATA"
    }
  ]
}

resource "google_compute_packet_mirroring" "mirror" {
  for_each    = module.vpc.subnets_self_links
  name        = "packet-mirror-config"
  description = "Packet Mirroring Example"
  project     = var.project_id
  region      = var.region

  network {
    url = module.vpc.network_self_link
  }
  collector_ilb {
    url = data.google_compute_forwarding_rule.monitoring_endpoint.self_link
  }
  filter {
    cidr_ranges  = []
    direction    = "BOTH"
    ip_protocols = []
  }
  mirrored_resources {
    dynamic "subnetworks" {
      for_each = each.value
      content {
        url = subnetworks.value
      }

    }
  }
}

 

Debug Output

No response

Expected Behavior

After running an initial terraform apply, subsequent terraform plan operations should result in a no-op and detect no changes.

Actual Behavior

Terraform detects changes required to the subnet blocks of the packet mirroring policy:

  ~ mirrored_resources {
        tags = []

      ~ subnetworks {
          ~ url = "https://www.googleapis.com/compute/v1/projects/my-project/regions/us-central1/subnetworks/b-subnet" -> "https://www.googleapis.com/compute/v1/projects/my-project/regions/us-central1/subnetworks/a-subnet"
        }
      ~ subnetworks {
          ~ url = "https://www.googleapis.com/compute/v1/projects/my-project/regions/us-central1/subnetworks/a-subnet" -> "https://www.googleapis.com/compute/v1/projects/my-project/regions/us-central1/subnetworks/b-subnet"
        }

        # (3 unchanged blocks hidden)
    }

The GCP API seems to be returning the list of subnet mirrors in numerical ascending order, but reverse lexicographical order.

Terraform stores the state in lexographical order, and as such is detecting a drift every time a plan is run:

API Response will look like:

"subnetworks": [
        {
          "url": "https://www.googleapis.com/compute/v1/projects/my-project-id/regions/us-central1/subnetworks/b-subnet"
        },
        {
          "url": "https://www.googleapis.com/compute/v1/projects/my-project-id/regions/us-central1/subnetworks/a-subnet"
        }
      ]

However the list used to construct the subnet mirror blocks will look like:

module.vpc.module.subnets.google_compute_subnetwork.subnetwork["us-central1/a-subnet"]
module.vpc.module.subnets.google_compute_subnetwork.subnetwork["us-central1/b-subnet"]

The list of subnets might be able to be manipulated to maintain a specific order, however since the mirrored_resources block contains one or more un-ordered nested blocks, the provider should ignore any differences in ordering as long as the block contents are the same.

Steps to reproduce

  1. terraform apply
  2. terraform plan

Important Factoids

No response

References

No response

b/383183182

@jmgreger jmgreger added the bug label Dec 9, 2024
@github-actions github-actions bot added forward/review In review; remove label to forward service/compute-packet-mirroring labels Dec 9, 2024
@c2thorn
Copy link
Collaborator

c2thorn commented Dec 9, 2024

Looks like this field should be a set instead of a list, similar to #17607 which was fixed by https://github.com/GoogleCloudPlatform/magic-modules/pull/11199/files

this unfortunately can only be done in a major release since retyping from a list to a set is a breaking change. Labelling as appropriate.

In the meantime @jmgreger, I'd recommend using the lifecycle.ignore_changes argument to silence the diffs related to the reordering.

@c2thorn c2thorn added breaking-change and removed forward/review In review; remove label to forward labels Dec 9, 2024
@c2thorn c2thorn added this to the Future Major Release milestone Dec 9, 2024
@jmgreger
Copy link
Author

Thanks! I'll submit a PR on the Magic Modules repo to address it. Appreciate the quick review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants