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.29k stars 1.72k forks source link

`google_cloud_run_v2_service`'s `volume_mounts` doesn't expose `read_only` boolean #16605

Open mattmoor opened 9 months ago

mattmoor commented 9 months ago

Community Note

Terraform Version

Affected Resource(s)

Terraform Configuration Files

resource "google_cloud_run_v2_service" "recorder-service" {
  project  = var.project_id
  name     = "${var.name}-recorder"
  location = var.region

  provider     = google-beta # For empty_dir
  launch_stage = "BETA"

  template {
    service_account = google_service_account.recorder.email
    containers {
      image = cosign_sign.recorder-image.signed_ref

      ports {
        container_port = 8080
      }

      env {
        name  = "LOG_PATH"
        value = "/logs"
      }
      volume_mounts {
        name = "logs"
        mount_path = "/logs"
        # read_only = false
      }
    }
    containers {
      image = cosign_sign.logrotate-image.signed_ref

      env {
        name  = "LOGROTATE_BUCKET"
        value = "gs://mattmoor-dev-recorder" # TODO: provision this.
      }
      volume_mounts {
        name = "logs"
        mount_path = "/logs"
        # read_only = false
      }
    }
    volumes {
      name = "logs"
      empty_dir {}
    }
  }
}

Debug Output

Panic Output

Expected Behavior

I can configure whether the volume mount is read-only.

Actual Behavior

The volume mount defaults to read-only, which makes it an unusable medium for inter-container message-passing.

Steps to Reproduce

Try to create a writeable empty_dir volume mount (multi-container or not, but it's pretty critical for multi-container scenarios)

Important Factoids

Being able to share state through writeable volumes is pretty critical for multi-container scenarios.

The only documented fields on volume_mounts today are name and mount_path, but no read_only: https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/cloud_run_v2_service#nested_volume_mounts

References

cc @steren

b/313620407

mattmoor commented 9 months ago

FWIW, after deploying the service via TF, I am able to edit the volume_mounts via the Cloud console to achieve the desired state, but I'm guessing it will revert the next time I apply.

edwardmedia commented 9 months ago

@mattmoor can you share the debug for the apply?

mattmoor commented 9 months ago

The field isn’t part of the schema, so it’s rejected outright. I’m on the latest release of the provider iirc (5.7?).

I can try to include it when I get to a real computer.

mattmoor commented 9 months ago

Here's the output without any debug logging enabled FWIW:

│ Error: Unsupported argument
│ 
│   on ../../../terraform/modules/gcp/eventing/recorder.tf line 53, in resource "google_cloud_run_v2_service" "recorder-service":
│   53:         read_only = false
│ 
│ An argument named "read_only" is not expected here.

I'm not sure the debug logging will tell us anything that this doesn't.

edwardmedia commented 9 months ago

@mattmoor I do not believe read_only can be added here as the api does not expose it under VolumeMount

Does this make sense?

mattmoor commented 9 months ago

Maybe this is a v1/v2 bug. I see it here (and more): https://cloud.google.com/run/docs/reference/rest/v1/Container#VolumeMount

The YAML view in the UI after I deploy shows the field:

image

This is looking more like a product bug, and less like a TF provider bug. cc @steren

steren commented 9 months ago

Looks like it's missing in v2 indeed. I'll forward internally.

steren commented 9 months ago

Internal bug ID: b/313457029

mattmoor commented 9 months ago

Hmm, I was hoping to work around this by using the v1 Service, which as I showed above does have these fields in VolumeMount, but it looks like the TF provider for the v1 service doesn't have these either: https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/cloud_run_service#nested_volume_mounts

mattmoor commented 9 months ago

The fields appear to be missing here, but I don't know what the source of truth for this "magic module" stuff is, so not sure if/how I could PR a fix: https://github.com/hashicorp/terraform-provider-google/blob/331733a6648407005c790a283f4e12ebc9216fc7/google/services/cloudrun/resource_cloud_run_service.go#L1995-L1998

steren commented 9 months ago

I do not recommend switching to v1 just for that, v2 is our recommended TF resource.

mattmoor commented 9 months ago

Yeah, v1 doesn't help me if it has the same issue (manual or not).

bskaplan commented 9 months ago

Cloud Run does not support volume_mount.read_only. The field exists in the v1 API but was never checked so we didn't add it to the Terraform providers or the v2 API.

mattmoor commented 9 months ago

@bskaplan so are you saying that it unconditionally mounts as read-only or read-write?

bskaplan commented 9 months ago

It unconditionally mounts them as read-write, although the underlying volume can still be read-only (as in the case of secrets)

mattmoor commented 9 months ago

@bskaplan awesome, so the UI reporting readOnly: true is superficial.

I could still see value in having the volume be read-only to one container, and read-write for the other in the sidecar pattern, but it's good to know that this isn't blocking the use-case I had in mind, so thanks.