Incorrect tags applied when tag is empty

[t0yv0]

What happened?

Customer reports a regression where having an empty tag value causes all tags to disappear with 6.5.x version of the provider (I also reproduced on 6.2.x).

This program:

package main

import (
	"fmt"

	"github.com/pulumi/pulumi-aws/sdk/v6/go/aws/s3"
	"github.com/pulumi/pulumi/sdk/v3/go/pulumi"
)

func main() {

	pulumi.Run(func(ctx *pulumi.Context) error {

		bucket, err := s3.NewBucketV2(ctx, "test-bucket-anton-4", &s3.BucketV2Args{
			Bucket:       pulumi.String("test-bucket-anton-4"),
			ForceDestroy: pulumi.Bool(false),
			Tags: pulumi.StringMap{
				"cust_app":       pulumi.String(""),
				"cust_ver":  pulumi.String("2020-04-29"),
				"cust_url": pulumi.String("https://cust.com/xywvBZgKsUsN"),
			},
		})

		fmt.Printf("%p\n", bucket)

		if err != nil {
			return err
		}

		ctx.Export("tags", bucket.Tags)
		return nil
	})
}

pulumi up produces an empty tags export. Commenting out cust_app with empty string makes it produce 2 tags.

Example

See above.

Output of pulumi about

CLI          
Version      3.86.0
Go Version   go1.21.1
Go Compiler  gc

Plugins
NAME  VERSION
aws   6.2.1
go    unknown

Host     
OS       darwin
Version  14.0
Arch     x86_64

This project is written in go: executable='/Users/t0yv0/bin/go' version='go version go1.21.0 darwin/amd64'

Current Stack: t0yv0/test-chris-bug/dev

TYPE                      URN
pulumi:pulumi:Stack       urn:pulumi:dev::test-chris-bug::pulumi:pulumi:Stack::test-chris-bug-dev
pulumi:providers:aws      urn:pulumi:dev::test-chris-bug::pulumi:providers:aws::default
aws:s3/bucketV2:BucketV2  urn:pulumi:dev::test-chris-bug::aws:s3/bucketV2:BucketV2::test-bucket-anton-4


Found no pending operations associated with dev

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

Dependencies:
NAME                                 VERSION
github.com/pulumi/pulumi-aws/sdk/v6  6.2.1
github.com/pulumi/pulumi/sdk/v3      3.86.0

Pulumi locates its logs in /var/folders/gk/cchgxh512m72f_dmkcc3d09h0000gp/T/ 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).

[t0yv0]

This appears to be an upstream issue hashicorp/terraform-provider-aws#31941

Pulumi versions affected:

 6.6.0 affected   v5.21.0 upstream
 6.5.0
 6.4.1
 6.4.0
 6.3.0
 6.2.1 affected
 6.2.0
 6.1.0
 6.0.4 affected
 6.0.3
 6.0.2 affected
 5.42.0 not affected

Repro in TF:

// build an aws s3 bucket in Terraform HCL

resource "aws_s3_bucket" "anton-tf-test-aws-h-9" {
  bucket = "anton-tf-test-aws-h-9"
  force_destroy = false
  tags = {
    cust_sys_app_id = ""
    cust_sys_tag_version = "2020-04-29"
    cust_tagging_standard = "https://cust.com/xywvBZgKsUsN"
  }
}

terraform {
  required_providers {
    aws = {
      source  = "hashicorp/aws"
      version = "= 5.21.0"
    }
  }
}

terraform apply followed by

aws s3api get-bucket-tagging --bucket anton-tf-test-aws-h-9

observe no tags reported.

[t0yv0]

I've been diving deeper here since Pulumi reimplements tags + default tags merging; however this does not seem to be affecting this bug.

This function:

https://github.com/hashicorp/terraform-provider-aws/blob/master/internal/service/s3/bucket.go#L701

Receives different values for tags_all dependent on the presence of an empty-valued tag.

    d TAGS =  map[cust_sys_app_id: cust_sys_tag_version:2020-04-29 cust_tagging_standard:https://cust.com/xywvBZgKsUsN]
    d TAGS_ALL =  map[]  ## WHY IS THIS?
    
    d TAGS =  map[cust_sys_tag_version:2020-04-29 cust_tagging_standard:https://cust.com/xywvBZgKsUsN]
    d TAGS_ALL =  map[cust_sys_tag_version:2020-04-29 cust_tagging_standard:https://cust.com/xywvBZgKsUsN]

The incorrect empty tags_all value makes the upstream provider skip updating tags in the cloud because it fails to observe tags_all changing (empty = empty).

[t0yv0]

Quick notes on why tags_all is empty in one case and not the other.

• The differential behavior can be tracked back to SetTagsDiff function https://github.com/hashicorp/terraform-provider-aws/blob/master/internal/verify/diff.go#L26 that special-cases for empty values

• When there are no empty values, SetTagsDiff copies tags to tags_all; it does not copy them when any of the tags are empty

• SetTagsDiff is invoked as a diff customizer which is part of the SDKv2 provider framework

[t0yv0]

t0yv0/terraform-provider-aws#1 I've been looking at surgically editing SetTagsDiff and put it under test. I think what I am seeing is that there are serious issues in schema.ResourceDiff that conflate empty values with unknowns. An educated guess here is that the complicated code is trying to compensate for state transitions with unknowns, but it's not getting it right for empty values. My attempts to edit it so far hit other footguns in the TF Plugin SDK V2.

[t0yv0]

Given that surgical fixes to SetTagsDiff run into some fundamental issues with TF Plugin SDK V2 confusing empty values with unknown, I'm evaluating a more brute-force approach in parallel. I've built another matrix #2932 that runs state transitions on a random sample of interesting tag states and compares expected vs actual. I've then attempted the following changes:

1. SetTagsDiff replaced with return nil
2. Pulumi PreCheckCallback copies tags to tagsAll before it hits TF
3. Scan SDKv2 provider and modify tags_all to not be computed, otherwise TF rejects program-originating values for tags_all

The end-result is passing a lot more, but not all, of the cases. The remaining failing cases can be summarized as "cannot add empty tag":

(x="a") --> (x="a",y="") produces (x="a") end-state unexpecedly
(x="a") --> (x="a",y="",z="b") produces (x="a",y="",z="b") end-state as expected

There is some remaining work to understand why adding an empty tag does not correctly recognize the need to update.

[t0yv0]

I've pursued this approach further, and there appears to be at least one bug in the bridge; with the appropriate fix #2944 is passing 100 randomly drawn tags state transition tests including adding/removing empty tags. I'm now waiting for CI to qualify this with all the other tests, then submit for review.

[t0yv0]

Updates. CI uncovered a number of issues but is now passing the 100 random test matrix. The remaining issues flagged by failing test pertain go supporting Plugin Framework resources correctly and avoiding secret leaks, handling unknown correctly for tags. I will be looking at those next.

[t0yv0]

Updates. Using pulumi/pulumi-terraform-bridge@ed35c1e as the preferred fix for empty tag handling. Fixed unknown tests and secrets tests via another bridge fix pulumi/pulumi-terraform-bridge#1503

What's remaining is making sure Plugin Framework based resources are handled correctly.

[t0yv0]

Looks like automation closed this before a release went out. Reopening until we can release.

[t0yv0]

Additional upgrade tests were requested before releasing (https://github.com/pulumi/home/issues/3130) to mitigate the risk of pulumi/pulumi-terraform-bridge#1502 bridge XSkipDetailedDiffForChanges flag required for this fix to work.

[t0yv0]

Fixed in v6.9.0.