Skip to content

Commit

Permalink
Deprecate PlanResourceChange flag (#2594)
Browse files Browse the repository at this point in the history
This change deprecates the PRC flag and turns off any resource overrides
providers specified on the SDKv2 bridge PlanResourceChange change
feature.

We also delete any tests which test without the flag as this change
makes it impossible for providers to not use PRC.

Opened #2593 to
clean up the unused code

Prompted by
#2575 (comment),
thanks for spotting!
  • Loading branch information
VenelinMartinov authored Nov 6, 2024
1 parent e5f6941 commit 180b924
Show file tree
Hide file tree
Showing 12 changed files with 100 additions and 860 deletions.
4 changes: 1 addition & 3 deletions pkg/internal/tests/cross-tests/puwrite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,7 @@ runtime: yaml
},
},
}
shimProvider := shimv2.NewProvider(tfp, shimv2.WithPlanResourceChange(
func(tfResourceType string) bool { return true },
))
shimProvider := shimv2.NewProvider(tfp)
schema := shimProvider.ResourcesMap().Get(rtype).Schema()
out, err := generateYaml(t, rtoken,
crosstestsimpl.InferPulumiValue(t, schema, nil, coalesceInputs(t, tc.schema, tc.tfConfig)))
Expand Down
9 changes: 2 additions & 7 deletions pkg/internal/tests/cross-tests/upgrade_state_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ type upgradeStateTestCase struct {
Config2 any
ExpectEqual bool
ObjectType *tftypes.Object

DisablePlanResourceChange bool
}

