GCP resource plugin 7.x broke using gcp:storage:Bucket

dcox-at-bgrove commented 9 months ago

What happened?

We use CircleCI's Pulumi Orb to execute Pulumi in a CI/CD context. The Linux containers used to execute Pulumi start off without Pulumi or its plugins installed. We direct the orb to install Pulumi, then upon pulumi update, pulumi itself installs needed resource plugins such as gcp.

For several months, we've been using our Pulumi program to build/update, among other things, instances of gcp:storage:Bucket. But with the advent of gcp plugin version 7.0 around 2023-11-14, our jobs have begun failing in the pulumi update step: pulumi update --yes --stack redacted --cwd redacted

...
Diagnostics:
  pulumi:pulumi:Stack (*********.redacted-redacted):
    panic: interface conversion: interface {} is bool, not string
    goroutine 109 [running]:
    github.com/hashicorp/terraform-provider-google-beta/google-beta/tpgresource.SetLabelsDiff({0x57f7920?, 0x9629a00?}, 0xc007877830?, {0x4c5a680?, 0xc000842a80})
        /home/runner/work/pulumi-gcp/pulumi-gcp/upstream/google-beta/tpgresource/labels.go:84 +0x827
    github.com/hashicorp/terraform-provider-google-beta/google-beta/services/storage.ResourceStorageBucket.All.func6({0x57f7920, 0x9629a00}, 0x4cb7e4f?, {0x4c5a680, 0xc000842a80})
        /home/runner/go/pkg/mod/github.com/pulumi/terraform-plugin-sdk/v2@v2.0.0-20230912190043-e6d96b3b8f7e/helper/customdiff/compose.go:53 +0xab
    github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.schemaMap.Diff(0xc001421890, {0x57f7920, 0x9629a00}, 0xc0016711e0, 0xc0076fa0f0, 0xc000d4f6c0, {0x4c5a680, 0xc000842a80}, 0x0)
        /home/runner/go/pkg/mod/github.com/pulumi/terraform-plugin-sdk/v2@v2.0.0-20230912190043-e6d96b3b8f7e/helper/schema/schema.go:698 +0x4b4
    github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).SimpleDiff(0x0?, {0x57f7920?, 0x9629a00?}, 0xc0016711e0, 0xc001670ea0?, {0x4c5a680?, 0xc000842a80?})
        /home/runner/go/pkg/mod/github.com/pulumi/terraform-plugin-sdk/v2@v2.0.0-20230912190043-e6d96b3b8f7e/helper/schema/resource.go:967 +0xd5
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2.v2Provider.simpleDiff({0x1?, {0xffffffffffffffff?, 0xc00256d338?, 0x15e5823?}}, 0x44c8a40?, 0xc0076fa000?, 0x0?, 0xc00256d408?, {{{0x57f8028, 0xc0076ce4e8}}, ...}, ...)
        /home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/v3@v3.66.0/pkg/tfshim/sdk-v2/provider_diff.go:101 +0x8da
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2.v2Provider.Diff({0xc000da9e60?, {0x0?, 0x0?, 0x0?}}, {0x4ce14a4, 0x15}, {0x57f7e68?, 0xc007680f30}, {0x57c7840, 0xc0076fa0f0})
        /home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/v3@v3.66.0/pkg/tfshim/sdk-v2/provider_diff.go:63 +0x2d2
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge.(*Provider).Diff(0xc000629340, {0x57f7840?, 0xc0076c91d0?}, 0xc0076d4100)
        /home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/v3@v3.66.0/pkg/tfbridge/provider.go:672 +0x5bc
    github.com/pulumi/pulumi-terraform-bridge/x/muxer.(*muxer).Diff.func1({0x5810dc8?, 0xc000629340?})
        /home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/x/muxer@v0.0.7-0.20230801203955-5d215c892096/muxer.go:338 +0x30
    github.com/pulumi/pulumi-terraform-bridge/x/muxer.resourceMethod[...](0xc0006a8730?, 0x40, 0xc00256d760?)
        /home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/x/muxer@v0.0.7-0.20230801203955-5d215c892096/muxer.go:303 +0xb5
    github.com/pulumi/pulumi-terraform-bridge/x/muxer.(*muxer).Diff(0x0?, {0x57f7840?, 0xc0076c91d0?}, 0x40?)
        /home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/x/muxer@v0.0.7-0.20230801203955-5d215c892096/muxer.go:337 +0x65
    github.com/pulumi/pulumi/sdk/v3/proto/go._ResourceProvider_Diff_Handler.func1({0x57f7840, 0xc0076c91d0}, {0x4a58680?, 0xc0076d4100})
        /home/runner/go/pkg/mod/github.com/pulumi/pulumi/sdk/v3@v3.94.2/proto/go/provider_grpc.pb.go:575 +0x72
    github.com/grpc-ecosystem/grpc-opentracing/go/otgrpc.OpenTracingServerInterceptor.func1({0x57f7840, 0xc0076c8bd0}, {0x4a58680, 0xc0076d4100}, 0xc0076c63c0, 0xc007680a98)
        /home/runner/go/pkg/mod/github.com/grpc-ecosystem/grpc-opentracing@v0.0.0-20180507213350-8e809c8a8645/go/otgrpc/server.go:57 +0x3d0
    github.com/pulumi/pulumi/sdk/v3/proto/go._ResourceProvider_Diff_Handler({0x4c0ec00?, 0xc0006a8730}, {0x57f7840, 0xc0076c8bd0}, 0xc007682850, 0xc0006be1e0)
        /home/runner/go/pkg/mod/github.com/pulumi/pulumi/sdk/v3@v3.94.2/proto/go/provider_grpc.pb.go:577 +0x135
    google.golang.org/grpc.(*Server).processUnaryRPC(0xc000608000, {0x580b0a0, 0xc005d3c1a0}, 0xc0075ff440, 0xc006873ad0, 0x95dc508, 0x0)
        /home/runner/go/pkg/mod/google.golang.org/grpc@v1.58.3/server.go:1374 +0xde7
    google.golang.org/grpc.(*Server).handleStream(0xc000608000, {0x580b0a0, 0xc005d3c1a0}, 0xc0075ff440, 0x0)
        /home/runner/go/pkg/mod/google.golang.org/grpc@v1.58.3/server.go:1751 +0x9e7
    google.golang.org/grpc.(*Server).serveStreams.func1.1()
        /home/runner/go/pkg/mod/google.golang.org/grpc@v1.58.3/server.go:986 +0xbb
    created by google.golang.org/grpc.(*Server).serveStreams.func1 in goroutine 38
        /home/runner/go/pkg/mod/google.golang.org/grpc@v1.58.3/server.go:997 +0x145

  gcp:storage:Bucket (redacted):
    error: error reading from server: EOF

