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

Error: log_config cannot be defined when enable_logging is false #7309

Open
MrVentzi opened this issue Sep 21, 2020 · 14 comments
Open

Error: log_config cannot be defined when enable_logging is false #7309

MrVentzi opened this issue Sep 21, 2020 · 14 comments

Comments

@MrVentzi
Copy link

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 the modular-magician user, it is either in the process of being autogenerated, or is planned to be autogenerated soon. If an issue is assigned to a user, that user is claiming responsibility for the issue. If an issue is assigned to hashibot, a community member has claimed the issue already.

Terraform Version

Terraform v0.12.18

Affected Resource(s)

  • google_compute_firewall

Terraform Configuration Files

# Copy-paste your Terraform configurations here.
#
# For large Terraform configs, please use a service like Dropbox and share a link to the ZIP file.
# For security, you can also encrypt the files using our GPG public key:
#    https://www.hashicorp.com/security
#
# If reproducing the bug involves modifying the config file (e.g., apply a config,
# change a value, apply the config again, see the bug), then please include both:
# * the version of the config before the change, and
# * the version of the config after the change.

Originally had this:

resource "google_compute_firewall" "XXXX_deny_all_egress" {
    count = var.deny_egress_XXXX_network
    name        = "XXXX-deny-all-egress"
    project = var.project_id

    network     = google_compute_network.ops_net.self_link
    description = "Denies all egress traffic for ${var.project_id} instances"
    priority    = 65534
    direction   = "EGRESS"

    deny {
        protocol    = "all"
    }
    enable_logging = var.enable_extra_network_logging == 1 ? true : false
}

Which gave us the deprecation warning:
Warning: "enable_logging": [DEPRECATED] Deprecated in favor of log_config

So we replaced it by this:

resource "google_compute_firewall" "XXX_deny_all_egress" {
    count = var.deny_egress_XXX_network
    name        = "XXX-deny-all-egress"
    project = var.project_id

    network     = google_compute_network.ops_net.self_link
    description = "Denies all egress traffic for ${var.project_id} instances"
    priority    = 65534
    direction   = "EGRESS"

    deny {
        protocol    = "all"
    }
    dynamic "log_config" {
        for_each = var.enable_extra_network_logging == 1 ? [1] : []
        content {
            metadata             = "INCLUDE_ALL_METADATA"
        }
    }
}

Debug Output

Panic Output

Expected Behavior

The resource should've been updated to use the new config.

Actual Behavior

Since some of the environments don't need this extra logging, they have it already set to false from previous config. Changing it gives the following error.

Error: log_config cannot be defined when enable_logging is false

Steps to Reproduce

  1. Create a FW Resource with enable_logging set to false.
  2. Terrafrom apply
  3. Update resource to use log_config
  4. Terraform apply

Important Factoids

References

  • #0000
@ghost ghost added the bug label Sep 21, 2020
@edwardmedia edwardmedia self-assigned this Sep 21, 2020
@edwardmedia
Copy link
Contributor

@MrVentzi I can repro the issue. But both log_config and enable_logging basically point to the same thing. It is fine with me when I added log_config and set enable_logging to true. If you leave enable_logging false, that conflicts the config with addition of log_config. Does this make sense to you?

@iangelov
Copy link

I can confirm the issue as well (just saw it in our environment). Using the deprecated field to have the new field work doesn't make sense.

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

@iangelov To workaround the issue, when you add log_config, you can set the enable_logging to true at the same time.

@MrVentzi
Copy link
Author

@edwardmedia Yes I did see that if I have both the log_config and enable_logging it works, but then that defeats the purpose of deprecation. If enable_logging is deprecated, surely log_config should be enough, otherwise there is no value of changing the current config.

@mark-00
Copy link

mark-00 commented Sep 22, 2020

I don think this is a dup of #7322

I have an other error message and I specify neither log_config or enable_logging

@c2thorn
Copy link
Collaborator

c2thorn commented Sep 25, 2020

Hey @MrVentzi, I don't believe we can do anything about this due to provider limitations. When enable_logging gets set in state, there isn't a way to remove it from state. You can change its value, but you can't unset it without modifying your state file directly, or reimporting resource.

In the provider we can't tell the difference between a false and nil boolean (closed in favor of this issue) nor can we determine what is coming from the config vs what is just leftover from state. Essentially, we don't have a way to differentiate your scenario from someone explicitly trying to set enable_logging=false while setting log_config in their config.

If you've previously set enable_logging to false, you will have to set it back to true until the Terraform SDK can give us some new tools. Definitely not ideal, but at least there is an actionable workaround for now.

@c2thorn c2thorn closed this as completed Sep 25, 2020
@c2thorn
Copy link
Collaborator

c2thorn commented Sep 25, 2020

Instead of closing, I will set this to upstream-terraform. Whenever my linked issues get resolved, we should check back on this issue to see if solving it is possible.

@hallvors
Copy link

The way I ran into this was by enabling logging briefly for a firewall rule in the web console. (I have neither log_config nor enable_logging in my Terraform source files). The first deploy afterwards went fine (but Terraform toggled logging off again since it was not configured to be on..) - subsequent deploys give me this error. Rather unexpected..

@ppuschmann
Copy link

ppuschmann commented Mar 11, 2021

The way I ran into this was by enabling logging briefly for a firewall rule in the web console. (I have neither log_config nor enable_logging in my Terraform source files). The first deploy afterwards went fine (but Terraform toggled logging off again since it was not configured to be on..) - subsequent deploys give me this error. Rather unexpected..

I experience this similarly:

We had added this to some fw-rules:

log_config {
  metadata = "EXCLUDE_ALL_METADATA"
}

And we expected that the removal of this block would just remove/disable logging.

The only thing that's currently working for us in the sense of having a) disabled logging and b) no error messages, is to use the deprecated attribute enable_logging = false.

Yes, this is more related to #7322.

A different weird workaround would possibly something like

lifecycle {
  ignore_changes = [log_config]
}

But I'd really like to prevent using this.

@suckowbiz
Copy link

suckowbiz commented Mar 19, 2021

We also hit this one :(!

Steps to reproduce:

  1. Create a firewall rule without having log_config or enable_logging (deprecated) defined.
  2. Terraform apply
  3. Edit that firewall rule by enabling logging with adding a log_config definition:
log_config {
  metadata = "INCLUDE_ALL_METADATA"
}
  1. Terraform apply
  2. Now disable logging by removing the entire log_config as documented here: https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/compute_firewall#log_config
  3. Terraform apply
  4. Edit the firewall rule with an inline change. Here: edit the source_ranges.
  5. Terraform apply.

The outcome is this error:

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # module.generator.google_compute_firewall.p01vpc01-p01vpc01f2 will be updated in-place
  ~ resource "google_compute_firewall" "p01vpc01-p01vpc01f2" {
        id                      = "projects/obfuscated/global/firewalls/allow-http-out"
        name                    = "allow-http-out"
      ~ source_ranges           = [
          + "0.0.0.0/0",
          - "10.10.10.10/32",
        ]
        # (14 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.
root@f930494b5bba:/local# terragrunt apply plan.out 
module.generator.google_compute_firewall.p01vpc01-p01vpc01f2: Modifying... [id=projects/obfuscated/global/firewalls/allow-http-out]

Error: Error updating Firewall "projects/obfuscated/global/firewalls/allow-http-out": googleapi: Error 400: Invalid value for field 'resource.logConfig': '{  "enable": false}'. Logging for this firewall rule must be enabled to toggle logging options., invalid


ERRO[0002] Hit multiple errors:
Hit multiple errors:
exit status 1 

Note

Describing this rule with gcloud shows:

$ gcloud compute firewall-rules describe deny-incoming
creationTimestamp: '2021-03-19T06:08:35.543-07:00'
denied:
- IPProtocol: all
description: terraform managed
direction: INGRESS
disabled: false
id: 'obfuscated'
kind: compute#firewall
logConfig:
  enable: false
name: deny-incoming
network: https://www.googleapis.com/compute/v1/projects/obfuscated/global/networks/vpc1
priority: 1000
selfLink: https://www.googleapis.com/compute/v1/projects/obfuscated/global/firewalls/deny-incoming
sourceRanges:
- 0.0.0.0/0

Unfortunately at the moment there is no way to resolve this issue other than deleting and re-creating the entire rule!

@suckowbiz
Copy link

Ping @ndmckinley can you help out on this one?

Once logging was enabled and disabled afterwards it leaves a "corrupt" firewall rule that no longer can be edited with Terraform.

@djsmiley2k
Copy link

djsmiley2k commented Aug 11, 2021

Unfortunately at the moment there is no way to resolve this issue other than deleting and re-creating the entire rule!

Any advice on how to achieve this easily?

I feel like if upstream aren't fixing this, it should at least blow away the old rule and re-create it with the newly set logging options (be that disabled or enabled). It might be there's something simple I can add to our firewall module to achieve this, but I'm unsure what.

@Lord-Y
Copy link

Lord-Y commented Feb 1, 2022

Still having this issue with provider 4.9.0. Any news about the fix? I put enable_logging in my tf files ...

@github-actions github-actions bot added the forward/review In review; remove label to forward label Oct 5, 2023
@jsadasivuni-CLGX
Copy link

Still having this issue with provider 5.25.0 (latest) Any news about the fix?

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

No branches or pull requests