Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove max items one defaults hacks #2049

Merged
merged 4 commits into from
May 31, 2024
Merged

remove max items one defaults hacks #2049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
735212e
remove hacks
VenelinMartinov May 31, 2024
6700c99
remove formatting changes
VenelinMartinov May 31, 2024
38e62fc
formatting
VenelinMartinov May 31, 2024
6b6de90
lint
VenelinMartinov May 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions pkg/tfbridge/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,6 @@ func ignoredTokens(info *info.Provider) map[string]bool {

// initResourceMaps creates maps from Pulumi types and tokens to Terraform resource type.
func (p *Provider) initResourceMaps() {

ignoredTokens := ignoredTokens(&p.info)

// Fetch a list of all resource types handled by this provider and make a map.
Expand Down Expand Up @@ -1130,9 +1129,8 @@ func (p *Provider) Create(ctx context.Context, req *pulumirpc.CreateRequest) (*p
}
// To get Terraform to create a new resource, the ID must be blank and existing state must be empty (since the
// resource does not exist yet), and the diff object should have no old state and all of the new state.
config, assets, err := makeTerraformConfigWithOpts(
config, assets, err := MakeTerraformConfig(
ctx, p, props, res.TF.Schema(), res.Schema.Fields,
makeTerraformConfigOpts{},
)
if err != nil {
return nil, errors.Wrapf(err, "preparing %s's new property inputs", urn)
Expand Down
62 changes: 18 additions & 44 deletions pkg/tfbridge/schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,9 +298,8 @@ type conversionContext struct {
}

type makeTerraformInputsOptions struct {
DisableDefaults bool
DisableTFDefaults bool
EnableMaxItemsOneDefaults bool
DisableDefaults bool
DisableTFDefaults bool
}

func makeTerraformInputsWithOptions(
Expand All @@ -319,13 +318,12 @@ func makeTerraformInputsWithOptions(
}

cctx := &conversionContext{
Ctx: ctx,
ComputeDefaultOptions: cdOptions,
ProviderConfig: config,
ApplyDefaults: !opts.DisableDefaults,
ApplyTFDefaults: !opts.DisableTFDefaults,
ApplyMaxItemsOneDefaults: opts.EnableMaxItemsOneDefaults,
Assets: AssetTable{},
Ctx: ctx,
ComputeDefaultOptions: cdOptions,
ProviderConfig: config,
ApplyDefaults: !opts.DisableDefaults,
ApplyTFDefaults: !opts.DisableTFDefaults,
Assets: AssetTable{},
}

inputs, err := cctx.makeTerraformInputs(olds, news, tfs, ps)
Expand Down Expand Up @@ -1232,22 +1230,6 @@ func MakeTerraformOutput(
return output
}

type makeTerraformConfigOpts struct {
EnableMaxItemsOneDefaults bool
}

func makeTerraformConfigWithOpts(ctx context.Context, p *Provider, m resource.PropertyMap,
tfs shim.SchemaMap, ps map[string]*SchemaInfo, opts makeTerraformConfigOpts) (shim.ResourceConfig, AssetTable, error) {
inputs, assets, err := makeTerraformInputsWithOptions(ctx, nil, p.configValues, nil, m, tfs, ps,
makeTerraformInputsOptions{
DisableDefaults: true, DisableTFDefaults: true, EnableMaxItemsOneDefaults: opts.EnableMaxItemsOneDefaults},
)
if err != nil {
return nil, nil, err
}
return MakeTerraformConfigFromInputs(ctx, p.tf, inputs), assets, nil
}

// MakeTerraformConfig creates a Terraform config map, used in state and diff calculations, from a Pulumi property map.
func MakeTerraformConfig(ctx context.Context, p *Provider, m resource.PropertyMap,
tfs shim.SchemaMap, ps map[string]*SchemaInfo) (shim.ResourceConfig, AssetTable, error) {
Expand Down Expand Up @@ -1307,12 +1289,11 @@ func MakeTerraformConfigFromInputs(
return p.NewResourceConfig(ctx, raw)
}

type makeTerraformStateOpts struct {
EnableMaxItemsOneDefaults bool
}

func makeTerraformStateWithOpts(
ctx context.Context, res Resource, id string, m resource.PropertyMap, opts makeTerraformStateOpts,
// MakeTerraformState converts a Pulumi property bag into its Terraform equivalent. This requires
// flattening everything and serializing individual properties as strings. This is a little awkward, but it's how
// Terraform represents resource properties (schemas are simply sugar on top).
func MakeTerraformState(
ctx context.Context, res Resource, id string, m resource.PropertyMap,
) (shim.InstanceState, error) {
// Parse out any metadata from the state.
var meta map[string]interface{}
Expand All @@ -1332,23 +1313,15 @@ func makeTerraformStateWithOpts(
// ints, to represent numbers.
inputs, _, err := makeTerraformInputsWithOptions(ctx, nil, nil, nil, m, res.TF.Schema(), res.Schema.Fields,
makeTerraformInputsOptions{
DisableDefaults: true, DisableTFDefaults: true, EnableMaxItemsOneDefaults: opts.EnableMaxItemsOneDefaults})
DisableDefaults: true, DisableTFDefaults: true,
})
if err != nil {
return nil, err
}

return res.TF.InstanceState(id, inputs, meta)
}

// MakeTerraformState converts a Pulumi property bag into its Terraform equivalent. This requires
// flattening everything and serializing individual properties as strings. This is a little awkward, but it's how
// Terraform represents resource properties (schemas are simply sugar on top).
func MakeTerraformState(
ctx context.Context, res Resource, id string, m resource.PropertyMap,
) (shim.InstanceState, error) {
return makeTerraformStateWithOpts(ctx, res, id, m, makeTerraformStateOpts{})
}

// UnmarshalTerraformState unmarshals a Terraform instance state from an RPC property map.
func UnmarshalTerraformState(
ctx context.Context, r Resource, id string, m *pbstruct.Struct, l string,
Expand All @@ -1366,7 +1339,7 @@ func UnmarshalTerraformState(
return nil, err
}

return makeTerraformStateWithOpts(ctx, r, id, props, makeTerraformStateOpts{})
return MakeTerraformState(ctx, r, id, props)
}

// IsMaxItemsOne returns true if the schema/info pair represents a TypeList or TypeSet which should project
Expand All @@ -1387,7 +1360,8 @@ func IsMaxItemsOne(tfs shim.Schema, info *SchemaInfo) bool {
// getInfoFromTerraformName does a map lookup to find the Pulumi name and schema info, if any.
func getInfoFromTerraformName(key string,
tfs shim.SchemaMap, ps map[string]*SchemaInfo, rawName bool) (resource.PropertyKey,
shim.Schema, *SchemaInfo) {
shim.Schema, *SchemaInfo,
) {
info := ps[key]

var name string
Expand Down
50 changes: 14 additions & 36 deletions pkg/tfbridge/schema_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,6 @@ func makeTerraformInputsForConfig(olds, news resource.PropertyMap,
makeTerraformInputsOptions{})
}

func makeTerraformInputsForCreate(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, EnableMaxItemsOneDefaults: true})
}

func makeTerraformInput(v resource.PropertyValue, tfs shim.Schema, ps *SchemaInfo) (interface{}, error) {
ctx := &conversionContext{}
return ctx.makeTerraformInput("v", resource.PropertyValue{}, v, tfs, ps)
Expand Down Expand Up @@ -233,7 +226,6 @@ func TestMakeTerraformInputsWithMaxItemsOne(t *testing.T) {
news resource.PropertyMap
expectedNoDefaults map[string]interface{}
expectedForConfig map[string]interface{}
expectedForCreate map[string]interface{}
}{
"empty-olds": {
olds: resource.PropertyMap{},
Expand All @@ -248,9 +240,6 @@ func TestMakeTerraformInputsWithMaxItemsOne(t *testing.T) {
expectedForConfig: map[string]interface{}{
"__defaults": []interface{}{},
},
expectedForCreate: map[string]interface{}{
"element": []interface{}{},
},
},
"non-empty-olds": {
olds: resource.PropertyMap{
Expand All @@ -272,9 +261,6 @@ func TestMakeTerraformInputsWithMaxItemsOne(t *testing.T) {
expectedForConfig: map[string]interface{}{
"__defaults": []interface{}{},
},
expectedForCreate: map[string]interface{}{
"element": []interface{}{},
},
},
"non-missing-news": {
olds: resource.PropertyMap{
Expand All @@ -299,9 +285,6 @@ func TestMakeTerraformInputsWithMaxItemsOne(t *testing.T) {
"__defaults": []interface{}{},
"element": []interface{}{"el"},
},
expectedForCreate: map[string]interface{}{
"element": []interface{}{"el"},
},
},
}

Expand All @@ -316,11 +299,6 @@ func TestMakeTerraformInputsWithMaxItemsOne(t *testing.T) {
tt.olds, tt.news, tfs, nil /* ps */)
require.NoError(t, err)
assert.Equal(t, tt.expectedForConfig, resultForConfig)

resultForCreate, _, err := makeTerraformInputsForCreate(
tt.olds, tt.news, tfs, nil /* ps */)
require.NoError(t, err)
assert.Equal(t, tt.expectedForCreate, resultForCreate)
})
}
}
Expand Down Expand Up @@ -711,8 +689,8 @@ func TestMetaProperties(t *testing.T) {
assert.NoError(t, err)
assert.NotNil(t, props)

state, err = makeTerraformStateWithOpts(
ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props, makeTerraformStateOpts{})
state, err = MakeTerraformState(
ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props)
assert.NoError(t, err)
assert.NotNil(t, state)

Expand All @@ -726,8 +704,8 @@ func TestMetaProperties(t *testing.T) {
// Delete the resource's meta-property and ensure that we re-populate its schema version.
delete(props, metaKey)

state, err = makeTerraformStateWithOpts(
ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props, makeTerraformStateOpts{})
state, err = MakeTerraformState(
ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props)
assert.NoError(t, err)
assert.NotNil(t, state)

Expand Down Expand Up @@ -765,8 +743,8 @@ func TestMetaProperties(t *testing.T) {
assert.NoError(t, err)
assert.NotNil(t, props)

state, err = makeTerraformStateWithOpts(
ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props, makeTerraformStateOpts{})
state, err = MakeTerraformState(
ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props)
assert.NoError(t, err)
assert.NotNil(t, state)

Expand All @@ -793,8 +771,8 @@ func TestInjectingCustomTimeouts(t *testing.T) {
assert.NoError(t, err)
assert.NotNil(t, props)

state, err = makeTerraformStateWithOpts(
ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props, makeTerraformStateOpts{})
state, err = MakeTerraformState(
ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props)
assert.NoError(t, err)
assert.NotNil(t, state)

Expand All @@ -808,8 +786,8 @@ func TestInjectingCustomTimeouts(t *testing.T) {
// Delete the resource's meta-property and ensure that we re-populate its schema version.
delete(props, metaKey)

state, err = makeTerraformStateWithOpts(
ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props, makeTerraformStateOpts{})
state, err = MakeTerraformState(
ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props)
assert.NoError(t, err)
assert.NotNil(t, state)

Expand Down Expand Up @@ -850,8 +828,8 @@ func TestInjectingCustomTimeouts(t *testing.T) {
assert.NoError(t, err)
assert.NotNil(t, props)

state, err = makeTerraformStateWithOpts(
ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props, makeTerraformStateOpts{})
state, err = MakeTerraformState(
ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props)
assert.NoError(t, err)
assert.NotNil(t, state)

Expand Down Expand Up @@ -906,8 +884,8 @@ func TestResultAttributesRoundTrip(t *testing.T) {
assert.NoError(t, err)
assert.NotNil(t, props)

state, err = makeTerraformStateWithOpts(
ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props, makeTerraformStateOpts{})
state, err = MakeTerraformState(
ctx, Resource{TF: res, Schema: &ResourceInfo{}}, state.ID(), props)
assert.NoError(t, err)
assert.NotNil(t, state)

Expand Down