hashicorp / terraform-provider-google

Terraform Provider for Google Cloud Platform
https://registry.terraform.io/providers/hashicorp/google/latest/docs
Mozilla Public License 2.0
2.31k stars 1.73k forks source link

PubSub Subscription filter length not validated on plan - causes deletion of subscription without creation when applied #19096

Open omrikiei opened 1 month ago

omrikiei commented 1 month ago

Community Note

I plan to submit a (tiny) fix for the issue

Terraform Version & Provider Version(s)

Terraform v1.9.3 on mac m1

Affected Resource(s)

google_pubsub_subscription

Terraform Configuration

provider "google" {
  project = "test"
  region  = "eu-west-3"
}

resource "google_pubsub_topic" "foo" {
  name = "foo_topic"
}

resource "google_pubsub_subscription" "foo" {
  name   = "foo_subscription"
  topic  = google_pubsub_topic.foo.id
  filter = "attributes.foo = \"barbarbarbarbarbarbarbararbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbarbar\""
  labels = {
    foo = "foo_subscription"
  }
  retry_policy {
    minimum_backoff = "60.0s"
  }
  ack_deadline_seconds = 30
  enable_exactly_once_delivery = true
}

Debug Output

No response

Expected Behavior

tf plan should fail if the filter's length exceeds 256 bytes

Actual Behavior

No response

Steps to reproduce

  1. terraform plan / terraform apply

Important Factoids

No response

References

No response

b/359238389

charan-kumar-137 commented 1 month ago

Since the Subscription argument filter has ForceNew set. Any modification to the argument will cause deletion and creation.

The error is while creating, API returns FILTER_EXPRESSION_TOO_LONG

Adding a validation at expandPubsubSubscriptionFilter might cause the plan to fail.

omrikiei commented 1 month ago

@charan-kumar-137 you are correct, however sometimes destruction and re-creation of the subscription is intended and in this case it would leave a "vacuum" as no subscription would get created. Thus I believe that the plan should indeed fail, regardless if for an existing subscription or new subscription. I've added the following PR to magic-modules as a quick way of identifying that. Optimally maybe the way to go is to parse the filter language as a more complex regex. i.e

pattern := `^(
        attributes:(?:[a-zA-Z0-9_-]+|"(?:[a-zA-Z0-9/_-]+)") |
        NOT attributes:(?:[a-zA-Z0-9_-]+|"(?:[a-zA-Z0-9/_-]+)") |
        attributes\.(?:[a-zA-Z0-9_-]+|"(?:[a-zA-Z0-9/_-]+)")\s*=\s*"(?:[^"\\]*(?:\\.[^"\\]*)*)" |
        attributes\.(?:[a-zA-Z0-9_-]+|"(?:[a-zA-Z0-9/_-]+)")\s*!=\s*"(?:[^"\\]*(?:\\.[^"\\]*)*)" |
        hasPrefix\(attributes\.(?:[a-zA-Z0-9_-]+|"(?:[a-zA-Z0-9/_-]+)"),\s*"(?:[^"\\]*(?:\\.[^"\\]*)*)"\) |
        NOT hasPrefix\(attributes\.(?:[a-zA-Z0-9_-]+|"(?:[a-zA-Z0-9/_-]+)"),\s*"(?:[^"\\]*(?:\\.[^"\\]*)*)"\) |
        \(.*\) |
        AND|OR|NOT
    )$`
ggtisc commented 1 month ago

Confirmed issue!

if the filter size exceeds 256 kb the terraform plan indicates that everything is ok, but when running a terraform apply it results in an error

kbl commented 4 days ago

Although the changed filter validation rule indeed checks if filter is shorter than 257 characters it has some minor flaw - it doesn't match some of the characters which are perfectly valid inside of the filter value. For example \n is not matched by ^.{1,256}$.