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

Errors disabling google_compute_region_backend_service.log_config #12582

Closed
AdamMAtWork opened this issue Sep 16, 2022 · 6 comments
Closed

Errors disabling google_compute_region_backend_service.log_config #12582

AdamMAtWork opened this issue Sep 16, 2022 · 6 comments
Assignees
Labels

Comments

@AdamMAtWork
Copy link

AdamMAtWork commented Sep 16, 2022

It's not currently possible to disable logging on a google_compute_region_backend_service resource, once it has been enabled with a non-zero sample rate.

Affected provider versions: 3.54, 4.36

If I create a

resource "google_compute_region_backend_service" "default" {
  log_config {
      enable = false
      sample_rate = 0
    }
}

, the resource is created with logging disabled. This is working as expected.

If I then update the resource using

resource "google_compute_region_backend_service" "default" {
  log_config {
      enable  = true
      sample_rate = 0
    }
}

then the log_config is enabled, but the sample_rate remains zero. This completes successfully, although serves no purpose as a sampling rate above zero is required. So, let's set it to some useful value:

resource "google_compute_region_backend_service" "default" {
  log_config {
      enable = true
      sample_rate = 0.25
    }
}

This sets the sampling_rate to 25%, and applies as expected.

So far, so good.

But now, we need to disable the logging. So we try:

resource "google_compute_region_backend_service" "default" {
  log_config {
      enable = false
      sample_rate = 0
    }
}

This results in the following error:

Error: Error updating RegionBackendService "projects/foo/regions/x/backendServices/bar": googleapi: Error 400: Invalid value for field 'resource.logConfig': '{  "enable": false,  "sampleRate": 0.25}'. Logging for this backend service must be enabled to toggle logging options., invalid

  on ../../../modules/main.tf line 00, in resource "google_compute_region_backend_service" "default":
  00: resource "google_compute_region_backend_service" "default" {

In an attempt to get around this, let's try setting the sampling_rate to zero first, and then setting enabled to false:

resource "google_compute_region_backend_service" "default" {

  log_config {
      enable          = true
      sample_rate = 0
    }
}

This takes about 30s to complete while the API processes the request, and completes without any errors. However, it seems the value of zero is ignored by the API, as the next plan indicates the same change still needs to be applied:

      ~ log_config {
            enable      = true
          ~ sample_rate = 0.25 -> 0
        }

Repeating this test with sample_rate = null yields an identical result.

This exposes 2 problems:

  1. It's not possible to set sample_rate to zero or null once it has been set to any non-zero value
  2. It's not possible to set enable to false as the provider produces a log_config object that always includes the sample_rate value.

A review of the provider code file resource_compute_region_backend_service.go reveals the following function which appears to be responsible for producing the value of the log_config object:

func expandComputeRegionBackendServiceLogConfig(v interface{}, d TerraformResourceData, config *Config) (interface{}, error) {
	l := v.([]interface{})
	if len(l) == 0 || l[0] == nil {
		return nil, nil
	}
	raw := l[0]
	original := raw.(map[string]interface{})
	transformed := make(map[string]interface{})

	transformedEnable, err := expandComputeRegionBackendServiceLogConfigEnable(original["enable"], d, config)
	if err != nil {
		return nil, err
	} else {
		transformed["enable"] = transformedEnable
	}

	transformedSampleRate, err := expandComputeRegionBackendServiceLogConfigSampleRate(original["sample_rate"], d, config)
	if err != nil {
		return nil, err
	} else if val := reflect.ValueOf(transformedSampleRate); val.IsValid() && !isEmptyValue(val) {
		transformed["sampleRate"] = transformedSampleRate
	}

	return transformed, nil
}

I'm a long-time software developer, but am not super familier with golang, so my assessment is probably inaccurate, but in this function, the expansion of the value for sample rate appears to contain the following errors:

  1. The value of the sample_rate is a number, therefore the test !isEmptyValue(val) will always be true, since isEmptyValue(val) will always be false - a number value will always have a value because there is no way in HCL or json to express an empty number. Unlike an empty string, which is quoted, numbers are not quoted. If I try to provide the value 'null', it is either ignored by the API or the provider is sending the previous value.
  2. The object attribute sample_rate should not be sent at all if the value of enable is false, but the tests deciding whether to include sample_rate omit this check. An extra check, something like the following, is missing:
   } else if val := reflect.ValueOf(transformedEnable); val.IsValid() && isFalse(val) {
   	/* do not add sampleRate */
   } else if val := reflect.ValueOf(transformedSampleRate); val.IsValid() && !isEmptyValue(val) {
   	transformed["sampleRate"] = transformedSampleRate
   }
@edwardmedia
Copy link
Contributor

@AdamMAtWork I see the provider sending the related data to API. I wonder if there is a rule that the config does not follow. Are you able to disable it (problem#2) via other tools?

Logging for this backend service must be enabled to toggle logging options

@AdamMAtWork
Copy link
Author

@edwardmedia

I see the provider sending the related data to API. I wonder if there is a rule that the config does not follow. Are you able to disable it (problem#2) via other tools?

Yes, the Google REST API accepts the following logConfig with a value of false:

  "logConfig": {
    "enable": false
  }

Submitting this to the update endpoint results in a success message.

@AdamMAtWork
Copy link
Author

@edwardmedia
Interestingly, submitting the following update to an ILB that already has logging enabled results in a success message, and a followup GET request retrieves the updated value:

  "logConfig": {
    "enable": true,
    "sampleRate": 0.0
  }

@edwardmedia
Copy link
Contributor

@AdamMAtWork we should be able to update the sampleRate to 0, but I am not able to update the enable from true to false with below error. I wondered if this is an API rule, and there is not much we can do with it at the provider level. Please let me know if you see the behavior differently.

Logging for this backend service must be enabled to toggle logging options

@edwardmedia
Copy link
Contributor

edwardmedia commented Oct 4, 2022

@AdamMAtWork closing this issue. I believe this is by design

@github-actions
Copy link

github-actions bot commented Nov 4, 2022

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 4, 2022
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

3 participants