Exited with code exit status 255

This still occurs today with gcp plugin version 7.2.1.

If I install the latest 6.x release of the gcp plugin (execute pulumi plugin rm resource gcp then pulumi plugin install resource gcp 6.67.1), then pulumi update works as before.

Example

Ours is a yaml-based Pulumi program. The error appears to be in processing this resource:

  redacted-bucket:
    type: gcp:storage:Bucket
    properties:
      labels:
        redacted
      lifecycleRules:
        - action:
            type: Delete
          condition:
            age: 3
            withState: ANY
      forceDestroy: true
      location: redacted
      name: redacted
      project: ${gcp_project}
      publicAccessPrevention: enforced
      uniformBucketLevelAccess: true
    options:
      protect: true

Output of `pulumi about`

(On my Windows developer machine, on which essentially the same error occurs)

CLI
Version      3.94.2
Go Version   go1.21.3
Go Compiler  gc

Plugins
NAME  VERSION
yaml  unknown

Host
OS       Microsoft Windows 10 Pro
Version  10.0.19045 Build 19045
Arch     x86_64

This project is written in yaml

Current Stack: redacted/redacted.redacted/redacted

TYPE                                                    URN
pulumi:pulumi:Stack                                     urn:pulumi:redacted::redacted.redacted::pulumi:pulumi:Stack::redacted
pulumi:providers:gcp                                    urn:pulumi:redacted::redacted.redacted::pulumi:providers:gcp::default
gcp:storage/bucket:Bucket                               urn:pulumi:redacted::redacted.redacted::gcp:storage/bucket:Bucket::redacted
gcp:storage/bucket:Bucket                               urn:pulumi:redacted::redacted.redacted::gcp:storage/bucket:Bucket::redacted
gcp:pubsub/topic:Topic                                  urn:pulumi:redacted::redacted.redacted::gcp:pubsub/topic:Topic::redacted
...
gcp:logging/metric:Metric                               urn:pulumi:redacted::redacted.redacted::gcp:logging/metric:Metric::redacted
gcp:logging/metric:Metric                               urn:pulumi:redacted::redacted.redacted::gcp:logging/metric:Metric::redacted
gcp:logging/metric:Metric                               urn:pulumi:redacted::redacted.redacted::gcp:logging/metric:Metric::redacted
gcp:logging/metric:Metric                               urn:pulumi:redacted::redacted.redacted::gcp:logging/metric:Metric::redacted
gcp:logging/metric:Metric                               urn:pulumi:redacted::redacted.redacted::gcp:logging/metric:Metric::redacted
gcp:logging/metric:Metric                               urn:pulumi:redacted::redacted.redacted::gcp:logging/metric:Metric::redacted
gcp:monitoring/notificationChannel:NotificationChannel  urn:pulumi:redacted::redacted.redacted::gcp:monitoring/notificationChannel:NotificationChannel::redacted
gcp:monitoring/notificationChannel:NotificationChannel  urn:pulumi:redacted::redacted.redacted::gcp:monitoring/notificationChannel:NotificationChannel::redacted
gcp:monitoring/alertPolicy:AlertPolicy                  urn:pulumi:redacted::redacted.redacted::gcp:monitoring/alertPolicy:AlertPolicy::redacted

