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

Permadiff in pulumiLabels and effectiveLabels with empty labels #2372

Closed
VenelinMartinov opened this issue Sep 9, 2024 · 8 comments · Fixed by #2386
Closed

Permadiff in pulumiLabels and effectiveLabels with empty labels #2372

VenelinMartinov opened this issue Sep 9, 2024 · 8 comments · Fixed by #2386
Assignees
Labels
kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Milestone

Comments

@VenelinMartinov
Copy link
Contributor

VenelinMartinov commented Sep 9, 2024

Describe what happened

A user reported seeing a diff in effectiveLabels/pulumiLabels under 7.38

~ gcp:storage:Bucket example-gcp-ais-adv-bucket updating (0s) [diff: ~effectiveLabels,pulumiLabels

Sample program

GRPC logs:

{
    "method": "/pulumirpc.ResourceProvider/Diff",
    "request": {
        "id": "ID",
        "urn": "URN",
        "olds": {
            "__meta": "{\"e2bfb730-ecaa-11e6-8f88-34363bc7c4c0\":{\"create\":600000000000,\"read\":240000000000,\"update\":240000000000},\"schema_version\":\"1\"}",
            "autoclass": null,
            "cors": [],
            "customPlacementConfig": null,
            "defaultEventBasedHold": false,
            "effectiveLabels": {
                "tag": "2020-04-29",
                "prov": "cloud"
            },
            "enableObjectRetention": false,
            "encryption": {
                "defaultKmsKeyName": "KEY_NAME"
            },
            "forceDestroy": false,
            "id": "ID",
            "labels": {
                "app_id": "",
                "app_name": "",
                "env": "",
                "owner": "",
                "div": "",
                "sub_env": "",
                "tag": "2020-04-29",
                "prov": "cloud"
            },
            "lifecycleRules": [
            ],
            "location": "US",
            "logging": null,
            "name": "name",
            "project": "project",
            "projectNumber": 0,
            "publicAccessPrevention": "enforced",
            "pulumiLabels": {
                "tag": "2020-04-29",
                "prov": "cloud"
            },
            "requesterPays": false,
            "retentionPolicy": null,
            "rpo": "DEFAULT",
            "selfLink": "link",
            "softDeletePolicy": {
                "effectiveTime": "2024-09-08T19:23:04.697Z",
                "retentionDurationSeconds": 604800
            },
            "storageClass": "STANDARD",
            "uniformBucketLevelAccess": true,
            "url": "url",
            "versioning": {
                "enabled": true
            },
            "website": null
        },
        "news": {
            "__defaults": [
                "name"
            ],
            "encryption": {
                "__defaults": [],
                "defaultKmsKeyName": "name"
            },
            "forceDestroy": false,
            "labels": {
                "app_id": "",
                "app_name": "",
                "env": "",
                "owner": "",
                "div": "",
                "sub_env": "",
                "tag": "2020-04-29",
                "prov": "cloud"
            },
            "lifecycleRules": [
            ],
            "location": "us",
            "name": "name",
            "project": "project",
            "publicAccessPrevention": "enforced",
            "storageClass": "STANDARD",
            "uniformBucketLevelAccess": true,
            "versioning": {
                "__defaults": [],
                "enabled": true
            }
        },
        "oldInputs": {
            "__defaults": [
                "name"
            ],
            "encryption": {
                "__defaults": [],
                "defaultKmsKeyName": "name"
            },
            "forceDestroy": false,
            "labels": {
                "app_id": "",
                "app_name": "",
                "env": "",
                "owner": "",
                "div": "",
                "sub_env": "",
                "tag": "2020-04-29",
                "prov": "cloud"
            },
            "lifecycleRules": [
            ],
            "location": "us",
            "name": "name",
            "project": "project",
            "publicAccessPrevention": "enforced",
            "storageClass": "STANDARD",
            "uniformBucketLevelAccess": true,
            "versioning": {
                "__defaults": [],
                "enabled": true
            }
        }
    },
    "response": {
        "stables": [
            "enableObjectRetention",
            "location",
            "name",
            "project"
        ],
        "changes": "DIFF_SOME",
        "diffs": [
            "effectiveLabels",
            "pulumiLabels"
        ],
        "detailedDiff": {
            "effectiveLabels": {
                "kind": "UPDATE"
            },
            "pulumiLabels": {
                "kind": "UPDATE"
            }
        },
        "hasDetailedDiff": true
    },
    "metadata": {
        "kind": "resource",
        "mode": "client",
        "name": "gcp"
    }
}

Log output

No response

Affected Resource(s)

No response

Output of pulumi about

.

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).

