diff --git a/pkg/tfbridge/diff_test.go b/pkg/tfbridge/diff_test.go index f3e17e72e..272241a08 100644 --- a/pkg/tfbridge/diff_test.go +++ b/pkg/tfbridge/diff_test.go @@ -150,9 +150,8 @@ func TestCustomizeDiff(t *testing.T) { }) t.Run("CustomDiffDoesNotPanicOnGetRawStateOrRawConfig", func(t *testing.T) { - for _, diffStrat := range []shimv2.DiffStrategy{shimv2.PlanState, shimv2.ClassicDiff} { - diffStrat := diffStrat - t.Run(fmt.Sprintf("%v", diffStrat), func(t *testing.T) { + { + { ctx := context.Background() customDiffRes := &v2Schema.Resource{ Schema: tfs, @@ -175,7 +174,7 @@ func TestCustomizeDiff(t *testing.T) { }, } - provider := shimv2.NewProvider(v2Provider, shimv2.WithDiffStrategy(diffStrat)) + provider := shimv2.NewProvider(v2Provider) // Convert the inputs and state to TF config and resource attributes. r := Resource{ @@ -191,7 +190,7 @@ func TestCustomizeDiff(t *testing.T) { // Calling Diff with the given CustomizeDiff used to panic, no more asserts needed. _, err = provider.Diff(ctx, "resource", tfState, config, shim.DiffOptions{}) assert.NoError(t, err) - }) + } } }) } diff --git a/pkg/tfbridge/provider_test.go b/pkg/tfbridge/provider_test.go index 9e35376e6..e543646c0 100644 --- a/pkg/tfbridge/provider_test.go +++ b/pkg/tfbridge/provider_test.go @@ -951,7 +951,11 @@ func TestProviderReadV2(t *testing.T) { }, } - testProviderRead(t, provider, "ExampleResource", true /* CheckRawConfig */) + // TODO[pulumi/pulumi-terraform-bridge#1977] currently un-schematized fields do not propagate to RawConfig which + // causes the test to panic as written. + checkRawConfig := false + + testProviderRead(t, provider, "ExampleResource", checkRawConfig) } func testProviderReadNestedSecret(t *testing.T, provider *Provider, typeName tokens.Type) { @@ -4738,7 +4742,8 @@ func TestUnknowns(t *testing.T) { }, "response": { "properties":{ - "id":"" + "id":"", + "setBlockProps": [{"prop": ""}] } } }`) diff --git a/pkg/tfshim/sdk-v2/cty.go b/pkg/tfshim/sdk-v2/cty.go index 1338d516a..5c88658c0 100644 --- a/pkg/tfshim/sdk-v2/cty.go +++ b/pkg/tfshim/sdk-v2/cty.go @@ -36,14 +36,9 @@ const ( // makeResourceRawConfig converts the decoded Go values in a terraform.ResourceConfig into a cty.Value that is // appropriate for Instance{Diff,State}.RawConfig. func makeResourceRawConfig( - strategy DiffStrategy, config *terraform.ResourceConfig, resource *schema.Resource, ) cty.Value { - if strategy == ClassicDiff { - return makeResourceRawConfigClassic(config, resource) - } - value, err := recoverAndCoerceCtyValue(resource, config.Raw) if err == nil { return value diff --git a/pkg/tfshim/sdk-v2/cty_test.go b/pkg/tfshim/sdk-v2/cty_test.go index fd835d5b0..ac6c99f3c 100644 --- a/pkg/tfshim/sdk-v2/cty_test.go +++ b/pkg/tfshim/sdk-v2/cty_test.go @@ -675,7 +675,7 @@ func TestMakeResourceRawConfig(t *testing.T) { } resourceConfig := &terraform.ResourceConfig{Raw: c.config} - config := makeResourceRawConfig(PlanState, resourceConfig, c.schema) + config := makeResourceRawConfig(resourceConfig, c.schema) actualMap := config.AsValueMap() expectedMap := c.expected.AsValueMap() diff --git a/pkg/tfshim/sdk-v2/diff_strategy.go b/pkg/tfshim/sdk-v2/diff_strategy.go index de0985b52..9ed5c71d7 100644 --- a/pkg/tfshim/sdk-v2/diff_strategy.go +++ b/pkg/tfshim/sdk-v2/diff_strategy.go @@ -21,6 +21,10 @@ import ( // Configures how the provider performs Diff. Since this is a sensitive method that can result in unexpected breaking // changes, using a configurable DiffStrategy as a feature flag assists gradual rollout. +// +// Deprecated. +// +// TODO[pulumi/pulumi-terraform-bridge#2062] clean up deprecation. type DiffStrategy int const ( @@ -49,6 +53,9 @@ func (s DiffStrategy) String() string { } } +// Deprecated. +// +// TODO[pulumi/pulumi-terraform-bridge#2062] clean up deprecation. func ParseDiffStrategy(text string) (DiffStrategy, error) { switch text { case "ClassicDiff": @@ -64,6 +71,9 @@ func ParseDiffStrategy(text string) (DiffStrategy, error) { const diffStrategyEnvVar = "PULUMI_DIFF_STRATEGY" +// Deprecated. +// +// TODO[pulumi/pulumi-terraform-bridge#2062] clean up deprecation. func ParseDiffStrategyFromEnv() (DiffStrategy, bool, error) { s := os.Getenv(diffStrategyEnvVar) if s == "" { diff --git a/pkg/tfshim/sdk-v2/provider.go b/pkg/tfshim/sdk-v2/provider.go index 3a40536cd..2eef34b72 100644 --- a/pkg/tfshim/sdk-v2/provider.go +++ b/pkg/tfshim/sdk-v2/provider.go @@ -138,11 +138,6 @@ func (p v2Provider) Refresh( s shim.InstanceState, c shim.ResourceConfig, ) (shim.InstanceState, error) { - opts, err := getProviderOptions(p.opts) - if err != nil { - return nil, err - } - r, ok := p.tf.ResourcesMap[t] if !ok { return nil, fmt.Errorf("unknown resource %v", t) @@ -154,7 +149,7 @@ func (p v2Provider) Refresh( } if c != nil { - state.RawConfig = makeResourceRawConfig(opts.diffStrategy, configFromShim(c), r) + state.RawConfig = makeResourceRawConfig(configFromShim(c), r) } state, diags := r.RefreshWithoutUpgrade(context.TODO(), state, p.tf.Meta()) @@ -171,12 +166,8 @@ func (p v2Provider) ReadDataDiff( return nil, fmt.Errorf("unknown resource %v", t) } - providerOpts, err := getProviderOptions(p.opts) - if err != nil { - return nil, err - } config := configFromShim(c) - rawConfig := makeResourceRawConfig(providerOpts.diffStrategy, config, resource) + rawConfig := makeResourceRawConfig(config, resource) diff, err := resource.Diff(ctx, nil, config, p.tf.Meta()) if diff != nil { diff --git a/pkg/tfshim/sdk-v2/provider_diff.go b/pkg/tfshim/sdk-v2/provider_diff.go index 02e2f183d..ca67a9b27 100644 --- a/pkg/tfshim/sdk-v2/provider_diff.go +++ b/pkg/tfshim/sdk-v2/provider_diff.go @@ -18,7 +18,6 @@ import ( "context" "fmt" - "github.com/golang/glog" hcty "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" @@ -43,18 +42,13 @@ func (p v2Provider) Diff( return diffToShim(&terraform.InstanceDiff{Destroy: true}), nil } - providerOpts, err := getProviderOptions(p.opts) - if err != nil { - return nil, err - } - r, ok := p.tf.ResourcesMap[t] if !ok { return nil, fmt.Errorf("unknown resource %v", t) } config, state := configFromShim(c), stateFromShim(s) - rawConfig := makeResourceRawConfig(providerOpts.diffStrategy, config, r) + rawConfig := makeResourceRawConfig(config, r) if state == nil { // When handling Create Pulumi passes nil for state, but this diverges from how Terraform does things, @@ -66,13 +60,14 @@ func (p v2Provider) Diff( } } else { // Upgrades are needed only if we have non-empty prior state. + var err error state, err = upgradeResourceState(ctx, t, p.tf, r, state) if err != nil { return nil, fmt.Errorf("failed to upgrade resource state: %w", err) } } - diff, err := p.simpleDiff(ctx, providerOpts.diffStrategy, r, state, config, rawConfig, p.tf.Meta()) + diff, err := p.simpleDiff(ctx, r, state, config, rawConfig, p.tf.Meta()) if err != nil { return nil, err } @@ -91,84 +86,12 @@ func (p v2Provider) Diff( func (p v2Provider) simpleDiff( ctx context.Context, - diffStrat DiffStrategy, res *schema.Resource, s *terraform.InstanceState, c *terraform.ResourceConfig, rawConfigVal hcty.Value, meta interface{}, ) (*terraform.InstanceDiff, error) { - - switch diffStrat { - case ClassicDiff: - state := s.DeepCopy() - if state.RawPlan.IsNull() { - // SimpleDiff may read RawPlan and panic if it is nil; while in the case of ClassicDiff we do - // not yet do what TF CLI does (that is PlanState), it is better to approximate and assume that - // the RawPlan is the same as RawConfig than to have the code panic. - state.RawPlan = rawConfigVal - } - if state.RawState.IsNull() { - // Same trick as for nil RawPlan. - priorStateVal, err := state.AttrsAsObjectValue(res.CoreConfigSchema().ImpliedType()) - if err != nil { - return nil, err - } - state.RawState = priorStateVal - } - if state.RawConfig.IsNull() { - // Same trick as above. - state.RawConfig = rawConfigVal - } - return res.SimpleDiff(ctx, state, c, meta) - case PlanState: - return simpleDiffViaPlanState(ctx, res, s, rawConfigVal, meta) - case TryPlanState: - classicResult, err := res.SimpleDiff(ctx, s, c, meta) - if err != nil { - return nil, err - } - planStateResult, err := simpleDiffViaPlanState(ctx, res, s, rawConfigVal, meta) - if err != nil { - glog.Errorf("Ignoring PlanState DiffStrategy that failed with an unexpected error. "+ - "You can set the environment variable %s to %q to avoid this message. "+ - "Please report the error details to github.com/pulumi/pulumi-terraform-bridge: %v", - diffStrategyEnvVar, ClassicDiff.String(), err) - return classicResult, nil - } - if planStateResult.ChangeType() != classicResult.ChangeType() { - glog.Warningf("Ignoring PlanState DiffStrategy that returns %q disagreeing "+ - " with ClassicDiff result %q. "+ - "You can set the environment variable %s to %q to avoid this message. "+ - "Please report this warning to github.com/pulumi/pulumi-terraform-bridge", - showDiffChangeType(byte(planStateResult.ChangeType())), - showDiffChangeType(byte(classicResult.ChangeType())), - diffStrategyEnvVar, ClassicDiff.String()) - return classicResult, nil - } - if planStateResult.RequiresNew() != classicResult.RequiresNew() { - glog.Warningf("Ignoring PlanState DiffStrategy that decided RequiresNew()=%v disagreeing "+ - " with ClassicDiff result RequiresNew()=%v. "+ - "You can set the environment variable %s to %q to avoid this message. "+ - "Please report this warning to github.com/pulumi/pulumi-terraform-bridge", - planStateResult.RequiresNew(), - classicResult.RequiresNew(), - diffStrategyEnvVar, ClassicDiff.String()) - return classicResult, nil - } - return classicResult, nil - default: - return res.SimpleDiff(ctx, s, c, meta) - } -} - -func simpleDiffViaPlanState( - ctx context.Context, - res *schema.Resource, - s *terraform.InstanceState, - rawConfigVal hcty.Value, - meta interface{}, -) (*terraform.InstanceDiff, error) { priorStateVal, err := s.AttrsAsObjectValue(res.CoreConfigSchema().ImpliedType()) if err != nil { return nil, err @@ -210,23 +133,3 @@ func simpleDiffViaPlanState( return diff, nil } - -func showDiffChangeType(b byte) string { - // based on diffChangeType enumeration from terraform.InstanceDiff ChangeType() result - switch b { - case 1: - return "diffNone" - case 2: - return "diffCreate" - case 3: - return "diffCreate" - case 4: - return "diffUpdate" - case 5: - return "diffDestroy" - case 6: - return "diffDestroyCreate" - default: - return "diffInvalid" - } -} diff --git a/pkg/tfshim/sdk-v2/provider_diff_test.go b/pkg/tfshim/sdk-v2/provider_diff_test.go index 72490d10c..adc214b53 100644 --- a/pkg/tfshim/sdk-v2/provider_diff_test.go +++ b/pkg/tfshim/sdk-v2/provider_diff_test.go @@ -41,7 +41,7 @@ func TestRawPlanSet(t *testing.T) { ResourcesMap: map[string]*schema.Resource{"myres": r}, } - wp := NewProvider(p, WithDiffStrategy(PlanState)) + wp := NewProvider(p) state := cty.ObjectVal(map[string]cty.Value{ "tags": cty.MapVal(map[string]cty.Value{"tag1": cty.StringVal("tag1v")}), diff --git a/pkg/tfshim/sdk-v2/provider_options.go b/pkg/tfshim/sdk-v2/provider_options.go index 5ba6229ad..0191973aa 100644 --- a/pkg/tfshim/sdk-v2/provider_options.go +++ b/pkg/tfshim/sdk-v2/provider_options.go @@ -15,26 +15,15 @@ package sdkv2 type providerOptions struct { - diffStrategy DiffStrategy planResourceChangeFilter func(string) bool } type providerOption func(providerOptions) (providerOptions, error) +// Deprecated. +// TODO[pulumi/pulumi-terraform-bridge#2062] clean up deprecation. func WithDiffStrategy(s DiffStrategy) providerOption { //nolint:revive return func(opts providerOptions) (providerOptions, error) { - - diffStrategyFromEnv, gotDiffStrategyFromEnv, err := ParseDiffStrategyFromEnv() - if err != nil { - return opts, err - } - - if gotDiffStrategyFromEnv { - opts.diffStrategy = diffStrategyFromEnv - return opts, nil - } - - opts.diffStrategy = s return opts, nil } }