From 11d6900a031d3e67ae481ce744f965b8baca989c Mon Sep 17 00:00:00 2001 From: Venelin Date: Mon, 3 Jun 2024 20:12:16 +0100 Subject: [PATCH 01/63] integration tests poc --- pkg/tests/cross-tests/input_check.go | 22 ++++++ pkg/tests/cross-tests/pu_driver.go | 38 +++++----- pkg/tests/cross-tests/pu_test.go | 102 +++++++++++++++++++++++++++ pkg/tests/cross-tests/tf_driver.go | 26 +------ 4 files changed, 146 insertions(+), 42 deletions(-) create mode 100644 pkg/tests/cross-tests/pu_test.go diff --git a/pkg/tests/cross-tests/input_check.go b/pkg/tests/cross-tests/input_check.go index a34552fad..ecb9ad2fa 100644 --- a/pkg/tests/cross-tests/input_check.go +++ b/pkg/tests/cross-tests/input_check.go @@ -71,6 +71,28 @@ func ensureProviderValid(t T, tfp *schema.Provider) { return nil } } + if r.DeleteContext == nil { + r.DeleteContext = func( + ctx context.Context, rd *schema.ResourceData, i interface{}, + ) diag.Diagnostics { + return diag.Diagnostics{} + } + } + + if r.CreateContext == nil { + r.CreateContext = func( + ctx context.Context, rd *schema.ResourceData, i interface{}, + ) diag.Diagnostics { + rd.SetId("newid") + return diag.Diagnostics{} + } + } + + r.UpdateContext = func( + ctx context.Context, rd *schema.ResourceData, i interface{}, + ) diag.Diagnostics { + return diag.Diagnostics{} + } } require.NoError(t, tfp.InternalValidate()) } diff --git a/pkg/tests/cross-tests/pu_driver.go b/pkg/tests/cross-tests/pu_driver.go index 884f93f92..a7493ded3 100644 --- a/pkg/tests/cross-tests/pu_driver.go +++ b/pkg/tests/cross-tests/pu_driver.go @@ -45,27 +45,13 @@ type pulumiDriver struct { objectType *tftypes.Object } -func (pd *pulumiDriver) providerInfo() tfbridge.ProviderInfo { - return tfbridge.ProviderInfo{ - Name: pd.name, - P: pd.shimProvider, - - Resources: map[string]*tfbridge.ResourceInfo{ - pd.tfResourceName: { - Tok: tokens.Type(pd.pulumiResourceToken), - }, - }, - } -} - -func (pd *pulumiDriver) startPulumiProvider(ctx context.Context) (*rpcutil.ServeHandle, error) { - info := pd.providerInfo() +func startPulumiProvider(ctx context.Context, name, version string, providerInfo tfbridge.ProviderInfo) (*rpcutil.ServeHandle, error) { sink := pulumidiag.DefaultSink(io.Discard, io.Discard, pulumidiag.FormatOptions{ Color: colors.Never, }) - schema, err := tfgen.GenerateSchema(info, sink) + schema, err := tfgen.GenerateSchema(providerInfo, sink) if err != nil { return nil, fmt.Errorf("tfgen.GenerateSchema failed: %w", err) } @@ -75,7 +61,7 @@ func (pd *pulumiDriver) startPulumiProvider(ctx context.Context) (*rpcutil.Serve return nil, fmt.Errorf("json.MarshalIndent(schema, ..) failed: %w", err) } - prov := tfbridge.NewProvider(ctx, nil, pd.name, pd.version, info.P, info, schemaBytes) + prov := tfbridge.NewProvider(ctx, nil, name, version, providerInfo.P, providerInfo, schemaBytes) handle, err := rpcutil.ServeWithOptions(rpcutil.ServeOptions{ Init: func(srv *grpc.Server) error { @@ -90,6 +76,24 @@ func (pd *pulumiDriver) startPulumiProvider(ctx context.Context) (*rpcutil.Serve return &handle, nil } +func (pd *pulumiDriver) providerInfo() tfbridge.ProviderInfo { + return tfbridge.ProviderInfo{ + Name: pd.name, + P: pd.shimProvider, + + Resources: map[string]*tfbridge.ResourceInfo{ + pd.tfResourceName: { + Tok: tokens.Type(pd.pulumiResourceToken), + }, + }, + } +} + +func (pd *pulumiDriver) startPulumiProvider(ctx context.Context) (*rpcutil.ServeHandle, error) { + info := pd.providerInfo() + return startPulumiProvider(ctx, pd.name, pd.version, info) +} + func (pd *pulumiDriver) writeYAML(t T, workdir string, tfConfig any) { res := pd.shimProvider.ResourcesMap().Get(pd.tfResourceName) schema := res.Schema() diff --git a/pkg/tests/cross-tests/pu_test.go b/pkg/tests/cross-tests/pu_test.go new file mode 100644 index 000000000..3a1f01306 --- /dev/null +++ b/pkg/tests/cross-tests/pu_test.go @@ -0,0 +1,102 @@ +package crosstests + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/pulumi/providertest/providers" + "github.com/pulumi/providertest/pulumitest" + "github.com/pulumi/providertest/pulumitest/opttest" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/tokens" + shimv2 "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2" + "github.com/stretchr/testify/require" +) + +func pulTest(t *testing.T, providerName string, resMap map[string]*schema.Resource, program string) *pulumitest.PulumiTest { + tfp := &schema.Provider{ResourcesMap: resMap} + ensureProviderValid(t, tfp) + + shimProvider := shimv2.NewProvider(tfp, shimv2.WithPlanResourceChange( + func(tfResourceType string) bool { return true }, + )) + + provider := tfbridge.ProviderInfo{ + P: shimProvider, + Name: providerName, + } + makeToken := func(module, name string) (string, error) { + return tokens.MakeStandard(providerName)(module, name) + } + provider.MustComputeTokens(tokens.SingleModule("prov", "index", makeToken)) + provider.MustApplyAutoAliases() + + puwd := t.TempDir() + p := filepath.Join(puwd, "Pulumi.yaml") + + err := os.WriteFile(p, []byte(program), 0o600) + require.NoError(t, err) + + opts := []opttest.Option{ + opttest.TestInPlace(), + opttest.SkipInstall(), + opttest.AttachProvider( + defProviderShortName, + func(ctx context.Context, pt providers.PulumiTest) (providers.Port, error) { + handle, err := startPulumiProvider(ctx, providerName, "0.0.1", provider) + require.NoError(t, err) + return providers.Port(handle.Port), nil + }, + ), + } + + return pulumitest.NewPulumiTest(t, puwd, opts...) +} + +func TestUnknownHandling(t *testing.T) { + resMap := map[string]*schema.Resource{ + "test": { + Schema: map[string]*schema.Schema{ + "test": { + Type: schema.TypeString, + Optional: true, + }, + }, + }, + "aux": { + Schema: map[string]*schema.Schema{ + "aux": { + Type: schema.TypeString, + Computed: true, + Optional: true, + }, + }, + CreateContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { + d.SetId("aux") + err := d.Set("aux", "aux") + require.NoError(t, err) + return nil + }, + }, + } + program := ` +name: test +runtime: yaml +resources: + auxRes: + type: prov:index:Aux + mainRes: + type: prov:index:Test + properties: + test: ${auxRes.aux} +outputs: + test: ${mainRes.test} +` + pt := pulTest(t, "prov", resMap, program) + res := pt.Up() + require.Equal(t, "aux", res.Outputs["test"]) +} diff --git a/pkg/tests/cross-tests/tf_driver.go b/pkg/tests/cross-tests/tf_driver.go index d44767f4d..ed1715a5e 100644 --- a/pkg/tests/cross-tests/tf_driver.go +++ b/pkg/tests/cross-tests/tf_driver.go @@ -31,10 +31,9 @@ import ( "github.com/hashicorp/terraform-plugin-go/tfprotov5" "github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server" "github.com/hashicorp/terraform-plugin-go/tftypes" - "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/convert" - "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2" + sdkv2 "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2" "github.com/pulumi/pulumi/sdk/v3/go/common/util/contract" "github.com/stretchr/testify/require" ) @@ -57,29 +56,6 @@ func newTfDriver(t T, dir, providerName, resName string, res *schema.Resource) * os.Setenv("TF_LOG_SDK", "off") os.Setenv("TF_LOG_SDK_PROTO", "off") - if res.DeleteContext == nil { - res.DeleteContext = func( - ctx context.Context, rd *schema.ResourceData, i interface{}, - ) diag.Diagnostics { - return diag.Diagnostics{} - } - } - - if res.CreateContext == nil { - res.CreateContext = func( - ctx context.Context, rd *schema.ResourceData, i interface{}, - ) diag.Diagnostics { - rd.SetId("newid") - return diag.Diagnostics{} - } - } - - res.UpdateContext = func( - ctx context.Context, rd *schema.ResourceData, i interface{}, - ) diag.Diagnostics { - return diag.Diagnostics{} - } - p := &schema.Provider{ ResourcesMap: map[string]*schema.Resource{ resName: res, From 85b85275f73db42b63302ead7491886d8318af17 Mon Sep 17 00:00:00 2001 From: Venelin Date: Mon, 3 Jun 2024 20:22:21 +0100 Subject: [PATCH 02/63] fix test --- pkg/tests/cross-tests/pu_test.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/pkg/tests/cross-tests/pu_test.go b/pkg/tests/cross-tests/pu_test.go index 3a1f01306..509777022 100644 --- a/pkg/tests/cross-tests/pu_test.go +++ b/pkg/tests/cross-tests/pu_test.go @@ -26,14 +26,14 @@ func pulTest(t *testing.T, providerName string, resMap map[string]*schema.Resour )) provider := tfbridge.ProviderInfo{ - P: shimProvider, - Name: providerName, + P: shimProvider, + Name: providerName, + MetadataInfo: &tfbridge.MetadataInfo{}, } makeToken := func(module, name string) (string, error) { return tokens.MakeStandard(providerName)(module, name) } provider.MustComputeTokens(tokens.SingleModule("prov", "index", makeToken)) - provider.MustApplyAutoAliases() puwd := t.TempDir() p := filepath.Join(puwd, "Pulumi.yaml") @@ -42,10 +42,11 @@ func pulTest(t *testing.T, providerName string, resMap map[string]*schema.Resour require.NoError(t, err) opts := []opttest.Option{ + opttest.Env("DISABLE_AUTOMATIC_PLUGIN_ACQUISITION", "true"), opttest.TestInPlace(), opttest.SkipInstall(), opttest.AttachProvider( - defProviderShortName, + providerName, func(ctx context.Context, pt providers.PulumiTest) (providers.Port, error) { handle, err := startPulumiProvider(ctx, providerName, "0.0.1", provider) require.NoError(t, err) @@ -59,7 +60,7 @@ func pulTest(t *testing.T, providerName string, resMap map[string]*schema.Resour func TestUnknownHandling(t *testing.T) { resMap := map[string]*schema.Resource{ - "test": { + "prov_test": { Schema: map[string]*schema.Schema{ "test": { Type: schema.TypeString, @@ -67,7 +68,7 @@ func TestUnknownHandling(t *testing.T) { }, }, }, - "aux": { + "prov_aux": { Schema: map[string]*schema.Schema{ "aux": { Type: schema.TypeString, @@ -91,12 +92,12 @@ resources: type: prov:index:Aux mainRes: type: prov:index:Test - properties: - test: ${auxRes.aux} + properties: + test: ${auxRes.aux} outputs: test: ${mainRes.test} ` pt := pulTest(t, "prov", resMap, program) res := pt.Up() - require.Equal(t, "aux", res.Outputs["test"]) + require.Equal(t, "aux", res.Outputs["test"].Value) } From 7dd99faa421a86032feaf6590b6a2ac1c2bbdebb Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 6 Jun 2024 10:42:36 +0100 Subject: [PATCH 03/63] refactor tests --- pkg/tests/cross-tests/diff_check.go | 45 ++---- pkg/tests/cross-tests/input_check.go | 39 +---- pkg/tests/cross-tests/pu_driver.go | 74 +--------- pkg/tests/cross-tests/pu_test.go | 103 ------------- pkg/tests/cross-tests/upgrade_state_check.go | 48 +----- pkg/tests/pulcheck/pulcheck.go | 147 +++++++++++++++++++ pkg/tests/schema_pulumi_test.go | 56 +++++++ 7 files changed, 230 insertions(+), 282 deletions(-) delete mode 100644 pkg/tests/cross-tests/pu_test.go create mode 100644 pkg/tests/pulcheck/pulcheck.go create mode 100644 pkg/tests/schema_pulumi_test.go diff --git a/pkg/tests/cross-tests/diff_check.go b/pkg/tests/cross-tests/diff_check.go index 4adfd8ef3..3faffd354 100644 --- a/pkg/tests/cross-tests/diff_check.go +++ b/pkg/tests/cross-tests/diff_check.go @@ -15,14 +15,12 @@ package crosstests import ( - "context" + "os" + "path/filepath" "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/pulumi/providertest/providers" - "github.com/pulumi/providertest/pulumitest" - "github.com/pulumi/providertest/pulumitest/opttest" - shimv2 "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tests/pulcheck" "github.com/pulumi/pulumi/sdk/v3/go/auto" "github.com/pulumi/pulumi/sdk/v3/go/common/apitype" "github.com/stretchr/testify/assert" @@ -52,7 +50,6 @@ func runDiffCheck(t T, tc diffTestCase) { rtype = "crossprovider_testres" rtok = "TestRes" rtoken = providerShortName + ":index:" + rtok - providerVer = "0.0.1" ) tfwd := t.TempDir() @@ -61,44 +58,24 @@ func runDiffCheck(t T, tc diffTestCase) { _ = tfd.writePlanApply(t, tc.Resource.Schema, rtype, "example", tc.Config1) tfDiffPlan := tfd.writePlanApply(t, tc.Resource.Schema, rtype, "example", tc.Config2) - tfp := &schema.Provider{ - ResourcesMap: map[string]*schema.Resource{ - rtype: tc.Resource, - }, - } - - shimProvider := shimv2.NewProvider(tfp, shimv2.WithPlanResourceChange( - func(tfResourceType string) bool { return true }, - )) + resMap := map[string]*schema.Resource{rtype: tc.Resource} + bridgedProvider := pulcheck.BridgedProvider(t, providerShortName, resMap) pd := &pulumiDriver{ name: providerShortName, - version: providerVer, - shimProvider: shimProvider, pulumiResourceToken: rtoken, tfResourceName: rtype, objectType: nil, } - - puwd := t.TempDir() - pd.writeYAML(t, puwd, tc.Config1) - - pt := pulumitest.NewPulumiTest(t, puwd, - opttest.TestInPlace(), - opttest.SkipInstall(), - opttest.AttachProvider( - providerShortName, - func(ctx context.Context, pt providers.PulumiTest) (providers.Port, error) { - handle, err := pd.startPulumiProvider(ctx) - require.NoError(t, err) - return providers.Port(handle.Port), nil - }, - ), - ) + yamlProgram := pd.generateYAML(t, bridgedProvider.P.ResourcesMap(),tc.Config1) + pt := pulcheck.PulCheck(t, bridgedProvider, string(yamlProgram)) pt.Up() - pd.writeYAML(t, puwd, tc.Config2) + yamlProgram = pd.generateYAML(t, bridgedProvider.P.ResourcesMap(), tc.Config2) + p := filepath.Join(pt.CurrentStack().Workspace().WorkDir(), "Pulumi.yaml") + err := os.WriteFile(p, yamlProgram, 0o600) + require.NoErrorf(t, err, "writing Pulumi.yaml") x := pt.Up() tfAction := tfd.parseChangesFromTFPlan(*tfDiffPlan) diff --git a/pkg/tests/cross-tests/input_check.go b/pkg/tests/cross-tests/input_check.go index ecb9ad2fa..71ebcee75 100644 --- a/pkg/tests/cross-tests/input_check.go +++ b/pkg/tests/cross-tests/input_check.go @@ -19,10 +19,7 @@ import ( "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/pulumi/providertest/providers" - "github.com/pulumi/providertest/pulumitest" - "github.com/pulumi/providertest/pulumitest/opttest" - shimv2 "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tests/pulcheck" "github.com/stretchr/testify/require" ) @@ -120,7 +117,6 @@ func runCreateInputCheck(t T, tc inputTestCase) { rtype = "crossprovider_testres" rtok = "TestRes" rtoken = providerShortName + ":index:" + rtok - providerVer = "0.0.1" ) tfwd := t.TempDir() @@ -128,42 +124,17 @@ func runCreateInputCheck(t T, tc inputTestCase) { tfd := newTfDriver(t, tfwd, providerShortName, rtype, tc.Resource) tfd.writePlanApply(t, tc.Resource.Schema, rtype, "example", tc.Config) - tfp := &schema.Provider{ - ResourcesMap: map[string]*schema.Resource{ - rtype: tc.Resource, - }, - } - ensureProviderValid(t, tfp) - - shimProvider := shimv2.NewProvider(tfp, shimv2.WithPlanResourceChange( - func(tfResourceType string) bool { return true }, - )) - + resMap := map[string]*schema.Resource{rtype: tc.Resource} + bridgedProvider := pulcheck.BridgedProvider(t, providerShortName, resMap) pd := &pulumiDriver{ name: providerShortName, - version: providerVer, - shimProvider: shimProvider, pulumiResourceToken: rtoken, tfResourceName: rtype, objectType: tc.ObjectType, } + yamlProgram := pd.generateYAML(t, bridgedProvider.P.ResourcesMap() ,tc.Config) - puwd := t.TempDir() - pd.writeYAML(t, puwd, tc.Config) - - pt := pulumitest.NewPulumiTest(t, puwd, - opttest.TestInPlace(), - opttest.SkipInstall(), - opttest.AttachProvider( - providerShortName, - func(ctx context.Context, pt providers.PulumiTest) (providers.Port, error) { - handle, err := pd.startPulumiProvider(ctx) - require.NoError(t, err) - return providers.Port(handle.Port), nil - }, - ), - opttest.Env("DISABLE_AUTOMATIC_PLUGIN_ACQUISITION", "true"), - ) + pt := pulcheck.PulCheck(t, bridgedProvider, string(yamlProgram)) pt.Up() diff --git a/pkg/tests/cross-tests/pu_driver.go b/pkg/tests/cross-tests/pu_driver.go index a7493ded3..493e9fac9 100644 --- a/pkg/tests/cross-tests/pu_driver.go +++ b/pkg/tests/cross-tests/pu_driver.go @@ -15,87 +15,21 @@ package crosstests import ( - "context" - "encoding/json" - "fmt" - "io" - "os" - "path/filepath" - "github.com/hashicorp/terraform-plugin-go/tftypes" - "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge" - "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfgen" shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" - pulumidiag "github.com/pulumi/pulumi/sdk/v3/go/common/diag" - "github.com/pulumi/pulumi/sdk/v3/go/common/diag/colors" - "github.com/pulumi/pulumi/sdk/v3/go/common/tokens" - "github.com/pulumi/pulumi/sdk/v3/go/common/util/rpcutil" - pulumirpc "github.com/pulumi/pulumi/sdk/v3/proto/go" "github.com/stretchr/testify/require" - "google.golang.org/grpc" "gopkg.in/yaml.v3" ) type pulumiDriver struct { name string - version string - shimProvider shim.Provider pulumiResourceToken string tfResourceName string objectType *tftypes.Object } - -func startPulumiProvider(ctx context.Context, name, version string, providerInfo tfbridge.ProviderInfo) (*rpcutil.ServeHandle, error) { - sink := pulumidiag.DefaultSink(io.Discard, io.Discard, pulumidiag.FormatOptions{ - Color: colors.Never, - }) - - schema, err := tfgen.GenerateSchema(providerInfo, sink) - if err != nil { - return nil, fmt.Errorf("tfgen.GenerateSchema failed: %w", err) - } - - schemaBytes, err := json.MarshalIndent(schema, "", " ") - if err != nil { - return nil, fmt.Errorf("json.MarshalIndent(schema, ..) failed: %w", err) - } - - prov := tfbridge.NewProvider(ctx, nil, name, version, providerInfo.P, providerInfo, schemaBytes) - - handle, err := rpcutil.ServeWithOptions(rpcutil.ServeOptions{ - Init: func(srv *grpc.Server) error { - pulumirpc.RegisterResourceProviderServer(srv, prov) - return nil - }, - }) - if err != nil { - return nil, fmt.Errorf("rpcutil.ServeWithOptions failed: %w", err) - } - - return &handle, nil -} - -func (pd *pulumiDriver) providerInfo() tfbridge.ProviderInfo { - return tfbridge.ProviderInfo{ - Name: pd.name, - P: pd.shimProvider, - - Resources: map[string]*tfbridge.ResourceInfo{ - pd.tfResourceName: { - Tok: tokens.Type(pd.pulumiResourceToken), - }, - }, - } -} - -func (pd *pulumiDriver) startPulumiProvider(ctx context.Context) (*rpcutil.ServeHandle, error) { - info := pd.providerInfo() - return startPulumiProvider(ctx, pd.name, pd.version, info) -} - -func (pd *pulumiDriver) writeYAML(t T, workdir string, tfConfig any) { - res := pd.shimProvider.ResourcesMap().Get(pd.tfResourceName) +func (pd *pulumiDriver) generateYAML(t T, resMap shim.ResourceMap, tfConfig any) []byte { + res := resMap.Get(pd.tfResourceName) schema := res.Schema() data, err := generateYaml(schema, pd.pulumiResourceToken, pd.objectType, tfConfig) @@ -104,7 +38,5 @@ func (pd *pulumiDriver) writeYAML(t T, workdir string, tfConfig any) { b, err := yaml.Marshal(data) require.NoErrorf(t, err, "marshaling Pulumi.yaml") t.Logf("\n\n%s", b) - p := filepath.Join(workdir, "Pulumi.yaml") - err = os.WriteFile(p, b, 0o600) - require.NoErrorf(t, err, "writing Pulumi.yaml") + return b } diff --git a/pkg/tests/cross-tests/pu_test.go b/pkg/tests/cross-tests/pu_test.go deleted file mode 100644 index 509777022..000000000 --- a/pkg/tests/cross-tests/pu_test.go +++ /dev/null @@ -1,103 +0,0 @@ -package crosstests - -import ( - "context" - "os" - "path/filepath" - "testing" - - "github.com/hashicorp/terraform-plugin-sdk/v2/diag" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/pulumi/providertest/providers" - "github.com/pulumi/providertest/pulumitest" - "github.com/pulumi/providertest/pulumitest/opttest" - "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge" - "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/tokens" - shimv2 "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2" - "github.com/stretchr/testify/require" -) - -func pulTest(t *testing.T, providerName string, resMap map[string]*schema.Resource, program string) *pulumitest.PulumiTest { - tfp := &schema.Provider{ResourcesMap: resMap} - ensureProviderValid(t, tfp) - - shimProvider := shimv2.NewProvider(tfp, shimv2.WithPlanResourceChange( - func(tfResourceType string) bool { return true }, - )) - - provider := tfbridge.ProviderInfo{ - P: shimProvider, - Name: providerName, - MetadataInfo: &tfbridge.MetadataInfo{}, - } - makeToken := func(module, name string) (string, error) { - return tokens.MakeStandard(providerName)(module, name) - } - provider.MustComputeTokens(tokens.SingleModule("prov", "index", makeToken)) - - puwd := t.TempDir() - p := filepath.Join(puwd, "Pulumi.yaml") - - err := os.WriteFile(p, []byte(program), 0o600) - require.NoError(t, err) - - opts := []opttest.Option{ - opttest.Env("DISABLE_AUTOMATIC_PLUGIN_ACQUISITION", "true"), - opttest.TestInPlace(), - opttest.SkipInstall(), - opttest.AttachProvider( - providerName, - func(ctx context.Context, pt providers.PulumiTest) (providers.Port, error) { - handle, err := startPulumiProvider(ctx, providerName, "0.0.1", provider) - require.NoError(t, err) - return providers.Port(handle.Port), nil - }, - ), - } - - return pulumitest.NewPulumiTest(t, puwd, opts...) -} - -func TestUnknownHandling(t *testing.T) { - resMap := map[string]*schema.Resource{ - "prov_test": { - Schema: map[string]*schema.Schema{ - "test": { - Type: schema.TypeString, - Optional: true, - }, - }, - }, - "prov_aux": { - Schema: map[string]*schema.Schema{ - "aux": { - Type: schema.TypeString, - Computed: true, - Optional: true, - }, - }, - CreateContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { - d.SetId("aux") - err := d.Set("aux", "aux") - require.NoError(t, err) - return nil - }, - }, - } - program := ` -name: test -runtime: yaml -resources: - auxRes: - type: prov:index:Aux - mainRes: - type: prov:index:Test - properties: - test: ${auxRes.aux} -outputs: - test: ${mainRes.test} -` - pt := pulTest(t, "prov", resMap, program) - res := pt.Up() - require.Equal(t, "aux", res.Outputs["test"].Value) -} diff --git a/pkg/tests/cross-tests/upgrade_state_check.go b/pkg/tests/cross-tests/upgrade_state_check.go index f6c119bfc..da3e3b084 100644 --- a/pkg/tests/cross-tests/upgrade_state_check.go +++ b/pkg/tests/cross-tests/upgrade_state_check.go @@ -5,10 +5,7 @@ import ( "fmt" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/pulumi/providertest/providers" - "github.com/pulumi/providertest/pulumitest" - "github.com/pulumi/providertest/pulumitest/opttest" - shimv2 "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tests/pulcheck" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -21,53 +18,24 @@ var ( DefProviderVer = "0.0.1" ) -func pulumiDriverFromRes(t T, res *schema.Resource) *pulumiDriver { - tfp := &schema.Provider{ - ResourcesMap: map[string]*schema.Resource{ - defRtype: res, - }, - } - ensureProviderValid(t, tfp) - shimProvider := shimv2.NewProvider(tfp, shimv2.WithPlanResourceChange( - func(tfResourceType string) bool { return true }, - )) +func runPulumiUpgrade(t T, res1, res2 *schema.Resource, config any) { + prov1 := pulcheck.BridgedProvider(t, defProviderShortName, map[string]*schema.Resource{defRtype: res1}) + prov2 := pulcheck.BridgedProvider(t, defProviderShortName, map[string]*schema.Resource{defRtype: res2}) - return &pulumiDriver{ + pd := &pulumiDriver{ name: defProviderShortName, - version: DefProviderVer, - shimProvider: shimProvider, pulumiResourceToken: defRtoken, tfResourceName: defRtype, objectType: nil, } -} - -func runPulumiUpgrade(t T, res1, res2 *schema.Resource, config any) { - pd := pulumiDriverFromRes(t, res1) - pd2 := pulumiDriverFromRes(t, res2) - - puwd := t.TempDir() - pd.writeYAML(t, puwd, config) - - opts := []opttest.Option{ - opttest.TestInPlace(), - opttest.SkipInstall(), - opttest.AttachProvider( - defProviderShortName, - func(ctx context.Context, pt providers.PulumiTest) (providers.Port, error) { - handle, err := pd.startPulumiProvider(ctx) - require.NoError(t, err) - return providers.Port(handle.Port), nil - }, - ), - } - pt := pulumitest.NewPulumiTest(t, puwd, opts...) + yamlProgram := pd.generateYAML(t, prov1.P.ResourcesMap(), config) + pt := pulcheck.PulCheck(t, prov1, string(yamlProgram)) pt.Up() - handle, err := pd2.startPulumiProvider(context.Background()) + handle, err := pulcheck.StartPulumiProvider(context.Background(), defProviderShortName, DefProviderVer, prov2) require.NoError(t, err) pt.CurrentStack().Workspace().SetEnvVar("PULUMI_DEBUG_PROVIDERS", fmt.Sprintf("%s:%d", defProviderShortName, handle.Port)) pt.Up() diff --git a/pkg/tests/pulcheck/pulcheck.go b/pkg/tests/pulcheck/pulcheck.go new file mode 100644 index 000000000..5e7b6c7fc --- /dev/null +++ b/pkg/tests/pulcheck/pulcheck.go @@ -0,0 +1,147 @@ +package pulcheck + +import ( + "context" + "encoding/json" + "fmt" + "io" + "os" + "path/filepath" + + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/pulumi/providertest/providers" + "github.com/pulumi/providertest/pulumitest" + "github.com/pulumi/providertest/pulumitest/opttest" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/info" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/tokens" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfgen" + shimv2 "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2" + pulumidiag "github.com/pulumi/pulumi/sdk/v3/go/common/diag" + "github.com/pulumi/pulumi/sdk/v3/go/common/diag/colors" + "github.com/pulumi/pulumi/sdk/v3/go/common/util/rpcutil" + pulumirpc "github.com/pulumi/pulumi/sdk/v3/proto/go" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "gotest.tools/assert" +) + +func CheckProviderValid(tfp *schema.Provider) error { + for _, r := range tfp.ResourcesMap { + //nolint:staticcheck + if r.Read == nil && r.ReadContext == nil { + r.ReadContext = func(_ context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics { + return nil + } + } + if r.DeleteContext == nil { + r.DeleteContext = func( + ctx context.Context, rd *schema.ResourceData, i interface{}, + ) diag.Diagnostics { + return diag.Diagnostics{} + } + } + + if r.CreateContext == nil { + r.CreateContext = func( + ctx context.Context, rd *schema.ResourceData, i interface{}, + ) diag.Diagnostics { + rd.SetId("newid") + return diag.Diagnostics{} + } + } + + r.UpdateContext = func( + ctx context.Context, rd *schema.ResourceData, i interface{}, + ) diag.Diagnostics { + return diag.Diagnostics{} + } + } + return tfp.InternalValidate() +} + +func StartPulumiProvider(ctx context.Context, name, version string, providerInfo tfbridge.ProviderInfo) (*rpcutil.ServeHandle, error) { + sink := pulumidiag.DefaultSink(io.Discard, io.Discard, pulumidiag.FormatOptions{ + Color: colors.Never, + }) + + schema, err := tfgen.GenerateSchema(providerInfo, sink) + if err != nil { + return nil, fmt.Errorf("tfgen.GenerateSchema failed: %w", err) + } + + schemaBytes, err := json.MarshalIndent(schema, "", " ") + if err != nil { + return nil, fmt.Errorf("json.MarshalIndent(schema, ..) failed: %w", err) + } + + prov := tfbridge.NewProvider(ctx, nil, name, version, providerInfo.P, providerInfo, schemaBytes) + + handle, err := rpcutil.ServeWithOptions(rpcutil.ServeOptions{ + Init: func(srv *grpc.Server) error { + pulumirpc.RegisterResourceProviderServer(srv, prov) + return nil + }, + }) + if err != nil { + return nil, fmt.Errorf("rpcutil.ServeWithOptions failed: %w", err) + } + + return &handle, nil +} + +type T interface { + Logf(string, ...any) + TempDir() string + require.TestingT + assert.TestingT + pulumitest.PT +} + +func BridgedProvider(t T, providerName string, resMap map[string]*schema.Resource) info.Provider { + tfp := &schema.Provider{ResourcesMap: resMap} + err := CheckProviderValid(tfp) + require.NoError(t, err) + + shimProvider := shimv2.NewProvider(tfp, shimv2.WithPlanResourceChange( + func(tfResourceType string) bool { return true }, + )) + + provider := tfbridge.ProviderInfo{ + P: shimProvider, + Name: providerName, + Version: "0.0.1", + MetadataInfo: &tfbridge.MetadataInfo{}, + } + makeToken := func(module, name string) (string, error) { + return tokens.MakeStandard(providerName)(module, name) + } + provider.MustComputeTokens(tokens.SingleModule("prov", "index", makeToken)) + + return provider +} + +func PulCheck(t T, bridgedProvider info.Provider, program string) *pulumitest.PulumiTest { + puwd := t.TempDir() + p := filepath.Join(puwd, "Pulumi.yaml") + + err := os.WriteFile(p, []byte(program), 0o600) + require.NoError(t, err) + + opts := []opttest.Option{ + opttest.Env("DISABLE_AUTOMATIC_PLUGIN_ACQUISITION", "true"), + opttest.TestInPlace(), + opttest.SkipInstall(), + opttest.AttachProvider( + bridgedProvider.Name, + func(ctx context.Context, pt providers.PulumiTest) (providers.Port, error) { + handle, err := StartPulumiProvider(ctx, bridgedProvider.Name, bridgedProvider.Version, bridgedProvider) + require.NoError(t, err) + return providers.Port(handle.Port), nil + }, + ), + } + + return pulumitest.NewPulumiTest(t, puwd, opts...) +} diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go new file mode 100644 index 000000000..5701d2c11 --- /dev/null +++ b/pkg/tests/schema_pulumi_test.go @@ -0,0 +1,56 @@ +package tests + +import ( + "context" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tests/pulcheck" + "github.com/stretchr/testify/require" +) + +func TestUnknownHandling(t *testing.T) { + resMap := map[string]*schema.Resource{ + "prov_test": { + Schema: map[string]*schema.Schema{ + "test": { + Type: schema.TypeString, + Optional: true, + }, + }, + }, + "prov_aux": { + Schema: map[string]*schema.Schema{ + "aux": { + Type: schema.TypeString, + Computed: true, + Optional: true, + }, + }, + CreateContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { + d.SetId("aux") + err := d.Set("aux", "aux") + require.NoError(t, err) + return nil + }, + }, + } + bridgedProvider := pulcheck.BridgedProvider(t, "prov", resMap) + program := ` +name: test +runtime: yaml +resources: + auxRes: + type: prov:index:Aux + mainRes: + type: prov:index:Test + properties: + test: ${auxRes.aux} +outputs: + test: ${mainRes.test} +` + pt := pulcheck.PulCheck(t, bridgedProvider, program) + res := pt.Up() + require.Equal(t, "aux", res.Outputs["test"].Value) +} From 87275865ebc4db46f41bd21f22da6034e96bdad8 Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 6 Jun 2024 10:49:29 +0100 Subject: [PATCH 04/63] lint --- pkg/tests/cross-tests/diff_check.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/tests/cross-tests/diff_check.go b/pkg/tests/cross-tests/diff_check.go index 3faffd354..3ffbaa21e 100644 --- a/pkg/tests/cross-tests/diff_check.go +++ b/pkg/tests/cross-tests/diff_check.go @@ -67,7 +67,7 @@ func runDiffCheck(t T, tc diffTestCase) { tfResourceName: rtype, objectType: nil, } - yamlProgram := pd.generateYAML(t, bridgedProvider.P.ResourcesMap(),tc.Config1) + yamlProgram := pd.generateYAML(t, bridgedProvider.P.ResourcesMap(), tc.Config1) pt := pulcheck.PulCheck(t, bridgedProvider, string(yamlProgram)) pt.Up() From 28bfadbd4db4bc036e74915bb178ef6323299378 Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 6 Jun 2024 10:52:02 +0100 Subject: [PATCH 05/63] go mod tidy --- pkg/tests/go.mod | 1 + pkg/tests/go.sum | 2 ++ 2 files changed, 3 insertions(+) diff --git a/pkg/tests/go.mod b/pkg/tests/go.mod index 399693e42..c168eb917 100644 --- a/pkg/tests/go.mod +++ b/pkg/tests/go.mod @@ -14,6 +14,7 @@ require ( github.com/pulumi/providertest v0.0.12 github.com/pulumi/pulumi-terraform-bridge/v3 v3.80.0 github.com/stretchr/testify v1.9.0 + gotest.tools v2.2.0+incompatible pgregory.net/rapid v0.6.1 ) diff --git a/pkg/tests/go.sum b/pkg/tests/go.sum index 048d504a0..881ecc7f1 100644 --- a/pkg/tests/go.sum +++ b/pkg/tests/go.sum @@ -2962,6 +2962,8 @@ gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo= +gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw= gotest.tools/v3 v3.0.3 h1:4AuOwCGf4lLR9u3YOe2awrHygurzhO/HeQ6laiA6Sx0= gotest.tools/v3 v3.0.3/go.mod h1:Z7Lb0S5l+klDB31fvDQX8ss/FlKDxtlFlw3Oa8Ymbl8= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= From 1ba4c87e6d6ef22af19c93fa48ee75eecc7be9ae Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 6 Jun 2024 10:54:25 +0100 Subject: [PATCH 06/63] hide function --- pkg/tests/pulcheck/pulcheck.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/tests/pulcheck/pulcheck.go b/pkg/tests/pulcheck/pulcheck.go index 5e7b6c7fc..56d9577fc 100644 --- a/pkg/tests/pulcheck/pulcheck.go +++ b/pkg/tests/pulcheck/pulcheck.go @@ -27,7 +27,7 @@ import ( "gotest.tools/assert" ) -func CheckProviderValid(tfp *schema.Provider) error { +func ensureProviderValid(t T, tfp *schema.Provider) { for _, r := range tfp.ResourcesMap { //nolint:staticcheck if r.Read == nil && r.ReadContext == nil { @@ -58,7 +58,7 @@ func CheckProviderValid(tfp *schema.Provider) error { return diag.Diagnostics{} } } - return tfp.InternalValidate() + require.NoError(t, tfp.InternalValidate()) } func StartPulumiProvider(ctx context.Context, name, version string, providerInfo tfbridge.ProviderInfo) (*rpcutil.ServeHandle, error) { @@ -101,8 +101,7 @@ type T interface { func BridgedProvider(t T, providerName string, resMap map[string]*schema.Resource) info.Provider { tfp := &schema.Provider{ResourcesMap: resMap} - err := CheckProviderValid(tfp) - require.NoError(t, err) + ensureProviderValid(t, tfp) shimProvider := shimv2.NewProvider(tfp, shimv2.WithPlanResourceChange( func(tfResourceType string) bool { return true }, From b9271d635ce3835a0aad60908deb5fed9e89e320 Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 6 Jun 2024 11:31:18 +0100 Subject: [PATCH 07/63] move things about --- pkg/tests/cross-tests/assert.go | 30 +++++++ pkg/tests/cross-tests/defaults.go | 9 +++ pkg/tests/cross-tests/diff_check.go | 23 ++---- pkg/tests/cross-tests/input_check.go | 82 ++------------------ pkg/tests/cross-tests/upgrade_state_check.go | 9 --- 5 files changed, 55 insertions(+), 98 deletions(-) create mode 100644 pkg/tests/cross-tests/assert.go create mode 100644 pkg/tests/cross-tests/defaults.go diff --git a/pkg/tests/cross-tests/assert.go b/pkg/tests/cross-tests/assert.go new file mode 100644 index 000000000..9d862baf2 --- /dev/null +++ b/pkg/tests/cross-tests/assert.go @@ -0,0 +1,30 @@ +package crosstests + +import ( + "github.com/hashicorp/go-cty/cty" + "github.com/stretchr/testify/require" +) + +func FailNotEqual(t T, name string, tfVal, pulVal any) { + t.Logf(name + " not equal!") + t.Logf("TF value %s", tfVal) + t.Logf("PU value %s", pulVal) + t.Fail() +} + +func assertCtyValEqual(t T, name string, tfVal, pulVal cty.Value) { + if !tfVal.RawEquals(pulVal) { + FailNotEqual(t, name, tfVal.GoString(), pulVal.GoString()) + } +} + +func assertValEqual(t T, name string, tfVal, pulVal any) { + // usually plugin-sdk schema types + if hasEqualTfVal, ok := tfVal.(interface{ Equal(interface{}) bool }); ok { + if !hasEqualTfVal.Equal(pulVal) { + FailNotEqual(t, name, tfVal, pulVal) + } + } else { + require.Equal(t, tfVal, pulVal, "Values for key %s do not match", name) + } +} diff --git a/pkg/tests/cross-tests/defaults.go b/pkg/tests/cross-tests/defaults.go new file mode 100644 index 000000000..da3cfba9f --- /dev/null +++ b/pkg/tests/cross-tests/defaults.go @@ -0,0 +1,9 @@ +package crosstests + +var ( + defProviderShortName = "crossprovider" + defRtype = "crossprovider_testres" + defRtok = "TestRes" + defRtoken = defProviderShortName + ":index:" + defRtok + DefProviderVer = "0.0.1" +) diff --git a/pkg/tests/cross-tests/diff_check.go b/pkg/tests/cross-tests/diff_check.go index 3ffbaa21e..d63aa141f 100644 --- a/pkg/tests/cross-tests/diff_check.go +++ b/pkg/tests/cross-tests/diff_check.go @@ -45,26 +45,19 @@ type diffTestCase struct { } func runDiffCheck(t T, tc diffTestCase) { - var ( - providerShortName = "crossprovider" - rtype = "crossprovider_testres" - rtok = "TestRes" - rtoken = providerShortName + ":index:" + rtok - ) - tfwd := t.TempDir() - tfd := newTfDriver(t, tfwd, providerShortName, rtype, tc.Resource) - _ = tfd.writePlanApply(t, tc.Resource.Schema, rtype, "example", tc.Config1) - tfDiffPlan := tfd.writePlanApply(t, tc.Resource.Schema, rtype, "example", tc.Config2) + tfd := newTfDriver(t, tfwd, defProviderShortName, defRtype, tc.Resource) + _ = tfd.writePlanApply(t, tc.Resource.Schema, defRtype, "example", tc.Config1) + tfDiffPlan := tfd.writePlanApply(t, tc.Resource.Schema, defRtype, "example", tc.Config2) - resMap := map[string]*schema.Resource{rtype: tc.Resource} - bridgedProvider := pulcheck.BridgedProvider(t, providerShortName, resMap) + resMap := map[string]*schema.Resource{defRtype: tc.Resource} + bridgedProvider := pulcheck.BridgedProvider(t, defProviderShortName, resMap) pd := &pulumiDriver{ - name: providerShortName, - pulumiResourceToken: rtoken, - tfResourceName: rtype, + name: defProviderShortName, + pulumiResourceToken: defRtoken, + tfResourceName: defRtype, objectType: nil, } yamlProgram := pd.generateYAML(t, bridgedProvider.P.ResourcesMap(), tc.Config1) diff --git a/pkg/tests/cross-tests/input_check.go b/pkg/tests/cross-tests/input_check.go index 71ebcee75..2ede4b609 100644 --- a/pkg/tests/cross-tests/input_check.go +++ b/pkg/tests/cross-tests/input_check.go @@ -15,12 +15,10 @@ package crosstests import ( "context" - "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tests/pulcheck" - "github.com/stretchr/testify/require" ) // Adapted from diff_check.go @@ -36,64 +34,6 @@ type inputTestCase struct { SkipCompareRawState bool } -func FailNotEqual(t T, name string, tfVal, pulVal any) { - t.Logf(name + " not equal!") - t.Logf("TF value %s", tfVal) - t.Logf("PU value %s", pulVal) - t.Fail() -} - -func assertCtyValEqual(t T, name string, tfVal, pulVal cty.Value) { - if !tfVal.RawEquals(pulVal) { - FailNotEqual(t, name, tfVal.GoString(), pulVal.GoString()) - } -} - -func assertValEqual(t T, name string, tfVal, pulVal any) { - // usually plugin-sdk schema types - if hasEqualTfVal, ok := tfVal.(interface{ Equal(interface{}) bool }); ok { - if !hasEqualTfVal.Equal(pulVal) { - FailNotEqual(t, name, tfVal, pulVal) - } - } else { - require.Equal(t, tfVal, pulVal, "Values for key %s do not match", name) - } -} - -func ensureProviderValid(t T, tfp *schema.Provider) { - for _, r := range tfp.ResourcesMap { - //nolint:staticcheck - if r.Read == nil && r.ReadContext == nil { - r.ReadContext = func(_ context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics { - return nil - } - } - if r.DeleteContext == nil { - r.DeleteContext = func( - ctx context.Context, rd *schema.ResourceData, i interface{}, - ) diag.Diagnostics { - return diag.Diagnostics{} - } - } - - if r.CreateContext == nil { - r.CreateContext = func( - ctx context.Context, rd *schema.ResourceData, i interface{}, - ) diag.Diagnostics { - rd.SetId("newid") - return diag.Diagnostics{} - } - } - - r.UpdateContext = func( - ctx context.Context, rd *schema.ResourceData, i interface{}, - ) diag.Diagnostics { - return diag.Diagnostics{} - } - } - require.NoError(t, tfp.InternalValidate()) -} - // Adapted from diff_check.go func runCreateInputCheck(t T, tc inputTestCase) { //nolint:staticcheck @@ -112,27 +52,21 @@ func runCreateInputCheck(t T, tc inputTestCase) { rd.SetId("someid") // CreateContext must pick an ID return make(diag.Diagnostics, 0) } - var ( - providerShortName = "crossprovider" - rtype = "crossprovider_testres" - rtok = "TestRes" - rtoken = providerShortName + ":index:" + rtok - ) tfwd := t.TempDir() - tfd := newTfDriver(t, tfwd, providerShortName, rtype, tc.Resource) - tfd.writePlanApply(t, tc.Resource.Schema, rtype, "example", tc.Config) + tfd := newTfDriver(t, tfwd, defProviderShortName, defRtype, tc.Resource) + tfd.writePlanApply(t, tc.Resource.Schema, defRtype, "example", tc.Config) - resMap := map[string]*schema.Resource{rtype: tc.Resource} - bridgedProvider := pulcheck.BridgedProvider(t, providerShortName, resMap) + resMap := map[string]*schema.Resource{defRtype: tc.Resource} + bridgedProvider := pulcheck.BridgedProvider(t, defProviderShortName, resMap) pd := &pulumiDriver{ - name: providerShortName, - pulumiResourceToken: rtoken, - tfResourceName: rtype, + name: defProviderShortName, + pulumiResourceToken: defRtoken, + tfResourceName: defRtype, objectType: tc.ObjectType, } - yamlProgram := pd.generateYAML(t, bridgedProvider.P.ResourcesMap() ,tc.Config) + yamlProgram := pd.generateYAML(t, bridgedProvider.P.ResourcesMap(), tc.Config) pt := pulcheck.PulCheck(t, bridgedProvider, string(yamlProgram)) diff --git a/pkg/tests/cross-tests/upgrade_state_check.go b/pkg/tests/cross-tests/upgrade_state_check.go index da3e3b084..af8fd8258 100644 --- a/pkg/tests/cross-tests/upgrade_state_check.go +++ b/pkg/tests/cross-tests/upgrade_state_check.go @@ -10,15 +10,6 @@ import ( "github.com/stretchr/testify/require" ) -var ( - defProviderShortName = "crossprovider" - defRtype = "crossprovider_testres" - defRtok = "TestRes" - defRtoken = defProviderShortName + ":index:" + defRtok - DefProviderVer = "0.0.1" -) - - func runPulumiUpgrade(t T, res1, res2 *schema.Resource, config any) { prov1 := pulcheck.BridgedProvider(t, defProviderShortName, map[string]*schema.Resource{defRtype: res1}) prov2 := pulcheck.BridgedProvider(t, defProviderShortName, map[string]*schema.Resource{defRtype: res2}) From 386478179851bed36c27b212e0c5a6fd15fc2971 Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 6 Jun 2024 12:08:07 +0100 Subject: [PATCH 08/63] fix invalid tf provider --- pkg/tests/cross-tests/defaults.go | 2 +- pkg/tests/cross-tests/input_check.go | 3 +-- pkg/tests/cross-tests/tf_driver.go | 2 ++ pkg/tests/pulcheck/pulcheck.go | 9 ++++----- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/tests/cross-tests/defaults.go b/pkg/tests/cross-tests/defaults.go index da3cfba9f..7790c1190 100644 --- a/pkg/tests/cross-tests/defaults.go +++ b/pkg/tests/cross-tests/defaults.go @@ -2,7 +2,7 @@ package crosstests var ( defProviderShortName = "crossprovider" - defRtype = "crossprovider_testres" + defRtype = "crossprovider_test_res" defRtok = "TestRes" defRtoken = defProviderShortName + ":index:" + defRtok DefProviderVer = "0.0.1" diff --git a/pkg/tests/cross-tests/input_check.go b/pkg/tests/cross-tests/input_check.go index 2ede4b609..4a10d9600 100644 --- a/pkg/tests/cross-tests/input_check.go +++ b/pkg/tests/cross-tests/input_check.go @@ -36,8 +36,7 @@ type inputTestCase struct { // Adapted from diff_check.go func runCreateInputCheck(t T, tc inputTestCase) { - //nolint:staticcheck - if tc.Resource.CreateContext != nil || tc.Resource.Create != nil { + if tc.Resource.CreateContext != nil { t.Errorf("Create methods should not be set for these tests!") } diff --git a/pkg/tests/cross-tests/tf_driver.go b/pkg/tests/cross-tests/tf_driver.go index ed1715a5e..68ec8bca6 100644 --- a/pkg/tests/cross-tests/tf_driver.go +++ b/pkg/tests/cross-tests/tf_driver.go @@ -33,6 +33,7 @@ import ( "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/convert" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tests/pulcheck" sdkv2 "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2" "github.com/pulumi/pulumi/sdk/v3/go/common/util/contract" "github.com/stretchr/testify/require" @@ -61,6 +62,7 @@ func newTfDriver(t T, dir, providerName, resName string, res *schema.Resource) * resName: res, }, } + pulcheck.EnsureProviderValid(t, p) serverFactory := func() tfprotov5.ProviderServer { return p.GRPCProvider() diff --git a/pkg/tests/pulcheck/pulcheck.go b/pkg/tests/pulcheck/pulcheck.go index 56d9577fc..127ee7d6f 100644 --- a/pkg/tests/pulcheck/pulcheck.go +++ b/pkg/tests/pulcheck/pulcheck.go @@ -27,10 +27,9 @@ import ( "gotest.tools/assert" ) -func ensureProviderValid(t T, tfp *schema.Provider) { +func EnsureProviderValid(t T, tfp *schema.Provider) { for _, r := range tfp.ResourcesMap { - //nolint:staticcheck - if r.Read == nil && r.ReadContext == nil { + if r.ReadContext == nil { r.ReadContext = func(_ context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics { return nil } @@ -101,7 +100,7 @@ type T interface { func BridgedProvider(t T, providerName string, resMap map[string]*schema.Resource) info.Provider { tfp := &schema.Provider{ResourcesMap: resMap} - ensureProviderValid(t, tfp) + EnsureProviderValid(t, tfp) shimProvider := shimv2.NewProvider(tfp, shimv2.WithPlanResourceChange( func(tfResourceType string) bool { return true }, @@ -116,7 +115,7 @@ func BridgedProvider(t T, providerName string, resMap map[string]*schema.Resourc makeToken := func(module, name string) (string, error) { return tokens.MakeStandard(providerName)(module, name) } - provider.MustComputeTokens(tokens.SingleModule("prov", "index", makeToken)) + provider.MustComputeTokens(tokens.SingleModule(providerName, "index", makeToken)) return provider } From db35f45b8d74509a7d5df70a6bb169d87ffa4d42 Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 6 Jun 2024 12:23:07 +0100 Subject: [PATCH 09/63] finish test --- pkg/tests/schema_pulumi_test.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index 5701d2c11..4ce831e67 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tests/pulcheck" + "github.com/pulumi/pulumi/sdk/v3/go/auto/optpreview" "github.com/stretchr/testify/require" ) @@ -48,9 +49,13 @@ resources: properties: test: ${auxRes.aux} outputs: - test: ${mainRes.test} + testOut: ${mainRes.test} ` pt := pulcheck.PulCheck(t, bridgedProvider, program) - res := pt.Up() - require.Equal(t, "aux", res.Outputs["test"].Value) + res := pt.Preview(optpreview.Diff()) + // Test that the test property is unknown at preview time + require.Contains(t, res.StdOut, "test : output") + resUp := pt.Up() + // assert that the property gets resolved + require.Equal(t, "aux", resUp.Outputs["testOut"].Value) } From 2f06781a9137dcf8cb12936524288158f0290143 Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 6 Jun 2024 12:24:36 +0100 Subject: [PATCH 10/63] add some experimental comments --- pkg/tests/pulcheck/pulcheck.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/tests/pulcheck/pulcheck.go b/pkg/tests/pulcheck/pulcheck.go index 127ee7d6f..65ab4c91f 100644 --- a/pkg/tests/pulcheck/pulcheck.go +++ b/pkg/tests/pulcheck/pulcheck.go @@ -27,6 +27,7 @@ import ( "gotest.tools/assert" ) +// This is an experimental API. func EnsureProviderValid(t T, tfp *schema.Provider) { for _, r := range tfp.ResourcesMap { if r.ReadContext == nil { @@ -60,6 +61,7 @@ func EnsureProviderValid(t T, tfp *schema.Provider) { require.NoError(t, tfp.InternalValidate()) } +// This is an experimental API. func StartPulumiProvider(ctx context.Context, name, version string, providerInfo tfbridge.ProviderInfo) (*rpcutil.ServeHandle, error) { sink := pulumidiag.DefaultSink(io.Discard, io.Discard, pulumidiag.FormatOptions{ Color: colors.Never, @@ -90,6 +92,7 @@ func StartPulumiProvider(ctx context.Context, name, version string, providerInfo return &handle, nil } +// This is an experimental API. type T interface { Logf(string, ...any) TempDir() string @@ -98,6 +101,7 @@ type T interface { pulumitest.PT } +// This is an experimental API. func BridgedProvider(t T, providerName string, resMap map[string]*schema.Resource) info.Provider { tfp := &schema.Provider{ResourcesMap: resMap} EnsureProviderValid(t, tfp) @@ -120,6 +124,7 @@ func BridgedProvider(t T, providerName string, resMap map[string]*schema.Resourc return provider } +// This is an experimental API. func PulCheck(t T, bridgedProvider info.Provider, program string) *pulumitest.PulumiTest { puwd := t.TempDir() p := filepath.Join(puwd, "Pulumi.yaml") From a6cc58330717d4ce1b858b468c21d82e985c3324 Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 6 Jun 2024 13:04:48 +0100 Subject: [PATCH 11/63] dirty map refresh repro --- pkg/tests/schema_pulumi_test.go | 47 +++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index 4ce831e67..7c553cef7 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -8,6 +8,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tests/pulcheck" "github.com/pulumi/pulumi/sdk/v3/go/auto/optpreview" + "github.com/pulumi/pulumi/sdk/v3/go/auto/optrefresh" "github.com/stretchr/testify/require" ) @@ -59,3 +60,49 @@ outputs: // assert that the property gets resolved require.Equal(t, "aux", resUp.Outputs["testOut"].Value) } + +func TestEmptyMapsRefreshClean(t *testing.T) { + resMap := map[string]*schema.Resource{ + "prov_test": { + Schema: map[string]*schema.Schema{ + "map_prop": { + Type: schema.TypeMap, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + "other_prop": { + Type: schema.TypeString, + Optional: true, + }, + }, + ReadContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { + err := d.Set("map_prop", map[string]interface{}{}) + require.NoError(t, err) + err = d.Set("other_prop", "test") + require.NoError(t, err) + return nil + }, + }, + } + bridgedProvider := pulcheck.BridgedProvider(t, "prov", resMap) + program := ` +name: test +runtime: yaml +resources: + mainRes: + type: prov:index:Test + properties: + otherProp: "test" +outputs: + mapPropOut: ${mainRes.mapProp} +` + pt := pulcheck.PulCheck(t, bridgedProvider, program) + + upRes := pt.Up() + require.Equal(t, nil, upRes.Outputs["mapPropOut"].Value) + + res := pt.Refresh(optrefresh.ExpectNoChanges()) + t.Logf(res.StdOut) +} From 8e50e2d47f9a7cfb13a34c3900adc903ff3381fd Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 6 Jun 2024 13:06:15 +0100 Subject: [PATCH 12/63] remove comments --- pkg/tests/pulcheck/pulcheck.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/tests/pulcheck/pulcheck.go b/pkg/tests/pulcheck/pulcheck.go index 65ab4c91f..127ee7d6f 100644 --- a/pkg/tests/pulcheck/pulcheck.go +++ b/pkg/tests/pulcheck/pulcheck.go @@ -27,7 +27,6 @@ import ( "gotest.tools/assert" ) -// This is an experimental API. func EnsureProviderValid(t T, tfp *schema.Provider) { for _, r := range tfp.ResourcesMap { if r.ReadContext == nil { @@ -61,7 +60,6 @@ func EnsureProviderValid(t T, tfp *schema.Provider) { require.NoError(t, tfp.InternalValidate()) } -// This is an experimental API. func StartPulumiProvider(ctx context.Context, name, version string, providerInfo tfbridge.ProviderInfo) (*rpcutil.ServeHandle, error) { sink := pulumidiag.DefaultSink(io.Discard, io.Discard, pulumidiag.FormatOptions{ Color: colors.Never, @@ -92,7 +90,6 @@ func StartPulumiProvider(ctx context.Context, name, version string, providerInfo return &handle, nil } -// This is an experimental API. type T interface { Logf(string, ...any) TempDir() string @@ -101,7 +98,6 @@ type T interface { pulumitest.PT } -// This is an experimental API. func BridgedProvider(t T, providerName string, resMap map[string]*schema.Resource) info.Provider { tfp := &schema.Provider{ResourcesMap: resMap} EnsureProviderValid(t, tfp) @@ -124,7 +120,6 @@ func BridgedProvider(t T, providerName string, resMap map[string]*schema.Resourc return provider } -// This is an experimental API. func PulCheck(t T, bridgedProvider info.Provider, program string) *pulumitest.PulumiTest { puwd := t.TempDir() p := filepath.Join(puwd, "Pulumi.yaml") From 08d78d3a82454f00ccd38c4d035805926b7830b4 Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 6 Jun 2024 13:10:17 +0100 Subject: [PATCH 13/63] make test schema valid --- pkg/tests/cross-tests/cross_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/tests/cross-tests/cross_test.go b/pkg/tests/cross-tests/cross_test.go index 92f5eec86..12098b67d 100644 --- a/pkg/tests/cross-tests/cross_test.go +++ b/pkg/tests/cross-tests/cross_test.go @@ -98,7 +98,6 @@ func TestSetReordering(t *testing.T) { Optional: true, Elem: &schema.Schema{ Type: schema.TypeString, - Optional: true, }, }, }, From 9761d636c4acdef930f60af6272bcc7ad8a2b813 Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 6 Jun 2024 13:13:05 +0100 Subject: [PATCH 14/63] accidental public var --- pkg/tests/cross-tests/defaults.go | 2 +- pkg/tests/cross-tests/upgrade_state_check.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/tests/cross-tests/defaults.go b/pkg/tests/cross-tests/defaults.go index 7790c1190..eb9664b91 100644 --- a/pkg/tests/cross-tests/defaults.go +++ b/pkg/tests/cross-tests/defaults.go @@ -5,5 +5,5 @@ var ( defRtype = "crossprovider_test_res" defRtok = "TestRes" defRtoken = defProviderShortName + ":index:" + defRtok - DefProviderVer = "0.0.1" + defProviderVer = "0.0.1" ) diff --git a/pkg/tests/cross-tests/upgrade_state_check.go b/pkg/tests/cross-tests/upgrade_state_check.go index af8fd8258..e17d92c08 100644 --- a/pkg/tests/cross-tests/upgrade_state_check.go +++ b/pkg/tests/cross-tests/upgrade_state_check.go @@ -26,7 +26,7 @@ func runPulumiUpgrade(t T, res1, res2 *schema.Resource, config any) { pt.Up() - handle, err := pulcheck.StartPulumiProvider(context.Background(), defProviderShortName, DefProviderVer, prov2) + handle, err := pulcheck.StartPulumiProvider(context.Background(), defProviderShortName, defProviderVer, prov2) require.NoError(t, err) pt.CurrentStack().Workspace().SetEnvVar("PULUMI_DEBUG_PROVIDERS", fmt.Sprintf("%s:%d", defProviderShortName, handle.Port)) pt.Up() From b775cc6ca222149aecc63a2e52ed6253d6070ff1 Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 6 Jun 2024 13:13:28 +0100 Subject: [PATCH 15/63] lint --- pkg/tests/cross-tests/cross_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/tests/cross-tests/cross_test.go b/pkg/tests/cross-tests/cross_test.go index 12098b67d..aad3d0714 100644 --- a/pkg/tests/cross-tests/cross_test.go +++ b/pkg/tests/cross-tests/cross_test.go @@ -97,7 +97,7 @@ func TestSetReordering(t *testing.T) { Type: schema.TypeSet, Optional: true, Elem: &schema.Schema{ - Type: schema.TypeString, + Type: schema.TypeString, }, }, }, From 36c44a8229770f33b5227fc536b7b3a42f8bca8f Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 6 Jun 2024 13:28:25 +0100 Subject: [PATCH 16/63] empty map input cross test --- pkg/tests/cross-tests/input_cross_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/pkg/tests/cross-tests/input_cross_test.go b/pkg/tests/cross-tests/input_cross_test.go index 359ee88c1..fa7943e10 100644 --- a/pkg/tests/cross-tests/input_cross_test.go +++ b/pkg/tests/cross-tests/input_cross_test.go @@ -553,3 +553,21 @@ func TestCreateDoesNotPanicWithStateUpgraders(t *testing.T) { }, }) } + +func TestEmptyMap(t *testing.T) { + skipUnlessLinux(t) + runCreateInputCheck(t, inputTestCase{ + Resource: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "tags": { + Type: schema.TypeMap, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + }, + }, + Config: tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{}), + }) +} From 6d817998fb8968a521bdb8d65f0cd75e080fff69 Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 6 Jun 2024 13:44:36 +0100 Subject: [PATCH 17/63] move skip unless linux --- pkg/tests/cross-tests/ci.go | 28 ------------------- pkg/tests/cross-tests/cross_test.go | 9 ------ pkg/tests/cross-tests/input_cross_test.go | 12 -------- .../cross-tests/upgrade_state_cross_test.go | 4 --- pkg/tests/pulcheck/pulcheck.go | 11 ++++++++ 5 files changed, 11 insertions(+), 53 deletions(-) delete mode 100644 pkg/tests/cross-tests/ci.go diff --git a/pkg/tests/cross-tests/ci.go b/pkg/tests/cross-tests/ci.go deleted file mode 100644 index 2565e8cfa..000000000 --- a/pkg/tests/cross-tests/ci.go +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright 2016-2024, 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 - -// Helpers to disable failing CI runs. -package crosstests - -import ( - "os" - "runtime" - "strings" - "testing" -) - -func skipUnlessLinux(t *testing.T) { - if ci, ok := os.LookupEnv("CI"); ok && ci == "true" && !strings.Contains(strings.ToLower(runtime.GOOS), "linux") { - t.Skip("Skipping on non-Linux platforms as our CI does not yet install Terraform CLI required for these tests") - } -} diff --git a/pkg/tests/cross-tests/cross_test.go b/pkg/tests/cross-tests/cross_test.go index aad3d0714..cb1bea846 100644 --- a/pkg/tests/cross-tests/cross_test.go +++ b/pkg/tests/cross-tests/cross_test.go @@ -30,7 +30,6 @@ import ( ) func TestUnchangedBasicObject(t *testing.T) { - skipUnlessLinux(t) cfg := map[string]any{"f0": []any{map[string]any{"x": "ok"}}} runDiffCheck(t, diffTestCase{ Resource: &schema.Resource{ @@ -53,7 +52,6 @@ func TestUnchangedBasicObject(t *testing.T) { } func TestSimpleStringNoChange(t *testing.T) { - skipUnlessLinux(t) config := map[string]any{"name": "A"} runDiffCheck(t, diffTestCase{ Resource: &schema.Resource{ @@ -70,7 +68,6 @@ func TestSimpleStringNoChange(t *testing.T) { } func TestSimpleStringRename(t *testing.T) { - skipUnlessLinux(t) runDiffCheck(t, diffTestCase{ Resource: &schema.Resource{ Schema: map[string]*schema.Schema{ @@ -90,7 +87,6 @@ func TestSimpleStringRename(t *testing.T) { } func TestSetReordering(t *testing.T) { - skipUnlessLinux(t) resource := &schema.Resource{ Schema: map[string]*schema.Schema{ "set": { @@ -121,7 +117,6 @@ func TestSetReordering(t *testing.T) { // │ 1: resource "crossprovider_testres" "example" { func TestEmptyRequiredList(t *testing.T) { t.Skip("TODO - fix panic and make a negative test here") - skipUnlessLinux(t) resource := &schema.Resource{ Schema: map[string]*schema.Schema{ "f0": { @@ -146,7 +141,6 @@ func TestEmptyRequiredList(t *testing.T) { } func TestAws2442(t *testing.T) { - skipUnlessLinux(t) hashes := map[int]string{} stringHashcode := func(s string) int { @@ -404,7 +398,6 @@ func TestAws2442(t *testing.T) { } func TestSimpleOptionalComputed(t *testing.T) { - skipUnlessLinux(t) emptyConfig := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{}) nonEmptyConfig := tftypes.NewValue( tftypes.Object{ @@ -451,7 +444,6 @@ func TestSimpleOptionalComputed(t *testing.T) { } func TestOptionalComputedAttrCollection(t *testing.T) { - skipUnlessLinux(t) emptyConfig := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{}) t0 := tftypes.List{ElementType: tftypes.String} t1 := tftypes.Object{ @@ -522,7 +514,6 @@ func TestOptionalComputedAttrCollection(t *testing.T) { } func TestOptionalComputedBlockCollection(t *testing.T) { - skipUnlessLinux(t) emptyConfig := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{}) t0 := tftypes.Object{ AttributeTypes: map[string]tftypes.Type{ diff --git a/pkg/tests/cross-tests/input_cross_test.go b/pkg/tests/cross-tests/input_cross_test.go index 359ee88c1..e1a90df33 100644 --- a/pkg/tests/cross-tests/input_cross_test.go +++ b/pkg/tests/cross-tests/input_cross_test.go @@ -10,7 +10,6 @@ import ( ) func TestInputsEqualStringBasic(t *testing.T) { - skipUnlessLinux(t) // Test both config representations. for _, tc := range []struct { name string @@ -45,7 +44,6 @@ func TestInputsEqualStringBasic(t *testing.T) { } func TestInputsEqualObjectBasic(t *testing.T) { - skipUnlessLinux(t) t1 := tftypes.Object{ AttributeTypes: map[string]tftypes.Type{ "x": tftypes.String, @@ -103,7 +101,6 @@ func TestInputsEqualObjectBasic(t *testing.T) { } func TestInputsConfigModeEqual(t *testing.T) { - skipUnlessLinux(t) t2 := tftypes.Object{AttributeTypes: map[string]tftypes.Type{ "x": tftypes.String, }} @@ -192,7 +189,6 @@ func TestInputsConfigModeEqual(t *testing.T) { // Isolated from rapid-generated tests func TestInputsEmptyString(t *testing.T) { - skipUnlessLinux(t) runCreateInputCheck(t, inputTestCase{ Resource: &schema.Resource{ Schema: map[string]*schema.Schema{ @@ -216,7 +212,6 @@ func TestInputsEmptyString(t *testing.T) { } func TestOptionalSetNotSpecified(t *testing.T) { - skipUnlessLinux(t) runCreateInputCheck(t, inputTestCase{ Resource: &schema.Resource{ Schema: map[string]*schema.Schema{ @@ -237,7 +232,6 @@ func TestOptionalSetNotSpecified(t *testing.T) { } func TestExplicitNilList(t *testing.T) { - skipUnlessLinux(t) t0 := tftypes.Map{ElementType: tftypes.Number} t1 := tftypes.Object{AttributeTypes: map[string]tftypes.Type{ "f0": tftypes.List{ElementType: t0}, @@ -269,7 +263,6 @@ func TestExplicitNilList(t *testing.T) { } func TestInputsEmptyCollections(t *testing.T) { - skipUnlessLinux(t) config := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{}) // signifies a block @@ -331,7 +324,6 @@ func TestInputsEmptyCollections(t *testing.T) { } func TestInputsNestedBlocksEmpty(t *testing.T) { - skipUnlessLinux(t) emptyConfig := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{}) @@ -425,7 +417,6 @@ func TestInputsNestedBlocksEmpty(t *testing.T) { } func TestEmptySetOfEmptyObjects(t *testing.T) { - skipUnlessLinux(t) t1 := tftypes.Object{} t0 := tftypes.Object{AttributeTypes: map[string]tftypes.Type{ "d3f0": tftypes.Set{ElementType: t1}, @@ -449,7 +440,6 @@ func TestEmptySetOfEmptyObjects(t *testing.T) { } func TestMap(t *testing.T) { - skipUnlessLinux(t) t0 := tftypes.Map{ElementType: tftypes.String} t1 := tftypes.Object{AttributeTypes: map[string]tftypes.Type{ "tags": t0, @@ -480,7 +470,6 @@ func TestMap(t *testing.T) { } func TestTimeouts(t *testing.T) { - skipUnlessLinux(t) emptyConfig := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{}) runCreateInputCheck(t, inputTestCase{ Resource: &schema.Resource{ @@ -505,7 +494,6 @@ func TestTimeouts(t *testing.T) { // TestAccCloudWatch failed with PlanResourceChange to do a simple Create preview because the state upgrade was // unexpectedly called with nil state. Emulate this here to test it does not fail. func TestCreateDoesNotPanicWithStateUpgraders(t *testing.T) { - skipUnlessLinux(t) resourceRuleV0 := func() *schema.Resource { return &schema.Resource{ diff --git a/pkg/tests/cross-tests/upgrade_state_cross_test.go b/pkg/tests/cross-tests/upgrade_state_cross_test.go index 54d6efd68..31f74147a 100644 --- a/pkg/tests/cross-tests/upgrade_state_cross_test.go +++ b/pkg/tests/cross-tests/upgrade_state_cross_test.go @@ -8,7 +8,6 @@ import ( ) func TestUpgradeInputsStringBasic(t *testing.T) { - skipUnlessLinux(t) t.Skipf("TODO[pulumi/pulumi-terraform-bridge#2039] - Zero schema version does not work") runUpgradeStateInputCheck(t, inputTestCase{ Resource: &schema.Resource{ @@ -30,8 +29,6 @@ func TestUpgradeInputsStringBasic(t *testing.T) { } func TestUpgradeInputsStringBasicNonZeroVersion(t *testing.T) { - skipUnlessLinux(t) - runUpgradeStateInputCheck(t, inputTestCase{ Resource: &schema.Resource{ Schema: map[string]*schema.Schema{ @@ -53,7 +50,6 @@ func TestUpgradeInputsStringBasicNonZeroVersion(t *testing.T) { } func TestUpgradeInputsObjectBasic(t *testing.T) { - skipUnlessLinux(t) t.Skipf("TODO[pulumi/pulumi-terraform-bridge#2039] - Zero schema version does not work") t1 := tftypes.Object{ AttributeTypes: map[string]tftypes.Type{ diff --git a/pkg/tests/pulcheck/pulcheck.go b/pkg/tests/pulcheck/pulcheck.go index 65ab4c91f..08d9f47c9 100644 --- a/pkg/tests/pulcheck/pulcheck.go +++ b/pkg/tests/pulcheck/pulcheck.go @@ -7,6 +7,9 @@ import ( "io" "os" "path/filepath" + "runtime" + "strings" + "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -124,8 +127,16 @@ func BridgedProvider(t T, providerName string, resMap map[string]*schema.Resourc return provider } +func skipUnlessLinux(t *testing.T) { + if ci, ok := os.LookupEnv("CI"); ok && ci == "true" && !strings.Contains(strings.ToLower(runtime.GOOS), "linux") { + t.Skip("Skipping on non-Linux platforms as our CI does not yet install Terraform CLI required for these tests") + } +} + + // This is an experimental API. func PulCheck(t T, bridgedProvider info.Provider, program string) *pulumitest.PulumiTest { + skipUnlessLinux(t) puwd := t.TempDir() p := filepath.Join(puwd, "Pulumi.yaml") From f0f0f6f55548b17fa23bbfb214d1db46452f1bbc Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 6 Jun 2024 14:09:35 +0100 Subject: [PATCH 18/63] add tests for explicit empty maps --- pkg/tests/cross-tests/input_cross_test.go | 25 +++++++++++- pkg/tests/schema_pulumi_test.go | 47 +++++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/pkg/tests/cross-tests/input_cross_test.go b/pkg/tests/cross-tests/input_cross_test.go index fa7943e10..4d023484b 100644 --- a/pkg/tests/cross-tests/input_cross_test.go +++ b/pkg/tests/cross-tests/input_cross_test.go @@ -554,7 +554,7 @@ func TestCreateDoesNotPanicWithStateUpgraders(t *testing.T) { }) } -func TestEmptyMap(t *testing.T) { +func TestUnspecifiedMap(t *testing.T) { skipUnlessLinux(t) runCreateInputCheck(t, inputTestCase{ Resource: &schema.Resource{ @@ -571,3 +571,26 @@ func TestEmptyMap(t *testing.T) { Config: tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{}), }) } + +func TestEmptyMap(t *testing.T) { + skipUnlessLinux(t) + t1 := tftypes.Map{ElementType: tftypes.String} + runCreateInputCheck(t, inputTestCase{ + Resource: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "tags": { + Type: schema.TypeMap, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + }, + }, + Config: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{"tags": t1}, + }, map[string]tftypes.Value{ + "tags": tftypes.NewValue(t1, map[string]tftypes.Value{}), + }), + }) +} diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index 7c553cef7..6e64700af 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -61,6 +61,52 @@ outputs: require.Equal(t, "aux", resUp.Outputs["testOut"].Value) } +func TestUnspecifiedMapsRefreshClean(t *testing.T) { + resMap := map[string]*schema.Resource{ + "prov_test": { + Schema: map[string]*schema.Schema{ + "map_prop": { + Type: schema.TypeMap, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + "other_prop": { + Type: schema.TypeString, + Optional: true, + }, + }, + ReadContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { + err := d.Set("map_prop", map[string]interface{}{}) + require.NoError(t, err) + err = d.Set("other_prop", "test") + require.NoError(t, err) + return nil + }, + }, + } + bridgedProvider := pulcheck.BridgedProvider(t, "prov", resMap) + program := ` +name: test +runtime: yaml +resources: + mainRes: + type: prov:index:Test + properties: + otherProp: "test" +outputs: + mapPropOut: ${mainRes.mapProp} +` + pt := pulcheck.PulCheck(t, bridgedProvider, program) + + upRes := pt.Up() + require.Equal(t, nil, upRes.Outputs["mapPropOut"].Value) + + res := pt.Refresh(optrefresh.ExpectNoChanges()) + t.Logf(res.StdOut) +} + func TestEmptyMapsRefreshClean(t *testing.T) { resMap := map[string]*schema.Resource{ "prov_test": { @@ -95,6 +141,7 @@ resources: type: prov:index:Test properties: otherProp: "test" + mapProp: {} outputs: mapPropOut: ${mainRes.mapProp} ` From 76b4d177bc1def3504950e9304b9ad9b9e509798 Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 6 Jun 2024 14:32:32 +0100 Subject: [PATCH 19/63] bad merge and fix types --- pkg/tests/cross-tests/input_cross_test.go | 6 ------ pkg/tests/cross-tests/t.go | 1 + pkg/tests/pulcheck/pulcheck.go | 4 ++-- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/pkg/tests/cross-tests/input_cross_test.go b/pkg/tests/cross-tests/input_cross_test.go index 31fbd9a3d..1cbf0a883 100644 --- a/pkg/tests/cross-tests/input_cross_test.go +++ b/pkg/tests/cross-tests/input_cross_test.go @@ -214,7 +214,6 @@ func TestInputsEmptyString(t *testing.T) { func TestInputsUnspecifiedMaxItemsOne(t *testing.T) { // Regression test for [pulumi/pulumi-terraform-bridge#1767] - skipUnlessLinux(t) runCreateInputCheck(t, inputTestCase{ Resource: &schema.Resource{ Schema: map[string]*schema.Schema{ @@ -238,11 +237,7 @@ func TestInputsUnspecifiedMaxItemsOne(t *testing.T) { } func TestOptionalSetNotSpecified(t *testing.T) { -<<<<<<< HEAD -======= // Regression test for [pulumi/pulumi-terraform-bridge#1970] and [pulumi/pulumi-terraform-bridge#1964] - skipUnlessLinux(t) ->>>>>>> master runCreateInputCheck(t, inputTestCase{ Resource: &schema.Resource{ Schema: map[string]*schema.Schema{ @@ -263,7 +258,6 @@ func TestOptionalSetNotSpecified(t *testing.T) { func TestInputsEqualEmptyList(t *testing.T) { // Regression test for [pulumi/pulumi-terraform-bridge#1915] - skipUnlessLinux(t) for _, maxItems := range []int{0, 1} { name := fmt.Sprintf("MaxItems: %v", maxItems) t.Run(name, func(t *testing.T) { diff --git a/pkg/tests/cross-tests/t.go b/pkg/tests/cross-tests/t.go index 92dacbcaf..56d31217d 100644 --- a/pkg/tests/cross-tests/t.go +++ b/pkg/tests/cross-tests/t.go @@ -24,6 +24,7 @@ import ( type T interface { Logf(string, ...any) TempDir() string + Skip(...any) require.TestingT assert.TestingT pulumitest.PT diff --git a/pkg/tests/pulcheck/pulcheck.go b/pkg/tests/pulcheck/pulcheck.go index 08d9f47c9..e48b45950 100644 --- a/pkg/tests/pulcheck/pulcheck.go +++ b/pkg/tests/pulcheck/pulcheck.go @@ -9,7 +9,6 @@ import ( "path/filepath" "runtime" "strings" - "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -99,6 +98,7 @@ func StartPulumiProvider(ctx context.Context, name, version string, providerInfo type T interface { Logf(string, ...any) TempDir() string + Skip(...any) require.TestingT assert.TestingT pulumitest.PT @@ -127,7 +127,7 @@ func BridgedProvider(t T, providerName string, resMap map[string]*schema.Resourc return provider } -func skipUnlessLinux(t *testing.T) { +func skipUnlessLinux(t T) { if ci, ok := os.LookupEnv("CI"); ok && ci == "true" && !strings.Contains(strings.ToLower(runtime.GOOS), "linux") { t.Skip("Skipping on non-Linux platforms as our CI does not yet install Terraform CLI required for these tests") } From 03e4b08c24d7bff390aedc919c65be548bc9e4be Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 6 Jun 2024 15:41:56 +0100 Subject: [PATCH 20/63] lint --- pkg/tests/pulcheck/pulcheck.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/tests/pulcheck/pulcheck.go b/pkg/tests/pulcheck/pulcheck.go index e48b45950..542cbe1f2 100644 --- a/pkg/tests/pulcheck/pulcheck.go +++ b/pkg/tests/pulcheck/pulcheck.go @@ -133,7 +133,6 @@ func skipUnlessLinux(t T) { } } - // This is an experimental API. func PulCheck(t T, bridgedProvider info.Provider, program string) *pulumitest.PulumiTest { skipUnlessLinux(t) From 6a7801629bd0e6cee922428b356238c3ca17bc19 Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 6 Jun 2024 16:15:54 +0100 Subject: [PATCH 21/63] Revert "move skip unless linux" This reverts commit 6d817998fb8968a521bdb8d65f0cd75e080fff69. --- pkg/tests/cross-tests/ci.go | 28 +++++++++++++++++++ pkg/tests/cross-tests/cross_test.go | 9 ++++++ pkg/tests/cross-tests/input_cross_test.go | 12 ++++++++ .../cross-tests/upgrade_state_cross_test.go | 4 +++ pkg/tests/pulcheck/pulcheck.go | 9 ------ 5 files changed, 53 insertions(+), 9 deletions(-) create mode 100644 pkg/tests/cross-tests/ci.go diff --git a/pkg/tests/cross-tests/ci.go b/pkg/tests/cross-tests/ci.go new file mode 100644 index 000000000..2565e8cfa --- /dev/null +++ b/pkg/tests/cross-tests/ci.go @@ -0,0 +1,28 @@ +// Copyright 2016-2024, 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 + +// Helpers to disable failing CI runs. +package crosstests + +import ( + "os" + "runtime" + "strings" + "testing" +) + +func skipUnlessLinux(t *testing.T) { + if ci, ok := os.LookupEnv("CI"); ok && ci == "true" && !strings.Contains(strings.ToLower(runtime.GOOS), "linux") { + t.Skip("Skipping on non-Linux platforms as our CI does not yet install Terraform CLI required for these tests") + } +} diff --git a/pkg/tests/cross-tests/cross_test.go b/pkg/tests/cross-tests/cross_test.go index cb1bea846..aad3d0714 100644 --- a/pkg/tests/cross-tests/cross_test.go +++ b/pkg/tests/cross-tests/cross_test.go @@ -30,6 +30,7 @@ import ( ) func TestUnchangedBasicObject(t *testing.T) { + skipUnlessLinux(t) cfg := map[string]any{"f0": []any{map[string]any{"x": "ok"}}} runDiffCheck(t, diffTestCase{ Resource: &schema.Resource{ @@ -52,6 +53,7 @@ func TestUnchangedBasicObject(t *testing.T) { } func TestSimpleStringNoChange(t *testing.T) { + skipUnlessLinux(t) config := map[string]any{"name": "A"} runDiffCheck(t, diffTestCase{ Resource: &schema.Resource{ @@ -68,6 +70,7 @@ func TestSimpleStringNoChange(t *testing.T) { } func TestSimpleStringRename(t *testing.T) { + skipUnlessLinux(t) runDiffCheck(t, diffTestCase{ Resource: &schema.Resource{ Schema: map[string]*schema.Schema{ @@ -87,6 +90,7 @@ func TestSimpleStringRename(t *testing.T) { } func TestSetReordering(t *testing.T) { + skipUnlessLinux(t) resource := &schema.Resource{ Schema: map[string]*schema.Schema{ "set": { @@ -117,6 +121,7 @@ func TestSetReordering(t *testing.T) { // │ 1: resource "crossprovider_testres" "example" { func TestEmptyRequiredList(t *testing.T) { t.Skip("TODO - fix panic and make a negative test here") + skipUnlessLinux(t) resource := &schema.Resource{ Schema: map[string]*schema.Schema{ "f0": { @@ -141,6 +146,7 @@ func TestEmptyRequiredList(t *testing.T) { } func TestAws2442(t *testing.T) { + skipUnlessLinux(t) hashes := map[int]string{} stringHashcode := func(s string) int { @@ -398,6 +404,7 @@ func TestAws2442(t *testing.T) { } func TestSimpleOptionalComputed(t *testing.T) { + skipUnlessLinux(t) emptyConfig := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{}) nonEmptyConfig := tftypes.NewValue( tftypes.Object{ @@ -444,6 +451,7 @@ func TestSimpleOptionalComputed(t *testing.T) { } func TestOptionalComputedAttrCollection(t *testing.T) { + skipUnlessLinux(t) emptyConfig := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{}) t0 := tftypes.List{ElementType: tftypes.String} t1 := tftypes.Object{ @@ -514,6 +522,7 @@ func TestOptionalComputedAttrCollection(t *testing.T) { } func TestOptionalComputedBlockCollection(t *testing.T) { + skipUnlessLinux(t) emptyConfig := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{}) t0 := tftypes.Object{ AttributeTypes: map[string]tftypes.Type{ diff --git a/pkg/tests/cross-tests/input_cross_test.go b/pkg/tests/cross-tests/input_cross_test.go index 1cbf0a883..71ecf30ac 100644 --- a/pkg/tests/cross-tests/input_cross_test.go +++ b/pkg/tests/cross-tests/input_cross_test.go @@ -11,6 +11,7 @@ import ( ) func TestInputsEqualStringBasic(t *testing.T) { + skipUnlessLinux(t) // Test both config representations. for _, tc := range []struct { name string @@ -45,6 +46,7 @@ func TestInputsEqualStringBasic(t *testing.T) { } func TestInputsEqualObjectBasic(t *testing.T) { + skipUnlessLinux(t) t1 := tftypes.Object{ AttributeTypes: map[string]tftypes.Type{ "x": tftypes.String, @@ -103,6 +105,7 @@ func TestInputsEqualObjectBasic(t *testing.T) { func TestInputsConfigModeEqual(t *testing.T) { // Regression test for [pulumi/pulumi-terraform-bridge#1762] + skipUnlessLinux(t) t2 := tftypes.Object{AttributeTypes: map[string]tftypes.Type{ "x": tftypes.String, }} @@ -190,6 +193,7 @@ func TestInputsConfigModeEqual(t *testing.T) { // Isolated from rapid-generated tests func TestInputsEmptyString(t *testing.T) { + skipUnlessLinux(t) runCreateInputCheck(t, inputTestCase{ Resource: &schema.Resource{ Schema: map[string]*schema.Schema{ @@ -238,6 +242,7 @@ func TestInputsUnspecifiedMaxItemsOne(t *testing.T) { func TestOptionalSetNotSpecified(t *testing.T) { // Regression test for [pulumi/pulumi-terraform-bridge#1970] and [pulumi/pulumi-terraform-bridge#1964] + skipUnlessLinux(t) runCreateInputCheck(t, inputTestCase{ Resource: &schema.Resource{ Schema: map[string]*schema.Schema{ @@ -294,6 +299,7 @@ func TestInputsEqualEmptyList(t *testing.T) { } func TestExplicitNilList(t *testing.T) { + skipUnlessLinux(t) t0 := tftypes.Map{ElementType: tftypes.Number} t1 := tftypes.Object{AttributeTypes: map[string]tftypes.Type{ "f0": tftypes.List{ElementType: t0}, @@ -325,6 +331,7 @@ func TestExplicitNilList(t *testing.T) { } func TestInputsEmptyCollections(t *testing.T) { + skipUnlessLinux(t) config := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{}) // signifies a block @@ -385,6 +392,7 @@ func TestInputsEmptyCollections(t *testing.T) { } func TestInputsNestedBlocksEmpty(t *testing.T) { + skipUnlessLinux(t) emptyConfig := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{}) @@ -477,6 +485,7 @@ func TestInputsNestedBlocksEmpty(t *testing.T) { } func TestEmptySetOfEmptyObjects(t *testing.T) { + skipUnlessLinux(t) t1 := tftypes.Object{} t0 := tftypes.Object{AttributeTypes: map[string]tftypes.Type{ "d3f0": tftypes.Set{ElementType: t1}, @@ -500,6 +509,7 @@ func TestEmptySetOfEmptyObjects(t *testing.T) { } func TestMap(t *testing.T) { + skipUnlessLinux(t) t0 := tftypes.Map{ElementType: tftypes.String} t1 := tftypes.Object{AttributeTypes: map[string]tftypes.Type{ "tags": t0, @@ -530,6 +540,7 @@ func TestMap(t *testing.T) { } func TestTimeouts(t *testing.T) { + skipUnlessLinux(t) emptyConfig := tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{}) runCreateInputCheck(t, inputTestCase{ Resource: &schema.Resource{ @@ -554,6 +565,7 @@ func TestTimeouts(t *testing.T) { // TestAccCloudWatch failed with PlanResourceChange to do a simple Create preview because the state upgrade was // unexpectedly called with nil state. Emulate this here to test it does not fail. func TestCreateDoesNotPanicWithStateUpgraders(t *testing.T) { + skipUnlessLinux(t) resourceRuleV0 := func() *schema.Resource { return &schema.Resource{ diff --git a/pkg/tests/cross-tests/upgrade_state_cross_test.go b/pkg/tests/cross-tests/upgrade_state_cross_test.go index 31f74147a..54d6efd68 100644 --- a/pkg/tests/cross-tests/upgrade_state_cross_test.go +++ b/pkg/tests/cross-tests/upgrade_state_cross_test.go @@ -8,6 +8,7 @@ import ( ) func TestUpgradeInputsStringBasic(t *testing.T) { + skipUnlessLinux(t) t.Skipf("TODO[pulumi/pulumi-terraform-bridge#2039] - Zero schema version does not work") runUpgradeStateInputCheck(t, inputTestCase{ Resource: &schema.Resource{ @@ -29,6 +30,8 @@ func TestUpgradeInputsStringBasic(t *testing.T) { } func TestUpgradeInputsStringBasicNonZeroVersion(t *testing.T) { + skipUnlessLinux(t) + runUpgradeStateInputCheck(t, inputTestCase{ Resource: &schema.Resource{ Schema: map[string]*schema.Schema{ @@ -50,6 +53,7 @@ func TestUpgradeInputsStringBasicNonZeroVersion(t *testing.T) { } func TestUpgradeInputsObjectBasic(t *testing.T) { + skipUnlessLinux(t) t.Skipf("TODO[pulumi/pulumi-terraform-bridge#2039] - Zero schema version does not work") t1 := tftypes.Object{ AttributeTypes: map[string]tftypes.Type{ diff --git a/pkg/tests/pulcheck/pulcheck.go b/pkg/tests/pulcheck/pulcheck.go index 542cbe1f2..a1ba13272 100644 --- a/pkg/tests/pulcheck/pulcheck.go +++ b/pkg/tests/pulcheck/pulcheck.go @@ -7,8 +7,6 @@ import ( "io" "os" "path/filepath" - "runtime" - "strings" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -127,15 +125,8 @@ func BridgedProvider(t T, providerName string, resMap map[string]*schema.Resourc return provider } -func skipUnlessLinux(t T) { - if ci, ok := os.LookupEnv("CI"); ok && ci == "true" && !strings.Contains(strings.ToLower(runtime.GOOS), "linux") { - t.Skip("Skipping on non-Linux platforms as our CI does not yet install Terraform CLI required for these tests") - } -} - // This is an experimental API. func PulCheck(t T, bridgedProvider info.Provider, program string) *pulumitest.PulumiTest { - skipUnlessLinux(t) puwd := t.TempDir() p := filepath.Join(puwd, "Pulumi.yaml") From 6e3f3ded05fc3979f6342e55a15bbc98573d3c8c Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Thu, 6 Jun 2024 15:42:40 -0400 Subject: [PATCH 22/63] Normalize Read result for empty collections This workaround is aimed to reduce dirty refresh incidence on diffs between empty and null collections that is benign in TF but projects worse to Pulumi. --- pkg/tfbridge/normalize.go | 74 +++++++++++++++++++++++++++++++++++++++ pkg/tfbridge/provider.go | 4 +++ 2 files changed, 78 insertions(+) create mode 100644 pkg/tfbridge/normalize.go diff --git a/pkg/tfbridge/normalize.go b/pkg/tfbridge/normalize.go new file mode 100644 index 000000000..626de1958 --- /dev/null +++ b/pkg/tfbridge/normalize.go @@ -0,0 +1,74 @@ +// Copyright 2016-2024, 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 tfbridge + +import ( + "context" + + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/info" + shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" + "github.com/pulumi/pulumi/sdk/v3/go/common/resource" +) + +// Compensate for a TF problem causing spurious diffs between null and empty collections highlighted as dirty refresh in +// Pulumi. When applying resource changes TF will call normalizeNullValues with apply=true which may erase the +// distinction between empty and null collections. This makes state returned from Read differ from the state returned by +// Create and Update and written to the state store. While the problem is present in both TF and Pulumi bridged +// providers, it is worse in Pulumi because refresh is emitting a warning that something is changing. +// +// To compensate for this problem, this method corrects the Pulumi Read result during `pulumi refresh` to also erase +// empty collections from the Read result if the old input is missing or null. +// +// This currently only handles top-level properties. +// +// See: https://github.com/pulumi/terraform-plugin-sdk/blob/upstream-v2.33.0/helper/schema/grpc_provider.go#L1514 +func normalizeNullValues( + ctx context.Context, + schemaMap shim.SchemaMap, + schemaInfos map[string]*info.Schema, + oldInputs, inputs resource.PropertyMap, +) resource.PropertyMap { + if oldInputs == nil { + return inputs + } + copy := inputs.Copy() + for _, k := range inputs.StableKeys() { + v := inputs[k] + oldInput, gotOldInput := oldInputs[k] + if gotOldInput && !oldInput.IsNull() { + continue + } + if !(v.IsArray() && len(v.ArrayValue()) == 0) { + continue + } + tfName := PulumiToTerraformName(string(k), schemaMap, schemaInfos) + schema, ok := schemaMap.GetOk(tfName) + if !ok { + continue + } + t := schema.Type() + isCollection := t == shim.TypeList || t == shim.TypeMap || t == shim.TypeSet + if !isCollection || IsMaxItemsOne(schema, schemaInfos[tfName]) { + continue + } + // An empty collection (not MaxItems=1) with missing/null oldInput is getting replaced to match. + if gotOldInput { + copy[k] = oldInput + } else { + delete(copy, k) + } + } + return copy +} diff --git a/pkg/tfbridge/provider.go b/pkg/tfbridge/provider.go index 533949205..38e43deb6 100755 --- a/pkg/tfbridge/provider.go +++ b/pkg/tfbridge/provider.go @@ -1296,6 +1296,10 @@ func (p *Provider) Read(ctx context.Context, req *pulumirpc.ReadRequest) (*pulum return nil, err } + if isRefresh { + inputs = normalizeNullValues(oldInputs, res.TF.Schema(), res.Schema.Fields, inputs) + } + cleanInputs := deconflict(ctx, res.TF.Schema(), res.Schema.Fields, inputs) minputs, err := plugin.MarshalProperties(cleanInputs, plugin.MarshalOptions{ From fc0e64f4d300f6b847ad3e46989d13e2cb8bcb36 Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Thu, 6 Jun 2024 15:54:24 -0400 Subject: [PATCH 23/63] First pass --- pkg/tfbridge/normalize.go | 6 ++++++ pkg/tfbridge/provider.go | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/tfbridge/normalize.go b/pkg/tfbridge/normalize.go index 626de1958..e775b9376 100644 --- a/pkg/tfbridge/normalize.go +++ b/pkg/tfbridge/normalize.go @@ -16,6 +16,7 @@ package tfbridge import ( "context" + "fmt" "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/info" shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" @@ -65,8 +66,13 @@ func normalizeNullValues( } // An empty collection (not MaxItems=1) with missing/null oldInput is getting replaced to match. if gotOldInput { + p := string(k) + msg := fmt.Sprintf("normalizeNullValues: replacing %s=[] with oldInputs.%s=null", p, p) + GetLogger(ctx).Debug(msg) copy[k] = oldInput } else { + msg := fmt.Sprintf("normalizeNullValues: removing %s=[] to match oldInputs", string(k)) + GetLogger(ctx).Debug(msg) delete(copy, k) } } diff --git a/pkg/tfbridge/provider.go b/pkg/tfbridge/provider.go index 38e43deb6..4e68c6fc3 100755 --- a/pkg/tfbridge/provider.go +++ b/pkg/tfbridge/provider.go @@ -1297,7 +1297,7 @@ func (p *Provider) Read(ctx context.Context, req *pulumirpc.ReadRequest) (*pulum } if isRefresh { - inputs = normalizeNullValues(oldInputs, res.TF.Schema(), res.Schema.Fields, inputs) + inputs = normalizeNullValues(ctx, res.TF.Schema(), res.Schema.Fields, oldInputs, inputs) } cleanInputs := deconflict(ctx, res.TF.Schema(), res.Schema.Fields, inputs) From 4acec2cdb5e559f57d4c04bff7bf816bae37675b Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 6 Jun 2024 21:16:50 +0100 Subject: [PATCH 24/63] skip on windows --- pkg/tests/pulcheck/pulcheck.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/tests/pulcheck/pulcheck.go b/pkg/tests/pulcheck/pulcheck.go index a1ba13272..542cbe1f2 100644 --- a/pkg/tests/pulcheck/pulcheck.go +++ b/pkg/tests/pulcheck/pulcheck.go @@ -7,6 +7,8 @@ import ( "io" "os" "path/filepath" + "runtime" + "strings" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -125,8 +127,15 @@ func BridgedProvider(t T, providerName string, resMap map[string]*schema.Resourc return provider } +func skipUnlessLinux(t T) { + if ci, ok := os.LookupEnv("CI"); ok && ci == "true" && !strings.Contains(strings.ToLower(runtime.GOOS), "linux") { + t.Skip("Skipping on non-Linux platforms as our CI does not yet install Terraform CLI required for these tests") + } +} + // This is an experimental API. func PulCheck(t T, bridgedProvider info.Provider, program string) *pulumitest.PulumiTest { + skipUnlessLinux(t) puwd := t.TempDir() p := filepath.Join(puwd, "Pulumi.yaml") From 103ed4ac247e44cd97c94a3ac4ba7cbea40e174d Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Thu, 6 Jun 2024 17:03:16 -0400 Subject: [PATCH 25/63] Scope to provider2.go --- pkg/tfbridge/normalize.go | 80 ---------------------------------- pkg/tfbridge/provider.go | 4 -- pkg/tfshim/sdk-v2/provider2.go | 67 +++++++++++++++++++++++++++- 3 files changed, 66 insertions(+), 85 deletions(-) delete mode 100644 pkg/tfbridge/normalize.go diff --git a/pkg/tfbridge/normalize.go b/pkg/tfbridge/normalize.go deleted file mode 100644 index e775b9376..000000000 --- a/pkg/tfbridge/normalize.go +++ /dev/null @@ -1,80 +0,0 @@ -// Copyright 2016-2024, 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 tfbridge - -import ( - "context" - "fmt" - - "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/info" - shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" - "github.com/pulumi/pulumi/sdk/v3/go/common/resource" -) - -// Compensate for a TF problem causing spurious diffs between null and empty collections highlighted as dirty refresh in -// Pulumi. When applying resource changes TF will call normalizeNullValues with apply=true which may erase the -// distinction between empty and null collections. This makes state returned from Read differ from the state returned by -// Create and Update and written to the state store. While the problem is present in both TF and Pulumi bridged -// providers, it is worse in Pulumi because refresh is emitting a warning that something is changing. -// -// To compensate for this problem, this method corrects the Pulumi Read result during `pulumi refresh` to also erase -// empty collections from the Read result if the old input is missing or null. -// -// This currently only handles top-level properties. -// -// See: https://github.com/pulumi/terraform-plugin-sdk/blob/upstream-v2.33.0/helper/schema/grpc_provider.go#L1514 -func normalizeNullValues( - ctx context.Context, - schemaMap shim.SchemaMap, - schemaInfos map[string]*info.Schema, - oldInputs, inputs resource.PropertyMap, -) resource.PropertyMap { - if oldInputs == nil { - return inputs - } - copy := inputs.Copy() - for _, k := range inputs.StableKeys() { - v := inputs[k] - oldInput, gotOldInput := oldInputs[k] - if gotOldInput && !oldInput.IsNull() { - continue - } - if !(v.IsArray() && len(v.ArrayValue()) == 0) { - continue - } - tfName := PulumiToTerraformName(string(k), schemaMap, schemaInfos) - schema, ok := schemaMap.GetOk(tfName) - if !ok { - continue - } - t := schema.Type() - isCollection := t == shim.TypeList || t == shim.TypeMap || t == shim.TypeSet - if !isCollection || IsMaxItemsOne(schema, schemaInfos[tfName]) { - continue - } - // An empty collection (not MaxItems=1) with missing/null oldInput is getting replaced to match. - if gotOldInput { - p := string(k) - msg := fmt.Sprintf("normalizeNullValues: replacing %s=[] with oldInputs.%s=null", p, p) - GetLogger(ctx).Debug(msg) - copy[k] = oldInput - } else { - msg := fmt.Sprintf("normalizeNullValues: removing %s=[] to match oldInputs", string(k)) - GetLogger(ctx).Debug(msg) - delete(copy, k) - } - } - return copy -} diff --git a/pkg/tfbridge/provider.go b/pkg/tfbridge/provider.go index 4e68c6fc3..533949205 100755 --- a/pkg/tfbridge/provider.go +++ b/pkg/tfbridge/provider.go @@ -1296,10 +1296,6 @@ func (p *Provider) Read(ctx context.Context, req *pulumirpc.ReadRequest) (*pulum return nil, err } - if isRefresh { - inputs = normalizeNullValues(ctx, res.TF.Schema(), res.Schema.Fields, oldInputs, inputs) - } - cleanInputs := deconflict(ctx, res.TF.Schema(), res.Schema.Fields, inputs) minputs, err := plugin.MarshalProperties(cleanInputs, plugin.MarshalOptions{ diff --git a/pkg/tfshim/sdk-v2/provider2.go b/pkg/tfshim/sdk-v2/provider2.go index 7836cc088..aba68eec7 100644 --- a/pkg/tfshim/sdk-v2/provider2.go +++ b/pkg/tfshim/sdk-v2/provider2.go @@ -254,13 +254,78 @@ func (p *planResourceChangeImpl) Refresh( if rr.stateValue.IsNull() { return nil, nil } + normStateValue := p.normalizeNullValues(res, c, rr.stateValue) return &v2InstanceState2{ resourceType: rr.resourceType, - stateValue: rr.stateValue, + stateValue: normStateValue, meta: rr.meta, }, nil } +// Compensate for a TF problem causing spurious diffs between null and empty collections highlighted as dirty refresh in +// Pulumi. When applying resource changes TF will call normalizeNullValues with apply=true which may erase the +// distinction between empty and null collections. This makes state returned from Read differ from the state returned by +// Create and Update and written to the state store. While the problem is present in both TF and Pulumi bridged +// providers, it is worse in Pulumi because refresh is emitting a warning that something is changing. +// +// To compensate for this problem, this method corrects the Pulumi Read result during `pulumi refresh` to also erase +// empty collections from the Read result if the old input is missing or null. +// +// This currently only handles top-level properties. +// +// See: https://github.com/pulumi/terraform-plugin-sdk/blob/upstream-v2.33.0/helper/schema/grpc_provider.go#L1514 +func (p *planResourceChangeImpl) normalizeNullValues( + res *schema.Resource, + config shim.ResourceConfig, + state cty.Value, +) cty.Value { + if config == nil { + return state + } + sm := res.SchemaMap() + m := state.AsValueMap() + // fmt.Println("oldInputs", resource.NewObjectProperty(oldInputs).String()) + // fmt.Println("inputs", resource.NewObjectProperty(inputs).String()) + for tfName, v := range m { + sch, ok := sm[tfName] + if !ok { + fmt.Println("SKIP", tfName, "because unknown schema") + continue + } + t := sch.Type + isCollection := t == schema.TypeList || t == schema.TypeMap || t == schema.TypeSet + if !isCollection { + fmt.Println("SKIP", tfName, "because not a collection") + continue + } + + if _, ok := sch.Elem.(*schema.Resource); ok || sch.ConfigMode == schema.SchemaConfigModeBlock { + fmt.Println("SKIP", tfName, "because it is a block in TF") + continue + } + if config.IsSet(tfName) { + fmt.Println("SKIP", tfName, "because it is set in config") + continue + } + + // oldInput, gotOldInput := oldInputs[k] + // if gotOldInput && !oldInput.IsNull() { + // fmt.Println("SKIP", k, "because gotOldInput that is not null") + // continue + // } + + if !(v.Type().IsCollectionType() && !v.IsNull() && v.LengthInt() == 0) { + fmt.Println("SKIP", tfName, "because not an empty collection") + continue + } + + msg := fmt.Sprintf("normalizeNullValues: replacing %s=[] not found in config", tfName) + fmt.Println(msg) + m[tfName] = cty.NullVal(v.Type()) + } + return cty.ObjectVal(m) +} + func (p *planResourceChangeImpl) NewDestroyDiff( ctx context.Context, t string, opts shim.TimeoutOptions, ) shim.InstanceDiff { From 5385d82ce52d70030843c8050c02d8bedd65fdc8 Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Thu, 6 Jun 2024 17:11:24 -0400 Subject: [PATCH 26/63] Use logger --- pkg/tfshim/sdk-v2/provider2.go | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/pkg/tfshim/sdk-v2/provider2.go b/pkg/tfshim/sdk-v2/provider2.go index aba68eec7..550cf5ea9 100644 --- a/pkg/tfshim/sdk-v2/provider2.go +++ b/pkg/tfshim/sdk-v2/provider2.go @@ -14,7 +14,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" - + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge" shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" "github.com/pulumi/pulumi/sdk/v3/go/common/util/contract" ) @@ -254,7 +254,7 @@ func (p *planResourceChangeImpl) Refresh( if rr.stateValue.IsNull() { return nil, nil } - normStateValue := p.normalizeNullValues(res, c, rr.stateValue) + normStateValue := p.normalizeNullValues(ctx, res, c, rr.stateValue) return &v2InstanceState2{ resourceType: rr.resourceType, stateValue: normStateValue, @@ -275,6 +275,7 @@ func (p *planResourceChangeImpl) Refresh( // // See: https://github.com/pulumi/terraform-plugin-sdk/blob/upstream-v2.33.0/helper/schema/grpc_provider.go#L1514 func (p *planResourceChangeImpl) normalizeNullValues( + ctx context.Context, res *schema.Resource, config shim.ResourceConfig, state cty.Value, @@ -284,18 +285,14 @@ func (p *planResourceChangeImpl) normalizeNullValues( } sm := res.SchemaMap() m := state.AsValueMap() - // fmt.Println("oldInputs", resource.NewObjectProperty(oldInputs).String()) - // fmt.Println("inputs", resource.NewObjectProperty(inputs).String()) for tfName, v := range m { sch, ok := sm[tfName] if !ok { - fmt.Println("SKIP", tfName, "because unknown schema") continue } t := sch.Type isCollection := t == schema.TypeList || t == schema.TypeMap || t == schema.TypeSet if !isCollection { - fmt.Println("SKIP", tfName, "because not a collection") continue } @@ -304,23 +301,15 @@ func (p *planResourceChangeImpl) normalizeNullValues( continue } if config.IsSet(tfName) { - fmt.Println("SKIP", tfName, "because it is set in config") continue } - // oldInput, gotOldInput := oldInputs[k] - // if gotOldInput && !oldInput.IsNull() { - // fmt.Println("SKIP", k, "because gotOldInput that is not null") - // continue - // } - if !(v.Type().IsCollectionType() && !v.IsNull() && v.LengthInt() == 0) { - fmt.Println("SKIP", tfName, "because not an empty collection") continue } msg := fmt.Sprintf("normalizeNullValues: replacing %s=[] not found in config", tfName) - fmt.Println(msg) + tfbridge.GetLogger(ctx).Debug(msg) m[tfName] = cty.NullVal(v.Type()) } return cty.ObjectVal(m) From 2044e8ddec6cadfd895894a6a5779a4d3795783d Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Thu, 6 Jun 2024 17:13:40 -0400 Subject: [PATCH 27/63] Remove fmt.Println --- pkg/tfshim/sdk-v2/provider2.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/tfshim/sdk-v2/provider2.go b/pkg/tfshim/sdk-v2/provider2.go index 550cf5ea9..2323fa4cd 100644 --- a/pkg/tfshim/sdk-v2/provider2.go +++ b/pkg/tfshim/sdk-v2/provider2.go @@ -297,7 +297,6 @@ func (p *planResourceChangeImpl) normalizeNullValues( } if _, ok := sch.Elem.(*schema.Resource); ok || sch.ConfigMode == schema.SchemaConfigModeBlock { - fmt.Println("SKIP", tfName, "because it is a block in TF") continue } if config.IsSet(tfName) { From 437997b834c3929963f1d3b915d10ef00b853022 Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Thu, 6 Jun 2024 17:22:46 -0400 Subject: [PATCH 28/63] Avoid import cycle --- pkg/tfshim/sdk-v2/provider2.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/tfshim/sdk-v2/provider2.go b/pkg/tfshim/sdk-v2/provider2.go index 2323fa4cd..21b9a1aef 100644 --- a/pkg/tfshim/sdk-v2/provider2.go +++ b/pkg/tfshim/sdk-v2/provider2.go @@ -14,7 +14,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" - "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge" + // "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge" shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" "github.com/pulumi/pulumi/sdk/v3/go/common/util/contract" ) @@ -307,8 +307,9 @@ func (p *planResourceChangeImpl) normalizeNullValues( continue } - msg := fmt.Sprintf("normalizeNullValues: replacing %s=[] not found in config", tfName) - tfbridge.GetLogger(ctx).Debug(msg) + // Cannot use GetLogger yet as that introduces an import cycle. + // msg := fmt.Sprintf("normalizeNullValues: replacing %s=[] not found in config", tfName) + // tfbridge.GetLogger(ctx).Debug(msg) m[tfName] = cty.NullVal(v.Type()) } return cty.ObjectVal(m) From 0bd1b5b0f32eda7aecb09c7e8a87f90286a38054 Mon Sep 17 00:00:00 2001 From: Venelin Date: Fri, 7 Jun 2024 10:57:36 +0100 Subject: [PATCH 29/63] bad merge --- pkg/tests/cross-tests/input_cross_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/tests/cross-tests/input_cross_test.go b/pkg/tests/cross-tests/input_cross_test.go index 71ecf30ac..5e4ddb32a 100644 --- a/pkg/tests/cross-tests/input_cross_test.go +++ b/pkg/tests/cross-tests/input_cross_test.go @@ -218,6 +218,7 @@ func TestInputsEmptyString(t *testing.T) { func TestInputsUnspecifiedMaxItemsOne(t *testing.T) { // Regression test for [pulumi/pulumi-terraform-bridge#1767] + skipUnlessLinux(t) runCreateInputCheck(t, inputTestCase{ Resource: &schema.Resource{ Schema: map[string]*schema.Schema{ @@ -263,6 +264,7 @@ func TestOptionalSetNotSpecified(t *testing.T) { func TestInputsEqualEmptyList(t *testing.T) { // Regression test for [pulumi/pulumi-terraform-bridge#1915] + skipUnlessLinux(t) for _, maxItems := range []int{0, 1} { name := fmt.Sprintf("MaxItems: %v", maxItems) t.Run(name, func(t *testing.T) { From 609305f7d6df541197bd16b1104c085c0b17dd90 Mon Sep 17 00:00:00 2001 From: Venelin Date: Fri, 7 Jun 2024 12:50:08 +0100 Subject: [PATCH 30/63] make normalize nulls work recursively --- pkg/tests/schema_pulumi_test.go | 52 ++++++++++++++++++++++++ pkg/tfshim/sdk-v2/provider2.go | 70 +++++++++++++++++++++------------ 2 files changed, 96 insertions(+), 26 deletions(-) diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index 6e64700af..b834fdd8c 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -153,3 +153,55 @@ outputs: res := pt.Refresh(optrefresh.ExpectNoChanges()) t.Logf(res.StdOut) } + +func TestNestedEmptyMapRefreshClean(t *testing.T) { + resMap := map[string]*schema.Resource{ + "prov_test": { + Schema: map[string]*schema.Schema{ + "prop": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "map_prop": { + Type: schema.TypeMap, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + }, + }, + }, + "other_prop": { + Type: schema.TypeString, + Optional: true, + }, + }, + ReadContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { + err := d.Set("prop", []map[string]interface{}{{"map_prop": map[string]interface{}{}}}) + require.NoError(t, err) + err = d.Set("other_prop", "test") + require.NoError(t, err) + return nil + }, + }, + } + bridgedProvider := pulcheck.BridgedProvider(t, "prov", resMap) + program := ` +name: test +runtime: yaml +resources: + mainRes: + type: prov:index:Test + properties: + otherProp: "test" + props: + - mapProp: {} +` + pt := pulcheck.PulCheck(t, bridgedProvider, program) + pt.Up() + + res := pt.Refresh(optrefresh.ExpectNoChanges()) + t.Logf(res.StdOut) +} diff --git a/pkg/tfshim/sdk-v2/provider2.go b/pkg/tfshim/sdk-v2/provider2.go index 21b9a1aef..ef900c0bb 100644 --- a/pkg/tfshim/sdk-v2/provider2.go +++ b/pkg/tfshim/sdk-v2/provider2.go @@ -14,7 +14,6 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" - // "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge" shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" "github.com/pulumi/pulumi/sdk/v3/go/common/util/contract" ) @@ -254,7 +253,10 @@ func (p *planResourceChangeImpl) Refresh( if rr.stateValue.IsNull() { return nil, nil } - normStateValue := p.normalizeNullValues(ctx, res, c, rr.stateValue) + normStateValue := rr.stateValue + if c != nil { + normStateValue = normalizeNullValues(res, rr.stateValue) + } return &v2InstanceState2{ resourceType: rr.resourceType, stateValue: normStateValue, @@ -274,47 +276,62 @@ func (p *planResourceChangeImpl) Refresh( // This currently only handles top-level properties. // // See: https://github.com/pulumi/terraform-plugin-sdk/blob/upstream-v2.33.0/helper/schema/grpc_provider.go#L1514 -func (p *planResourceChangeImpl) normalizeNullValues( - ctx context.Context, - res *schema.Resource, - config shim.ResourceConfig, - state cty.Value, -) cty.Value { - if config == nil { +func normalizeNullValues(res *schema.Resource, state cty.Value) cty.Value { + if !state.Type().IsObjectType() { return state } - sm := res.SchemaMap() m := state.AsValueMap() - for tfName, v := range m { - sch, ok := sm[tfName] + sch := res.CoreConfigSchema() + for key, attr := range sch.Attributes { + stateVal, ok := m[key] if !ok { continue } - t := sch.Type - isCollection := t == schema.TypeList || t == schema.TypeMap || t == schema.TypeSet - if !isCollection { + + m[key] = normalizeNullValuesAttr(attr.Type, stateVal) + } + + for key := range sch.BlockTypes { + subBlockRes := res.SchemaMap()[key] + if subBlockRes == nil { continue } - if _, ok := sch.Elem.(*schema.Resource); ok || sch.ConfigMode == schema.SchemaConfigModeBlock { + elemRes, ok := subBlockRes.Elem.(*schema.Resource) + if !ok { continue } - if config.IsSet(tfName) { + if !m[key].CanIterateElements() { continue } - - if !(v.Type().IsCollectionType() && !v.IsNull() && v.LengthInt() == 0) { - continue + it := m[key].ElementIterator() + newElems := make([]cty.Value, 0) + for it.Next() { + _, elemVal := it.Element() + newElems = append(newElems, normalizeNullValues(elemRes, elemVal)) + } + if subBlockRes.Type == schema.TypeSet { + m[key] = cty.SetVal(newElems) + } else { + m[key] = cty.ListVal(newElems) } - - // Cannot use GetLogger yet as that introduces an import cycle. - // msg := fmt.Sprintf("normalizeNullValues: replacing %s=[] not found in config", tfName) - // tfbridge.GetLogger(ctx).Debug(msg) - m[tfName] = cty.NullVal(v.Type()) } return cty.ObjectVal(m) } +func normalizeNullValuesAttr(attrType cty.Type, stateVal cty.Value) cty.Value { + if !attrType.IsCollectionType() { + return stateVal + } + if !stateVal.Type().IsCollectionType() { + return stateVal + } + if !stateVal.IsNull() && stateVal.LengthInt() > 0 { + return stateVal + } + return cty.NullVal(attrType) +} + func (p *planResourceChangeImpl) NewDestroyDiff( ctx context.Context, t string, opts shim.TimeoutOptions, ) shim.InstanceDiff { @@ -461,7 +478,8 @@ func (s *grpcServer) PlanResourceChange( PlannedState cty.Value PlannedMeta map[string]interface{} PlannedDiff *terraform.InstanceDiff -}, error) { +}, error, +) { configVal, err := msgpack.Marshal(config, ty) if err != nil { return nil, err From 8bc12097b7839b3ce91ce0272c635b3eb4b8bb1c Mon Sep 17 00:00:00 2001 From: Venelin Date: Fri, 7 Jun 2024 12:52:45 +0100 Subject: [PATCH 31/63] revert bad merge --- pkg/tests/pulcheck/pulcheck.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/tests/pulcheck/pulcheck.go b/pkg/tests/pulcheck/pulcheck.go index 26dea1822..542cbe1f2 100644 --- a/pkg/tests/pulcheck/pulcheck.go +++ b/pkg/tests/pulcheck/pulcheck.go @@ -29,6 +29,7 @@ import ( "gotest.tools/assert" ) +// This is an experimental API. func EnsureProviderValid(t T, tfp *schema.Provider) { for _, r := range tfp.ResourcesMap { if r.ReadContext == nil { @@ -62,6 +63,7 @@ func EnsureProviderValid(t T, tfp *schema.Provider) { require.NoError(t, tfp.InternalValidate()) } +// This is an experimental API. func StartPulumiProvider(ctx context.Context, name, version string, providerInfo tfbridge.ProviderInfo) (*rpcutil.ServeHandle, error) { sink := pulumidiag.DefaultSink(io.Discard, io.Discard, pulumidiag.FormatOptions{ Color: colors.Never, @@ -92,6 +94,7 @@ func StartPulumiProvider(ctx context.Context, name, version string, providerInfo return &handle, nil } +// This is an experimental API. type T interface { Logf(string, ...any) TempDir() string @@ -101,6 +104,7 @@ type T interface { pulumitest.PT } +// This is an experimental API. func BridgedProvider(t T, providerName string, resMap map[string]*schema.Resource) info.Provider { tfp := &schema.Provider{ResourcesMap: resMap} EnsureProviderValid(t, tfp) From 134c45fc5ad3a042bb0155f27343095368eafea4 Mon Sep 17 00:00:00 2001 From: Venelin Date: Fri, 7 Jun 2024 12:55:08 +0100 Subject: [PATCH 32/63] revert formatting change --- pkg/tfshim/sdk-v2/provider2.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/tfshim/sdk-v2/provider2.go b/pkg/tfshim/sdk-v2/provider2.go index ef900c0bb..6cda1bb0b 100644 --- a/pkg/tfshim/sdk-v2/provider2.go +++ b/pkg/tfshim/sdk-v2/provider2.go @@ -478,8 +478,7 @@ func (s *grpcServer) PlanResourceChange( PlannedState cty.Value PlannedMeta map[string]interface{} PlannedDiff *terraform.InstanceDiff -}, error, -) { +}, error) { configVal, err := msgpack.Marshal(config, ty) if err != nil { return nil, err From 318090d885f14dafeda1db4ad26b66928e3682e8 Mon Sep 17 00:00:00 2001 From: Venelin Date: Fri, 7 Jun 2024 13:27:01 +0100 Subject: [PATCH 33/63] tests --- pkg/tests/schema_pulumi_test.go | 196 +++++++++++++++++++------------- 1 file changed, 116 insertions(+), 80 deletions(-) diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index b834fdd8c..5d8c1a7a3 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -2,6 +2,7 @@ package tests import ( "context" + "fmt" "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" @@ -61,79 +62,114 @@ outputs: require.Equal(t, "aux", resUp.Outputs["testOut"].Value) } -func TestUnspecifiedMapsRefreshClean(t *testing.T) { - resMap := map[string]*schema.Resource{ - "prov_test": { - Schema: map[string]*schema.Schema{ - "map_prop": { - Type: schema.TypeMap, - Optional: true, - Elem: &schema.Schema{ - Type: schema.TypeString, - }, - }, - "other_prop": { - Type: schema.TypeString, - Optional: true, - }, - }, - ReadContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { - err := d.Set("map_prop", map[string]interface{}{}) - require.NoError(t, err) - err = d.Set("other_prop", "test") - require.NoError(t, err) - return nil - }, +func TestCollectionsRefreshClean(t *testing.T) { + for _, tc := range []struct { + name string + schemaType schema.ValueType + readVal interface{} + // Note maps are not pluralized in the program while lists and sets are. + programVal string + outputString string + expectedOutput interface{} + }{ + { + name: "map null", + schemaType: schema.TypeMap, + readVal: map[string]interface{}{}, + programVal: "collectionProp: null", + outputString: "${mainRes.collectionProp}", + expectedOutput: nil, }, - } - bridgedProvider := pulcheck.BridgedProvider(t, "prov", resMap) - program := ` -name: test -runtime: yaml -resources: - mainRes: - type: prov:index:Test - properties: - otherProp: "test" -outputs: - mapPropOut: ${mainRes.mapProp} -` - pt := pulcheck.PulCheck(t, bridgedProvider, program) - - upRes := pt.Up() - require.Equal(t, nil, upRes.Outputs["mapPropOut"].Value) - - res := pt.Refresh(optrefresh.ExpectNoChanges()) - t.Logf(res.StdOut) -} - -func TestEmptyMapsRefreshClean(t *testing.T) { - resMap := map[string]*schema.Resource{ - "prov_test": { - Schema: map[string]*schema.Schema{ - "map_prop": { - Type: schema.TypeMap, - Optional: true, - Elem: &schema.Schema{ - Type: schema.TypeString, + { + name: "map empty", + schemaType: schema.TypeMap, + readVal: map[string]interface{}{}, + programVal: "collectionProp: {}", + outputString: "${mainRes.collectionProp}", + expectedOutput: nil, + }, + { + name: "map nonempty", + schemaType: schema.TypeMap, + readVal: map[string]interface{}{"val": "test"}, + programVal: `collectionProp: {"val": "test"}`, + outputString: "${mainRes.collectionProp}", + expectedOutput: map[string]interface{}{"val": "test"}, + }, + { + name: "list null", + schemaType: schema.TypeList, + readVal: []interface{}{}, + programVal: "collectionProps: null", + outputString: "${mainRes.collectionProps}", + expectedOutput: nil, + }, + { + name: "list empty", + schemaType: schema.TypeList, + readVal: []interface{}{}, + programVal: "collectionProps: []", + outputString: "${mainRes.collectionProps}", + expectedOutput: nil, + }, + { + name: "list nonempty", + schemaType: schema.TypeList, + readVal: []interface{}{"val"}, + programVal: `collectionProps: ["val"]`, + outputString: "${mainRes.collectionProps}", + expectedOutput: []interface{}{"val"}, + }, + { + name: "set null", + schemaType: schema.TypeSet, + readVal: []interface{}{}, + programVal: "collectionProps: null", + outputString: "${mainRes.collectionProps}", + expectedOutput: nil, + }, + { + name: "set empty", + schemaType: schema.TypeSet, + readVal: []interface{}{}, + programVal: "collectionProps: []", + outputString: "${mainRes.collectionProps}", + expectedOutput: nil, + }, + { + name: "set nonempty", + schemaType: schema.TypeSet, + readVal: []interface{}{"val"}, + programVal: `collectionProps: ["val"]`, + outputString: "${mainRes.collectionProps}", + expectedOutput: []interface{}{"val"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + resMap := map[string]*schema.Resource{ + "prov_test": { + Schema: map[string]*schema.Schema{ + "collection_prop": { + Type: tc.schemaType, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, + "other_prop": { + Type: schema.TypeString, + Optional: true, + }, + }, + ReadContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { + err := d.Set("collection_prop", tc.readVal) + require.NoError(t, err) + err = d.Set("other_prop", "test") + require.NoError(t, err) + return nil }, }, - "other_prop": { - Type: schema.TypeString, - Optional: true, - }, - }, - ReadContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { - err := d.Set("map_prop", map[string]interface{}{}) - require.NoError(t, err) - err = d.Set("other_prop", "test") - require.NoError(t, err) - return nil - }, - }, - } - bridgedProvider := pulcheck.BridgedProvider(t, "prov", resMap) - program := ` + } + bridgedProvider := pulcheck.BridgedProvider(t, "prov", resMap) + program := fmt.Sprintf(` name: test runtime: yaml resources: @@ -141,17 +177,17 @@ resources: type: prov:index:Test properties: otherProp: "test" - mapProp: {} + %s outputs: - mapPropOut: ${mainRes.mapProp} -` - pt := pulcheck.PulCheck(t, bridgedProvider, program) - - upRes := pt.Up() - require.Equal(t, nil, upRes.Outputs["mapPropOut"].Value) - - res := pt.Refresh(optrefresh.ExpectNoChanges()) - t.Logf(res.StdOut) + collectionOutput: %s +`, tc.programVal, tc.outputString) + pt := pulcheck.PulCheck(t, bridgedProvider, program) + upRes := pt.Up() + require.Equal(t, tc.expectedOutput, upRes.Outputs["collectionOutput"].Value) + res := pt.Refresh(optrefresh.ExpectNoChanges()) + t.Logf(res.StdOut) + }) + } } func TestNestedEmptyMapRefreshClean(t *testing.T) { From 84cab44496bf8e822adf1fa8a1c6a721c7e5ba96 Mon Sep 17 00:00:00 2001 From: Venelin Date: Fri, 7 Jun 2024 13:27:14 +0100 Subject: [PATCH 34/63] comment about blocks --- pkg/tfshim/sdk-v2/provider2.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/tfshim/sdk-v2/provider2.go b/pkg/tfshim/sdk-v2/provider2.go index 6cda1bb0b..793478b89 100644 --- a/pkg/tfshim/sdk-v2/provider2.go +++ b/pkg/tfshim/sdk-v2/provider2.go @@ -310,6 +310,7 @@ func normalizeNullValues(res *schema.Resource, state cty.Value) cty.Value { _, elemVal := it.Element() newElems = append(newElems, normalizeNullValues(elemRes, elemVal)) } + // Blocks are either lists or sets. if subBlockRes.Type == schema.TypeSet { m[key] = cty.SetVal(newElems) } else { From 9bce708fb44f210759fd0aaaed45d52d69f4a15a Mon Sep 17 00:00:00 2001 From: Venelin Date: Fri, 7 Jun 2024 13:29:54 +0100 Subject: [PATCH 35/63] add comment about refresh vs import --- pkg/tfshim/sdk-v2/provider2.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/tfshim/sdk-v2/provider2.go b/pkg/tfshim/sdk-v2/provider2.go index 793478b89..59db42589 100644 --- a/pkg/tfshim/sdk-v2/provider2.go +++ b/pkg/tfshim/sdk-v2/provider2.go @@ -255,6 +255,7 @@ func (p *planResourceChangeImpl) Refresh( } normStateValue := rr.stateValue if c != nil { + // This means we are doing a refresh and not an import. normStateValue = normalizeNullValues(res, rr.stateValue) } return &v2InstanceState2{ From c5be08b6ed33e7521042362730950d2d87593cb9 Mon Sep 17 00:00:00 2001 From: Venelin Date: Fri, 7 Jun 2024 13:34:54 +0100 Subject: [PATCH 36/63] add note about tf divergence --- pkg/tests/schema_pulumi_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index 5d8c1a7a3..b2cbc5c03 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -62,6 +62,7 @@ outputs: require.Equal(t, "aux", resUp.Outputs["testOut"].Value) } +// The clean refresh on empty/nil collections is an intentional divergence from TF behaviour. func TestCollectionsRefreshClean(t *testing.T) { for _, tc := range []struct { name string From 8925c0ece70798f569ddb6168ebf3e7089cd6460 Mon Sep 17 00:00:00 2001 From: Venelin Date: Fri, 7 Jun 2024 13:37:12 +0100 Subject: [PATCH 37/63] remove old note --- pkg/tfshim/sdk-v2/provider2.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/tfshim/sdk-v2/provider2.go b/pkg/tfshim/sdk-v2/provider2.go index 59db42589..b4ea60ea7 100644 --- a/pkg/tfshim/sdk-v2/provider2.go +++ b/pkg/tfshim/sdk-v2/provider2.go @@ -274,8 +274,6 @@ func (p *planResourceChangeImpl) Refresh( // To compensate for this problem, this method corrects the Pulumi Read result during `pulumi refresh` to also erase // empty collections from the Read result if the old input is missing or null. // -// This currently only handles top-level properties. -// // See: https://github.com/pulumi/terraform-plugin-sdk/blob/upstream-v2.33.0/helper/schema/grpc_provider.go#L1514 func normalizeNullValues(res *schema.Resource, state cty.Value) cty.Value { if !state.Type().IsObjectType() { From d5908f72a202e0e872931bf3acbf6db9da1068bb Mon Sep 17 00:00:00 2001 From: Venelin Date: Fri, 7 Jun 2024 13:50:00 +0100 Subject: [PATCH 38/63] disable plugin auto plugin acq --- pkg/tfgen/convert_cli_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/tfgen/convert_cli_test.go b/pkg/tfgen/convert_cli_test.go index 7d53acc85..a4a11e594 100644 --- a/pkg/tfgen/convert_cli_test.go +++ b/pkg/tfgen/convert_cli_test.go @@ -57,6 +57,7 @@ func TestConvertViaPulumiCLI(t *testing.T) { t.Skipf("Skipping on Windows due to a test setup issue") } t.Setenv("PULUMI_CONVERT", "1") + t.Setenv("DISABLE_AUTOMATIC_PLUGIN_ACQUISITION", "true") simpleResourceTF := ` resource "simple_resource" "a_resource" { From 2f5858b696327729a94e7d111a094cb0a86925eb Mon Sep 17 00:00:00 2001 From: Venelin Date: Fri, 7 Jun 2024 14:11:21 +0100 Subject: [PATCH 39/63] non prc tests --- pkg/tests/pulcheck/pulcheck.go | 23 +++- pkg/tests/schema_pulumi_test.go | 200 +++++++++++++++++++++++--------- 2 files changed, 168 insertions(+), 55 deletions(-) diff --git a/pkg/tests/pulcheck/pulcheck.go b/pkg/tests/pulcheck/pulcheck.go index 542cbe1f2..3ddf97454 100644 --- a/pkg/tests/pulcheck/pulcheck.go +++ b/pkg/tests/pulcheck/pulcheck.go @@ -104,13 +104,32 @@ type T interface { pulumitest.PT } +type bridgedProviderOpts struct { + DisablePlanResourceChange bool +} + +// BridgedProviderOpts +type BridgedProviderOpt func(*bridgedProviderOpts) + +// WithPlanResourceChange +func DisablePlanResourceChange() BridgedProviderOpt { + return func(o *bridgedProviderOpts) { + o.DisablePlanResourceChange = true + } +} + // This is an experimental API. -func BridgedProvider(t T, providerName string, resMap map[string]*schema.Resource) info.Provider { +func BridgedProvider(t T, providerName string, resMap map[string]*schema.Resource, opts ...BridgedProviderOpt) info.Provider { + options := &bridgedProviderOpts{} + for _, opt := range opts { + opt(options) + } + tfp := &schema.Provider{ResourcesMap: resMap} EnsureProviderValid(t, tfp) shimProvider := shimv2.NewProvider(tfp, shimv2.WithPlanResourceChange( - func(tfResourceType string) bool { return true }, + func(tfResourceType string) bool { return !options.DisablePlanResourceChange }, )) provider := tfbridge.ProviderInfo{ diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index b2cbc5c03..341797980 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -65,86 +65,176 @@ outputs: // The clean refresh on empty/nil collections is an intentional divergence from TF behaviour. func TestCollectionsRefreshClean(t *testing.T) { for _, tc := range []struct { - name string - schemaType schema.ValueType - readVal interface{} + name string + planResourceChange bool + schemaType schema.ValueType + readVal interface{} // Note maps are not pluralized in the program while lists and sets are. programVal string outputString string expectedOutput interface{} }{ { - name: "map null", - schemaType: schema.TypeMap, - readVal: map[string]interface{}{}, - programVal: "collectionProp: null", - outputString: "${mainRes.collectionProp}", - expectedOutput: nil, + name: "map null with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeMap, + readVal: map[string]interface{}{}, + programVal: "collectionProp: null", + outputString: "${mainRes.collectionProp}", + expectedOutput: nil, }, { - name: "map empty", - schemaType: schema.TypeMap, - readVal: map[string]interface{}{}, - programVal: "collectionProp: {}", - outputString: "${mainRes.collectionProp}", - expectedOutput: nil, + name: "map null without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeMap, + readVal: map[string]interface{}{}, + programVal: "collectionProp: null", + outputString: "${mainRes.collectionProp}", + expectedOutput: nil, }, { - name: "map nonempty", - schemaType: schema.TypeMap, - readVal: map[string]interface{}{"val": "test"}, - programVal: `collectionProp: {"val": "test"}`, - outputString: "${mainRes.collectionProp}", - expectedOutput: map[string]interface{}{"val": "test"}, + name: "map empty with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeMap, + readVal: map[string]interface{}{}, + programVal: "collectionProp: {}", + outputString: "${mainRes.collectionProp}", + expectedOutput: nil, }, { - name: "list null", - schemaType: schema.TypeList, - readVal: []interface{}{}, - programVal: "collectionProps: null", - outputString: "${mainRes.collectionProps}", - expectedOutput: nil, + name: "map empty without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeMap, + readVal: map[string]interface{}{}, + programVal: "collectionProp: {}", + outputString: "${mainRes.collectionProp}", + expectedOutput: nil, }, { - name: "list empty", - schemaType: schema.TypeList, - readVal: []interface{}{}, - programVal: "collectionProps: []", - outputString: "${mainRes.collectionProps}", - expectedOutput: nil, + name: "map nonempty with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeMap, + readVal: map[string]interface{}{"val": "test"}, + programVal: `collectionProp: {"val": "test"}`, + outputString: "${mainRes.collectionProp}", + expectedOutput: map[string]interface{}{"val": "test"}, }, { - name: "list nonempty", - schemaType: schema.TypeList, - readVal: []interface{}{"val"}, - programVal: `collectionProps: ["val"]`, - outputString: "${mainRes.collectionProps}", - expectedOutput: []interface{}{"val"}, + name: "map nonempty without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeMap, + readVal: map[string]interface{}{"val": "test"}, + programVal: `collectionProp: {"val": "test"}`, + outputString: "${mainRes.collectionProp}", + expectedOutput: map[string]interface{}{"val": "test"}, }, { - name: "set null", - schemaType: schema.TypeSet, - readVal: []interface{}{}, - programVal: "collectionProps: null", - outputString: "${mainRes.collectionProps}", - expectedOutput: nil, + name: "list null with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeList, + readVal: []interface{}{}, + programVal: "collectionProps: null", + outputString: "${mainRes.collectionProps}", + expectedOutput: nil, }, { - name: "set empty", - schemaType: schema.TypeSet, - readVal: []interface{}{}, - programVal: "collectionProps: []", - outputString: "${mainRes.collectionProps}", - expectedOutput: nil, + name: "list null without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeList, + readVal: []interface{}{}, + programVal: "collectionProps: null", + outputString: "${mainRes.collectionProps}", + expectedOutput: nil, + }, + { + name: "list empty with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeList, + readVal: []string{}, + programVal: "collectionProps: []", + outputString: "${mainRes.collectionProps}", + expectedOutput: []interface{}{}, }, { - name: "set nonempty", + name: "list empty without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeList, + readVal: []string{}, + programVal: "collectionProps: []", + outputString: "${mainRes.collectionProps}", + expectedOutput: []interface{}{}, + }, + { + name: "list nonempty with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeList, + readVal: []interface{}{"val"}, + programVal: `collectionProps: ["val"]`, + outputString: "${mainRes.collectionProps}", + expectedOutput: []interface{}{"val"}, + }, + { + name: "list nonempty without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeList, + readVal: []interface{}{"val"}, + programVal: `collectionProps: ["val"]`, + outputString: "${mainRes.collectionProps}", + expectedOutput: []interface{}{"val"}, + }, + { + name: "set null with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeSet, + readVal: []interface{}{}, + programVal: "collectionProps: null", + outputString: "${mainRes.collectionProps}", + expectedOutput: nil, + }, + { + name: "set null without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeSet, + readVal: []interface{}{}, + programVal: "collectionProps: null", + outputString: "${mainRes.collectionProps}", + expectedOutput: nil, + }, + { + name: "set empty with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeSet, + readVal: []interface{}{}, + programVal: "collectionProps: []", + outputString: "${mainRes.collectionProps}", + expectedOutput: nil, + }, + { + name: "set empty without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeSet, + readVal: []interface{}{}, + programVal: "collectionProps: []", + outputString: "${mainRes.collectionProps}", + expectedOutput: nil, + }, + { + name: "set nonempty with planResourceChange", schemaType: schema.TypeSet, readVal: []interface{}{"val"}, programVal: `collectionProps: ["val"]`, outputString: "${mainRes.collectionProps}", expectedOutput: []interface{}{"val"}, }, + { + name: "set nonempty without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeSet, + readVal: []interface{}{"val"}, + programVal: `collectionProps: ["val"]`, + outputString: "${mainRes.collectionProps}", + expectedOutput: []interface{}{"val"}, + }, } { t.Run(tc.name, func(t *testing.T) { resMap := map[string]*schema.Resource{ @@ -169,7 +259,11 @@ func TestCollectionsRefreshClean(t *testing.T) { }, }, } - bridgedProvider := pulcheck.BridgedProvider(t, "prov", resMap) + opts := []pulcheck.BridgedProviderOpt{} + if !tc.planResourceChange { + opts = append(opts, pulcheck.DisablePlanResourceChange()) + } + bridgedProvider := pulcheck.BridgedProvider(t, "prov", resMap, opts...) program := fmt.Sprintf(` name: test runtime: yaml From 2f93f9f026397f89ac85c84a9029f98363f22ba6 Mon Sep 17 00:00:00 2001 From: Venelin Date: Fri, 7 Jun 2024 15:50:59 +0100 Subject: [PATCH 40/63] collection refresh tests --- pkg/tests/pulcheck/pulcheck.go | 23 ++- pkg/tests/schema_pulumi_test.go | 277 ++++++++++++++++++++++++++++++++ 2 files changed, 298 insertions(+), 2 deletions(-) diff --git a/pkg/tests/pulcheck/pulcheck.go b/pkg/tests/pulcheck/pulcheck.go index 542cbe1f2..3ddf97454 100644 --- a/pkg/tests/pulcheck/pulcheck.go +++ b/pkg/tests/pulcheck/pulcheck.go @@ -104,13 +104,32 @@ type T interface { pulumitest.PT } +type bridgedProviderOpts struct { + DisablePlanResourceChange bool +} + +// BridgedProviderOpts +type BridgedProviderOpt func(*bridgedProviderOpts) + +// WithPlanResourceChange +func DisablePlanResourceChange() BridgedProviderOpt { + return func(o *bridgedProviderOpts) { + o.DisablePlanResourceChange = true + } +} + // This is an experimental API. -func BridgedProvider(t T, providerName string, resMap map[string]*schema.Resource) info.Provider { +func BridgedProvider(t T, providerName string, resMap map[string]*schema.Resource, opts ...BridgedProviderOpt) info.Provider { + options := &bridgedProviderOpts{} + for _, opt := range opts { + opt(options) + } + tfp := &schema.Provider{ResourcesMap: resMap} EnsureProviderValid(t, tfp) shimProvider := shimv2.NewProvider(tfp, shimv2.WithPlanResourceChange( - func(tfResourceType string) bool { return true }, + func(tfResourceType string) bool { return !options.DisablePlanResourceChange }, )) provider := tfbridge.ProviderInfo{ diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index 4ce831e67..757544b1e 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -2,12 +2,14 @@ package tests import ( "context" + "fmt" "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tests/pulcheck" "github.com/pulumi/pulumi/sdk/v3/go/auto/optpreview" + "github.com/pulumi/pulumi/sdk/v3/go/auto/optrefresh" "github.com/stretchr/testify/require" ) @@ -59,3 +61,278 @@ outputs: // assert that the property gets resolved require.Equal(t, "aux", resUp.Outputs["testOut"].Value) } + +// The clean refresh on empty/nil collections is an intentional divergence from TF behaviour. +func TestCollectionsRefreshClean(t *testing.T) { + for _, tc := range []struct { + name string + planResourceChange bool + schemaType schema.ValueType + readVal interface{} + // Note maps are not pluralized in the program while lists and sets are. + programVal string + outputString string + expectedOutput interface{} + }{ + { + name: "map null with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeMap, + readVal: map[string]interface{}{}, + programVal: "collectionProp: null", + outputString: "${mainRes.collectionProp}", + expectedOutput: nil, + }, + { + name: "map null without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeMap, + readVal: map[string]interface{}{}, + programVal: "collectionProp: null", + outputString: "${mainRes.collectionProp}", + expectedOutput: nil, + }, + { + name: "map empty with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeMap, + readVal: map[string]interface{}{}, + programVal: "collectionProp: {}", + outputString: "${mainRes.collectionProp}", + expectedOutput: nil, + }, + { + name: "map empty without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeMap, + readVal: map[string]interface{}{}, + programVal: "collectionProp: {}", + outputString: "${mainRes.collectionProp}", + expectedOutput: nil, + }, + { + name: "map nonempty with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeMap, + readVal: map[string]interface{}{"val": "test"}, + programVal: `collectionProp: {"val": "test"}`, + outputString: "${mainRes.collectionProp}", + expectedOutput: map[string]interface{}{"val": "test"}, + }, + { + name: "map nonempty without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeMap, + readVal: map[string]interface{}{"val": "test"}, + programVal: `collectionProp: {"val": "test"}`, + outputString: "${mainRes.collectionProp}", + expectedOutput: map[string]interface{}{"val": "test"}, + }, + { + name: "list null with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeList, + readVal: []interface{}{}, + programVal: "collectionProps: null", + outputString: "${mainRes.collectionProps}", + expectedOutput: nil, + }, + { + name: "list null without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeList, + readVal: []interface{}{}, + programVal: "collectionProps: null", + outputString: "${mainRes.collectionProps}", + expectedOutput: nil, + }, + { + name: "list empty with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeList, + readVal: []string{}, + programVal: "collectionProps: []", + outputString: "${mainRes.collectionProps}", + expectedOutput: []interface{}{}, + }, + { + name: "list empty without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeList, + readVal: []string{}, + programVal: "collectionProps: []", + outputString: "${mainRes.collectionProps}", + expectedOutput: []interface{}{}, + }, + { + name: "list nonempty with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeList, + readVal: []interface{}{"val"}, + programVal: `collectionProps: ["val"]`, + outputString: "${mainRes.collectionProps}", + expectedOutput: []interface{}{"val"}, + }, + { + name: "list nonempty without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeList, + readVal: []interface{}{"val"}, + programVal: `collectionProps: ["val"]`, + outputString: "${mainRes.collectionProps}", + expectedOutput: []interface{}{"val"}, + }, + { + name: "set null with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeSet, + readVal: []interface{}{}, + programVal: "collectionProps: null", + outputString: "${mainRes.collectionProps}", + expectedOutput: nil, + }, + { + name: "set null without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeSet, + readVal: []interface{}{}, + programVal: "collectionProps: null", + outputString: "${mainRes.collectionProps}", + expectedOutput: nil, + }, + { + name: "set empty with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeSet, + readVal: []interface{}{}, + programVal: "collectionProps: []", + outputString: "${mainRes.collectionProps}", + expectedOutput: nil, + }, + { + name: "set empty without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeSet, + readVal: []interface{}{}, + programVal: "collectionProps: []", + outputString: "${mainRes.collectionProps}", + expectedOutput: nil, + }, + { + name: "set nonempty with planResourceChange", + schemaType: schema.TypeSet, + readVal: []interface{}{"val"}, + programVal: `collectionProps: ["val"]`, + outputString: "${mainRes.collectionProps}", + expectedOutput: []interface{}{"val"}, + }, + { + name: "set nonempty without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeSet, + readVal: []interface{}{"val"}, + programVal: `collectionProps: ["val"]`, + outputString: "${mainRes.collectionProps}", + expectedOutput: []interface{}{"val"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + resMap := map[string]*schema.Resource{ + "prov_test": { + Schema: map[string]*schema.Schema{ + "collection_prop": { + Type: tc.schemaType, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, + "other_prop": { + Type: schema.TypeString, + Optional: true, + }, + }, + ReadContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { + err := d.Set("collection_prop", tc.readVal) + require.NoError(t, err) + err = d.Set("other_prop", "test") + require.NoError(t, err) + return nil + }, + }, + } + opts := []pulcheck.BridgedProviderOpt{} + if !tc.planResourceChange { + opts = append(opts, pulcheck.DisablePlanResourceChange()) + } + bridgedProvider := pulcheck.BridgedProvider(t, "prov", resMap, opts...) + program := fmt.Sprintf(` +name: test +runtime: yaml +resources: + mainRes: + type: prov:index:Test + properties: + otherProp: "test" + %s +outputs: + collectionOutput: %s +`, tc.programVal, tc.outputString) + pt := pulcheck.PulCheck(t, bridgedProvider, program) + upRes := pt.Up() + require.Equal(t, tc.expectedOutput, upRes.Outputs["collectionOutput"].Value) + res := pt.Refresh(optrefresh.ExpectNoChanges()) + t.Logf(res.StdOut) + }) + } +} + +func TestNestedEmptyMapRefreshClean(t *testing.T) { + resMap := map[string]*schema.Resource{ + "prov_test": { + Schema: map[string]*schema.Schema{ + "prop": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "map_prop": { + Type: schema.TypeMap, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + }, + }, + }, + "other_prop": { + Type: schema.TypeString, + Optional: true, + }, + }, + ReadContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { + err := d.Set("prop", []map[string]interface{}{{"map_prop": map[string]interface{}{}}}) + require.NoError(t, err) + err = d.Set("other_prop", "test") + require.NoError(t, err) + return nil + }, + }, + } + bridgedProvider := pulcheck.BridgedProvider(t, "prov", resMap) + program := ` +name: test +runtime: yaml +resources: + mainRes: + type: prov:index:Test + properties: + otherProp: "test" + props: + - mapProp: {} +` + pt := pulcheck.PulCheck(t, bridgedProvider, program) + pt.Up() + + res := pt.Refresh(optrefresh.ExpectNoChanges()) + t.Logf(res.StdOut) +} \ No newline at end of file From 3202055a015da9f261ae5c6ffa8057b9fcd8b1c8 Mon Sep 17 00:00:00 2001 From: Venelin Date: Mon, 10 Jun 2024 12:14:05 +0100 Subject: [PATCH 41/63] fix non-prc tests --- pkg/tests/schema_pulumi_test.go | 59 ++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index ba8c191d3..4f4ab195f 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -63,12 +63,13 @@ outputs: } // The clean refresh on empty/nil collections is an intentional divergence from TF behaviour. -func TestCollectionsRefreshClean(t *testing.T) { +// This is behaviour observed in both AWS and GCP providers, as well as a few others +func TestCollectionsNullEmptyRefreshClean(t *testing.T) { for _, tc := range []struct { name string planResourceChange bool schemaType schema.ValueType - readVal interface{} + cloudVal interface{} // Note maps are not pluralized in the program while lists and sets are. programVal string outputString string @@ -78,7 +79,7 @@ func TestCollectionsRefreshClean(t *testing.T) { name: "map null with planResourceChange", planResourceChange: true, schemaType: schema.TypeMap, - readVal: map[string]interface{}{}, + cloudVal: map[string]interface{}{}, programVal: "collectionProp: null", outputString: "${mainRes.collectionProp}", expectedOutput: nil, @@ -87,16 +88,16 @@ func TestCollectionsRefreshClean(t *testing.T) { name: "map null without planResourceChange", planResourceChange: false, schemaType: schema.TypeMap, - readVal: map[string]interface{}{}, + cloudVal: map[string]interface{}{}, programVal: "collectionProp: null", outputString: "${mainRes.collectionProp}", - expectedOutput: nil, + expectedOutput: map[string]interface{}{}, }, { name: "map empty with planResourceChange", planResourceChange: true, schemaType: schema.TypeMap, - readVal: map[string]interface{}{}, + cloudVal: map[string]interface{}{}, programVal: "collectionProp: {}", outputString: "${mainRes.collectionProp}", expectedOutput: nil, @@ -105,16 +106,16 @@ func TestCollectionsRefreshClean(t *testing.T) { name: "map empty without planResourceChange", planResourceChange: false, schemaType: schema.TypeMap, - readVal: map[string]interface{}{}, + cloudVal: map[string]interface{}{}, programVal: "collectionProp: {}", outputString: "${mainRes.collectionProp}", - expectedOutput: nil, + expectedOutput: map[string]interface{}{}, }, { name: "map nonempty with planResourceChange", planResourceChange: true, schemaType: schema.TypeMap, - readVal: map[string]interface{}{"val": "test"}, + cloudVal: map[string]interface{}{"val": "test"}, programVal: `collectionProp: {"val": "test"}`, outputString: "${mainRes.collectionProp}", expectedOutput: map[string]interface{}{"val": "test"}, @@ -123,7 +124,7 @@ func TestCollectionsRefreshClean(t *testing.T) { name: "map nonempty without planResourceChange", planResourceChange: false, schemaType: schema.TypeMap, - readVal: map[string]interface{}{"val": "test"}, + cloudVal: map[string]interface{}{"val": "test"}, programVal: `collectionProp: {"val": "test"}`, outputString: "${mainRes.collectionProp}", expectedOutput: map[string]interface{}{"val": "test"}, @@ -132,7 +133,7 @@ func TestCollectionsRefreshClean(t *testing.T) { name: "list null with planResourceChange", planResourceChange: true, schemaType: schema.TypeList, - readVal: []interface{}{}, + cloudVal: []interface{}{}, programVal: "collectionProps: null", outputString: "${mainRes.collectionProps}", expectedOutput: nil, @@ -141,16 +142,16 @@ func TestCollectionsRefreshClean(t *testing.T) { name: "list null without planResourceChange", planResourceChange: false, schemaType: schema.TypeList, - readVal: []interface{}{}, + cloudVal: []interface{}{}, programVal: "collectionProps: null", outputString: "${mainRes.collectionProps}", - expectedOutput: nil, + expectedOutput: []interface{}{}, }, { name: "list empty with planResourceChange", planResourceChange: true, schemaType: schema.TypeList, - readVal: []string{}, + cloudVal: []string{}, programVal: "collectionProps: []", outputString: "${mainRes.collectionProps}", expectedOutput: []interface{}{}, @@ -159,7 +160,7 @@ func TestCollectionsRefreshClean(t *testing.T) { name: "list empty without planResourceChange", planResourceChange: false, schemaType: schema.TypeList, - readVal: []string{}, + cloudVal: []string{}, programVal: "collectionProps: []", outputString: "${mainRes.collectionProps}", expectedOutput: []interface{}{}, @@ -168,7 +169,7 @@ func TestCollectionsRefreshClean(t *testing.T) { name: "list nonempty with planResourceChange", planResourceChange: true, schemaType: schema.TypeList, - readVal: []interface{}{"val"}, + cloudVal: []interface{}{"val"}, programVal: `collectionProps: ["val"]`, outputString: "${mainRes.collectionProps}", expectedOutput: []interface{}{"val"}, @@ -177,7 +178,7 @@ func TestCollectionsRefreshClean(t *testing.T) { name: "list nonempty without planResourceChange", planResourceChange: false, schemaType: schema.TypeList, - readVal: []interface{}{"val"}, + cloudVal: []interface{}{"val"}, programVal: `collectionProps: ["val"]`, outputString: "${mainRes.collectionProps}", expectedOutput: []interface{}{"val"}, @@ -186,7 +187,7 @@ func TestCollectionsRefreshClean(t *testing.T) { name: "set null with planResourceChange", planResourceChange: true, schemaType: schema.TypeSet, - readVal: []interface{}{}, + cloudVal: []interface{}{}, programVal: "collectionProps: null", outputString: "${mainRes.collectionProps}", expectedOutput: nil, @@ -195,16 +196,16 @@ func TestCollectionsRefreshClean(t *testing.T) { name: "set null without planResourceChange", planResourceChange: false, schemaType: schema.TypeSet, - readVal: []interface{}{}, + cloudVal: []interface{}{}, programVal: "collectionProps: null", outputString: "${mainRes.collectionProps}", - expectedOutput: nil, + expectedOutput: []interface{}{}, }, { name: "set empty with planResourceChange", planResourceChange: true, schemaType: schema.TypeSet, - readVal: []interface{}{}, + cloudVal: []interface{}{}, programVal: "collectionProps: []", outputString: "${mainRes.collectionProps}", expectedOutput: nil, @@ -213,15 +214,15 @@ func TestCollectionsRefreshClean(t *testing.T) { name: "set empty without planResourceChange", planResourceChange: false, schemaType: schema.TypeSet, - readVal: []interface{}{}, + cloudVal: []interface{}{}, programVal: "collectionProps: []", outputString: "${mainRes.collectionProps}", - expectedOutput: nil, + expectedOutput: []interface{}{}, }, { name: "set nonempty with planResourceChange", schemaType: schema.TypeSet, - readVal: []interface{}{"val"}, + cloudVal: []interface{}{"val"}, programVal: `collectionProps: ["val"]`, outputString: "${mainRes.collectionProps}", expectedOutput: []interface{}{"val"}, @@ -230,7 +231,7 @@ func TestCollectionsRefreshClean(t *testing.T) { name: "set nonempty without planResourceChange", planResourceChange: false, schemaType: schema.TypeSet, - readVal: []interface{}{"val"}, + cloudVal: []interface{}{"val"}, programVal: `collectionProps: ["val"]`, outputString: "${mainRes.collectionProps}", expectedOutput: []interface{}{"val"}, @@ -251,12 +252,18 @@ func TestCollectionsRefreshClean(t *testing.T) { }, }, ReadContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { - err := d.Set("collection_prop", tc.readVal) + err := d.Set("collection_prop", tc.cloudVal) require.NoError(t, err) err = d.Set("other_prop", "test") require.NoError(t, err) return nil }, + CreateContext: func(ctx context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics { + err := rd.Set("collection_prop", tc.cloudVal) + require.NoError(t, err) + rd.SetId("id0") + return nil + }, }, } opts := []pulcheck.BridgedProviderOpt{} From fc6d8a5cff9f7e0e23a13f7ab44f655b4a850d8a Mon Sep 17 00:00:00 2001 From: Venelin Date: Mon, 10 Jun 2024 14:24:37 +0100 Subject: [PATCH 42/63] add nested tests, refactor a bit, add expected failures --- pkg/tests/schema_pulumi_test.go | 619 +++++++++++++++++++++++--------- 1 file changed, 443 insertions(+), 176 deletions(-) diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index 4f4ab195f..dcab2e33c 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -62,182 +62,419 @@ outputs: require.Equal(t, "aux", resUp.Outputs["testOut"].Value) } -// The clean refresh on empty/nil collections is an intentional divergence from TF behaviour. // This is behaviour observed in both AWS and GCP providers, as well as a few others +// where the provider does not distinguish between nil and empty collections. func TestCollectionsNullEmptyRefreshClean(t *testing.T) { for _, tc := range []struct { name string planResourceChange bool schemaType schema.ValueType cloudVal interface{} - // Note maps are not pluralized in the program while lists and sets are. - programVal string - outputString string - expectedOutput interface{} + programVal string + // If true, the cloud value will be set in the CreateContext + createCloudValOverride bool + expectedOutputTopLevel interface{} + expectedOutputNested interface{} + expectFailTopLevel bool + expectFailNested bool }{ { - name: "map null with planResourceChange", - planResourceChange: true, - schemaType: schema.TypeMap, - cloudVal: map[string]interface{}{}, - programVal: "collectionProp: null", - outputString: "${mainRes.collectionProp}", - expectedOutput: nil, + name: "map null with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeMap, + cloudVal: map[string]interface{}{}, + programVal: "null", + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, }, { - name: "map null without planResourceChange", - planResourceChange: false, - schemaType: schema.TypeMap, - cloudVal: map[string]interface{}{}, - programVal: "collectionProp: null", - outputString: "${mainRes.collectionProp}", - expectedOutput: map[string]interface{}{}, + name: "map null without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeMap, + cloudVal: map[string]interface{}{}, + programVal: "null", + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, }, { - name: "map empty with planResourceChange", - planResourceChange: true, - schemaType: schema.TypeMap, - cloudVal: map[string]interface{}{}, - programVal: "collectionProp: {}", - outputString: "${mainRes.collectionProp}", - expectedOutput: nil, + name: "map null with planResourceChange with cloud override", + planResourceChange: true, + schemaType: schema.TypeMap, + cloudVal: map[string]interface{}{}, + programVal: "null", + createCloudValOverride: true, + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, }, { - name: "map empty without planResourceChange", - planResourceChange: false, - schemaType: schema.TypeMap, - cloudVal: map[string]interface{}{}, - programVal: "collectionProp: {}", - outputString: "${mainRes.collectionProp}", - expectedOutput: map[string]interface{}{}, + name: "map null without planResourceChange with cloud override", + planResourceChange: false, + schemaType: schema.TypeMap, + cloudVal: map[string]interface{}{}, + programVal: "null", + createCloudValOverride: true, + // Note the difference in expected output between top level and nested properties + expectedOutputTopLevel: map[string]interface{}{}, + expectedOutputNested: nil, + // Note only fails at the nested level! + expectFailNested: true, }, { - name: "map nonempty with planResourceChange", - planResourceChange: true, - schemaType: schema.TypeMap, - cloudVal: map[string]interface{}{"val": "test"}, - programVal: `collectionProp: {"val": "test"}`, - outputString: "${mainRes.collectionProp}", - expectedOutput: map[string]interface{}{"val": "test"}, + name: "map empty with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeMap, + cloudVal: map[string]interface{}{}, + programVal: "{}", + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, }, { - name: "map nonempty without planResourceChange", - planResourceChange: false, - schemaType: schema.TypeMap, - cloudVal: map[string]interface{}{"val": "test"}, - programVal: `collectionProp: {"val": "test"}`, - outputString: "${mainRes.collectionProp}", - expectedOutput: map[string]interface{}{"val": "test"}, + name: "map empty without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeMap, + cloudVal: map[string]interface{}{}, + programVal: "{}", + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, }, { - name: "list null with planResourceChange", - planResourceChange: true, - schemaType: schema.TypeList, - cloudVal: []interface{}{}, - programVal: "collectionProps: null", - outputString: "${mainRes.collectionProps}", - expectedOutput: nil, + name: "map empty with planResourceChange with cloud override", + planResourceChange: true, + schemaType: schema.TypeMap, + cloudVal: map[string]interface{}{}, + programVal: "{}", + createCloudValOverride: true, + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, }, { - name: "list null without planResourceChange", - planResourceChange: false, - schemaType: schema.TypeList, - cloudVal: []interface{}{}, - programVal: "collectionProps: null", - outputString: "${mainRes.collectionProps}", - expectedOutput: []interface{}{}, + name: "map empty without planResourceChange with cloud override", + planResourceChange: false, + schemaType: schema.TypeMap, + cloudVal: map[string]interface{}{}, + programVal: "{}", + createCloudValOverride: true, + // Note the difference in expected output between top level and nested properties + expectedOutputTopLevel: map[string]interface{}{}, + expectedOutputNested: nil, + // Note only fails at the nested level! + expectFailNested: true, }, { - name: "list empty with planResourceChange", - planResourceChange: true, - schemaType: schema.TypeList, - cloudVal: []string{}, - programVal: "collectionProps: []", - outputString: "${mainRes.collectionProps}", - expectedOutput: []interface{}{}, + name: "map nonempty with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeMap, + cloudVal: map[string]interface{}{"val": "test"}, + programVal: `{"val": "test"}`, + expectedOutputTopLevel: map[string]interface{}{"val": "test"}, + expectedOutputNested: map[string]interface{}{"val": "test"}, + }, + { + name: "map nonempty without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeMap, + cloudVal: map[string]interface{}{"val": "test"}, + programVal: `{"val": "test"}`, + expectedOutputTopLevel: map[string]interface{}{"val": "test"}, + expectedOutputNested: map[string]interface{}{"val": "test"}, + }, + { + name: "map nonempty with planResourceChange with cloud override", + planResourceChange: true, + schemaType: schema.TypeMap, + cloudVal: map[string]interface{}{"val": "test"}, + programVal: `{"val": "test"}`, + createCloudValOverride: true, + expectedOutputTopLevel: map[string]interface{}{"val": "test"}, + expectedOutputNested: map[string]interface{}{"val": "test"}, + }, + { + name: "map nonempty without planResourceChange with cloud override", + planResourceChange: false, + schemaType: schema.TypeMap, + cloudVal: map[string]interface{}{"val": "test"}, + programVal: `{"val": "test"}`, + createCloudValOverride: true, + expectedOutputTopLevel: map[string]interface{}{"val": "test"}, + expectedOutputNested: map[string]interface{}{"val": "test"}, + }, + { + name: "list null with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeList, + cloudVal: []interface{}{}, + programVal: "null", + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, + }, + { + name: "list null without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeList, + cloudVal: []interface{}{}, + programVal: "null", + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, + }, + { + name: "list null with planResourceChange with cloud override", + planResourceChange: true, + schemaType: schema.TypeList, + cloudVal: []interface{}{}, + programVal: "null", + createCloudValOverride: true, + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, + }, + { + name: "list null without planResourceChange with cloud override", + planResourceChange: false, + schemaType: schema.TypeList, + cloudVal: []interface{}{}, + programVal: "null", + createCloudValOverride: true, + // Note the difference in expected output between top level and nested properties + expectedOutputTopLevel: []interface{}{}, + expectedOutputNested: nil, + // Note only fails at the nested level! + expectFailNested: true, + }, + { + name: "list empty with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeList, + cloudVal: []string{}, + programVal: "[]", + expectedOutputTopLevel: []interface{}{}, + expectedOutputNested: []interface{}{}, }, { name: "list empty without planResourceChange", planResourceChange: false, schemaType: schema.TypeList, cloudVal: []string{}, - programVal: "collectionProps: []", - outputString: "${mainRes.collectionProps}", - expectedOutput: []interface{}{}, + programVal: "[]", + // Note the difference in expected output between top level and nested properties + // This is the opposite of all other cases of difference - the top level is nil and the nested is empty + expectedOutputTopLevel: nil, + expectedOutputNested: []interface{}{}, + // Note only fails at the top level! + expectFailTopLevel: true, }, { - name: "list nonempty with planResourceChange", - planResourceChange: true, - schemaType: schema.TypeList, - cloudVal: []interface{}{"val"}, - programVal: `collectionProps: ["val"]`, - outputString: "${mainRes.collectionProps}", - expectedOutput: []interface{}{"val"}, + name: "list empty with planResourceChange with cloud override", + planResourceChange: true, + schemaType: schema.TypeList, + cloudVal: []string{}, + programVal: "[]", + createCloudValOverride: true, + expectedOutputTopLevel: []interface{}{}, + expectedOutputNested: []interface{}{}, }, { - name: "list nonempty without planResourceChange", - planResourceChange: false, - schemaType: schema.TypeList, - cloudVal: []interface{}{"val"}, - programVal: `collectionProps: ["val"]`, - outputString: "${mainRes.collectionProps}", - expectedOutput: []interface{}{"val"}, + name: "list empty without planResourceChange with cloud override", + planResourceChange: false, + schemaType: schema.TypeList, + cloudVal: []string{}, + programVal: "[]", + createCloudValOverride: true, + expectedOutputTopLevel: []interface{}{}, + expectedOutputNested: []interface{}{}, }, { - name: "set null with planResourceChange", - planResourceChange: true, - schemaType: schema.TypeSet, - cloudVal: []interface{}{}, - programVal: "collectionProps: null", - outputString: "${mainRes.collectionProps}", - expectedOutput: nil, + name: "list nonempty with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeList, + cloudVal: []interface{}{"val"}, + programVal: `["val"]`, + expectedOutputTopLevel: []interface{}{"val"}, + expectedOutputNested: []interface{}{"val"}, }, { - name: "set null without planResourceChange", - planResourceChange: false, - schemaType: schema.TypeSet, - cloudVal: []interface{}{}, - programVal: "collectionProps: null", - outputString: "${mainRes.collectionProps}", - expectedOutput: []interface{}{}, + name: "list nonempty without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeList, + cloudVal: []interface{}{"val"}, + programVal: `["val"]`, + expectedOutputTopLevel: []interface{}{"val"}, + expectedOutputNested: []interface{}{"val"}, }, { - name: "set empty with planResourceChange", - planResourceChange: true, - schemaType: schema.TypeSet, - cloudVal: []interface{}{}, - programVal: "collectionProps: []", - outputString: "${mainRes.collectionProps}", - expectedOutput: nil, + name: "list nonempty with planResourceChange with cloud override", + planResourceChange: true, + schemaType: schema.TypeList, + cloudVal: []interface{}{"val"}, + programVal: `["val"]`, + createCloudValOverride: true, + expectedOutputTopLevel: []interface{}{"val"}, + expectedOutputNested: []interface{}{"val"}, }, { - name: "set empty without planResourceChange", - planResourceChange: false, - schemaType: schema.TypeSet, - cloudVal: []interface{}{}, - programVal: "collectionProps: []", - outputString: "${mainRes.collectionProps}", - expectedOutput: []interface{}{}, + name: "list nonempty without planResourceChange with cloud override", + planResourceChange: false, + schemaType: schema.TypeList, + cloudVal: []interface{}{"val"}, + programVal: `["val"]`, + createCloudValOverride: true, + expectedOutputTopLevel: []interface{}{"val"}, + expectedOutputNested: []interface{}{"val"}, }, { - name: "set nonempty with planResourceChange", - schemaType: schema.TypeSet, - cloudVal: []interface{}{"val"}, - programVal: `collectionProps: ["val"]`, - outputString: "${mainRes.collectionProps}", - expectedOutput: []interface{}{"val"}, + name: "set null with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeSet, + cloudVal: []interface{}{}, + programVal: "null", + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, }, { - name: "set nonempty without planResourceChange", - planResourceChange: false, - schemaType: schema.TypeSet, - cloudVal: []interface{}{"val"}, - programVal: `collectionProps: ["val"]`, - outputString: "${mainRes.collectionProps}", - expectedOutput: []interface{}{"val"}, + name: "set null without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeSet, + cloudVal: []interface{}{}, + programVal: "null", + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, + }, + { + name: "set null with planResourceChange with cloud override", + planResourceChange: true, + schemaType: schema.TypeSet, + cloudVal: []interface{}{}, + programVal: "null", + createCloudValOverride: true, + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, + }, + { + name: "set null without planResourceChange with cloud override", + planResourceChange: false, + schemaType: schema.TypeSet, + cloudVal: []interface{}{}, + programVal: "null", + createCloudValOverride: true, + // Note the difference in expected output between top level and nested properties + expectedOutputTopLevel: []interface{}{}, + expectedOutputNested: nil, + // Note only fails at the nested level! + expectFailNested: true, + }, + { + name: "set empty with planResourceChange", + planResourceChange: true, + schemaType: schema.TypeSet, + cloudVal: []interface{}{}, + programVal: "[]", + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, + }, + { + name: "set empty without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeSet, + cloudVal: []interface{}{}, + programVal: "[]", + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, + }, + { + name: "set empty with planResourceChange with cloud override", + planResourceChange: true, + schemaType: schema.TypeSet, + cloudVal: []interface{}{}, + programVal: "[]", + createCloudValOverride: true, + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, + }, + { + name: "set empty without planResourceChange with cloud override", + planResourceChange: false, + schemaType: schema.TypeSet, + cloudVal: []interface{}{}, + programVal: "[]", + createCloudValOverride: true, + // Note the difference in expected output between top level and nested properties + expectedOutputTopLevel: []interface{}{}, + expectedOutputNested: nil, + // Note only fails at the nested level! + expectFailNested: true, + }, + { + name: "set nonempty with planResourceChange", + schemaType: schema.TypeSet, + cloudVal: []interface{}{"val"}, + programVal: `["val"]`, + expectedOutputTopLevel: []interface{}{"val"}, + expectedOutputNested: []interface{}{"val"}, + }, + { + name: "set nonempty without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeSet, + cloudVal: []interface{}{"val"}, + programVal: `["val"]`, + expectedOutputTopLevel: []interface{}{"val"}, + expectedOutputNested: []interface{}{"val"}, + }, + { + name: "set nonempty with planResourceChange with cloud override", + schemaType: schema.TypeSet, + cloudVal: []interface{}{"val"}, + programVal: `["val"]`, + createCloudValOverride: true, + expectedOutputTopLevel: []interface{}{"val"}, + expectedOutputNested: []interface{}{"val"}, + }, + { + name: "set nonempty without planResourceChange with cloud override", + planResourceChange: false, + schemaType: schema.TypeSet, + cloudVal: []interface{}{"val"}, + programVal: `["val"]`, + createCloudValOverride: true, + expectedOutputTopLevel: []interface{}{"val"}, + expectedOutputNested: []interface{}{"val"}, }, } { - t.Run(tc.name, func(t *testing.T) { + collectionPropPlural := "" + pluralized := tc.schemaType == schema.TypeList || tc.schemaType == schema.TypeSet + if pluralized { + collectionPropPlural += "s" + } + + t.Run(tc.name+" top level", func(t *testing.T) { resMap := map[string]*schema.Resource{ "prov_test": { Schema: map[string]*schema.Schema{ @@ -251,16 +488,19 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { Optional: true, }, }, - ReadContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { - err := d.Set("collection_prop", tc.cloudVal) + ReadContext: func(_ context.Context, rd *schema.ResourceData, _ interface{}) diag.Diagnostics { + err := rd.Set("collection_prop", tc.cloudVal) require.NoError(t, err) - err = d.Set("other_prop", "test") + err = rd.Set("other_prop", "test") require.NoError(t, err) return nil }, - CreateContext: func(ctx context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics { - err := rd.Set("collection_prop", tc.cloudVal) - require.NoError(t, err) + CreateContext: func(_ context.Context, rd *schema.ResourceData, _ interface{}) diag.Diagnostics { + if tc.createCloudValOverride { + err := rd.Set("collection_prop", tc.cloudVal) + require.NoError(t, err) + } + rd.SetId("id0") return nil }, @@ -279,54 +519,69 @@ resources: type: prov:index:Test properties: otherProp: "test" - %s + collectionProp%s: %s outputs: - collectionOutput: %s -`, tc.programVal, tc.outputString) + collectionOutput: ${mainRes.collectionProp%s} +`, collectionPropPlural, tc.programVal, collectionPropPlural) pt := pulcheck.PulCheck(t, bridgedProvider, program) upRes := pt.Up() - require.Equal(t, tc.expectedOutput, upRes.Outputs["collectionOutput"].Value) - res := pt.Refresh(optrefresh.ExpectNoChanges()) + require.Equal(t, tc.expectedOutputTopLevel, upRes.Outputs["collectionOutput"].Value) + res, err := pt.CurrentStack().Refresh(pt.Context(), optrefresh.ExpectNoChanges()) + if tc.expectFailTopLevel { + require.Error(t, err) + } else { + require.NoError(t, err) + } t.Logf(res.StdOut) }) - } -} -func TestNestedEmptyMapRefreshClean(t *testing.T) { - resMap := map[string]*schema.Resource{ - "prov_test": { - Schema: map[string]*schema.Schema{ - "prop": { - Type: schema.TypeList, - Optional: true, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "map_prop": { - Type: schema.TypeMap, - Optional: true, - Elem: &schema.Schema{ - Type: schema.TypeString, + t.Run(tc.name+" nested", func(t *testing.T) { + resMap := map[string]*schema.Resource{ + "prov_test": { + Schema: map[string]*schema.Schema{ + "prop": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "collection_prop": { + Type: tc.schemaType, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, + "other_nested_prop": { + Type: schema.TypeString, + Optional: true, + }, }, }, }, + "other_prop": { + Type: schema.TypeString, + Optional: true, + }, + }, + ReadContext: func(_ context.Context, rd *schema.ResourceData, _ interface{}) diag.Diagnostics { + err := rd.Set("prop", []map[string]interface{}{{"collection_prop": tc.cloudVal, "other_nested_prop": "test"}}) + require.NoError(t, err) + err = rd.Set("other_prop", "test") + require.NoError(t, err) + + return nil + }, + CreateContext: func(_ context.Context, rd *schema.ResourceData, _ interface{}) diag.Diagnostics { + if tc.createCloudValOverride { + err := rd.Set("prop", []map[string]interface{}{{"collection_prop": tc.cloudVal, "other_nested_prop": "test"}}) + require.NoError(t, err) + } + rd.SetId("id0") + return nil }, }, - "other_prop": { - Type: schema.TypeString, - Optional: true, - }, - }, - ReadContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { - err := d.Set("prop", []map[string]interface{}{{"map_prop": map[string]interface{}{}}}) - require.NoError(t, err) - err = d.Set("other_prop", "test") - require.NoError(t, err) - return nil - }, - }, - } - bridgedProvider := pulcheck.BridgedProvider(t, "prov", resMap) - program := ` + } + + bridgedProvider := pulcheck.BridgedProvider(t, "prov", resMap) + program := fmt.Sprintf(` name: test runtime: yaml resources: @@ -335,11 +590,23 @@ resources: properties: otherProp: "test" props: - - mapProp: {} -` - pt := pulcheck.PulCheck(t, bridgedProvider, program) - pt.Up() + - collectionProp%s: %s + otherNestedProp: "test" +outputs: + collectionOutput: ${mainRes.props[0].collectionProp%s} +`, collectionPropPlural, tc.programVal, collectionPropPlural) + pt := pulcheck.PulCheck(t, bridgedProvider, program) + upRes := pt.Up() + require.Equal(t, tc.expectedOutputNested, upRes.Outputs["collectionOutput"].Value) - res := pt.Refresh(optrefresh.ExpectNoChanges()) - t.Logf(res.StdOut) + res, err := pt.CurrentStack().Refresh(pt.Context(), optrefresh.ExpectNoChanges()) + if tc.expectFailNested { + require.Error(t, err) + } else { + require.NoError(t, err) + } + + t.Logf(res.StdOut) + }) + } } From dd22d141e38c8cf1c4326cfe77d8157ffce842e3 Mon Sep 17 00:00:00 2001 From: Venelin Date: Mon, 10 Jun 2024 14:34:23 +0100 Subject: [PATCH 43/63] expand note --- pkg/tests/schema_pulumi_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index dcab2e33c..1fdad3d2a 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -62,8 +62,6 @@ outputs: require.Equal(t, "aux", resUp.Outputs["testOut"].Value) } -// This is behaviour observed in both AWS and GCP providers, as well as a few others -// where the provider does not distinguish between nil and empty collections. func TestCollectionsNullEmptyRefreshClean(t *testing.T) { for _, tc := range []struct { name string @@ -72,6 +70,9 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { cloudVal interface{} programVal string // If true, the cloud value will be set in the CreateContext + // This is behaviour observed in both AWS and GCP providers, as well as a few others + // where the provider returns an empty collections when a nil one was specified in inputs. + // See [pulumi/pulumi-terraform-bridge#2047] for more details around this behavior createCloudValOverride bool expectedOutputTopLevel interface{} expectedOutputNested interface{} From b916333e51e4421306bb7d1af3f77a62ab86ce16 Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Fri, 7 Jun 2024 14:49:52 -0400 Subject: [PATCH 44/63] Experiments in schema walkers --- pkg/tfshim/sdk-v2/provider2.go | 57 ++++++++-------------- pkg/tfshim/sdk-v2/walk.go | 87 ++++++++++++++++++++++++++++++++++ pkg/tfshim/sdk-v2/walk_test.go | 68 ++++++++++++++++++++++++++ 3 files changed, 176 insertions(+), 36 deletions(-) create mode 100644 pkg/tfshim/sdk-v2/walk.go create mode 100644 pkg/tfshim/sdk-v2/walk_test.go diff --git a/pkg/tfshim/sdk-v2/provider2.go b/pkg/tfshim/sdk-v2/provider2.go index b4ea60ea7..3acb2b000 100644 --- a/pkg/tfshim/sdk-v2/provider2.go +++ b/pkg/tfshim/sdk-v2/provider2.go @@ -15,6 +15,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/walk" "github.com/pulumi/pulumi/sdk/v3/go/common/util/contract" ) @@ -279,44 +280,28 @@ func normalizeNullValues(res *schema.Resource, state cty.Value) cty.Value { if !state.Type().IsObjectType() { return state } - m := state.AsValueMap() - sch := res.CoreConfigSchema() - for key, attr := range sch.Attributes { - stateVal, ok := m[key] - if !ok { - continue + tr, err := cty.Transform(state, func(p cty.Path, v cty.Value) (cty.Value, error) { + // Only interested in non-null empty collection values. + if v.IsNull() || !v.Type().IsCollectionType() || v.LengthInt() > 0 { + return v, nil } - - m[key] = normalizeNullValuesAttr(attr.Type, stateVal) - } - - for key := range sch.BlockTypes { - subBlockRes := res.SchemaMap()[key] - if subBlockRes == nil { - continue - } - - elemRes, ok := subBlockRes.Elem.(*schema.Resource) - if !ok { - continue - } - if !m[key].CanIterateElements() { - continue - } - it := m[key].ElementIterator() - newElems := make([]cty.Value, 0) - for it.Next() { - _, elemVal := it.Element() - newElems = append(newElems, normalizeNullValues(elemRes, elemVal)) - } - // Blocks are either lists or sets. - if subBlockRes.Type == schema.TypeSet { - m[key] = cty.SetVal(newElems) - } else { - m[key] = cty.ListVal(newElems) + sp := walk.FromHCtyPath(p) + sc := findSchemaContext(res, sp) + switch sc := sc.(type) { + // Only interested in attributes. + case *attrSchemaContext: + attr := sc.resource.CoreConfigSchema().Attributes[sc.name] + if !attr.Type.IsCollectionType() { + return v, nil + } + // Do substitute a null for the empty collection here. + return cty.NullVal(attr.Type), nil + default: + return v, nil } - } - return cty.ObjectVal(m) + }) + contract.AssertNoErrorf(err, "Transform never errors") + return tr } func normalizeNullValuesAttr(attrType cty.Type, stateVal cty.Value) cty.Value { diff --git a/pkg/tfshim/sdk-v2/walk.go b/pkg/tfshim/sdk-v2/walk.go new file mode 100644 index 000000000..832a31dcd --- /dev/null +++ b/pkg/tfshim/sdk-v2/walk.go @@ -0,0 +1,87 @@ +package sdkv2 + +import ( + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/walk" + "github.com/pulumi/pulumi/sdk/v3/go/common/util/contract" +) + +func findSchemaContext(topLevel *schema.Resource, path walk.SchemaPath) schemaContext { + var c schemaContext = &blockSchemaContext{topLevel} + for _, step := range path { + switch step := step.(type) { + case walk.ElementStep: + c = c.element() + if c == nil { + return nil + } + case walk.GetAttrStep: + c = c.attribute(step.Name) + if c == nil { + return nil + } + } + } + return c +} + +type schemaContext interface { + element() schemaContext + attribute(name string) schemaContext +} + +type blockSchemaContext struct { + resource *schema.Resource +} + +func (*blockSchemaContext) element() schemaContext { return nil } + +func (b *blockSchemaContext) attribute(name string) schemaContext { + s := b.resource.CoreConfigSchema() + _, isAttr := s.Attributes[name] + if isAttr { + return &attrSchemaContext{b.resource, name} + } + blk, isBlock := s.BlockTypes[name] + if isBlock { + switch int(blk.Nesting) { + case 1, 2: // single, group + // One exception seems to be timeout blocks but presumably they do not matter here. + contract.Failf("NestingMode={Single,Group} blocks not expressible with SDKv2: %v", blk.Nesting) + case 3, 4: // list, set + x, ok := b.resource.SchemaMap()[name] + contract.Assertf(ok, "expected to find %q in SchemaMap()", name) + subr, ok := x.Elem.(*schema.Resource) + contract.Assertf(ok, "expected Elem() to be a *schema.Resource") + return &blockNestingSchemaContext{subr} + case 5: // map + contract.Failf("NestingMode={Map} blocks not expressible with SDKv2: %v", blk.Nesting) + default: + contract.Failf("invalid block type %v", blk.Nesting) + panic("invalid block type") + } + } + return nil +} + +// Intermediate node for stepping through collection-nested blocks. +type blockNestingSchemaContext struct { + elem *schema.Resource +} + +func (b *blockNestingSchemaContext) element() schemaContext { return &blockSchemaContext{b.elem} } +func (*blockNestingSchemaContext) attribute(name string) schemaContext { return nil } + +var _ schemaContext = (*blockSchemaContext)(nil) + +// This is a leaf node of the schema context tree. While value types can be further nested, including maps and objects, +// these will not be attributes in the Terraform sense of supporting Required, Computed etc annotations. +type attrSchemaContext struct { + resource *schema.Resource + name string +} + +func (*attrSchemaContext) element() schemaContext { return nil } +func (*attrSchemaContext) attribute(name string) schemaContext { return nil } + +var _ schemaContext = (*attrSchemaContext)(nil) diff --git a/pkg/tfshim/sdk-v2/walk_test.go b/pkg/tfshim/sdk-v2/walk_test.go new file mode 100644 index 000000000..430ce9e01 --- /dev/null +++ b/pkg/tfshim/sdk-v2/walk_test.go @@ -0,0 +1,68 @@ +package sdkv2 + +import ( + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/walk" + "github.com/stretchr/testify/require" +) + +func TestFindSchemaContext(t *testing.T) { + topLevel := &schema.Resource{ + Schema: map[string]*schema.Schema{ + "attr": { + Type: schema.TypeString, + Optional: true, + }, + "blk": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "foo": { + Type: schema.TypeString, + Optional: true, + }, + }, + }, + MaxItems: 1, // should not matter + }, + "mattr": { + Type: schema.TypeMap, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + Optional: true, + }, + }, + }, + } + + require.NoError(t, topLevel.InternalValidate(nil, true)) + + t.Run("attr", func(t *testing.T) { + c := findSchemaContext(topLevel, walk.NewSchemaPath().GetAttr("attr")) + a := c.(*attrSchemaContext) + require.True(t, a.resource.CoreConfigSchema().Attributes[a.name].Optional) + }) + + t.Run("blk", func(t *testing.T) { + c := findSchemaContext(topLevel, walk.NewSchemaPath().GetAttr("blk").Element()) + bb := c.(*blockSchemaContext) + _, ok := bb.resource.Schema["foo"] + require.True(t, ok) + }) + + t.Run("blk.foo", func(t *testing.T) { + c := findSchemaContext(topLevel, walk.NewSchemaPath().GetAttr("blk").Element().GetAttr("foo")) + a := c.(*attrSchemaContext) + require.True(t, a.resource.CoreConfigSchema().Attributes[a.name].Optional) + }) + + t.Run("mblk", func(t *testing.T) { + c := findSchemaContext(topLevel, walk.NewSchemaPath().GetAttr("mattr")) + a := c.(*attrSchemaContext) + require.True(t, a.resource.CoreConfigSchema().Attributes[a.name].Optional) + }) +} From 9e641aa200ffe5ae587c054b3a35f2f694e41083 Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Fri, 7 Jun 2024 16:20:38 -0400 Subject: [PATCH 45/63] Pass all planResourceChange test cases --- pkg/tests/schema_pulumi_test.go | 24 +++++++++ pkg/tfshim/sdk-v2/provider2.go | 89 ++++++++++++++++++++++++++------- 2 files changed, 94 insertions(+), 19 deletions(-) diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index 1fdad3d2a..cbbed56ff 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -506,6 +506,30 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { return nil }, }, + CreateContext: func(ctx context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics { + err := rd.Set("collection_prop", tc.readVal) + require.NoError(t, err) + rd.SetId("id0") + return nil + }, + ReadContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { + err := d.Set("collection_prop", tc.readVal) + require.NoError(t, err) + err = d.Set("other_prop", "test") + require.NoError(t, err) + return nil + }, + UpdateContext: func(ctx context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics { + return nil + }, + DeleteContext: func(ctx context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics { + return nil + }, + } + require.NoError(t, resource.InternalValidate(nil, true)) + + resMap := map[string]*schema.Resource{ + "prov_test": resource, } opts := []pulcheck.BridgedProviderOpt{} if !tc.planResourceChange { diff --git a/pkg/tfshim/sdk-v2/provider2.go b/pkg/tfshim/sdk-v2/provider2.go index 3acb2b000..455a0729b 100644 --- a/pkg/tfshim/sdk-v2/provider2.go +++ b/pkg/tfshim/sdk-v2/provider2.go @@ -147,6 +147,16 @@ type planResourceChangeImpl struct { var _ planResourceChangeProvider = (*planResourceChangeImpl)(nil) +func (p *planResourceChangeImpl) recoverConfig(t string, c shim.ResourceConfig) (cty.Value, error) { + res := p.tf.ResourcesMap[t] + config := configFromShim(c) + cfg, err := recoverAndCoerceCtyValueWithSchema(res.CoreConfigSchema(), config.Config) + if err != nil { + return cty.NilVal, fmt.Errorf("Resource %q: %w", t, err) + } + return cfg, nil +} + func (p *planResourceChangeImpl) Diff( ctx context.Context, t string, @@ -154,7 +164,6 @@ func (p *planResourceChangeImpl) Diff( c shim.ResourceConfig, opts shim.DiffOptions, ) (shim.InstanceDiff, error) { - config := configFromShim(c) s, err := p.upgradeState(ctx, t, s) if err != nil { return nil, err @@ -168,9 +177,9 @@ func (p *planResourceChangeImpl) Diff( if err != nil { return nil, err } - cfg, err := recoverAndCoerceCtyValueWithSchema(res.CoreConfigSchema(), config.Config) + cfg, err := p.recoverConfig(t, c) if err != nil { - return nil, fmt.Errorf("Resource %q: %w", t, err) + return nil, err } cfg = normalizeBlockCollections(cfg, res) @@ -257,7 +266,7 @@ func (p *planResourceChangeImpl) Refresh( normStateValue := rr.stateValue if c != nil { // This means we are doing a refresh and not an import. - normStateValue = normalizeNullValues(res, rr.stateValue) + normStateValue = normalizeNullValues(res, state.stateValue, rr.stateValue) } return &v2InstanceState2{ resourceType: rr.resourceType, @@ -276,30 +285,72 @@ func (p *planResourceChangeImpl) Refresh( // empty collections from the Read result if the old input is missing or null. // // See: https://github.com/pulumi/terraform-plugin-sdk/blob/upstream-v2.33.0/helper/schema/grpc_provider.go#L1514 -func normalizeNullValues(res *schema.Resource, state cty.Value) cty.Value { - if !state.Type().IsObjectType() { +func normalizeNullValues(res *schema.Resource, oldState, state cty.Value) cty.Value { + if !oldState.Type().IsObjectType() || !state.Type().IsObjectType() { return state } + + // Only interested in nulls and non-null empty collection values. + interesting := func(v cty.Value) bool { + if !v.IsKnown() { + return false + } + if v.IsNull() { + return true + } + return v.Type().IsCollectionType() && v.LengthInt() == 0 + } + + matchingOldStateValue := func(t cty.Type, p cty.Path) cty.Value { + if v, err := p.Apply(oldState); err == nil { + return v + } + return cty.NullVal(t) + } + + fmt.Println("WALK", state.GoString()) + tr, err := cty.Transform(state, func(p cty.Path, v cty.Value) (cty.Value, error) { - // Only interested in non-null empty collection values. - if v.IsNull() || !v.Type().IsCollectionType() || v.LengthInt() > 0 { + sp := walk.FromHCtyPath(p) + fmt.Println("walking", sp.MustEncodeSchemaPath()) + if !interesting(v) { + fmt.Println("value not interesting", sp.MustEncodeSchemaPath()) return v, nil } - sp := walk.FromHCtyPath(p) - sc := findSchemaContext(res, sp) - switch sc := sc.(type) { + // Only interested in attributes. - case *attrSchemaContext: - attr := sc.resource.CoreConfigSchema().Attributes[sc.name] - if !attr.Type.IsCollectionType() { - return v, nil - } - // Do substitute a null for the empty collection here. - return cty.NullVal(attr.Type), nil - default: + sc, ok := findSchemaContext(res, sp).(*attrSchemaContext) + if !ok { + fmt.Println("no schema context") + return v, nil + } + + // Must be a collection attribute. + attr := sc.resource.CoreConfigSchema().Attributes[sc.name] + if !attr.Type.IsCollectionType() { + fmt.Println("attr.Type NOT A COLLECTION") return v, nil } + + // Matching config value must be interesting. + mv := matchingOldStateValue(v.Type(), p) + if !interesting(mv) { + fmt.Println("mv not interesting") + return v, nil + } + + // Config value must be different from state. + if mv.Equals(v).True() { + fmt.Println("SAME", mv.GoString(), v.GoString()) + return v, nil + } + + // Do prefer the config value at this point. + fmt.Println("SUBSTITUTE", sp.MustEncodeSchemaPath()) + return mv, nil }) + fmt.Println("WALKED", tr.GoString()) + contract.AssertNoErrorf(err, "Transform never errors") return tr } From 142ba6e4150716b18707b73fb8164bef839a8439 Mon Sep 17 00:00:00 2001 From: Venelin Date: Mon, 10 Jun 2024 15:44:54 +0100 Subject: [PATCH 46/63] bad merge --- pkg/tests/schema_pulumi_test.go | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index cbbed56ff..1fdad3d2a 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -506,30 +506,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { return nil }, }, - CreateContext: func(ctx context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics { - err := rd.Set("collection_prop", tc.readVal) - require.NoError(t, err) - rd.SetId("id0") - return nil - }, - ReadContext: func(_ context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { - err := d.Set("collection_prop", tc.readVal) - require.NoError(t, err) - err = d.Set("other_prop", "test") - require.NoError(t, err) - return nil - }, - UpdateContext: func(ctx context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics { - return nil - }, - DeleteContext: func(ctx context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics { - return nil - }, - } - require.NoError(t, resource.InternalValidate(nil, true)) - - resMap := map[string]*schema.Resource{ - "prov_test": resource, } opts := []pulcheck.BridgedProviderOpt{} if !tc.planResourceChange { From a2b8c162e614c4706fa2ccbc9423c4f363793b93 Mon Sep 17 00:00:00 2001 From: Venelin Date: Mon, 10 Jun 2024 16:06:11 +0100 Subject: [PATCH 47/63] fix nested tests --- pkg/tests/schema_pulumi_test.go | 111 ++++++++++++++------------------ 1 file changed, 49 insertions(+), 62 deletions(-) diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index 1fdad3d2a..782a5ba0c 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -91,15 +91,16 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { expectFailNested: true, }, { - name: "map null without planResourceChange", - planResourceChange: false, - schemaType: schema.TypeMap, - cloudVal: map[string]interface{}{}, - programVal: "null", + name: "map null without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeMap, + cloudVal: map[string]interface{}{}, + programVal: "null", + // Note the difference in expected output between top level and nested properties expectedOutputTopLevel: nil, - expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, + expectedOutputNested: map[string]interface{}{}, + // Note only fails at the top level + expectFailTopLevel: true, }, { name: "map null with planResourceChange with cloud override", @@ -120,11 +121,8 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { cloudVal: map[string]interface{}{}, programVal: "null", createCloudValOverride: true, - // Note the difference in expected output between top level and nested properties expectedOutputTopLevel: map[string]interface{}{}, - expectedOutputNested: nil, - // Note only fails at the nested level! - expectFailNested: true, + expectedOutputNested: map[string]interface{}{}, }, { name: "map empty with planResourceChange", @@ -138,15 +136,15 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { expectFailNested: true, }, { - name: "map empty without planResourceChange", - planResourceChange: false, - schemaType: schema.TypeMap, - cloudVal: map[string]interface{}{}, - programVal: "{}", + name: "map empty without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeMap, + cloudVal: map[string]interface{}{}, + programVal: "{}", + // Note the difference in expected output between top level and nested properties expectedOutputTopLevel: nil, - expectedOutputNested: nil, + expectedOutputNested: map[string]interface{}{}, expectFailTopLevel: true, - expectFailNested: true, }, { name: "map empty with planResourceChange with cloud override", @@ -167,11 +165,8 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { cloudVal: map[string]interface{}{}, programVal: "{}", createCloudValOverride: true, - // Note the difference in expected output between top level and nested properties expectedOutputTopLevel: map[string]interface{}{}, - expectedOutputNested: nil, - // Note only fails at the nested level! - expectFailNested: true, + expectedOutputNested: map[string]interface{}{}, }, { name: "map nonempty with planResourceChange", @@ -223,15 +218,15 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { expectFailNested: true, }, { - name: "list null without planResourceChange", - planResourceChange: false, - schemaType: schema.TypeList, - cloudVal: []interface{}{}, - programVal: "null", + name: "list null without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeList, + cloudVal: []interface{}{}, + programVal: "null", + // Note the difference in expected output between top level and nested properties expectedOutputTopLevel: nil, - expectedOutputNested: nil, + expectedOutputNested: []interface{}{}, expectFailTopLevel: true, - expectFailNested: true, }, { name: "list null with planResourceChange with cloud override", @@ -252,11 +247,8 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { cloudVal: []interface{}{}, programVal: "null", createCloudValOverride: true, - // Note the difference in expected output between top level and nested properties expectedOutputTopLevel: []interface{}{}, - expectedOutputNested: nil, - // Note only fails at the nested level! - expectFailNested: true, + expectedOutputNested: []interface{}{}, }, { name: "list empty with planResourceChange", @@ -274,7 +266,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { cloudVal: []string{}, programVal: "[]", // Note the difference in expected output between top level and nested properties - // This is the opposite of all other cases of difference - the top level is nil and the nested is empty expectedOutputTopLevel: nil, expectedOutputNested: []interface{}{}, // Note only fails at the top level! @@ -350,15 +341,15 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { expectFailNested: true, }, { - name: "set null without planResourceChange", - planResourceChange: false, - schemaType: schema.TypeSet, - cloudVal: []interface{}{}, - programVal: "null", + name: "set null without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeSet, + cloudVal: []interface{}{}, + programVal: "null", + // Note the difference in expected output between top level and nested properties expectedOutputTopLevel: nil, - expectedOutputNested: nil, + expectedOutputNested: []interface{}{}, expectFailTopLevel: true, - expectFailNested: true, }, { name: "set null with planResourceChange with cloud override", @@ -379,11 +370,8 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { cloudVal: []interface{}{}, programVal: "null", createCloudValOverride: true, - // Note the difference in expected output between top level and nested properties expectedOutputTopLevel: []interface{}{}, - expectedOutputNested: nil, - // Note only fails at the nested level! - expectFailNested: true, + expectedOutputNested: []interface{}{}, }, { name: "set empty with planResourceChange", @@ -397,15 +385,15 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { expectFailNested: true, }, { - name: "set empty without planResourceChange", - planResourceChange: false, - schemaType: schema.TypeSet, - cloudVal: []interface{}{}, - programVal: "[]", + name: "set empty without planResourceChange", + planResourceChange: false, + schemaType: schema.TypeSet, + cloudVal: []interface{}{}, + programVal: "[]", + // Note the difference in expected output between top level and nested properties expectedOutputTopLevel: nil, - expectedOutputNested: nil, + expectedOutputNested: []interface{}{}, expectFailTopLevel: true, - expectFailNested: true, }, { name: "set empty with planResourceChange with cloud override", @@ -426,11 +414,8 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { cloudVal: []interface{}{}, programVal: "[]", createCloudValOverride: true, - // Note the difference in expected output between top level and nested properties expectedOutputTopLevel: []interface{}{}, - expectedOutputNested: nil, - // Note only fails at the nested level! - expectFailNested: true, + expectedOutputNested: []interface{}{}, }, { name: "set nonempty with planResourceChange", @@ -475,6 +460,11 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { collectionPropPlural += "s" } + opts := []pulcheck.BridgedProviderOpt{} + if !tc.planResourceChange { + opts = append(opts, pulcheck.DisablePlanResourceChange()) + } + t.Run(tc.name+" top level", func(t *testing.T) { resMap := map[string]*schema.Resource{ "prov_test": { @@ -507,10 +497,7 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { }, }, } - opts := []pulcheck.BridgedProviderOpt{} - if !tc.planResourceChange { - opts = append(opts, pulcheck.DisablePlanResourceChange()) - } + bridgedProvider := pulcheck.BridgedProvider(t, "prov", resMap, opts...) program := fmt.Sprintf(` name: test @@ -581,7 +568,7 @@ outputs: }, } - bridgedProvider := pulcheck.BridgedProvider(t, "prov", resMap) + bridgedProvider := pulcheck.BridgedProvider(t, "prov", resMap, opts...) program := fmt.Sprintf(` name: test runtime: yaml From 8ca5c2b77a591a3cbc4d3c46a3b0010dcc914c2f Mon Sep 17 00:00:00 2001 From: Venelin Date: Mon, 10 Jun 2024 16:08:04 +0100 Subject: [PATCH 48/63] more notes --- pkg/tests/schema_pulumi_test.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index 782a5ba0c..ade63fcca 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -144,7 +144,8 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { // Note the difference in expected output between top level and nested properties expectedOutputTopLevel: nil, expectedOutputNested: map[string]interface{}{}, - expectFailTopLevel: true, + // Note only fails at the top level + expectFailTopLevel: true, }, { name: "map empty with planResourceChange with cloud override", @@ -226,7 +227,8 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { // Note the difference in expected output between top level and nested properties expectedOutputTopLevel: nil, expectedOutputNested: []interface{}{}, - expectFailTopLevel: true, + // Note only fails at the top level + expectFailTopLevel: true, }, { name: "list null with planResourceChange with cloud override", @@ -268,7 +270,7 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { // Note the difference in expected output between top level and nested properties expectedOutputTopLevel: nil, expectedOutputNested: []interface{}{}, - // Note only fails at the top level! + // Note only fails at the top level expectFailTopLevel: true, }, { @@ -349,7 +351,8 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { // Note the difference in expected output between top level and nested properties expectedOutputTopLevel: nil, expectedOutputNested: []interface{}{}, - expectFailTopLevel: true, + // Note only fails at the top level + expectFailTopLevel: true, }, { name: "set null with planResourceChange with cloud override", @@ -393,7 +396,8 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { // Note the difference in expected output between top level and nested properties expectedOutputTopLevel: nil, expectedOutputNested: []interface{}{}, - expectFailTopLevel: true, + // Note only fails at the top level + expectFailTopLevel: true, }, { name: "set empty with planResourceChange with cloud override", From 569a9b6c80e8c6e76ae00e522c6cd92acbc47886 Mon Sep 17 00:00:00 2001 From: Venelin Date: Mon, 10 Jun 2024 16:32:11 +0100 Subject: [PATCH 49/63] additional tests on null cloud values --- pkg/tests/schema_pulumi_test.go | 133 ++++++++++++++++++++++++++++++++ 1 file changed, 133 insertions(+) diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index ade63fcca..4b6cb5459 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -102,6 +102,28 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { // Note only fails at the top level expectFailTopLevel: true, }, + { + name: "map null with planResourceChange with nil cloud value", + planResourceChange: true, + schemaType: schema.TypeMap, + cloudVal: nil, + programVal: "null", + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, + }, + { + name: "map null without planResourceChange with nil cloud value", + planResourceChange: false, + schemaType: schema.TypeMap, + cloudVal: nil, + programVal: "null", + expectedOutputTopLevel: nil, + expectedOutputNested: map[string]interface{}{}, + // Note only fails at the top level + expectFailTopLevel: true, + }, { name: "map null with planResourceChange with cloud override", planResourceChange: true, @@ -124,6 +146,28 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { expectedOutputTopLevel: map[string]interface{}{}, expectedOutputNested: map[string]interface{}{}, }, + { + name: "map null with planResourceChange with nil cloud value and cloud override", + planResourceChange: true, + schemaType: schema.TypeMap, + cloudVal: nil, + programVal: "null", + createCloudValOverride: true, + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, + }, + { + name: "map null without planResourceChange with nil cloud value and cloud override", + planResourceChange: false, + schemaType: schema.TypeMap, + cloudVal: nil, + programVal: "null", + createCloudValOverride: true, + expectedOutputTopLevel: map[string]interface{}{}, + expectedOutputNested: map[string]interface{}{}, + }, { name: "map empty with planResourceChange", planResourceChange: true, @@ -230,6 +274,29 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { // Note only fails at the top level expectFailTopLevel: true, }, + { + name: "list null with planResourceChange with nil cloud value", + planResourceChange: true, + schemaType: schema.TypeList, + cloudVal: nil, + programVal: "null", + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, + }, + { + name: "list null without planResourceChange with nil cloud value", + planResourceChange: false, + schemaType: schema.TypeList, + cloudVal: nil, + programVal: "null", + // Note the difference in expected output between top level and nested properties + expectedOutputTopLevel: nil, + expectedOutputNested: []interface{}{}, + // Note only fails at the top level + expectFailTopLevel: true, + }, { name: "list null with planResourceChange with cloud override", planResourceChange: true, @@ -252,6 +319,28 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { expectedOutputTopLevel: []interface{}{}, expectedOutputNested: []interface{}{}, }, + { + name: "list null with planResourceChange with nil cloud value and cloud override", + planResourceChange: true, + schemaType: schema.TypeList, + cloudVal: nil, + programVal: "null", + createCloudValOverride: true, + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, + }, + { + name: "list null without planResourceChange with nil cloud value and cloud override", + planResourceChange: false, + schemaType: schema.TypeList, + cloudVal: nil, + programVal: "null", + createCloudValOverride: true, + expectedOutputTopLevel: []interface{}{}, + expectedOutputNested: []interface{}{}, + }, { name: "list empty with planResourceChange", planResourceChange: true, @@ -354,6 +443,28 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { // Note only fails at the top level expectFailTopLevel: true, }, + { + name: "set null with planResourceChange with nil cloud value", + planResourceChange: true, + schemaType: schema.TypeSet, + cloudVal: nil, + programVal: "null", + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, + }, + { + name: "set null without planResourceChange with nil cloud value", + planResourceChange: false, + schemaType: schema.TypeSet, + cloudVal: nil, + programVal: "null", + expectedOutputTopLevel: nil, + expectedOutputNested: []interface{}{}, + // Note only fails at the top level + expectFailTopLevel: true, + }, { name: "set null with planResourceChange with cloud override", planResourceChange: true, @@ -376,6 +487,28 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { expectedOutputTopLevel: []interface{}{}, expectedOutputNested: []interface{}{}, }, + { + name: "set null with planResourceChange with nil cloud value and cloud override", + planResourceChange: true, + schemaType: schema.TypeSet, + cloudVal: nil, + programVal: "null", + createCloudValOverride: true, + expectedOutputTopLevel: nil, + expectedOutputNested: nil, + expectFailTopLevel: true, + expectFailNested: true, + }, + { + name: "set null without planResourceChange with nil cloud value and cloud override", + planResourceChange: false, + schemaType: schema.TypeSet, + cloudVal: nil, + programVal: "null", + createCloudValOverride: true, + expectedOutputTopLevel: []interface{}{}, + expectedOutputNested: []interface{}{}, + }, { name: "set empty with planResourceChange", planResourceChange: true, From 4c06b7e5fbe95a50f421c2132da8fac1ec042293 Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Fri, 7 Jun 2024 17:01:53 -0400 Subject: [PATCH 50/63] Fix timeouts panic --- pkg/tfshim/sdk-v2/walk.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/tfshim/sdk-v2/walk.go b/pkg/tfshim/sdk-v2/walk.go index 832a31dcd..ec25525f8 100644 --- a/pkg/tfshim/sdk-v2/walk.go +++ b/pkg/tfshim/sdk-v2/walk.go @@ -45,9 +45,13 @@ func (b *blockSchemaContext) attribute(name string) schemaContext { blk, isBlock := s.BlockTypes[name] if isBlock { switch int(blk.Nesting) { - case 1, 2: // single, group - // One exception seems to be timeout blocks but presumably they do not matter here. - contract.Failf("NestingMode={Single,Group} blocks not expressible with SDKv2: %v", blk.Nesting) + case 1: + // The only SDKv2- expressible blocks of NestingMode=Single seem to be the special "timeout" + // blocks that are not part of the user-defined schema. The code cannot panic on these but opts + // to silently skip processing. + return nil + case 2: // group + contract.Failf("NestingMode=Group blocks not expressible with SDKv2: %v", blk.Nesting) case 3, 4: // list, set x, ok := b.resource.SchemaMap()[name] contract.Assertf(ok, "expected to find %q in SchemaMap()", name) From ebe3c5e0c4f3d0caac87bcba5ae304544e1778c6 Mon Sep 17 00:00:00 2001 From: Venelin Date: Mon, 10 Jun 2024 16:48:51 +0100 Subject: [PATCH 51/63] remove skips from PRC tests --- pkg/tests/schema_pulumi_test.go | 35 +++------------------------------ 1 file changed, 3 insertions(+), 32 deletions(-) diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index 4b6cb5459..4d1e8a349 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -87,8 +87,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { programVal: "null", expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "map null without planResourceChange", @@ -110,8 +108,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { programVal: "null", expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "map null without planResourceChange with nil cloud value", @@ -133,8 +129,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { createCloudValOverride: true, expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "map null without planResourceChange with cloud override", @@ -155,8 +149,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { createCloudValOverride: true, expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "map null without planResourceChange with nil cloud value and cloud override", @@ -176,8 +168,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { programVal: "{}", expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "map empty without planResourceChange", @@ -200,8 +190,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { createCloudValOverride: true, expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "map empty without planResourceChange with cloud override", @@ -259,8 +247,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { programVal: "null", expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "list null without planResourceChange", @@ -282,8 +268,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { programVal: "null", expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "list null without planResourceChange with nil cloud value", @@ -306,8 +290,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { createCloudValOverride: true, expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "list null without planResourceChange with cloud override", @@ -328,8 +310,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { createCloudValOverride: true, expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "list null without planResourceChange with nil cloud value and cloud override", @@ -428,8 +408,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { programVal: "null", expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "set null without planResourceChange", @@ -451,8 +429,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { programVal: "null", expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "set null without planResourceChange with nil cloud value", @@ -474,8 +450,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { createCloudValOverride: true, expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "set null without planResourceChange with cloud override", @@ -496,8 +470,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { createCloudValOverride: true, expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "set null without planResourceChange with nil cloud value and cloud override", @@ -517,8 +489,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { programVal: "[]", expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "set empty without planResourceChange", @@ -541,8 +511,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { createCloudValOverride: true, expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "set empty without planResourceChange with cloud override", @@ -591,6 +559,7 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { expectedOutputNested: []interface{}{"val"}, }, } { + tc := tc collectionPropPlural := "" pluralized := tc.schemaType == schema.TypeList || tc.schemaType == schema.TypeSet if pluralized { @@ -603,6 +572,7 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { } t.Run(tc.name+" top level", func(t *testing.T) { + t.Parallel() resMap := map[string]*schema.Resource{ "prov_test": { Schema: map[string]*schema.Schema{ @@ -661,6 +631,7 @@ outputs: }) t.Run(tc.name+" nested", func(t *testing.T) { + t.Parallel() resMap := map[string]*schema.Resource{ "prov_test": { Schema: map[string]*schema.Schema{ From 2fc9d7a3c43eecd9700fb33a42a55059b295a70b Mon Sep 17 00:00:00 2001 From: Venelin Date: Mon, 10 Jun 2024 17:02:15 +0100 Subject: [PATCH 52/63] remove unused function --- pkg/tfshim/sdk-v2/provider2.go | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/pkg/tfshim/sdk-v2/provider2.go b/pkg/tfshim/sdk-v2/provider2.go index 455a0729b..3317a30af 100644 --- a/pkg/tfshim/sdk-v2/provider2.go +++ b/pkg/tfshim/sdk-v2/provider2.go @@ -355,19 +355,6 @@ func normalizeNullValues(res *schema.Resource, oldState, state cty.Value) cty.Va return tr } -func normalizeNullValuesAttr(attrType cty.Type, stateVal cty.Value) cty.Value { - if !attrType.IsCollectionType() { - return stateVal - } - if !stateVal.Type().IsCollectionType() { - return stateVal - } - if !stateVal.IsNull() && stateVal.LengthInt() > 0 { - return stateVal - } - return cty.NullVal(attrType) -} - func (p *planResourceChangeImpl) NewDestroyDiff( ctx context.Context, t string, opts shim.TimeoutOptions, ) shim.InstanceDiff { @@ -514,7 +501,8 @@ func (s *grpcServer) PlanResourceChange( PlannedState cty.Value PlannedMeta map[string]interface{} PlannedDiff *terraform.InstanceDiff -}, error) { +}, error, +) { configVal, err := msgpack.Marshal(config, ty) if err != nil { return nil, err From 7b2efb772ff9af4bcb188ab467bd6d50566acde2 Mon Sep 17 00:00:00 2001 From: Venelin Date: Tue, 11 Jun 2024 12:52:20 +0100 Subject: [PATCH 53/63] bad merge --- pkg/tests/schema_pulumi_test.go | 32 -------------------------------- 1 file changed, 32 deletions(-) diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index 89c77ecf8..132714cc3 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -87,8 +87,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { programVal: "null", expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "map null without planResourceChange", @@ -110,8 +108,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { programVal: "null", expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "map null without planResourceChange with nil cloud value", @@ -133,8 +129,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { createCloudValOverride: true, expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "map null without planResourceChange with cloud override", @@ -155,8 +149,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { createCloudValOverride: true, expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "map null without planResourceChange with nil cloud value and cloud override", @@ -176,8 +168,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { programVal: "{}", expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "map empty without planResourceChange", @@ -200,8 +190,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { createCloudValOverride: true, expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "map empty without planResourceChange with cloud override", @@ -259,8 +247,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { programVal: "null", expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "list null without planResourceChange", @@ -282,8 +268,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { programVal: "null", expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "list null without planResourceChange with nil cloud value", @@ -306,8 +290,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { createCloudValOverride: true, expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "list null without planResourceChange with cloud override", @@ -328,8 +310,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { createCloudValOverride: true, expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "list null without planResourceChange with nil cloud value and cloud override", @@ -428,8 +408,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { programVal: "null", expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "set null without planResourceChange", @@ -451,8 +429,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { programVal: "null", expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "set null without planResourceChange with nil cloud value", @@ -474,8 +450,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { createCloudValOverride: true, expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "set null without planResourceChange with cloud override", @@ -496,8 +470,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { createCloudValOverride: true, expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "set null without planResourceChange with nil cloud value and cloud override", @@ -517,8 +489,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { programVal: "[]", expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "set empty without planResourceChange", @@ -541,8 +511,6 @@ func TestCollectionsNullEmptyRefreshClean(t *testing.T) { createCloudValOverride: true, expectedOutputTopLevel: nil, expectedOutputNested: nil, - expectFailTopLevel: true, - expectFailNested: true, }, { name: "set empty without planResourceChange with cloud override", From d01e5d35129f8a18f7b6df9f3c6822c930da7860 Mon Sep 17 00:00:00 2001 From: Venelin Date: Tue, 11 Jun 2024 12:55:53 +0100 Subject: [PATCH 54/63] TEST ONLY :enable PRC by default --- pkg/tfshim/sdk-v2/provider.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/pkg/tfshim/sdk-v2/provider.go b/pkg/tfshim/sdk-v2/provider.go index 2eef34b72..6a69ab0db 100644 --- a/pkg/tfshim/sdk-v2/provider.go +++ b/pkg/tfshim/sdk-v2/provider.go @@ -3,14 +3,12 @@ package sdkv2 import ( "context" "fmt" - "os" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/logging" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" testing "github.com/mitchellh/go-testing-interface" shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" - "github.com/pulumi/pulumi/sdk/v3/go/common/util/cmdutil" ) var _ = shim.Provider(v2Provider{}) @@ -62,13 +60,14 @@ func NewProvider(p *schema.Provider, opts ...providerOption) shim.Provider { tf: p, opts: opts, } - if cmdutil.IsTruthy(os.Getenv("PULUMI_ENABLE_PLAN_RESOURCE_CHANGE")) { - return newProviderWithPlanResourceChange(p, prov, func(s string) bool { return true }) - } - if opts, err := getProviderOptions(opts); err == nil && opts.planResourceChangeFilter != nil { - return newProviderWithPlanResourceChange(p, prov, opts.planResourceChangeFilter) - } - return prov + return newProviderWithPlanResourceChange(p, prov, func(s string) bool { return true }) + // if cmdutil.IsTruthy(os.Getenv("PULUMI_ENABLE_PLAN_RESOURCE_CHANGE")) { + // return newProviderWithPlanResourceChange(p, prov, func(s string) bool { return true }) + // } + // if opts, err := getProviderOptions(opts); err == nil && opts.planResourceChangeFilter != nil { + // return newProviderWithPlanResourceChange(p, prov, opts.planResourceChangeFilter) + // } + // return prov } func (p v2Provider) Schema() shim.SchemaMap { From c0e013f61e46fbd7deea7d19f5c3eaa98a0ae3a7 Mon Sep 17 00:00:00 2001 From: Venelin Date: Tue, 11 Jun 2024 12:56:34 +0100 Subject: [PATCH 55/63] revert test only change --- pkg/tfshim/sdk-v2/provider.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/pkg/tfshim/sdk-v2/provider.go b/pkg/tfshim/sdk-v2/provider.go index 6a69ab0db..2eef34b72 100644 --- a/pkg/tfshim/sdk-v2/provider.go +++ b/pkg/tfshim/sdk-v2/provider.go @@ -3,12 +3,14 @@ package sdkv2 import ( "context" "fmt" + "os" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/logging" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" testing "github.com/mitchellh/go-testing-interface" shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" + "github.com/pulumi/pulumi/sdk/v3/go/common/util/cmdutil" ) var _ = shim.Provider(v2Provider{}) @@ -60,14 +62,13 @@ func NewProvider(p *schema.Provider, opts ...providerOption) shim.Provider { tf: p, opts: opts, } - return newProviderWithPlanResourceChange(p, prov, func(s string) bool { return true }) - // if cmdutil.IsTruthy(os.Getenv("PULUMI_ENABLE_PLAN_RESOURCE_CHANGE")) { - // return newProviderWithPlanResourceChange(p, prov, func(s string) bool { return true }) - // } - // if opts, err := getProviderOptions(opts); err == nil && opts.planResourceChangeFilter != nil { - // return newProviderWithPlanResourceChange(p, prov, opts.planResourceChangeFilter) - // } - // return prov + if cmdutil.IsTruthy(os.Getenv("PULUMI_ENABLE_PLAN_RESOURCE_CHANGE")) { + return newProviderWithPlanResourceChange(p, prov, func(s string) bool { return true }) + } + if opts, err := getProviderOptions(opts); err == nil && opts.planResourceChangeFilter != nil { + return newProviderWithPlanResourceChange(p, prov, opts.planResourceChangeFilter) + } + return prov } func (p v2Provider) Schema() shim.SchemaMap { From a9caa286b74a634d8b17b4e62f545ed6a67fa6b8 Mon Sep 17 00:00:00 2001 From: Venelin Date: Tue, 11 Jun 2024 16:25:33 +0100 Subject: [PATCH 56/63] remove debug logging --- pkg/tfshim/sdk-v2/provider2.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/pkg/tfshim/sdk-v2/provider2.go b/pkg/tfshim/sdk-v2/provider2.go index 3317a30af..3756255ee 100644 --- a/pkg/tfshim/sdk-v2/provider2.go +++ b/pkg/tfshim/sdk-v2/provider2.go @@ -308,48 +308,38 @@ func normalizeNullValues(res *schema.Resource, oldState, state cty.Value) cty.Va return cty.NullVal(t) } - fmt.Println("WALK", state.GoString()) - tr, err := cty.Transform(state, func(p cty.Path, v cty.Value) (cty.Value, error) { sp := walk.FromHCtyPath(p) - fmt.Println("walking", sp.MustEncodeSchemaPath()) if !interesting(v) { - fmt.Println("value not interesting", sp.MustEncodeSchemaPath()) return v, nil } // Only interested in attributes. sc, ok := findSchemaContext(res, sp).(*attrSchemaContext) if !ok { - fmt.Println("no schema context") return v, nil } // Must be a collection attribute. attr := sc.resource.CoreConfigSchema().Attributes[sc.name] if !attr.Type.IsCollectionType() { - fmt.Println("attr.Type NOT A COLLECTION") return v, nil } // Matching config value must be interesting. mv := matchingOldStateValue(v.Type(), p) if !interesting(mv) { - fmt.Println("mv not interesting") return v, nil } // Config value must be different from state. if mv.Equals(v).True() { - fmt.Println("SAME", mv.GoString(), v.GoString()) return v, nil } // Do prefer the config value at this point. - fmt.Println("SUBSTITUTE", sp.MustEncodeSchemaPath()) return mv, nil }) - fmt.Println("WALKED", tr.GoString()) contract.AssertNoErrorf(err, "Transform never errors") return tr From 4b0f66736889e2ab13f1d31ca46718a3ab254603 Mon Sep 17 00:00:00 2001 From: Venelin Date: Tue, 11 Jun 2024 16:26:10 +0100 Subject: [PATCH 57/63] TEST enable PRC by default --- pkg/tfshim/sdk-v2/provider.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/tfshim/sdk-v2/provider.go b/pkg/tfshim/sdk-v2/provider.go index 2eef34b72..84c1c63c5 100644 --- a/pkg/tfshim/sdk-v2/provider.go +++ b/pkg/tfshim/sdk-v2/provider.go @@ -62,6 +62,7 @@ func NewProvider(p *schema.Provider, opts ...providerOption) shim.Provider { tf: p, opts: opts, } + return newProviderWithPlanResourceChange(p, prov, func(s string) bool { return true }) if cmdutil.IsTruthy(os.Getenv("PULUMI_ENABLE_PLAN_RESOURCE_CHANGE")) { return newProviderWithPlanResourceChange(p, prov, func(s string) bool { return true }) } From 170c52d2e7b0b91133deadba3d2e0e3164a27c9a Mon Sep 17 00:00:00 2001 From: Venelin Date: Tue, 11 Jun 2024 16:27:59 +0100 Subject: [PATCH 58/63] revert test change --- pkg/tfshim/sdk-v2/provider.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/tfshim/sdk-v2/provider.go b/pkg/tfshim/sdk-v2/provider.go index 84c1c63c5..2eef34b72 100644 --- a/pkg/tfshim/sdk-v2/provider.go +++ b/pkg/tfshim/sdk-v2/provider.go @@ -62,7 +62,6 @@ func NewProvider(p *schema.Provider, opts ...providerOption) shim.Provider { tf: p, opts: opts, } - return newProviderWithPlanResourceChange(p, prov, func(s string) bool { return true }) if cmdutil.IsTruthy(os.Getenv("PULUMI_ENABLE_PLAN_RESOURCE_CHANGE")) { return newProviderWithPlanResourceChange(p, prov, func(s string) bool { return true }) } From 82214b34be86e04c41dd543c4448ce8ddcceca11 Mon Sep 17 00:00:00 2001 From: Venelin Date: Wed, 12 Jun 2024 18:19:28 +0100 Subject: [PATCH 59/63] config mode attr tests --- pkg/tests/schema_pulumi_test.go | 110 ++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index 132714cc3..9763d3c5e 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -707,3 +707,113 @@ outputs: }) } } + +func TestConfigModeAttrNullEmptyRefresh(t *testing.T) { + for _, cloudOverrideEnabled := range []bool{true, false} { + t.Run(fmt.Sprintf("cloudOverride=%v", cloudOverrideEnabled), func(t *testing.T) { + for _, tc := range []struct { + name string + cloudVal interface{} + programVal string + expectedVal interface{} + }{ + { + name: "null", + cloudVal: nil, + programVal: "null", + expectedVal: nil, + }, + { + name: "null with empty cloud value", + cloudVal: []interface{}{}, + programVal: "null", + expectedVal: nil, + }, + { + name: "empty", + cloudVal: []interface{}{}, + programVal: "[]", + expectedVal: []interface{}{}, + }, + { + name: "empty with nil cloud value", + cloudVal: nil, + programVal: "[]", + expectedVal: []interface{}{}, + }, + { + name: "non-empty with empty obj", + cloudVal: []interface{}{map[string]interface{}{}}, + programVal: `[{}]`, + expectedVal: []interface{}{map[string]interface{}{}}, + }, + { + name: "nonempty", + cloudVal: []interface{}{map[string]interface{}{"foo": "test"}}, + programVal: `[{"foo": "test"}]`, + expectedVal: []interface{}{map[string]interface{}{"foo": "test"}}, + }, + } { + cloudVal := tc.cloudVal + programVal := tc.programVal + expectedVal := tc.expectedVal + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + resMap := map[string]*schema.Resource{ + "prov_test": { + Schema: map[string]*schema.Schema{ + "blk": { + Type: schema.TypeList, + Optional: true, + ConfigMode: schema.SchemaConfigModeAttr, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "foo": { + Type: schema.TypeString, + Optional: true, + }, + }, + }, + }, + }, + ReadContext: func(_ context.Context, rd *schema.ResourceData, _ interface{}) diag.Diagnostics { + err := rd.Set("blk", cloudVal) + require.NoError(t, err) + return nil + }, + CreateContext: func(_ context.Context, rd *schema.ResourceData, _ interface{}) diag.Diagnostics { + if cloudOverrideEnabled { + err := rd.Set("blk", cloudVal) + require.NoError(t, err) + } + + rd.SetId("id0") + return nil + }, + }, + } + + bridgedProvider := pulcheck.BridgedProvider(t, "prov", resMap) + program := fmt.Sprintf(` +name: test +runtime: yaml +resources: + mainRes: + type: prov:index:Test + properties: + blks: %s +outputs: + blkOut: ${mainRes.blks} +`, programVal) + pt := pulcheck.PulCheck(t, bridgedProvider, program) + upRes := pt.Up() + require.Equal(t, expectedVal, upRes.Outputs["blkOut"].Value) + + res, err := pt.CurrentStack().Refresh(pt.Context(), optrefresh.ExpectNoChanges()) + require.NoError(t, err) + t.Logf(res.StdOut) + }) + } + }) + } +} From 3758fb5b74b9a73c576137e1cc0eb4f22c504bc5 Mon Sep 17 00:00:00 2001 From: Venelin Date: Wed, 12 Jun 2024 20:35:17 +0100 Subject: [PATCH 60/63] set repro --- pkg/tests/schema_pulumi_test.go | 95 +++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index 9763d3c5e..641d19a0d 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -1,8 +1,11 @@ package tests import ( + "bytes" "context" "fmt" + "sort" + "strings" "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" @@ -817,3 +820,95 @@ outputs: }) } } + +func TestRefreshReorderEmptyNull(t *testing.T) { + resourceComputeFirewallRuleHash := func(v interface{}) int { + var buf bytes.Buffer + m := v.(map[string]interface{}) + buf.WriteString(fmt.Sprintf("%s-", strings.ToLower(m["protocol"].(string)))) + + // We need to make sure to sort the strings below so that we always + // generate the same hash code no matter what is in the set. + if v, ok := m["ports"]; ok && v != nil { + s := make([]string, 0, len(v.([]interface{}))) + for _, p := range v.([]interface{}) { + s = append(s, fmt.Sprintf("%d", p.(int))) + } + sort.Strings(s) + + for _, v := range s { + buf.WriteString(fmt.Sprintf("%s-", v)) + } + } + + return schema.HashString(buf.String()) + } + resMap := map[string]*schema.Resource{ + "prov_test": { + Schema: map[string]*schema.Schema{ + "allow": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "protocol": { + Type: schema.TypeString, + Required: true, + }, + "ports": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeInt, + }, + }, + }, + }, + }, + }, + ReadContext: func(_ context.Context, rd *schema.ResourceData, _ interface{}) diag.Diagnostics { + err := rd.Set("allow", schema.NewSet(resourceComputeFirewallRuleHash, + []interface{}{ + map[string]interface{}{"protocol": "tcp", "ports": []interface{}{80, 443}}, + map[string]interface{}{"protocol": "icmp", "ports": []interface{}{}}, + }, + )) + require.NoError(t, err) + return nil + }, + CreateContext: func(_ context.Context, rd *schema.ResourceData, _ interface{}) diag.Diagnostics { + err := rd.Set("allow", schema.NewSet(resourceComputeFirewallRuleHash, + []interface{}{ + map[string]interface{}{"protocol": "icmp", "ports": []interface{}{}}, + map[string]interface{}{"protocol": "tcp", "ports": []interface{}{80, 443}}, + }, + )) + require.NoError(t, err) + + rd.SetId("id0") + return nil + }, + }, + } + bridgedProvider := pulcheck.BridgedProvider(t, "prov", resMap) + program := ` +name: test +runtime: yaml +resources: + mainRes: + type: prov:index:Test + properties: + allows: + - protocol: "tcp" + ports: [80, 443] + - protocol: "icmp" + ports: [] +outputs: + allowOut: ${mainRes.allows} +` + pt := pulcheck.PulCheck(t, bridgedProvider, program) + pt.Up() + res, err := pt.CurrentStack().Refresh(pt.Context(), optrefresh.ExpectNoChanges()) + require.NoError(t, err) + t.Logf(res.StdOut) +} From b890f3ede26cd9be5e1b951068d96296ece517a3 Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Wed, 12 Jun 2024 17:04:57 -0400 Subject: [PATCH 61/63] Skip cty.Value indexing failures --- pkg/tfshim/sdk-v2/provider2.go | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/pkg/tfshim/sdk-v2/provider2.go b/pkg/tfshim/sdk-v2/provider2.go index 3756255ee..ae7935ab4 100644 --- a/pkg/tfshim/sdk-v2/provider2.go +++ b/pkg/tfshim/sdk-v2/provider2.go @@ -301,13 +301,6 @@ func normalizeNullValues(res *schema.Resource, oldState, state cty.Value) cty.Va return v.Type().IsCollectionType() && v.LengthInt() == 0 } - matchingOldStateValue := func(t cty.Type, p cty.Path) cty.Value { - if v, err := p.Apply(oldState); err == nil { - return v - } - return cty.NullVal(t) - } - tr, err := cty.Transform(state, func(p cty.Path, v cty.Value) (cty.Value, error) { sp := walk.FromHCtyPath(p) if !interesting(v) { @@ -326,18 +319,28 @@ func normalizeNullValues(res *schema.Resource, oldState, state cty.Value) cty.Va return v, nil } - // Matching config value must be interesting. - mv := matchingOldStateValue(v.Type(), p) + mv, err := p.Apply(v) + if err != nil { + // Apply may fail if the paths index through set elements, see: + // See https://github.com/zclconf/go-cty/issues/180 + // + // It may also fail on type mismatches due to changing schema. + // + // In either case just ignore this and bail. + return v, nil + } + + // Matching old state value must be interesting. if !interesting(mv) { return v, nil } - // Config value must be different from state. + // Old state value must be different from the read state. if mv.Equals(v).True() { return v, nil } - // Do prefer the config value at this point. + // Do prefer the old state value at this point. return mv, nil }) From 6e907e38493aee382ed537af55ae1e33f567904d Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Wed, 12 Jun 2024 22:43:20 -0400 Subject: [PATCH 62/63] Fix typo --- pkg/tfshim/sdk-v2/provider2.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/tfshim/sdk-v2/provider2.go b/pkg/tfshim/sdk-v2/provider2.go index ae7935ab4..0096854ed 100644 --- a/pkg/tfshim/sdk-v2/provider2.go +++ b/pkg/tfshim/sdk-v2/provider2.go @@ -319,7 +319,7 @@ func normalizeNullValues(res *schema.Resource, oldState, state cty.Value) cty.Va return v, nil } - mv, err := p.Apply(v) + mv, err := p.Apply(oldState) if err != nil { // Apply may fail if the paths index through set elements, see: // See https://github.com/zclconf/go-cty/issues/180 From 5d60418308180cdc7cf33bdb7328f3dd6348a086 Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Wed, 12 Jun 2024 22:47:57 -0400 Subject: [PATCH 63/63] Address PR feedback --- pkg/tfshim/sdk-v2/walk.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/tfshim/sdk-v2/walk.go b/pkg/tfshim/sdk-v2/walk.go index ec25525f8..c1c4ba480 100644 --- a/pkg/tfshim/sdk-v2/walk.go +++ b/pkg/tfshim/sdk-v2/walk.go @@ -2,6 +2,7 @@ package sdkv2 import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2/internal/tf/configs/configschema" "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/walk" "github.com/pulumi/pulumi/sdk/v3/go/common/util/contract" ) @@ -44,21 +45,21 @@ func (b *blockSchemaContext) attribute(name string) schemaContext { } blk, isBlock := s.BlockTypes[name] if isBlock { - switch int(blk.Nesting) { - case 1: + switch configschema.NestingMode(blk.Nesting) { + case configschema.NestingSingle: // The only SDKv2- expressible blocks of NestingMode=Single seem to be the special "timeout" // blocks that are not part of the user-defined schema. The code cannot panic on these but opts // to silently skip processing. return nil - case 2: // group + case configschema.NestingGroup: contract.Failf("NestingMode=Group blocks not expressible with SDKv2: %v", blk.Nesting) - case 3, 4: // list, set + case configschema.NestingList, configschema.NestingSet: // list, set x, ok := b.resource.SchemaMap()[name] contract.Assertf(ok, "expected to find %q in SchemaMap()", name) subr, ok := x.Elem.(*schema.Resource) contract.Assertf(ok, "expected Elem() to be a *schema.Resource") return &blockNestingSchemaContext{subr} - case 5: // map + case configschema.NestingMap: // map contract.Failf("NestingMode={Map} blocks not expressible with SDKv2: %v", blk.Nesting) default: contract.Failf("invalid block type %v", blk.Nesting)