From 6b95c60de71a628bf0d2dbed288198c9d91474f2 Mon Sep 17 00:00:00 2001 From: VenelinMartinov Date: Wed, 31 Jul 2024 18:58:37 +0300 Subject: [PATCH] Match upstream fix for Configure config to allow access to RawConfig by providers (#2263) This matches an upstream fix in the terraform-plugin-sdk: https://github.com/hashicorp/terraform-plugin-sdk/pull/1271 for https://github.com/hashicorp/terraform-plugin-sdk/issues/1270 We fail to inherit it as we do not use the top-level GRPC methods but hook in deeper. The problem is that the `GetRawConfig` methods expect the `CtyValue` on the `ResourceData` object to be filled https://github.com/hashicorp/terraform-plugin-sdk/pull/1271/files#diff-7f0240b3a899f37961cbce63b21bcc4395864395501e3231cd0e0f15a11a9c47R599 This happens in https://github.com/hashicorp/terraform-plugin-sdk/pull/1271/files#diff-cb044934d551a7bd462735476d13f6a21549a098d631f9ba5a2be806657d333fR595 but that is in the top-level GRPC method, which we do not use. This PR aims to mimic the same for the bridge - when making the config for the provider `Configure` method, we make sure to fill in the `CtyValue`. As this is only done for `Configure` in the tf-plugin-sdk, we match that. To do that we add a new method to the `shim.Provider` interface `NewProviderConfig`, in contrast with the old NewResourceConfig. This is then used in the new schema.go function `MakeTerraformConfigFromInputsWithOpts`, which has `ProviderConfig` option, which is to be used by `Configure` and `CheckConfig`. fixes https://github.com/pulumi/pulumi-terraform-bridge/issues/2262 --- pf/internal/schemashim/provider.go | 3 + pf/proto/unsupported.go | 4 + pkg/tests/schema_pulumi_test.go | 261 +++++++++++++++++++++++++++++ pkg/tfbridge/provider.go | 2 +- pkg/tfbridge/schema.go | 17 +- pkg/tfshim/schema/provider.go | 6 + pkg/tfshim/sdk-v1/provider.go | 9 + pkg/tfshim/sdk-v2/provider.go | 19 +++ pkg/tfshim/shim.go | 1 + pkg/tfshim/tfplugin5/provider.go | 4 + pkg/tfshim/util/filter.go | 6 + pkg/tfshim/util/util.go | 6 + 12 files changed, 335 insertions(+), 3 deletions(-) diff --git a/pf/internal/schemashim/provider.go b/pf/internal/schemashim/provider.go index 72366c1fe..33ac5e374 100644 --- a/pf/internal/schemashim/provider.go +++ b/pf/internal/schemashim/provider.go @@ -167,6 +167,9 @@ func (p *SchemaOnlyProvider) NewResourceConfig(context.Context, map[string]inter panic("schemaOnlyProvider does not implement runtime operation ResourceConfig") } +func (p *SchemaOnlyProvider) NewProviderConfig(context.Context, map[string]interface{}) shim.ResourceConfig { + panic("schemaOnlyProvider does not implement runtime operation ProviderConfig") +} func (p *SchemaOnlyProvider) IsSet(context.Context, interface{}) ([]interface{}, bool) { panic("schemaOnlyProvider does not implement runtime operation IsSet") } diff --git a/pf/proto/unsupported.go b/pf/proto/unsupported.go index 2e3692b7d..ce2d2c6b3 100644 --- a/pf/proto/unsupported.go +++ b/pf/proto/unsupported.go @@ -78,6 +78,10 @@ func (Provider) NewResourceConfig(ctx context.Context, object map[string]interfa panic("Unimplemented") } +func (Provider) NewProviderConfig(ctx context.Context, object map[string]interface{}) shim.ResourceConfig { + panic("Unimplemented") +} + // Checks if a value is representing a Set, and unpacks its elements on success. func (Provider) IsSet(ctx context.Context, v interface{}) ([]interface{}, bool) { panic("Unimplemented") diff --git a/pkg/tests/schema_pulumi_test.go b/pkg/tests/schema_pulumi_test.go index 5572652b6..7c36f2af9 100644 --- a/pkg/tests/schema_pulumi_test.go +++ b/pkg/tests/schema_pulumi_test.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hexops/autogold/v2" "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tests/internal/pulcheck" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tests/internal/tfcheck" "github.com/pulumi/pulumi/sdk/v3/go/auto/optpreview" "github.com/pulumi/pulumi/sdk/v3/go/auto/optrefresh" "github.com/stretchr/testify/require" @@ -1508,3 +1509,263 @@ runtime: yaml }) } } + +func TestConfigureGetRawConfigDoesNotPanic(t *testing.T) { + // Regression test for [pulumi/pulumi-terraform-bridge#2262] + getOkExists := func(d *schema.ResourceData, key string) (interface{}, bool) { + v := d.GetRawConfig().GetAttr(key) + if v.IsNull() { + return nil, false + } + return d.Get(key), true + } + resMap := map[string]*schema.Resource{ + "prov_test": { + Schema: map[string]*schema.Schema{ + "test": { + Type: schema.TypeString, + Optional: true, + }, + }, + }, + } + + runConfigureTest := func(t *testing.T, configPresent bool) { + tfp := &schema.Provider{ + ResourcesMap: resMap, + Schema: map[string]*schema.Schema{ + "config": { + Type: schema.TypeString, + Optional: true, + }, + }, + ConfigureContextFunc: func(ctx context.Context, rd *schema.ResourceData) (interface{}, diag.Diagnostics) { + _, ok := getOkExists(rd, "config") + require.Equal(t, configPresent, ok, "Unexpected config value") + return nil, nil + }, + } + bridgedProvider := pulcheck.BridgedProvider(t, "prov", tfp) + configVal := "val" + if !configPresent { + configVal = "null" + } + program := fmt.Sprintf(` +name: test +runtime: yaml +resources: + prov: + type: pulumi:providers:prov + defaultProvider: true + properties: + config: %s + mainRes: + type: prov:index:Test + properties: + test: "hello" +outputs: + testOut: ${mainRes.test} +`, configVal) + pt := pulcheck.PulCheck(t, bridgedProvider, program) + pt.Up() + } + + t.Run("config exists", func(t *testing.T) { + runConfigureTest(t, true) + }) + + t.Run("config does not exist", func(t *testing.T) { + runConfigureTest(t, false) + }) +} + +// TODO[pulumi/pulumi-terraform-bridge#2274]: Move to actual cross-test suite once the plumbing is done +func TestConfigureCrossTest(t *testing.T) { + resMap := map[string]*schema.Resource{ + "prov_test": { + Schema: map[string]*schema.Schema{ + "test": { + Type: schema.TypeString, + Optional: true, + }, + }, + }, + } + + runTest := func(t *testing.T, sch map[string]*schema.Schema, pulumiProgram, tfProgram string) { + var tfRd *schema.ResourceData + var puRd *schema.ResourceData + _ = puRd // ignore unused warning + tfp := &schema.Provider{ + ResourcesMap: resMap, + Schema: sch, + ConfigureContextFunc: func(ctx context.Context, rd *schema.ResourceData) (interface{}, diag.Diagnostics) { + if tfRd == nil { + tfRd = rd + } else { + puRd = rd + } + + return nil, nil + }, + } + + tfdriver := tfcheck.NewTfDriver(t, t.TempDir(), "prov", tfp) + tfdriver.Write(t, tfProgram) + tfdriver.Plan(t) + require.NotNil(t, tfRd) + require.Nil(t, puRd) + + bridgedProvider := pulcheck.BridgedProvider(t, "prov", tfp) + + pt := pulcheck.PulCheck(t, bridgedProvider, pulumiProgram) + pt.Preview() + require.NotNil(t, puRd) + require.Equal(t, tfRd.GetRawConfig(), puRd.GetRawConfig()) + } + + t.Run("string attr", func(t *testing.T) { + runTest(t, + map[string]*schema.Schema{ + "config": { + Type: schema.TypeString, + Optional: true, + }, + }, + ` +name: test +runtime: yaml +resources: + prov: + type: pulumi:providers:prov + defaultProvider: true + properties: + config: val + mainRes: + type: prov:index:Test + properties: + test: "val" +`, + ` +provider "prov" { + config = "val" +} + +resource "prov_test" "test" { + test = "val" +}`) + }) + + t.Run("object block", func(t *testing.T) { + runTest(t, + map[string]*schema.Schema{ + "config": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "prop": { + Type: schema.TypeString, + Optional: true, + }, + }, + }, + MaxItems: 1, + }, + }, + ` +name: test +runtime: yaml +resources: + prov: + type: pulumi:providers:prov + defaultProvider: true + properties: + config: {"prop": "val"} + mainRes: + type: prov:index:Test + properties: + test: "val" +`, + ` +provider "prov" { + config { + prop = "val" + } +} + +resource "prov_test" "test" { + test = "val" +}`) + }) + + t.Run("list config", func(t *testing.T) { + runTest(t, + map[string]*schema.Schema{ + "config": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + }, + ` +name: test +runtime: yaml +resources: + prov: + type: pulumi:providers:prov + defaultProvider: true + properties: + configs: ["val"] + mainRes: + type: prov:index:Test + properties: + test: "val" +`, + ` +provider "prov" { + config = ["val"] +} + +resource "prov_test" "test" { + test = "val" +}`) + }) + + t.Run("set config", func(t *testing.T) { + runTest(t, + map[string]*schema.Schema{ + "config": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + }, + ` +name: test +runtime: yaml +resources: + prov: + type: pulumi:providers:prov + defaultProvider: true + properties: + configs: ["val"] + mainRes: + type: prov:index:Test + properties: + test: "val" +`, + ` +provider "prov" { + config = ["val"] +} + +resource "prov_test" "test" { + test = "val" +}`) + }) +} diff --git a/pkg/tfbridge/provider.go b/pkg/tfbridge/provider.go index 0279ff55d..a1c00739e 100644 --- a/pkg/tfbridge/provider.go +++ b/pkg/tfbridge/provider.go @@ -671,7 +671,7 @@ func buildTerraformConfig(ctx context.Context, p *Provider, vars resource.Proper return nil, err } - return MakeTerraformConfigFromInputs(ctx, p.tf, inputs), nil + return MakeTerraformConfigFromInputsWithOpts(ctx, p.tf, inputs, MakeTerraformInputsOptions{ProviderConfig: true}), nil } func validateProviderConfig( diff --git a/pkg/tfbridge/schema.go b/pkg/tfbridge/schema.go index f3849ddad..4018c0b12 100644 --- a/pkg/tfbridge/schema.go +++ b/pkg/tfbridge/schema.go @@ -1272,12 +1272,25 @@ func makeConfig(v interface{}) interface{} { } } +type MakeTerraformInputsOptions struct { + ProviderConfig bool +} + +func MakeTerraformConfigFromInputsWithOpts( + ctx context.Context, p shim.Provider, inputs map[string]interface{}, opts MakeTerraformInputsOptions, +) shim.ResourceConfig { + raw := makeConfig(inputs).(map[string]interface{}) + if opts.ProviderConfig { + return p.NewProviderConfig(ctx, raw) + } + return p.NewResourceConfig(ctx, raw) +} + // MakeTerraformConfigFromInputs creates a new Terraform configuration object from a set of Terraform inputs. func MakeTerraformConfigFromInputs( ctx context.Context, p shim.Provider, inputs map[string]interface{}, ) shim.ResourceConfig { - raw := makeConfig(inputs).(map[string]interface{}) - return p.NewResourceConfig(ctx, raw) + return MakeTerraformConfigFromInputsWithOpts(ctx, p, inputs, MakeTerraformInputsOptions{}) } type makeTerraformStateOptions struct { diff --git a/pkg/tfshim/schema/provider.go b/pkg/tfshim/schema/provider.go index db53d0921..f41d1f0d9 100644 --- a/pkg/tfshim/schema/provider.go +++ b/pkg/tfshim/schema/provider.go @@ -120,6 +120,12 @@ func (ProviderShim) NewResourceConfig( panic("this provider is schema-only and does not support runtime operations") } +func (ProviderShim) NewProviderConfig( + ctx context.Context, object map[string]interface{}, +) shim.ResourceConfig { + panic("this provider is schema-only and does not support runtime operations") +} + func (ProviderShim) IsSet(ctx context.Context, v interface{}) ([]interface{}, bool) { panic("this provider is schema-only and does not support runtime operations") } diff --git a/pkg/tfshim/sdk-v1/provider.go b/pkg/tfshim/sdk-v1/provider.go index 5b64e4ec7..6192c8f9b 100644 --- a/pkg/tfshim/sdk-v1/provider.go +++ b/pkg/tfshim/sdk-v1/provider.go @@ -171,6 +171,15 @@ func (p v1Provider) NewResourceConfig( }} } +func (p v1Provider) NewProviderConfig( + ctx context.Context, object map[string]interface{}, +) shim.ResourceConfig { + return v1ResourceConfig{&terraform.ResourceConfig{ + Raw: object, + Config: object, + }} +} + func (p v1Provider) IsSet(_ context.Context, v interface{}) ([]interface{}, bool) { if set, ok := v.(*schema.Set); ok { return set.List(), true diff --git a/pkg/tfshim/sdk-v2/provider.go b/pkg/tfshim/sdk-v2/provider.go index 7ab87eb37..fb1213d04 100644 --- a/pkg/tfshim/sdk-v2/provider.go +++ b/pkg/tfshim/sdk-v2/provider.go @@ -5,6 +5,7 @@ import ( "fmt" "os" + "github.com/golang/glog" "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" @@ -217,6 +218,24 @@ func (p v2Provider) NewResourceConfig( }} } +func (p v2Provider) NewProviderConfig( + _ context.Context, object map[string]interface{}, +) shim.ResourceConfig { + tfConfig := &terraform.ResourceConfig{ + Raw: object, + Config: object, + } + typ := schema.InternalMap(p.tf.Schema).CoreConfigSchema().ImpliedType() + ctyVal, err := recoverCtyValueOfObjectType(typ, object) + if err != nil { + glog.V(9).Infof("Failed to recover cty value of object type: %v, falling back to old behaviour", err) + return v2ResourceConfig{tfConfig} + } + + tfConfig.CtyValue = ctyVal + return v2ResourceConfig{tfConfig} +} + func (p v2Provider) IsSet(_ context.Context, v interface{}) ([]interface{}, bool) { if set, ok := v.(*schema.Set); ok { return set.List(), true diff --git a/pkg/tfshim/shim.go b/pkg/tfshim/shim.go index ad4d15d77..ff600bb26 100644 --- a/pkg/tfshim/shim.go +++ b/pkg/tfshim/shim.go @@ -249,6 +249,7 @@ type Provider interface { NewDestroyDiff(ctx context.Context, t string, opts TimeoutOptions) InstanceDiff NewResourceConfig(ctx context.Context, object map[string]interface{}) ResourceConfig + NewProviderConfig(ctx context.Context, object map[string]interface{}) ResourceConfig // Checks if a value is representing a Set, and unpacks its elements on success. IsSet(ctx context.Context, v interface{}) ([]interface{}, bool) diff --git a/pkg/tfshim/tfplugin5/provider.go b/pkg/tfshim/tfplugin5/provider.go index 0255dd6cb..c174a6e5d 100644 --- a/pkg/tfshim/tfplugin5/provider.go +++ b/pkg/tfshim/tfplugin5/provider.go @@ -572,6 +572,10 @@ func (p *provider) NewResourceConfig(ctx context.Context, object map[string]inte return resourceConfig(object) } +func (p *provider) NewProviderConfig(ctx context.Context, object map[string]interface{}) shim.ResourceConfig { + return resourceConfig(object) +} + func (p *provider) IsSet(ctx context.Context, v interface{}) ([]interface{}, bool) { val, ok := v.(cty.Value) if !ok { diff --git a/pkg/tfshim/util/filter.go b/pkg/tfshim/util/filter.go index c34fc91e6..2b679a92c 100644 --- a/pkg/tfshim/util/filter.go +++ b/pkg/tfshim/util/filter.go @@ -137,6 +137,12 @@ func (p *FilteringProvider) NewResourceConfig( return p.Provider.NewResourceConfig(ctx, object) } +func (p *FilteringProvider) NewProviderConfig( + ctx context.Context, object map[string]interface{}, +) shim.ResourceConfig { + return p.Provider.NewProviderConfig(ctx, object) +} + func (p *FilteringProvider) IsSet(ctx context.Context, v interface{}) ([]interface{}, bool) { return p.Provider.IsSet(ctx, v) } diff --git a/pkg/tfshim/util/util.go b/pkg/tfshim/util/util.go index c91fafb2b..2894d6a74 100644 --- a/pkg/tfshim/util/util.go +++ b/pkg/tfshim/util/util.go @@ -100,6 +100,12 @@ func (UnimplementedProvider) NewResourceConfig( panic("unimplemented") } +func (UnimplementedProvider) NewProviderConfig( + ctx context.Context, object map[string]interface{}, +) shim.ResourceConfig { + panic("unimplemented") +} + func (UnimplementedProvider) IsSet(ctx context.Context, v interface{}) ([]interface{}, bool) { panic("unimplemented") }