@VenelinMartinov VenelinMartinov added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Sep 9, 2024
@VenelinMartinov
Copy link
Contributor Author

Attempted to repro

__main__.py:

import pulumi
import pulumi_gcp as gcp
from pulumi_gcp import storage


prov = gcp.Provider(
    "my-provider",
    default_labels={
        "cloud": "my-cloud",
        "empty_default_label": "",
    },
)

# Create a GCP resource (Storage Bucket)
bucket = storage.Bucket(
    "my-bucket",
    location="US",
    labels={
        "environment": "dev",
        "team": "data",
        "empty": "",
    },
    opts=pulumi.ResourceOptions(provider=prov),
)

pulumi.export("bucket_name", bucket.url)

repro.sh:

#! /bin/bash
set -e

rm -f grpc.json 
pulumi down --yes

sed -i '' 's/7.36.0/7.35.0/g' requirements.txt
pulumi install
PULUMI_DEBUG_GRPC="grpc.json" pulumi up --yes

eval "$(pulumi stack output --shell)"
gcloud storage buckets update $bucket_name --update-labels=unmanaged=value,unmanaged_empty=

sed -i '' 's/7.35.0/7.36.0/g' requirements.txt
pulumi install
PULUMI_DEBUG_GRPC="grpc.json" pulumi up --yes

pulumi preview --diff

Note here that I have tried:

  • Labels on the bucket
  • Empty labels on the bucket
  • Default labels
  • Empty default labels
  • Unmanaged labels
  • Empty unmanaged labels

This should cover all the ways to set a label on a GCP bucket but I have been unable to reproduce the issue so far.

@VenelinMartinov VenelinMartinov added needs-repro Needs repro steps before it can be triaged or fixed and removed needs-triage Needs attention from the triage team labels Sep 9, 2024
@VenelinMartinov
Copy link
Contributor Author

VenelinMartinov commented Sep 9, 2024

A few new notes on the issue:

  • The issue happens when running 7.38 -> 7.38 as well - so it is NOT an upgrade issue.
  • The user is using the GCP bucket from an MLC
  • When going from 7.38 -> 7.38 the effectiveLabels and pulumiLabels properties are actually listed as deleted.
  • The diff happens every time, even after accepting it.

@VenelinMartinov VenelinMartinov changed the title Diff in pulumiLabels and effectiveLabels when upgrading to 7.36 without code changes Permadiff in pulumiLabels and effectiveLabels Sep 9, 2024
@VenelinMartinov
Copy link
Contributor Author

I believe I have a repro:

import pulumi
from pulumi_gcp import storage


bucketOne = storage.Bucket(
    "my-bucket-one",
    location="US",
    labels={
        "environment": "dev",
        "team": "data",
        "empty": "",
    },
)

# Create a GCP resource (Storage Bucket)
bucket = storage.Bucket(
    "my-bucket",
    location="US",
    labels={
        "environment": "dev",
        "team": "data",
        "empty": "",
        "other_bucket": bucketOne.name,
    },
)

pulumi.export("bucket_name", bucket.url)

Seems like the permadiff happens if the labels contain an output. Confirmed this does not happen in 7.35

@VenelinMartinov VenelinMartinov changed the title Permadiff in pulumiLabels and effectiveLabels Permadiff in pulumiLabels and effectiveLabels when value is an output Sep 9, 2024
@VenelinMartinov VenelinMartinov removed the needs-repro Needs repro steps before it can be triaged or fixed label Sep 9, 2024
@VenelinMartinov
Copy link
Contributor Author

Simplified the repro with some help - the second resource is not necessary:

import pulumi
from pulumi_gcp import storage


bucket = storage.Bucket(
    "my-bucket",
    location="US",
    labels={
        "tag": pulumi.Output.from_input("val"),
        "empty": "",
    },
)

@VenelinMartinov
Copy link
Contributor Author

Simplified it a bit more:

from pulumi_gcp import storage


bucket = storage.Bucket(
    "my-bucket",
    location="US",
    labels={
        "static": "static",
        "empty": "",
    },
)

Looks like the output type is not necessary either - we need to have an empty label and at least one more label for the permadiff to occur.

