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

TF never removes GCP FW logs when log_config is removed from the resource #7551

Closed
ghost opened this issue Oct 16, 2020 · 4 comments
Closed
Assignees
Labels

Comments

@ghost
Copy link

ghost commented Oct 16, 2020

This issue was originally opened by @bmsareias as hashicorp/terraform#26609. It was migrated here as a result of the provider split. The original body of the issue is below.


Terraform Version

# terraform --version                                                                                                                                                                                                                                                                         DEVOPS-33963-legacytooling!
Terraform v0.12.28
+ provider.google v3.43.0
+ provider.google-beta v3.43.0

Your version of Terraform is out of date! The latest version
is 0.13.4. You can update by downloading from https://www.terraform.io/downloads.html

Terraform Configuration Files

Initial config

resource "google_compute_firewall" "myFWRule" {
  name           = "myFWRule"
  network        = google_compute_network.my-network.self_link
  project        = google_compute_network.my-network.project
  source_ranges  = ["0.0.0.0/0"]
  direction      = "INGRESS"

  log_config {
    metadata = "INCLUDE_ALL_METADATA"
  }

  deny {
    protocol = "all"
  }
}

later-config

resource "google_compute_firewall" "myFWRule" {
  name           = "myFWRule"
  network        = google_compute_network.my-network.self_link
  project        = google_compute_network.my-network.project
  source_ranges  = ["0.0.0.0/0"]
  direction      = "INGRESS"

#  log_config {
#    metadata = "INCLUDE_ALL_METADATA"
#  }

  deny {
    protocol = "all"
  }
}

Expected Behavior

The log_config removal should disable the logs all together.

Actual Behavior

The log_config simply deletes it's "metadata" (required field) and never actually removes the log_config object, therefore GCP still marks the FW Rule's log as enabled.

Steps to Reproduce

terraform init
terraform apply (initial config)
terraform apply (later config)

Additional Context

So far the only method that properly fixes this is to use the already deprecated method of

resource "google_compute_firewall" "myFWRule" {
  name           = "myFWRule"
  network        = google_compute_network.my-network.self_link
  project        = google_compute_network.my-network.project
  source_ranges  = ["0.0.0.0/0"]
  direction      = "INGRESS"
  enable_logging = "false"  ## THIS IS A DEPRECATED METHOD!

#  log_config {
#    metadata = "INCLUDE_ALL_METADATA"
#  }

  deny {
    protocol = "all"
  }
}
@edwardmedia
Copy link
Contributor

edwardmedia commented Oct 19, 2020

@fd I see Logs turning to *Off on the Console UI after I updated the config. Can you post the debug logs so I can take a close look?

@bmsareias
Copy link

@edwardmedia Thanks for the quick response. Further investigating the issue I can confirm that you're correct on this aspect. The issue only appears when you have at least used once the old deprecated declaration:

enable_logging = "true" # "false", seems to not really matter.

once the deprecated declaration was used, if we try to invert that state with the new declarations it will always fail or do nothing.

Example 1:

Initial config 1

resource "google_compute_firewall" "myFWRule" {
  name           = "myFWRule"
  network        = google_compute_network.my-network.self_link
  project        = google_compute_network.my-network.project
  source_ranges  = ["0.0.0.0/0"]
  direction      = "INGRESS"
  enable_logging = "true"

  deny {
    protocol = "all"
  }
}

initial config 1 (declaration update)

resource "google_compute_firewall" "myFWRule" {
  name           = "myFWRule"
  network        = google_compute_network.my-network.self_link
  project        = google_compute_network.my-network.project
  source_ranges  = ["0.0.0.0/0"]
  direction      = "INGRESS"
#  enable_logging = "true" 

  log_config {
    metadata = "INCLUDE_ALL_METADATA"
  }

  deny {
    protocol = "all"
  }
}

later config 1 (log disable)

resource "google_compute_firewall" "myFWRule" {
  name           = "myFWRule"
  network        = google_compute_network.my-network.self_link
  project        = google_compute_network.my-network.project
  source_ranges  = ["0.0.0.0/0"]
  direction      = "INGRESS"
#  enable_logging = "true" 

#  log_config {
#    metadata = "INCLUDE_ALL_METADATA"
#  }

  deny {
    protocol = "all"
  }
}

This will never actually remove the logs.

Example 2

Initial config 2

resource "google_compute_firewall" "myFWRule" {
  name           = "myFWRule"
  network        = google_compute_network.my-network.self_link
  project        = google_compute_network.my-network.project
  source_ranges  = ["0.0.0.0/0"]
  direction      = "INGRESS"
  enable_logging = "false"

  deny {
    protocol = "all"
  }
}

Later config 2

resource "google_compute_firewall" "myFWRule" {
  name           = "myFWRule"
  network        = google_compute_network.my-network.self_link
  project        = google_compute_network.my-network.project
  source_ranges  = ["0.0.0.0/0"]
  direction      = "INGRESS"
#  enable_logging = "false"

  log_config {
    metadata = "INCLUDE_ALL_METADATA"
  }

  deny {
    protocol = "all"
  }
}

Renders into an error stating: "Error: log_config cannot be defined when enable_logging is false", while we clearly removed it from the configuration declarations.

Final considerations

It seems that, if at any time in the past the rule had any enable_logging declaration in it, it will override the newer method for eternity. Seems like the only real way to properly fix this state is to either destroy the rule and/or modify the state file to remove it's knowledge about the previous enable_logging statement.

Further note, this might be even made worst when using statefile backends like we do (but was unable to further investigate into this part).

@ghost ghost removed waiting-response labels Oct 21, 2020
@edwardmedia
Copy link
Contributor

@bmsareias here is another open issue that appears to address the same issue. I would consider to let that one for tracking. Feel free to reopen this if you see a need. Thank you

@ghost
Copy link
Author

ghost commented Nov 21, 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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

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

No branches or pull requests

2 participants