diff --git a/examples/examples_test.go b/examples/examples_test.go index f26ec341527..9a741620ed9 100644 --- a/examples/examples_test.go +++ b/examples/examples_test.go @@ -251,3 +251,37 @@ func TestMigrateRdsInstance(t *testing.T) { } ]`) } + +func TestRegressUnknownTags(t *testing.T) { + repro := ` + [ + { + "method": "/pulumirpc.ResourceProvider/Check", + "request": { + "urn": "urn:pulumi:p1::example-tags::eks:index:NodeGroupV2$aws:ec2/securityGroup:SecurityGroup::example-ng-tags-ng2-nodeSecurityGroup", + "olds": {}, + "news": { + "description": "Managed by Pulumi", + "revokeRulesOnDelete": true, + "tags": "04da6b54-80e4-46f7-96ec-b56ff0331ba9", + "vpcId": "vpc-4b82e033" + }, + "randomSeed": "pm3N78209q8Aq/BJU17gDsIRv2BvC/geMb0WK/pMRQg=" + }, + "response": { + "inputs": { + "__defaults": [ + "name" + ], + "description": "Managed by Pulumi", + "name": "example-ng-tags-ng2-nodeSecurityGroup-8012419", + "revokeRulesOnDelete": true, + "vpcId": "vpc-4b82e033", + "tags": "04da6b54-80e4-46f7-96ec-b56ff0331ba9" + } + } + } + ] + ` + replay(t, repro) +} diff --git a/provider/resources.go b/provider/resources.go index a9d6d29a3b4..1eb83c3286c 100644 --- a/provider/resources.go +++ b/provider/resources.go @@ -7152,96 +7152,3 @@ func Provider() *tfbridge.ProviderInfo { return &prov } - -// Apply provider tags to an individual resource. -// -// Historically, Pulumi has struggles to handle the "tags" and "tags_all" fields correctly: -// - https://github.com/pulumi/pulumi-aws/issues/2633 -// - https://github.com/pulumi/pulumi-aws/issues/1655 -// -// terraform-provider-aws has also struggled with implementing their desired behavior: -// - https://github.com/hashicorp/terraform-provider-aws/issues/29747 -// - https://github.com/hashicorp/terraform-provider-aws/issues/29842 -// - https://github.com/hashicorp/terraform-provider-aws/issues/24449 -// -// The Terraform lifecycle simply does not have a good way to map provider configuration -// onto resource values, so terraform-provider-aws is forced to work around limitations in -// unreliable ways. For example, terraform-provider-aws does not apply tags correctly with -// -refresh=false. -// -// This gives pulumi the same limitations by default. However, unlike Terraform, Pulumi -// does have a clear way to insert provider configuration into resource properties: -// Check. By writing a custom check function that applies "default_tags" to "tags" before -// the Terraform provider sees any resource configuration, we can give a consistent, -// reliable and good experience for Pulumi users. -func applyTags( - ctx context.Context, config resource.PropertyMap, meta resource.PropertyMap, -) (resource.PropertyMap, error) { - var defaultTags awsShim.TagConfig - - unknown := func() (resource.PropertyMap, error) { - current := config["tags"] - if current.IsOutput() { - output := current.OutputValue() - output.Known = false - config["tags"] = resource.NewOutputProperty(output) - } else { - config["tags"] = resource.MakeOutput(current) - } - return config, nil - } - - // awsShim.NewTagConfig accepts (context.Context, i interface{}) where i can be - // one of map[string]interface{} among other types. .Mappable() produces a - // map[string]interface{} where every value is of type string. This is well - // handled by awsShim.NewTagConfig. - // - // config values are guaranteed to be of the correct type because they have - // already been seen and approved of by the provider, which verifies its - // configuration is well typed. - - if defaults, ok := meta["defaultTags"]; ok { - if defaults.ContainsUnknowns() { - return unknown() - } - if defaults.IsObject() { - defaults := defaults.ObjectValue() - tags, ok := defaults["tags"] - if ok { - defaultTags = awsShim.NewTagConfig(ctx, tags.Mappable()) - } - } - } - - ignoredTags := &awsShim.TagIgnoreConfig{} - if ignores, ok := meta["ignoreTags"]; ok { - if ignores.ContainsUnknowns() { - return unknown() - } - if keys, ok := ignores.ObjectValue()["keys"]; ok { - ignoredTags.Keys = awsShim.NewTagConfig(ctx, keys.Mappable()).Tags - } - if keys, ok := ignores.ObjectValue()["keyPrefixes"]; ok { - ignoredTags.KeyPrefixes = awsShim.NewTagConfig(ctx, keys.Mappable()).Tags - } - } - - var resourceTags awsShim.TagConfig - if tags, ok := config["tags"]; ok { - resourceTags = awsShim.NewTagConfig(ctx, tags.Mappable().(map[string]interface{})) - } - - allTags := defaultTags.MergeTags(resourceTags.Tags).IgnoreConfig(ignoredTags) - - if len(allTags) > 0 { - allTagProperties := make(resource.PropertyMap, len(allTags)) - for k, v := range allTags { - allTagProperties[resource.PropertyKey(k)] = resource.NewStringProperty(v.ValueString()) - } - config["tags"] = resource.NewObjectProperty(allTagProperties) - } else { - delete(config, "tags") - } - - return config, nil -} diff --git a/provider/tags.go b/provider/tags.go new file mode 100644 index 00000000000..11425109e6f --- /dev/null +++ b/provider/tags.go @@ -0,0 +1,125 @@ +// Copyright 2016-2023, Pulumi Corporation. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package provider + +import ( + "context" + + awsShim "github.com/hashicorp/terraform-provider-aws/shim" + "github.com/pulumi/pulumi/sdk/v3/go/common/resource" + "github.com/pulumi/pulumi/sdk/v3/go/common/util/contract" +) + +// Apply provider tags to an individual resource. +// +// Historically, Pulumi has struggles to handle the "tags" and "tags_all" fields correctly: +// - https://github.com/pulumi/pulumi-aws/issues/2633 +// - https://github.com/pulumi/pulumi-aws/issues/1655 +// +// terraform-provider-aws has also struggled with implementing their desired behavior: +// - https://github.com/hashicorp/terraform-provider-aws/issues/29747 +// - https://github.com/hashicorp/terraform-provider-aws/issues/29842 +// - https://github.com/hashicorp/terraform-provider-aws/issues/24449 +// +// The Terraform lifecycle simply does not have a good way to map provider configuration +// onto resource values, so terraform-provider-aws is forced to work around limitations in +// unreliable ways. For example, terraform-provider-aws does not apply tags correctly with +// -refresh=false. +// +// This gives pulumi the same limitations by default. However, unlike Terraform, Pulumi +// does have a clear way to insert provider configuration into resource properties: +// Check. By writing a custom check function that applies "default_tags" to "tags" before +// the Terraform provider sees any resource configuration, we can give a consistent, +// reliable and good experience for Pulumi users. +func applyTags( + ctx context.Context, config resource.PropertyMap, meta resource.PropertyMap, +) (resource.PropertyMap, error) { + ret := config.Copy() + configTags := resource.NewObjectProperty(resource.PropertyMap{}) + if t, ok := config["tags"]; ok { + configTags = t + } + allTags, err := mergeTags(ctx, configTags, meta) + if err != nil { + return nil, err + } + if allTags.IsNull() { + delete(ret, "tags") + return ret, nil + } + ret["tags"] = allTags + return ret, nil +} + +// Wrap mergeTagsSimple with taking care of unknowns, secrets and outputs. +func mergeTags( + ctx context.Context, tags resource.PropertyValue, meta resource.PropertyMap, +) (resource.PropertyValue, error) { + // Any unknowns make the result unknown. + if resource.NewObjectProperty(meta).ContainsUnknowns() || tags.ContainsUnknowns() { + return resource.NewOutputProperty(resource.Output{Known: false}), nil + } + + // Expect the Pulumi CLI to be shielding Check from secrets. + contract.Assertf(!tags.ContainsSecrets(), "PreCheckCallback got secrets in config") + contract.Assertf(!meta.ContainsSecrets(), "PreCheckCallback got secrets in meta") + + var defaultTags awsShim.TagConfig + + // awsShim.NewTagConfig accepts (context.Context, i interface{}) where i can be one of + // map[string]interface{} among other types. .Mappable() produces a map[string]interface{} + // where every value is of type string. This is well handled by awsShim.NewTagConfig. + // + // config values are guaranteed to be of the correct type because they have already been + // seen and approved of by the provider, which verifies its configuration is well typed. + + if defaults, ok := meta["defaultTags"]; ok { + if defaults.IsObject() { + defaults := defaults.ObjectValue() + tags, ok := defaults["tags"] + if ok { + defaultTags = awsShim.NewTagConfig(ctx, tags.Mappable()) + } + } + } + + ignoredTags := &awsShim.TagIgnoreConfig{} + if ignores, ok := meta["ignoreTags"]; ok { + if keys, ok := ignores.ObjectValue()["keys"]; ok { + ignoredTags.Keys = awsShim.NewTagConfig(ctx, keys.Mappable()).Tags + } + if keys, ok := ignores.ObjectValue()["keyPrefixes"]; ok { + ignoredTags.KeyPrefixes = awsShim.NewTagConfig(ctx, keys.Mappable()).Tags + } + } + + var resourceTags awsShim.TagConfig + if tags.IsObject() { + resourceTags = awsShim.NewTagConfig(ctx, tags.Mappable()) + } + + allTags := defaultTags.MergeTags(resourceTags.Tags).IgnoreConfig(ignoredTags) + + if len(allTags) > 0 { + allTagProperties := make(resource.PropertyMap, len(allTags)) + for k, v := range allTags { + pk := resource.PropertyKey(k) + allTagProperties[pk] = resource.NewStringProperty(v.ValueString()) + } + return resource.NewObjectProperty(allTagProperties), nil + } else { + return resource.NewNullProperty(), nil + } +} diff --git a/provider/tags_test.go b/provider/tags_test.go new file mode 100644 index 00000000000..6c2c26fea0c --- /dev/null +++ b/provider/tags_test.go @@ -0,0 +1,217 @@ +// Copyright 2016-2023, Pulumi Corporation. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package provider + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + "pgregory.net/rapid" + + "github.com/pulumi/pulumi/sdk/v3/go/common/resource" +) + +func TestApplyTags(t *testing.T) { + ctx := context.Background() + + type gen = *rapid.Generator[resource.PropertyValue] + type pk = resource.PropertyKey + type pv = resource.PropertyValue + type pm = resource.PropertyMap + + maybeNullOrUnknown := func(x gen) gen { + return rapid.OneOf( + rapid.Just(resource.NewNullProperty()), + x, + rapid.Map(x, resource.MakeComputed), + ) + } + + str := maybeNullOrUnknown(rapid.OneOf( + rapid.Just(resource.NewStringProperty("")), + rapid.Just(resource.NewStringProperty("foo")), + rapid.Just(resource.NewStringProperty("bar")), + )) + + keys := rapid.Map(rapid.OneOf[string]( + rapid.Just(""), + rapid.Just("a"), + rapid.Just("b"), + ), func(s string) pk { + return resource.PropertyKey(s) + }) + + makeObj := func(m map[pk]resource.PropertyValue) resource.PropertyValue { + return resource.NewObjectProperty(resource.PropertyMap(m)) + } + + keyValueTags := maybeNullOrUnknown( + rapid.Map(rapid.MapOfN[pk, pv](keys, str, 0, 3), makeObj)) + + config := rapid.Map(keyValueTags, func(tags pv) pm { + return resource.PropertyMap{ + "tags": tags, + } + }) + + defaultConfig := maybeNullOrUnknown(rapid.Map(config, + resource.NewObjectProperty)) + + ignoreConfig := rapid.Custom[pv](func(t *rapid.T) pv { + keys := keyValueTags.Draw(t, "keys") + keyPrefixes := keyValueTags.Draw(t, "keyPrefixes") + m := resource.PropertyMap{} + if !keys.IsNull() { + m["keys"] = keys + } + if !keyPrefixes.IsNull() { + m["keyPrefixes"] = keyPrefixes + } + return resource.NewObjectProperty(m) + }) + + meta := rapid.Custom[pm](func(t *rapid.T) pm { + i := ignoreConfig.Draw(t, "ignoreConfig") + d := defaultConfig.Draw(t, "defaultConfig") + m := resource.PropertyMap{} + if !i.IsNull() { + m["ignoreConfig"] = i + } + if !d.IsNull() { + m["defaultConfig"] = d + } + return m + }) + + type args struct { + meta resource.PropertyMap + config resource.PropertyMap + } + + argsGen := rapid.Custom[args](func(t *rapid.T) args { + m := meta.Draw(t, "meta") + c := config.Draw(t, "config") + return args{meta: m, config: c} + }) + + t.Run("no panics", func(t *testing.T) { + rapid.Check(t, func(t *rapid.T) { + args := argsGen.Draw(t, "args") + _, err := applyTags(ctx, args.config, args.meta) + require.NoError(t, err) + }) + }) + + type testCase struct { + name string + config resource.PropertyMap + meta resource.PropertyMap + expect resource.PropertyMap + } + + testCases := []testCase{ + { + name: "resource tags propagate", + config: resource.PropertyMap{ + "tags": resource.NewObjectProperty(resource.PropertyMap{ + "tag1": resource.NewStringProperty("tag1v"), + }), + }, + meta: resource.PropertyMap{}, + expect: resource.PropertyMap{ + "tags": resource.NewObjectProperty(resource.PropertyMap{ + "tag1": resource.NewStringProperty("tag1v"), + }), + }, + }, + { + name: "provier sets a tag", + config: resource.PropertyMap{}, + meta: resource.PropertyMap{ + "defaultTags": resource.NewObjectProperty(resource.PropertyMap{ + "tags": resource.NewObjectProperty(resource.PropertyMap{ + "tag2": resource.NewStringProperty("tag2v"), + }), + }), + }, + expect: resource.PropertyMap{ + "tags": resource.NewObjectProperty(resource.PropertyMap{ + "tag2": resource.NewStringProperty("tag2v"), + }), + }, + }, + { + name: "provider adds a tag to resource tags", + config: resource.PropertyMap{ + "tags": resource.NewObjectProperty(resource.PropertyMap{ + "tag1": resource.NewStringProperty("tag1v"), + }), + }, + meta: resource.PropertyMap{ + "defaultTags": resource.NewObjectProperty(resource.PropertyMap{ + "tags": resource.NewObjectProperty(resource.PropertyMap{ + "tag2": resource.NewStringProperty("tag2v"), + }), + }), + }, + expect: resource.PropertyMap{ + "tags": resource.NewObjectProperty(resource.PropertyMap{ + "tag1": resource.NewStringProperty("tag1v"), + "tag2": resource.NewStringProperty("tag2v"), + }), + }, + }, + { + name: "provider cannot change a resource tag", + config: resource.PropertyMap{ + "tags": resource.NewObjectProperty(resource.PropertyMap{ + "tag1": resource.NewStringProperty("tag1v"), + }), + }, + meta: resource.PropertyMap{ + "defaultTags": resource.NewObjectProperty(resource.PropertyMap{ + "tags": resource.NewObjectProperty(resource.PropertyMap{ + "tag1": resource.NewStringProperty("tag2v"), + }), + }), + }, + expect: resource.PropertyMap{ + "tags": resource.NewObjectProperty(resource.PropertyMap{ + "tag1": resource.NewStringProperty("tag1v"), + }), + }, + }, + { + name: "unknowns mark the entire computation unknown", + config: resource.PropertyMap{ + "tags": resource.NewObjectProperty(resource.PropertyMap{ + "tag1": resource.MakeComputed(resource.PropertyValue{}), + }), + }, + expect: resource.PropertyMap{ + "tags": resource.NewOutputProperty(resource.Output{Known: false}), + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + r, err := applyTags(ctx, tc.config, tc.meta) + require.NoError(t, err) + require.Equal(t, tc.expect, r) + }) + } +}