func getVersionInState(t T, stack apitype.UntypedDeployment) int {
Expand Down Expand Up @@ -71,11 +69,8 @@ func getVersionInState(t T, stack apitype.UntypedDeployment) int {
return int(schemaVersion)
}

func runPulumiUpgrade(t T, res1, res2 *schema.Resource, config1, config2 cty.Value, disablePlanResourceChange bool) (int, int) {
func runPulumiUpgrade(t T, res1, res2 *schema.Resource, config1, config2 cty.Value) (int, int) {
opts := []pulcheck.BridgedProviderOpt{}
if disablePlanResourceChange {
opts = append(opts, pulcheck.DisablePlanResourceChange())
}

tfp1 := &schema.Provider{ResourcesMap: map[string]*schema.Resource{defRtype: res1}}
prov1 := pulcheck.BridgedProvider(t, defProviderShortName, tfp1, opts...)
Expand Down Expand Up @@ -160,7 +155,7 @@ func runUpgradeStateInputCheck(t T, tc upgradeStateTestCase) {
tfd2 := newTFResDriver(t, tfwd, defProviderShortName, defRtype, &upgradeRes)
_ = tfd2.writePlanApply(t, tc.Resource.Schema, defRtype, "example", config2, lifecycleArgs{})

schemaVersion1, schemaVersion2 := runPulumiUpgrade(t, tc.Resource, &upgradeRes, config1, config2, tc.DisablePlanResourceChange)
schemaVersion1, schemaVersion2 := runPulumiUpgrade(t, tc.Resource, &upgradeRes, config1, config2)

if tc.ExpectEqual {
assert.Equal(t, schemaVersion1, tc.Resource.SchemaVersion)
Expand Down
103 changes: 42 additions & 61 deletions pkg/internal/tests/cross-tests/upgrade_state_cross_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package crosstests

import (
"fmt"
"testing"

"github.com/hashicorp/terraform-plugin-go/tftypes"
Expand All @@ -28,28 +27,22 @@ func TestUpgradeInputsStringBasic(t *testing.T) {
"f0": tftypes.NewValue(tftypes.String, val),
})
}
for _, PRC := range []bool{true, false} {
t.Run(fmt.Sprintf("PRC=%v", PRC), func(t *testing.T) {
t.Run("same", func(t *testing.T) {
runUpgradeStateInputCheck(t, upgradeStateTestCase{
Resource: res,
Config1: configVal("val"),
Config2: configVal("val"),
DisablePlanResourceChange: !PRC,
ExpectEqual: true,
})
})
t.Run("same", func(t *testing.T) {
runUpgradeStateInputCheck(t, upgradeStateTestCase{
Resource: res,
Config1: configVal("val"),
Config2: configVal("val"),
ExpectEqual: true,
})
})

t.Run("different", func(t *testing.T) {
runUpgradeStateInputCheck(t, upgradeStateTestCase{
Resource: res,
Config1: configVal("val1"),
Config2: configVal("val2"),
DisablePlanResourceChange: !PRC,
})
})
t.Run("different", func(t *testing.T) {
runUpgradeStateInputCheck(t, upgradeStateTestCase{
Resource: res,
Config1: configVal("val1"),
Config2: configVal("val2"),
})
}
})
}

func TestUpgradeInputsStringBasicNonZeroVersion(t *testing.T) {
Expand All @@ -73,28 +66,22 @@ func TestUpgradeInputsStringBasicNonZeroVersion(t *testing.T) {
"f0": tftypes.NewValue(tftypes.String, val),
})
}
for _, PRC := range []bool{true, false} {
t.Run(fmt.Sprintf("PRC=%v", PRC), func(t *testing.T) {
t.Run("same", func(t *testing.T) {
runUpgradeStateInputCheck(t, upgradeStateTestCase{
Resource: res,
Config1: configVal("val"),
Config2: configVal("val"),
DisablePlanResourceChange: !PRC,
ExpectEqual: true,
})
})
t.Run("same", func(t *testing.T) {
runUpgradeStateInputCheck(t, upgradeStateTestCase{
Resource: res,
Config1: configVal("val"),
Config2: configVal("val"),
ExpectEqual: true,
})
})

t.Run("different", func(t *testing.T) {
runUpgradeStateInputCheck(t, upgradeStateTestCase{
Resource: res,
Config1: configVal("val1"),
Config2: configVal("val2"),
DisablePlanResourceChange: !PRC,
})
})
t.Run("different", func(t *testing.T) {
runUpgradeStateInputCheck(t, upgradeStateTestCase{
Resource: res,
Config1: configVal("val1"),
Config2: configVal("val2"),
})
}
})
}

func TestUpgradeInputsObjectBasic(t *testing.T) {
Expand Down Expand Up @@ -140,26 +127,20 @@ func TestUpgradeInputsObjectBasic(t *testing.T) {
},
)
}
for _, PRC := range []bool{true, false} {
t.Run(fmt.Sprintf("PRC=%v", PRC), func(t *testing.T) {
t.Run("same", func(t *testing.T) {
runUpgradeStateInputCheck(t, upgradeStateTestCase{
Resource: res,
Config1: configVal("val"),
Config2: configVal("val"),
DisablePlanResourceChange: !PRC,
ExpectEqual: true,
})
})
t.Run("same", func(t *testing.T) {
runUpgradeStateInputCheck(t, upgradeStateTestCase{
Resource: res,
Config1: configVal("val"),
Config2: configVal("val"),
ExpectEqual: true,
})
})

t.Run("different", func(t *testing.T) {
runUpgradeStateInputCheck(t, upgradeStateTestCase{
Resource: res,
Config1: configVal("val1"),
Config2: configVal("val2"),
DisablePlanResourceChange: !PRC,
})
})
t.Run("different", func(t *testing.T) {
runUpgradeStateInputCheck(t, upgradeStateTestCase{
Resource: res,
Config1: configVal("val1"),
Config2: configVal("val2"),
})
}
})
}
2 changes: 1 addition & 1 deletion pkg/tests/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ func TestFailedValidatorOnReadHandling(t *testing.T) {
},
}
tfp := &schema.Provider{ResourcesMap: resMap}
bridgedProvider := pulcheck.BridgedProvider(t, "prov", tfp, pulcheck.DisablePlanResourceChange())
bridgedProvider := pulcheck.BridgedProvider(t, "prov", tfp)
program := `
name: test
runtime: yaml
Expand Down
17 changes: 3 additions & 14 deletions pkg/tests/pulcheck/pulcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,22 +127,14 @@ type T interface {
}

type bridgedProviderOpts struct {
DisablePlanResourceChange bool
StateEdit shimv2.PlanStateEditFunc
resourceInfo map[string]*info.Resource
configInfo map[string]*info.Schema
StateEdit shimv2.PlanStateEditFunc
resourceInfo map[string]*info.Resource
configInfo map[string]*info.Schema
}

// BridgedProviderOpts
type BridgedProviderOpt func(*bridgedProviderOpts)

// WithPlanResourceChange
func DisablePlanResourceChange() BridgedProviderOpt {
return func(o *bridgedProviderOpts) {
o.DisablePlanResourceChange = true
}
}

func WithStateEdit(f shimv2.PlanStateEditFunc) BridgedProviderOpt {
return func(o *bridgedProviderOpts) {
o.StateEdit = f
Expand Down Expand Up @@ -175,9 +167,6 @@ func BridgedProvider(t T, providerName string, tfp *schema.Provider, opts ...Bri
EnsureProviderValid(t, tfp)

shimProvider := shimv2.NewProvider(tfp,
shimv2.WithPlanResourceChange(
func(tfResourceType string) bool { return !options.DisablePlanResourceChange },
),
shimv2.WithPlanStateEdit(options.StateEdit),
)

Expand Down
Loading

0 comments on commit 180b924

Please sign in to comment.