pulumi / pulumi-gcp

A Google Cloud Platform (GCP) Pulumi resource package, providing multi-language access to GCP
Apache License 2.0
179 stars 52 forks source link

Handle new default label #2357

Closed guineveresaenger closed 4 days ago

guineveresaenger commented 2 weeks ago

Upstream has added a new default attribution label.

Currently tests on storage.Bucket (but only Bucket) are failing due to a bug (also present upstream) on that resource:

  1. pulumi up successfully creates bucket and adds default label goog-terraform-provisioned
  2. A subsequent pulumi up shows a diff on the (non-authoritative)labels field, alerting the user that labels has a diff on it and will remove goog-terraform-provisioned: true.
  3. The actual bucket in the cloud, however, retains the label. Read-only labels fiels pulumiLabels andeffectiveLabelsare unchanged and continue to display thegoog-terraform-provisioned` label.

It appears that Bucket is the only resource affected in this way. Upstream issue is filed In the meantime, we'll want to explore user-friendly workarounds for this resource:

We also want to explore if and how we'd like to rename this label on our users' behalf, for clarity.

guineveresaenger commented 1 week ago

Update here.

Provider setting is a config value rename in resources.go: addTerraformAttributionLabel --> addPulumiAttributionLabel

Label itself is a patch: goog-terraform-provisioned --> goog-pulumi-provisioned

However, we do see some unexpected behavior under certain conditions.

Running a simple storage.Bucket pulumi up throws a diff on labels the first preview after create, showing the removal of this label from pulumi state. Notice that this label does not get removed from the cloud, and it does get encrypted to pulumiLabels and effectiveLabels - but not retained in labels. This also only happens if the labels field is otherwise unset - otherwise, the labels field remains untouched.

Import is affected - when importing a resource, this label is suggested in the import code. Adding or removing it, however, has no effect on the presence of the label. The only way to remove this label from the cloud resource is to set addPulumiAttributionLabel to false.

Sample code:

import * as pulumi from "@pulumi/pulumi";
import * as gcp from "@pulumi/gcp";

const bucket = new gcp.storage.Bucket("my-bucket", {
    location: "US",
});
export const labels = bucket.labels;

hashicorp/terraform-provider-google/issues/19323 has revealed this to be a Terraform core bug that for them only appears if you set Labels in an output block, and they suggest giving a default label as a workaround. On our end, it appears that our detailed diff output is our downfall - terraform only shows an anonymous diff on labels{}. In Terraform, this behavior was true in older versions as well. On our end, this diff is new. It does not feel like a desirable experience for our users, especially given the unintuitive behavior of goog-pulumi-provisioned only being removed when the global setting is set as false.

I wonder if https://github.com/pulumi/pulumi-gcp/pull/1946 is involved here - I will verify if this behavior replicates for resources other than Bucket.

guineveresaenger commented 1 week ago

Update: It is definitely the Bucket patch that is causing this. A Compute Instance does not have this issue.

guineveresaenger commented 1 week ago

Hm, the patch fix is now resulting in a diff on pulumiLabels for import.

guineveresaenger commented 1 week ago

It turned out that the conditions set in SetLabelsNoDefaults were accidentally writing the goog-pulumi-provisioned label to the resource labels field in the patch - goog-pulumi-provisioned is never in the provider-set defaultLabels, but the incoming set of labels to partition out according to lineage is also never empty. Additionally, this label setting happens during Create, so that a newly created Bucket always had an extra label on the resource.

An extra check for label key = goog-pulumi-provisioned and lineage = labels addresses this discrepancy. Here is the updated patch, with correct behavior: https://github.com/pulumi/pulumi-gcp/blob/e19dbc14d43a9304b6fc46c10bdd819b6a005eb8/patches/0009-Bucket-Skip-default-labels-for-Import-and-Create.patch

Finally, the randomized labels tests are verifying against pulumiLabels. We do expect this new label to appear in pulumiLabels, so this labels was set as an additional expectation in the label tests.

cleverguy25 commented 1 week ago

Added to epic https://github.com/pulumi/pulumi-gcp/issues/2290

pulumi-bot commented 4 days ago

This issue has been addressed in PR #2355 and shipped in release v8.0.0.