Found no pending operations associated with redacted

Backend
Name           pulumi.com
URL            https://app.pulumi.com/redacted
User           redacted
Organizations  redacted, redacted
Token type     personal

No dependencies found

Pulumi locates its logs in C:\Users\redacted\AppData\Local\Temp by default

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction. To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

iwahbe commented 9 months ago

Hi @dcox-at-bgrove. I'm sorry to hear that. I tried to reproduce with gcp 7.2.1 against this program:

name: secret-random-yaml
runtime: yaml
variables:
  gcp_project: pulumi-development
resources:
  redacted-bucket:
    type: gcp:storage:Bucket
    properties:
      labels:
        foo: bar
      lifecycleRules:
        - action:
            type: Delete
          condition:
            age: 3
            withState: ANY
      forceDestroy: true
      location: us-west1
      name: redacted-140879
      project: ${gcp_project}
      publicAccessPrevention: enforced
      uniformBucketLevelAccess: true
    options:
      protect: true

I was unable to trigger the panic. From the stack trace, it looks like this has something to do with labels. Can you provide a less redacted view of the labels you were using?[^1] Are you using default labels at the provider level?

[^1]: I don't think I need the values you are using, just the shape.

dcox-at-bgrove commented 9 months ago

redacted-bucket:
  type: gcp:storage:Bucket
  properties:
    labels:
      vanta-contains-user-data: false
      vanta-description: redacted
      vanta-non-prod: ${redacted}
      vanta-owner: redacted
      purpose: redacted

iwahbe commented 9 months ago

Thanks @dcox-at-bgrove. I have a repro.

To repro, run pulumi up with this program. Then switch out the version used (6.55.1 to 7.2.1) and run pulumi preview. This will panic.

name: dev-yaml
runtime: yaml
resources:
  gcp:
    type: pulumi:providers:gcp
    properties:
      project: pulumi-development
    options:
      version: 6.55.1
      # version: 7.2.1
    defaultProvider: true
  bucket:
    type: gcp:storage:Bucket
    properties:
      location: US
      labels:
        bad-bool: true

iwahbe commented 9 months ago

@dcox-at-bgrove You are passing vanta-contains-user-data: false to labels. pulumi-gcp should convert that false (of type bool) into "false" (of type string). For some reason, it's missing the conversion and that is causing the panic.

As a work-around, you can replace the bool value with a string value:

-      vanta-contains-user-data: false
+      vanta-contains-user-data: "false"

This fixes the panic.

VenelinMartinov commented 9 months ago

It seems the bug was introduced in https://github.com/pulumi/pulumi-gcp/releases/tag/v7.0.0 - I successfully reproduced the issue there but not in https://github.com/pulumi/pulumi-gcp/releases/tag/v6.67.1

Also as far as I could see, we are passing approximately the same parameters into upstream's SimpleDiff, both attached here. new_params.txt old_params.txt

VenelinMartinov commented 9 months ago

Looks like something changed on the TF side - as far as I can see we use the same parameters for SimpleDiff but it now throws an error.
I tried replicating this in a TF program but they don't have the same issue - it seems that bool(true) was converted to "true" on TF GCP provider versions as well. They likely have a separate conversion step before passing to the provider.

We have existing code for implicit type conversion of boolean values in the bridge but it looks like it doesn't apply for elements of a map - https://github.com/pulumi/pulumi-terraform-bridge/pull/1557 should fix this.
The downside is that it converts bool(true) into "true" while it was previously converted to "1", which causes an unnecessary diff when upgrading.

t0yv0 commented 9 months ago

It appears that we have a choice here around how to coalesce boolean values to the expected string type. The canonical mapping (how it usually happens in bridged providers) translates true => "true"; false => "false". We found a bug in the bridge that fails to do it for the code in question. We definitely will fix that. However that introduces another wrinkle. For users of 6.x provider that manifested the true => "1" and false => "0" translation, this will introduce an observable change that the "1" label will be relabelled as "true".

Wondering here if it is worth it to put some additional logic in the GCP provider, perhaps in PreCheckCallback across all resources that would help with preserving this 6.x behavior for labels specifically, making it the new canonical behavior? Or will be be OK with the slightly breaking change that switches "1" to "true"?

pulumi / pulumi-gcp