From 1a6c9b1a2041ad151d94e5baf9da3c7bfeee9ca0 Mon Sep 17 00:00:00 2001 From: Venelin Date: Mon, 10 Feb 2025 22:15:22 +0200 Subject: [PATCH] refactor unknown collection supported flag --- pkg/tfbridge/diff_test.go | 14 +++----- pkg/tfbridge/provider.go | 22 +++++-------- pkg/tfbridge/schema.go | 66 ++++++++++++++++--------------------- pkg/tfbridge/schema_test.go | 32 +++++------------- pkg/tfshim/sdk-v2/schema.go | 3 ++ pkg/tfshim/shim.go | 5 +++ 6 files changed, 60 insertions(+), 82 deletions(-) diff --git a/pkg/tfbridge/diff_test.go b/pkg/tfbridge/diff_test.go index 5cba5ec1b..cfa4b403d 100644 --- a/pkg/tfbridge/diff_test.go +++ b/pkg/tfbridge/diff_test.go @@ -91,7 +91,7 @@ func TestCustomizeDiff(t *testing.T) { Schema: &ResourceInfo{Fields: info}, } tfState, err := makeTerraformStateWithOpts(ctx, r, "id", stateMap, - makeTerraformStateOptions{defaultZeroSchemaVersion: true, unknownCollectionsSupported: true}) + makeTerraformStateOptions{defaultZeroSchemaVersion: true}) assert.NoError(t, err) config, _, err := MakeTerraformConfig(ctx, &Provider{tf: provider}, inputsMap, sch, info) @@ -135,7 +135,7 @@ func TestCustomizeDiff(t *testing.T) { Schema: &ResourceInfo{Fields: info}, } tfState, err := makeTerraformStateWithOpts(ctx, r, "id", stateMap, - makeTerraformStateOptions{defaultZeroSchemaVersion: true, unknownCollectionsSupported: true}) + makeTerraformStateOptions{defaultZeroSchemaVersion: true}) assert.NoError(t, err) config, _, err := MakeTerraformConfig(ctx, &Provider{tf: provider}, inputsMap, sch, info) @@ -190,7 +190,7 @@ func TestCustomizeDiff(t *testing.T) { Schema: &ResourceInfo{Fields: info}, } tfState, err := makeTerraformStateWithOpts(ctx, r, "id", stateMap, - makeTerraformStateOptions{defaultZeroSchemaVersion: true, unknownCollectionsSupported: true}) + makeTerraformStateOptions{defaultZeroSchemaVersion: true}) assert.NoError(t, err) config, _, err := MakeTerraformConfig(ctx, &Provider{tf: provider}, inputsMap, sch, info) @@ -297,9 +297,7 @@ func diffTest(t *testing.T, tfs map[string]*v2Schema.Schema, inputs, sch, r, provider, info := s.setup(tfs) tfState, err := makeTerraformStateWithOpts(ctx, r, "id", stateMap, - makeTerraformStateOptions{ - defaultZeroSchemaVersion: true, unknownCollectionsSupported: provider.SupportsUnknownCollections(), - }) + makeTerraformStateOptions{defaultZeroSchemaVersion: true}) assert.NoError(t, err) config, _, err := MakeTerraformConfig(ctx, &Provider{tf: provider}, inputsMap, sch, info) @@ -329,9 +327,7 @@ func diffTest(t *testing.T, tfs map[string]*v2Schema.Schema, inputs, t.Parallel() sch, r, provider, info := s.setup(tfs) tfState, err := makeTerraformStateWithOpts(ctx, r, "id", stateMap, - makeTerraformStateOptions{ - defaultZeroSchemaVersion: true, unknownCollectionsSupported: provider.SupportsUnknownCollections(), - }) + makeTerraformStateOptions{defaultZeroSchemaVersion: true}) assert.NoError(t, err) config, _, err := MakeTerraformConfig(ctx, &Provider{tf: provider}, inputsMap, sch, info) diff --git a/pkg/tfbridge/provider.go b/pkg/tfbridge/provider.go index 4d3a1bfa4..76b97dba9 100644 --- a/pkg/tfbridge/provider.go +++ b/pkg/tfbridge/provider.go @@ -721,7 +721,7 @@ func buildTerraformConfig(ctx context.Context, p *Provider, vars resource.Proper } inputs, _, err := makeTerraformInputsWithOptions(ctx, nil, tfVars, nil, tfVars, p.config, p.info.Config, - makeTerraformInputsOptions{UnknownCollectionsSupported: p.tf.SupportsUnknownCollections()}) + makeTerraformInputsOptions{}) if err != nil { return nil, err } @@ -1034,7 +1034,7 @@ func (p *Provider) Check(ctx context.Context, req *pulumirpc.CheckRequest) (*pul inputs, _, err := makeTerraformInputsWithOptions(ctx, &PulumiResource{URN: urn, Properties: news, Seed: req.RandomSeed, Autonaming: autonaming}, p.configValues, olds, news, schemaMap, res.Schema.Fields, - makeTerraformInputsOptions{DisableTFDefaults: true, UnknownCollectionsSupported: p.tf.SupportsUnknownCollections()}) + makeTerraformInputsOptions{DisableTFDefaults: true}) if err != nil { return nil, err } @@ -1053,7 +1053,7 @@ func (p *Provider) Check(ctx context.Context, req *pulumirpc.CheckRequest) (*pul inputs, assets, err := makeTerraformInputsWithOptions(ctx, &PulumiResource{URN: urn, Properties: news, Seed: req.RandomSeed, Autonaming: autonaming}, p.configValues, olds, news, schemaMap, res.Schema.Fields, - makeTerraformInputsOptions{UnknownCollectionsSupported: p.tf.SupportsUnknownCollections()}) + makeTerraformInputsOptions{}) if err != nil { return nil, err } @@ -1138,8 +1138,7 @@ func (p *Provider) Diff(ctx context.Context, req *pulumirpc.DiffRequest) (*pulum state, err := makeTerraformStateWithOpts(ctx, res, req.GetId(), olds, makeTerraformStateOptions{ - defaultZeroSchemaVersion: opts.defaultZeroSchemaVersion, - unknownCollectionsSupported: p.tf.SupportsUnknownCollections(), + defaultZeroSchemaVersion: opts.defaultZeroSchemaVersion, }, ) if err != nil { @@ -1441,8 +1440,7 @@ func (p *Provider) Read(ctx context.Context, req *pulumirpc.ReadRequest) (*pulum } state, err := unmarshalTerraformStateWithOpts(ctx, res, id, req.GetProperties(), fmt.Sprintf("%s.state", label), unmarshalTerraformStateOptions{ - defaultZeroSchemaVersion: opts.defaultZeroSchemaVersion, - unknownCollectionsSupported: p.tf.SupportsUnknownCollections(), + defaultZeroSchemaVersion: opts.defaultZeroSchemaVersion, }) if err != nil { return nil, errors.Wrapf(err, "unmarshaling %s's instance state", urn) @@ -1558,7 +1556,7 @@ func (p *Provider) processImportValidationErrors( tfInputs, _, err := makeTerraformInputsWithOptions(ctx, &PulumiResource{URN: urn, Properties: inputs}, p.configValues, inputsWithoutSecrets, inputsWithoutSecrets, schema, schemaInfos, - makeTerraformInputsOptions{DisableTFDefaults: true, UnknownCollectionsSupported: p.tf.SupportsUnknownCollections()}) + makeTerraformInputsOptions{DisableTFDefaults: true}) if err != nil { logger.Debug(fmt.Sprintf("Failed to makeTerraformInputsOptions."+ " This could lead to validation errors during resource import:\nError: %s", err.Error())) @@ -1640,8 +1638,7 @@ func (p *Provider) Update(ctx context.Context, req *pulumirpc.UpdateRequest) (*p state, err := makeTerraformStateWithOpts(ctx, res, req.GetId(), olds, makeTerraformStateOptions{ - defaultZeroSchemaVersion: opts.defaultZeroSchemaVersion, - unknownCollectionsSupported: p.tf.SupportsUnknownCollections(), + defaultZeroSchemaVersion: opts.defaultZeroSchemaVersion, }) if err != nil { return nil, errors.Wrapf(err, "unmarshaling %s's instance state", urn) @@ -1775,8 +1772,7 @@ func (p *Provider) Delete(ctx context.Context, req *pulumirpc.DeleteRequest) (*p // Fetch the resource attributes since many providers need more than just the ID to perform the delete. state, err := unmarshalTerraformStateWithOpts(ctx, res, req.GetId(), req.GetProperties(), label, unmarshalTerraformStateOptions{ - defaultZeroSchemaVersion: opts.defaultZeroSchemaVersion, - unknownCollectionsSupported: p.tf.SupportsUnknownCollections(), + defaultZeroSchemaVersion: opts.defaultZeroSchemaVersion, }) if err != nil { return nil, err @@ -1838,7 +1834,7 @@ func (p *Provider) Invoke(ctx context.Context, req *pulumirpc.InvokeRequest) (*p nil, args, ds.TF.Schema(), ds.Schema.Fields, - makeTerraformInputsOptions{UnknownCollectionsSupported: p.tf.SupportsUnknownCollections()}) + makeTerraformInputsOptions{}) if err != nil { return nil, errors.Wrapf(err, "couldn't prepare resource %v input state", tfname) } diff --git a/pkg/tfbridge/schema.go b/pkg/tfbridge/schema.go index 4a34cd038..94f06d3d6 100644 --- a/pkg/tfbridge/schema.go +++ b/pkg/tfbridge/schema.go @@ -287,21 +287,19 @@ func elemSchemas(sch shim.Schema, ps *SchemaInfo) (shim.Schema, *SchemaInfo) { } type conversionContext struct { - Ctx context.Context - ComputeDefaultOptions ComputeDefaultOptions - ProviderConfig resource.PropertyMap - ApplyDefaults bool - ApplyTFDefaults bool - Assets AssetTable - UnknownCollectionsSupported bool + Ctx context.Context + ComputeDefaultOptions ComputeDefaultOptions + ProviderConfig resource.PropertyMap + ApplyDefaults bool + ApplyTFDefaults bool + Assets AssetTable // UseTFSetTypes will output TF Set types when converting sets. UseTFSetTypes bool } type makeTerraformInputsOptions struct { - DisableDefaults bool - DisableTFDefaults bool - UnknownCollectionsSupported bool + DisableDefaults bool + DisableTFDefaults bool } func makeTerraformInputsWithOptions( @@ -321,13 +319,12 @@ func makeTerraformInputsWithOptions( } cctx := &conversionContext{ - Ctx: ctx, - ComputeDefaultOptions: cdOptions, - ProviderConfig: config, - ApplyDefaults: !opts.DisableDefaults, - ApplyTFDefaults: !opts.DisableTFDefaults, - Assets: AssetTable{}, - UnknownCollectionsSupported: opts.UnknownCollectionsSupported, + Ctx: ctx, + ComputeDefaultOptions: cdOptions, + ProviderConfig: config, + ApplyDefaults: !opts.DisableDefaults, + ApplyTFDefaults: !opts.DisableTFDefaults, + Assets: AssetTable{}, } inputs, err := cctx.makeTerraformInputs(olds, news, tfs, ps) @@ -351,14 +348,13 @@ func makeSingleTerraformInputForSetElement( ctx context.Context, name string, val resource.PropertyValue, tfs shim.Schema, ps *SchemaInfo, ) (interface{}, error) { cctx := &conversionContext{ - Ctx: ctx, - ComputeDefaultOptions: ComputeDefaultOptions{}, - ProviderConfig: nil, - ApplyDefaults: false, - ApplyTFDefaults: false, - Assets: AssetTable{}, - UnknownCollectionsSupported: false, - UseTFSetTypes: true, + Ctx: ctx, + ComputeDefaultOptions: ComputeDefaultOptions{}, + ProviderConfig: nil, + ApplyDefaults: false, + ApplyTFDefaults: false, + Assets: AssetTable{}, + UseTFSetTypes: true, } return cctx.makeTerraformInput(name, resource.NewNullProperty(), val, tfs, ps) @@ -559,7 +555,7 @@ func (ctx *conversionContext) makeTerraformInput( // If any variables are unknown, we need to mark them in the inputs so the config map treats it right. This // requires the use of the special UnknownVariableValue sentinel in Terraform, which is how it internally stores // interpolated variables whose inputs are currently unknown. - return makeTerraformUnknown(tfs, ctx.UnknownCollectionsSupported), nil + return makeTerraformUnknown(tfs), nil default: contract.Failf("Unexpected value marshaled: %v", v) return nil, nil @@ -986,13 +982,13 @@ func makeTerraformUnknownElement(elem interface{}) interface{} { switch e := elem.(type) { case shim.Schema: // If the element uses a normal schema, defer to makeTerraformUnknown. - return makeTerraformUnknown(e, false) + return makeTerraformUnknown(e) case shim.Resource: // If the element uses a resource schema, fill in unknown values for any required properties. res := make(map[string]interface{}) e.Schema().Range(func(k string, v shim.Schema) bool { if v.Required() { - res[k] = makeTerraformUnknown(v, false) + res[k] = makeTerraformUnknown(v) } return true }) @@ -1006,7 +1002,8 @@ func makeTerraformUnknownElement(elem interface{}) interface{} { // // It is important that we use the TF schema (if available) to decide what shape the unknown value should have: // e.g. TF does not play nicely with unknown lists, instead expecting a list of unknowns. -func makeTerraformUnknown(tfs shim.Schema, unknownCollectionsSupported bool) interface{} { +func makeTerraformUnknown(tfs shim.Schema) interface{} { + _, unknownCollectionsSupported := tfs.(shim.SchemaWithUnknownCollectionSupported) if unknownCollectionsSupported { return TerraformUnknownVariableValue } @@ -1253,7 +1250,6 @@ func MakeTerraformConfig(ctx context.Context, p *Provider, m resource.PropertyMa inputs, assets, err := makeTerraformInputsWithOptions(ctx, nil, p.configValues, nil, m, tfs, ps, makeTerraformInputsOptions{ DisableDefaults: true, DisableTFDefaults: true, - UnknownCollectionsSupported: p.tf.SupportsUnknownCollections(), }) if err != nil { return nil, nil, err @@ -1323,8 +1319,7 @@ func MakeTerraformConfigFromInputs( } type makeTerraformStateOptions struct { - defaultZeroSchemaVersion bool - unknownCollectionsSupported bool + defaultZeroSchemaVersion bool } func makeTerraformStateWithOpts( @@ -1351,9 +1346,7 @@ func makeTerraformStateWithOpts( // Mappable, but we use MapReplace because we use float64s and Terraform uses // ints, to represent numbers. inputs, _, err := makeTerraformInputsWithOptions(ctx, nil, nil, nil, m, res.TF.Schema(), res.Schema.Fields, - makeTerraformInputsOptions{ - DisableDefaults: true, DisableTFDefaults: true, UnknownCollectionsSupported: opts.unknownCollectionsSupported, - }) + makeTerraformInputsOptions{DisableDefaults: true, DisableTFDefaults: true}) if err != nil { return nil, err } @@ -1372,8 +1365,7 @@ func MakeTerraformState( } type unmarshalTerraformStateOptions struct { - defaultZeroSchemaVersion bool - unknownCollectionsSupported bool + defaultZeroSchemaVersion bool } func unmarshalTerraformStateWithOpts( diff --git a/pkg/tfbridge/schema_test.go b/pkg/tfbridge/schema_test.go index 0afa49e80..1347884b9 100644 --- a/pkg/tfbridge/schema_test.go +++ b/pkg/tfbridge/schema_test.go @@ -46,14 +46,14 @@ func makeTerraformInputsNoDefaults(olds, news resource.PropertyMap, tfs shim.SchemaMap, ps map[string]*SchemaInfo, ) (map[string]interface{}, AssetTable, error) { return makeTerraformInputsWithOptions(context.Background(), nil, nil, olds, news, tfs, ps, - makeTerraformInputsOptions{DisableDefaults: true, DisableTFDefaults: true, UnknownCollectionsSupported: true}) + makeTerraformInputsOptions{DisableDefaults: true, DisableTFDefaults: true}) } func makeTerraformInputsForConfig(olds, news resource.PropertyMap, tfs shim.SchemaMap, ps map[string]*SchemaInfo, ) (map[string]interface{}, AssetTable, error) { return makeTerraformInputsWithOptions(context.Background(), nil, nil, olds, news, tfs, ps, - makeTerraformInputsOptions{UnknownCollectionsSupported: true}) + makeTerraformInputsOptions{}) } func makeTerraformInput(v resource.PropertyValue, tfs shim.Schema, ps *SchemaInfo) (interface{}, error) { @@ -668,9 +668,7 @@ func TestMetaProperties(t *testing.T) { state, err = makeTerraformStateWithOpts( ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props, - makeTerraformStateOptions{ - defaultZeroSchemaVersion: true, unknownCollectionsSupported: prov.SupportsUnknownCollections(), - }) + makeTerraformStateOptions{defaultZeroSchemaVersion: true}) assert.NoError(t, err) assert.NotNil(t, state) @@ -686,9 +684,7 @@ func TestMetaProperties(t *testing.T) { state, err = makeTerraformStateWithOpts( ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props, - makeTerraformStateOptions{ - defaultZeroSchemaVersion: true, unknownCollectionsSupported: prov.SupportsUnknownCollections(), - }) + makeTerraformStateOptions{defaultZeroSchemaVersion: true}) assert.NoError(t, err) assert.NotNil(t, state) @@ -718,9 +714,7 @@ func TestMetaProperties(t *testing.T) { state, err = makeTerraformStateWithOpts( ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props, - makeTerraformStateOptions{ - defaultZeroSchemaVersion: true, unknownCollectionsSupported: prov.SupportsUnknownCollections(), - }) + makeTerraformStateOptions{defaultZeroSchemaVersion: true}) assert.NoError(t, err) assert.NotNil(t, state) @@ -751,9 +745,7 @@ func TestInjectingCustomTimeouts(t *testing.T) { state, err = makeTerraformStateWithOpts( ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props, - makeTerraformStateOptions{ - defaultZeroSchemaVersion: true, unknownCollectionsSupported: prov.SupportsUnknownCollections(), - }) + makeTerraformStateOptions{defaultZeroSchemaVersion: true}) assert.NoError(t, err) assert.NotNil(t, state) @@ -769,9 +761,7 @@ func TestInjectingCustomTimeouts(t *testing.T) { state, err = makeTerraformStateWithOpts( ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props, - makeTerraformStateOptions{ - defaultZeroSchemaVersion: true, unknownCollectionsSupported: prov.SupportsUnknownCollections(), - }) + makeTerraformStateOptions{defaultZeroSchemaVersion: true}) assert.NoError(t, err) assert.NotNil(t, state) @@ -804,9 +794,7 @@ func TestInjectingCustomTimeouts(t *testing.T) { state, err = makeTerraformStateWithOpts( ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props, - makeTerraformStateOptions{ - defaultZeroSchemaVersion: true, unknownCollectionsSupported: prov.SupportsUnknownCollections(), - }) + makeTerraformStateOptions{defaultZeroSchemaVersion: true}) assert.NoError(t, err) assert.NotNil(t, state) @@ -854,9 +842,7 @@ func TestResultAttributesRoundTrip(t *testing.T) { state, err = makeTerraformStateWithOpts( ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props, - makeTerraformStateOptions{ - defaultZeroSchemaVersion: true, unknownCollectionsSupported: prov.SupportsUnknownCollections(), - }) + makeTerraformStateOptions{defaultZeroSchemaVersion: true}) assert.NoError(t, err) assert.NotNil(t, state) diff --git a/pkg/tfshim/sdk-v2/schema.go b/pkg/tfshim/sdk-v2/schema.go index 46d9ec553..464a64482 100644 --- a/pkg/tfshim/sdk-v2/schema.go +++ b/pkg/tfshim/sdk-v2/schema.go @@ -10,6 +10,7 @@ import ( var ( _ = shim.Schema(v2Schema{}) _ = shim.SchemaWithNewSet(v2Schema{}) + _ = shim.SchemaWithUnknownCollectionSupported(v2Schema{}) _ = shim.SchemaMap(v2SchemaMap{}) ) @@ -185,3 +186,5 @@ func (m v2SchemaMap) Range(each func(key string, value shim.Schema) bool) { } } } + +func (s v2Schema) SupportsUnknownCollections() {} diff --git a/pkg/tfshim/shim.go b/pkg/tfshim/shim.go index ed02505c6..f3467761b 100644 --- a/pkg/tfshim/shim.go +++ b/pkg/tfshim/shim.go @@ -189,6 +189,10 @@ type SchemaWithNewSet interface { Schema NewSet(v []interface{}) interface{} } +type SchemaWithUnknownCollectionSupported interface { + Schema + SupportsUnknownCollections() +} type SchemaMap interface { Len() int @@ -297,6 +301,7 @@ type Provider interface { // Checks if a value is representing a Set, and unpacks its elements on success. IsSet(ctx context.Context, v interface{}) ([]interface{}, bool) + // Deprecated: use SchemaWithUnknownCollectionSupported instead. // SupportsUnknownCollections returns false if the provider needs special handling of unknown collections. // False for the sdkv1 provider. SupportsUnknownCollections() bool