What happens is that the state has:

"terraform_labels":cty.MapVal(map[string]cty.Value{"static":cty.StringVal("static")})

while the plan is:

"terraform_labels":cty.MapVal(map[string]cty.Value{"empty":cty.UnknownVal(cty.String), "static":cty.StringVal("static")})

Unfortunately this reproduces in TF too:

provider "google" {
  region       = "us-central1"
}

resource "google_storage_bucket" "bucket" {
  name     = "example-bucket12312322314"
  location = "US"

  labels = {
    "static" = "static"
    "empty" = ""
  }
}

run terraform apply -auto-approve then terraform plan and observe the diff:

Terraform will perform the following actions:

  # google_storage_bucket.bucket will be updated in-place
  ~ resource "google_storage_bucket" "bucket" {
      ~ effective_labels            = {
          + "empty"                      = (known after apply)
            # (2 unchanged elements hidden)
        }
        id                          = "example-bucket12312322314"
        name                        = "example-bucket12312322314"
      ~ terraform_labels            = {
          + "empty"                      = (known after apply)
            # (2 unchanged elements hidden)
        }
        # (14 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Note that this happens under -refresh=true as well.

@VenelinMartinov VenelinMartinov changed the title Permadiff in pulumiLabels and effectiveLabels when value is an output Permadiff in pulumiLabels and effectiveLabels with empty labels Sep 10, 2024
@VenelinMartinov
Copy link
Contributor Author

Upstream issue: hashicorp/terraform-provider-google#16750

@iwahbe iwahbe self-assigned this Sep 11, 2024
@VenelinMartinov
Copy link
Contributor Author

Built a synthetic repro for the issue here: pulumi/pulumi-terraform-bridge#2412

Ran it with and without PRC to compare the values we produce and see why non-PRC passes while PRC produces the perma-diff. Very little difference, only:

RawState.v:

nil(in PRC) vs non-PRC:

v:  map[string]interface {}{
	"id":               nil,
	"labels":           nil,
	"terraform_labels": nil,
},

in both the state and plan.

@mjeffryes mjeffryes added this to the 0.110 milestone Sep 12, 2024
@iwahbe iwahbe mentioned this issue Sep 12, 2024
7 tasks
iwahbe added a commit to pulumi/pulumi-terraform-bridge that referenced this issue Sep 17, 2024
PlanStateEdit allows a bridged provider author to correct the diff generated by the TF
provider with additional information: the original Pulumi encoded inputs. This allows
Pulumi to recover from information lost in the Terraform encoding.

One example of this information loss is pulumi/pulumi-gcp#2372, where SDKv2's
representation of an unknown value `""` is not distinguished from an actual empty
string. A sister PR uses this capability to resolve pulumi/pulumi-gcp#2372.
iwahbe added a commit to pulumi/pulumi-terraform-bridge that referenced this issue Sep 18, 2024
PlanStateEdit allows a bridged provider author to correct the diff
generated by the TF
provider with additional information: the original Pulumi encoded
inputs. This allows
Pulumi to recover from information lost in the Terraform encoding.

The ability to recover information from Pulumi's representation is only
available outside the normal TF information flow. This is why diff
customizers can't fill the role of PlanStateEdit.

One example of this information loss is pulumi/pulumi-gcp#2372, where
SDKv2's
representation of an unknown value `""` is not distinguished from an
actual empty
string. A sister PR uses this capability to resolve (not this one GH)
pulumi/pulumi-gcp#2372.
iwahbe added a commit that referenced this issue Sep 18, 2024
This PR takes advantage of pulumi/pulumi-terraform-bridge#2417 to fixup the incorrect
planned state caused by empty labels.

Fixes #2372
@iwahbe iwahbe closed this as completed in f8d6d51 Sep 18, 2024
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Sep 18, 2024
@pulumi-bot
Copy link
Contributor

This issue has been addressed in PR #2386 and shipped in release v8.2.0.

VenelinMartinov added a commit that referenced this issue Sep 25, 2024
This fixes the empty label handling in the GCP Cluster resource.

In the fix for #2372
(#2386 and
pulumi/pulumi-terraform-bridge#2417) we did not
know that the labels property in GCP is sometimes overloaded, ex GCP
Custer.

For the Cluster resource, the GCP labels are under `resource_labels`,
not `labels`

This PR adds the logic to the empty labels fix and adds a regression
test.

fixes #2395
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants