Skip to content

Commit

Permalink
Fix empty labels perma-diff
Browse files Browse the repository at this point in the history 
This PR takes advantage of pulumi/pulumi-terraform-bridge#2417 to fixup the incorrect
planned state caused by empty labels.

Fixes #2372
  • Loading branch information
@iwahbe
iwahbe committed Sep 18, 2024
1 parent 99484f2 commit f8d6d51
Show file tree
Hide file tree
Showing 11 changed files with 500 additions and 16 deletions.
12 changes: 4 additions & 8 deletions examples/examples_go_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ func expectedLabelValue(arg expectedLabelValueArg) string {
func (st labelsState) expectedLabels(prev labelsState) map[string]string {
// Note that the upstream provider actually takes a "" value for a label to mean "keep the previous value".
// This behaviour is exposed under PlanResourceChange
r := map[string]string{}
expected := map[string]string{}

keys := map[string]struct{}{}
for k := range st.DefaultLabels {
Expand All @@ -495,24 +495,20 @@ func (st labelsState) expectedLabels(prev labelsState) map[string]string {
prevLabelsVal, inPrevLabels := prev.Labels[k]
prevDefaultsVal, inPrevDefaults := prev.DefaultLabels[k]

expectedVal := expectedLabelValue(
expected[k] = expectedLabelValue(
expectedLabelValueArg{
labels: valuePresentTuple{value: labelsVal, present: inLabels},
defaults: valuePresentTuple{value: defaultsVal, present: inDefaults},
prevLabels: valuePresentTuple{value: prevLabelsVal, present: inPrevLabels},
prevDefaults: valuePresentTuple{value: prevDefaultsVal, present: inPrevDefaults},
},
)

if expectedVal != "" {
r[k] = expectedVal
}
}

// Because we're reading pulumiLabels, we need to expect the default provisioning label.
r["goog-pulumi-provisioned"] = "true"
expected["goog-pulumi-provisioned"] = "true"

return r
return expected
}

func validateStateResult(phase int, st1, st2 labelsState) func(
Expand Down
11 changes: 8 additions & 3 deletions provider/go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ go 1.22
toolchain go1.22.7

require (
github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320
github.com/hashicorp/terraform-plugin-sdk/v2 v2.33.0
github.com/hashicorp/terraform-provider-google-beta v0.0.0
github.com/hexops/autogold/v2 v2.2.1
github.com/pulumi/providertest v0.0.14
github.com/pulumi/pulumi-terraform-bridge/pf v0.43.0
github.com/pulumi/pulumi-terraform-bridge/v3 v3.90.0
github.com/pulumi/pulumi-terraform-bridge/pf v0.43.1-0.20240918114950-4954882afe70
github.com/pulumi/pulumi-terraform-bridge/v3 v3.90.1-0.20240918114950-4954882afe70
github.com/pulumi/pulumi/pkg/v3 v3.130.0
github.com/pulumi/pulumi/sdk/v3 v3.130.0
github.com/stretchr/testify v1.9.0
Expand Down Expand Up @@ -129,7 +131,6 @@ require (
github.com/hashicorp/errwrap v1.1.0 // indirect
github.com/hashicorp/go-checkpoint v0.5.0 // indirect
github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320 // indirect
github.com/hashicorp/go-getter v1.7.5 // indirect
github.com/hashicorp/go-hclog v1.6.3 // indirect
github.com/hashicorp/go-multierror v1.1.1 // indirect
Expand Down Expand Up @@ -158,6 +159,8 @@ require (
github.com/hashicorp/terraform-svchost v0.1.1 // indirect
github.com/hashicorp/vault/api v1.12.0 // indirect
github.com/hashicorp/yamux v0.1.1 // indirect
github.com/hexops/gotextdiff v1.0.3 // indirect
github.com/hexops/valast v1.4.4 // indirect
github.com/huandu/xstrings v1.4.0 // indirect
github.com/iancoleman/strcase v0.3.0 // indirect
github.com/imdario/mergo v0.3.15 // indirect
Expand Down Expand Up @@ -189,6 +192,7 @@ require (
github.com/muesli/reflow v0.3.0 // indirect
github.com/muesli/termenv v0.15.2 // indirect
github.com/natefinch/atomic v1.0.1 // indirect
github.com/nightlyone/lockfile v1.0.0 // indirect
github.com/nxadm/tail v1.4.11 // indirect
github.com/oklog/run v1.1.0 // indirect
github.com/opentracing/basictracer-go v1.1.0 // indirect
Expand Down Expand Up @@ -274,4 +278,5 @@ require (
gopkg.in/warnings.v0 v0.1.2 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
lukechampine.com/frand v1.4.2 // indirect
mvdan.cc/gofumpt v0.5.0 // indirect
)
15 changes: 11 additions & 4 deletions provider/go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1401,12 +1401,14 @@ github.com/ettle/strcase v0.1.1/go.mod h1:hzDLsPC7/lwKyBOywSHEP89nt2pDgdy+No1NBA
github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4=
github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL+zU=
github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk=
github.com/fatih/color v1.15.0/go.mod h1:0h5ZqXfHYED7Bhv2ZJamyIOUej9KtShiJESRwBDUSsw=
github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM=
github.com/fatih/color v1.16.0/go.mod h1:fL2Sau1YI5c0pdGEVCbKQbLXB6edEj1ZgiY4NijnWvE=
github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg=
github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
github.com/fogleman/gg v1.2.1-0.20190220221249-0403632d5b90/go.mod h1:R/bRT+9gY/C5z7JzPU0zXsXHKM4/ayA+zqcVNZzPa1k=
github.com/fogleman/gg v1.3.0/go.mod h1:R/bRT+9gY/C5z7JzPU0zXsXHKM4/ayA+zqcVNZzPa1k=
github.com/frankban/quicktest v1.14.3/go.mod h1:mgiwOwqx65TmIk1wJ6Q7wvnVMocbUorkibMOrVTHZps=
github.com/frankban/quicktest v1.14.4 h1:g2rn0vABPOOXmZUj+vbmUp0lPoXEMuhTpIluN0XL9UY=
github.com/frankban/quicktest v1.14.4/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0=
github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=
Expand Down Expand Up @@ -1733,6 +1735,7 @@ github.com/hashicorp/yamux v0.0.0-20180604194846-3520598351bb/go.mod h1:+NfK9FKe
github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d/go.mod h1:+NfK9FKeTrX5uv1uIXGdwYDTeHna2qgaIlx54MXqjAM=
github.com/hashicorp/yamux v0.1.1 h1:yrQxtgseBDrq9Y652vSRDvsKCJKOUD+GzTS4Y0Y8pvE=
github.com/hashicorp/yamux v0.1.1/go.mod h1:CtWFDAQgb7dxtzFs4tWbplKIe2jSi3+5vKbgIO0SLnQ=
github.com/hexops/autogold v0.8.1/go.mod h1:97HLDXyG23akzAoRYJh/2OBs3kd80eHyKPvZw0S5ZBY=
github.com/hexops/autogold v1.3.0 h1:IEtGNPxBeBu8RMn8eKWh/Ll9dVNgSnJ7bp/qHgMQ14o=
github.com/hexops/autogold v1.3.0/go.mod h1:d4hwi2rid66Sag+BVuHgwakW/EmaFr8vdTSbWDbrDRI=
github.com/hexops/autogold/v2 v2.2.1 h1:JPUXuZQGkcQMv7eeDXuNMovjfoRYaa0yVcm+F3voaGY=
Expand Down Expand Up @@ -1827,6 +1830,7 @@ github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Ky
github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94=
github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM=
github.com/mattn/go-isatty v0.0.17/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM=
github.com/mattn/go-isatty v0.0.19/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY=
github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
github.com/mattn/go-localereader v0.0.1 h1:ygSAOl7ZXTx4RdPYinUpg6W99U8jWvWi9Ye2JC/oIi4=
Expand Down Expand Up @@ -1976,10 +1980,10 @@ github.com/pulumi/providertest v0.0.14 h1:5QlAPAAs82jkQraHsJvq1xgVfC7xtW8sFJwv2p
github.com/pulumi/providertest v0.0.14/go.mod h1:GcsqEGgSngwaNOD+kICJPIUQlnA911fGBU8HDlJvVL0=
github.com/pulumi/pulumi-java/pkg v0.14.0 h1:CKL7lLF81Fq6VRhA5TNFsSMnHraTNCUzIhqCzYX8Wzk=
github.com/pulumi/pulumi-java/pkg v0.14.0/go.mod h1:VybuJMWJtJc9ZNbt1kcYH4TbpocMx9mEi7YWL2Co99c=
github.com/pulumi/pulumi-terraform-bridge/pf v0.43.0 h1:g15WgVKJBhFtzhLqOky6R77QIU3x4KkunrLHDSkK6CM=
github.com/pulumi/pulumi-terraform-bridge/pf v0.43.0/go.mod h1:xdU2rcUBjPX/alXMiywUK1GvN4goUHZxos8ZfT6sVXM=
github.com/pulumi/pulumi-terraform-bridge/v3 v3.90.0 h1:e7xfYAiXCE8LCwfKvbGeNAjdPmKwPM3kavEXECt3wvs=
github.com/pulumi/pulumi-terraform-bridge/v3 v3.90.0/go.mod h1:dIVp4qG+GsUwmpz40L7Z+PZnzHf3cQq1CAFwhz++ris=
github.com/pulumi/pulumi-terraform-bridge/pf v0.43.1-0.20240918114950-4954882afe70 h1:NhS1OHDyvpRyBUaajYIgVUtdjbP8ZY740huQDi2YjHs=
github.com/pulumi/pulumi-terraform-bridge/pf v0.43.1-0.20240918114950-4954882afe70/go.mod h1:xdU2rcUBjPX/alXMiywUK1GvN4goUHZxos8ZfT6sVXM=
github.com/pulumi/pulumi-terraform-bridge/v3 v3.90.1-0.20240918114950-4954882afe70 h1:DWZMy2RK0qc53kyvJMu2Gnn5aEBW1oz6Qjz9JPuSuy4=
github.com/pulumi/pulumi-terraform-bridge/v3 v3.90.1-0.20240918114950-4954882afe70/go.mod h1:dIVp4qG+GsUwmpz40L7Z+PZnzHf3cQq1CAFwhz++ris=
github.com/pulumi/pulumi-terraform-bridge/x/muxer v0.0.9-0.20240227144008-2da15b3d6f6e h1:yON1jwN6gg4cjnOQF607I3fTiFyIDr9WSsQNXHD6wbM=
github.com/pulumi/pulumi-terraform-bridge/x/muxer v0.0.9-0.20240227144008-2da15b3d6f6e/go.mod h1:AvlZujvfRiEu+60frCGEhaLeGttjHwM/g+47+IdPZXw=
github.com/pulumi/pulumi-yaml v1.9.2 h1:BCUuRPA1USmFXrExiHRU8yJ+OiphLYnroPxKRgGCJrs=
Expand Down Expand Up @@ -2007,6 +2011,7 @@ github.com/rogpeppe/go-charset v0.0.0-20180617210344-2471d30d28b4/go.mod h1:qgYe
github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc=
github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
github.com/rogpeppe/go-internal v1.10.0/go.mod h1:UQnix2H7Ngw/k4C5ijL5+65zddjncjaFoBhdsK/akog=
github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA=
github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=
github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4=
Expand Down Expand Up @@ -2658,6 +2663,7 @@ golang.org/x/tools v0.8.0/go.mod h1:JxBZ99ISMI5ViVkT1tr6tdNmXeTrcpVSD3vZ1RsRdN4=
golang.org/x/tools v0.9.1/go.mod h1:owI94Op576fPu3cIGQeHs3joujW/2Oc6MtlxbF5dfNc=
golang.org/x/tools v0.9.3/go.mod h1:owI94Op576fPu3cIGQeHs3joujW/2Oc6MtlxbF5dfNc=
golang.org/x/tools v0.10.0/go.mod h1:UJwyiVBsOA2uwvK/e5OY3GTpDUJriEd+/YlqAwLPmyM=
golang.org/x/tools v0.12.0/go.mod h1:Sc0INKfu04TlqNoRA1hgpFZbhYXHPr4V5DzpSBTPqQM=
golang.org/x/tools v0.13.0/go.mod h1:HvlwmtVNQAhOuCjW7xxvovg8wbNq7LwfXh/k7wXUl58=
golang.org/x/tools v0.17.0/go.mod h1:xsh6VxdV005rRVaS6SSAf9oiAqljS7UZUacMZ8Bnsps=
golang.org/x/tools v0.22.0 h1:gqSGLZqv+AI9lIQzniJ0nZDRG5GBPsSi+DRNHWNz6yA=
Expand Down Expand Up @@ -3112,6 +3118,7 @@ modernc.org/token v1.0.0/go.mod h1:UGzOrNV1mAFSEB63lOFHIpNRUVMvYTc6yu1SMY/XTDM=
modernc.org/token v1.0.1/go.mod h1:UGzOrNV1mAFSEB63lOFHIpNRUVMvYTc6yu1SMY/XTDM=
modernc.org/token v1.1.0/go.mod h1:UGzOrNV1mAFSEB63lOFHIpNRUVMvYTc6yu1SMY/XTDM=
modernc.org/z v1.5.1/go.mod h1:eWFB510QWW5Th9YGZT81s+LwvaAs3Q2yr4sP0rmLkv8=
mvdan.cc/gofumpt v0.4.0/go.mod h1:PljLOHDeZqgS8opHRKLzp2It2VBuSdteAgqUfzMTxlQ=
mvdan.cc/gofumpt v0.5.0 h1:0EQ+Z56k8tXjj/6TQD25BFNKQXpCvT0rnansIc7Ug5E=
mvdan.cc/gofumpt v0.5.0/go.mod h1:HBeVDtMKRZpXyxFciAirzdKklDlGu8aAy1wEbH5Y9js=
pgregory.net/rapid v0.6.1 h1:4eyrDxyht86tT4Ztm+kvlyNBLIk071gR+ZQdhphc9dQ=
Expand Down
80 changes: 80 additions & 0 deletions provider/labels.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,91 @@ package gcp
import (
"context"

"github.com/hashicorp/go-cty/cty"

"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge"
shimv2 "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2"
"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/walk"
"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
"github.com/pulumi/pulumi/sdk/v3/go/property"
)

// fixEmptyLabels applies a state edit to fix
// https://github.com/pulumi/pulumi-gcp/issues/2372.
func fixEmptyLabels(_ context.Context, req shimv2.PlanStateEditRequest) (cty.Value, error) {
// programLabels holds the labels that we expect to be applied. It is the union of
// labels (from the resource) and defaultLabels (from the provider).
//
// programLabels may not be the full set of labels on the resource, since
// effective_labels can include labels read from the cloud provider.
programLabels := property.Map{}

// Apply default labels first.
if pConfig := resource.FromResourcePropertyValue(resource.NewProperty(req.ProviderConfig)); pConfig.IsMap() {
l := pConfig.AsMap()["defaultLabels"]
if l.IsMap() {
programLabels = l.AsMap()
}
}

// Apply labels next, allowing labels to override defaultLabels.
if inputs, ok := (resource.PropertyPath{"labels"}.Get(resource.NewProperty(req.NewInputs))); ok {
if labels := resource.FromResourcePropertyValue(inputs); labels.IsMap() {
for k, v := range labels.AsMap() {
programLabels[k] = v
}
}
}

fixMap := func(f func(map[string]cty.Value) map[string]cty.Value) func(cty.Value) cty.Value {
return func(v cty.Value) cty.Value {
if !v.Type().IsMapType() || !v.IsKnown() || v.IsNull() {
return v
}

result := f(v.AsValueMap())
if len(result) == 0 {
return cty.MapValEmpty(v.Type().ElementType())
}
return cty.MapVal(result)
}
}

// fixOutputLabels fixes falsely unknown values label values.
fixOutputLabels := fixMap(func(m map[string]cty.Value) map[string]cty.Value {
for k, v := range m {
if v.IsKnown() {
// If v is known, so no correction is needed.
continue
}
label, ok := programLabels[k] // labels is in the Pulumi namespace
if !ok || label.IsComputed() || !label.IsString() {
// If we didn't inherit label from the program or the
// label is actually unknown, then just continue.
continue
}

// v is incorrectly unknown, so set it to the known value from the
// program.
m[k] = cty.StringVal(label.AsString())
}
return m
})

// Apply f to m[k] if k in m.
mapIfExists := func(m map[string]cty.Value, k string, f func(cty.Value) cty.Value) {
v, ok := m[k]
if ok {
m[k] = f(v)
}
}

planState := req.PlanState.AsValueMap()
mapIfExists(planState, "effective_labels", fixOutputLabels)
mapIfExists(planState, "terraform_labels", fixOutputLabels)
return cty.ObjectVal(planState), nil
}

// GCP has many resources with labels called "terraform_labels". We want to change those
// to "pulumiLabels".
//
Expand Down
89 changes: 89 additions & 0 deletions provider/labels_impl_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ import (
"context"
"testing"

"github.com/hashicorp/go-cty/cty"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

shimv2 "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2"
"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
)

Expand Down Expand Up @@ -225,3 +227,90 @@ func TestEnsureLabelPathsExist(t *testing.T) {
})
}
}

func TestFixEmptyLabels(t *testing.T) {
tests := []struct {
name string
inputs shimv2.PlanStateEditRequest
expected cty.Value
}{
{
name: "adjust incorrectly unknown labels",
inputs: shimv2.PlanStateEditRequest{
PlanState: cty.ObjectVal(map[string]cty.Value{
"effective_labels": cty.MapVal(map[string]cty.Value{
"empty": cty.UnknownVal(cty.String),
}),
"terraform_labels": cty.MapVal(map[string]cty.Value{
"empty": cty.UnknownVal(cty.String),
}),
}),
NewInputs: resource.PropertyMap{
"labels": resource.NewProperty(resource.PropertyMap{
"empty": resource.NewProperty(""),
}),
},
},
expected: cty.ObjectVal(map[string]cty.Value{
"effective_labels": cty.MapVal(map[string]cty.Value{
"empty": cty.StringVal(""),
}),
"terraform_labels": cty.MapVal(map[string]cty.Value{
"empty": cty.StringVal(""),
}),
}),
},
{
name: "adjust incorrectly unknown default labels",
inputs: shimv2.PlanStateEditRequest{
PlanState: cty.ObjectVal(map[string]cty.Value{
"effective_labels": cty.MapVal(map[string]cty.Value{
"empty": cty.UnknownVal(cty.String),
}),
}),
ProviderConfig: resource.PropertyMap{
"defaultLabels": resource.NewProperty(resource.PropertyMap{
"empty": resource.NewProperty(""),
}),
},
},
expected: cty.ObjectVal(map[string]cty.Value{
"effective_labels": cty.MapVal(map[string]cty.Value{
"empty": cty.StringVal(""),
}),
}),
},
{
name: "ignore valid labels",
inputs: shimv2.PlanStateEditRequest{
PlanState: cty.ObjectVal(map[string]cty.Value{
"effective_labels": cty.MapVal(map[string]cty.Value{
"non-empty": cty.StringVal("val"),
"actually-unknown": cty.UnknownVal(cty.String),
}),
}),
NewInputs: resource.PropertyMap{
"labels": resource.NewProperty(resource.PropertyMap{
"non-empty": resource.NewProperty("val"),
"actually-unknown": resource.MakeComputed(resource.NewProperty("")),
}),
},
},
expected: cty.ObjectVal(map[string]cty.Value{
"effective_labels": cty.MapVal(map[string]cty.Value{
"non-empty": cty.StringVal("val"),
"actually-unknown": cty.UnknownVal(cty.String),
}),
}),
},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
result, err := fixEmptyLabels(context.Background(), tt.inputs)
require.NoError(t, err)
assert.Equal(t, tt.expected, result)
})
}
}
Loading

0 comments on commit f8d6d51

Please sign in to comment.