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 rawNames from makeObjectTerraformInputs #1851

Merged
merged 7 commits into from
Apr 13, 2024
Merged

Remove rawNames from makeObjectTerraformInputs #1851

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
261df01
Remove `rawNames` from `makeObjectTerraformInputs`
iwahbe Apr 10, 2024
991e816
Remove rawNames from `*conversionContext.makeTerraformInput`
iwahbe Apr 10, 2024
e1d6178
Remove rawNames from `*conversionContext.applyDefaults`
iwahbe Apr 10, 2024
701fac6
Breaking: Remove rawNames from `MakeTerraformOutput` & `MakeTerraform…
iwahbe Apr 10, 2024
f90c041
Remove rawNames from `visitPropertyValue`
iwahbe Apr 10, 2024
f13bd3c
Remove rawNames from `getInfoFromTerraformName`
iwahbe Apr 10, 2024
7df97a6
Add some additional test cases for "Test_makeTerraformInputsNoDefaults"
iwahbe Apr 12, 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
62 changes: 36 additions & 26 deletions pkg/tfbridge/diff.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
pulumirpc "github.com/pulumi/pulumi/sdk/v3/proto/go"

shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim"
shimutil "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/util"
)

// containsComputedValues returns true if the given property value is or contains a computed value.
Expand Down Expand Up @@ -70,7 +69,7 @@ type propertyVisitor func(attributeKey, propertyPath string, value resource.Prop
// check to see if the InstanceDiff has an entry for that path.
func visitPropertyValue(
ctx context.Context, name, path string, v resource.PropertyValue, tfs shim.Schema,
ps *SchemaInfo, rawNames bool, visitor propertyVisitor) {
ps *SchemaInfo, visitor propertyVisitor) {

if IsMaxItemsOne(tfs, ps) {
if v.IsNull() {
Expand Down Expand Up @@ -103,7 +102,7 @@ func visitPropertyValue(
// fill in default values for empty fields (note that this is a property of the field reader, not of
// the schema) as it does when computing the hash code for a set element.
ctx := &conversionContext{Ctx: ctx}
ev, err := ctx.makeTerraformInput(ep, resource.PropertyValue{}, e, etfs, eps, rawNames)
ev, err := ctx.makeTerraformInput(ep, resource.PropertyValue{}, e, etfs, eps)
if err != nil {
return
}
Expand All @@ -127,21 +126,33 @@ func visitPropertyValue(
}

en := name + "." + ti
visitPropertyValue(ctx, en, ep, e, etfs, eps, rawNames, visitor)
visitPropertyValue(ctx, en, ep, e, etfs, eps, visitor)
}
case v.IsObject():
var tfflds shim.SchemaMap
if tfs != nil {
if res, isres := tfs.Elem().(shim.Resource); isres {
tfflds = res.Schema()
if res, ok := tfs.Elem().(shim.Resource); ok {
tfflds := res.Schema()
var psflds map[string]*SchemaInfo
if ps != nil {
psflds = ps.Fields
}

for k, e := range v.ObjectValue() {
var elementPath string
if strings.ContainsAny(string(k), `."[]`) {
elementPath = fmt.Sprintf(`%s.["%s"]`, path, strings.ReplaceAll(string(k), `"`, `\"`))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Umm.. How is elementPath used? Should this be a type distinct from string that encapsulates stepping down like this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure it should be a resource.PropertyPath. I didn't change it because I didn't want to expand the scope of this PR any more. I'll stack another PR on top of this to refactor elementPath.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm yeah.

} else {
elementPath = fmt.Sprintf("%s.%s", path, k)
}

en, etf, eps := getInfoFromPulumiName(k, tfflds, psflds)
visitPropertyValue(ctx, name+"."+en, elementPath, e, etf, eps, visitor)
}
return
}
}
var psflds map[string]*SchemaInfo
if ps != nil {
psflds = ps.Fields
}

rawElementNames := rawNames || shimutil.IsOfTypeMap(tfs)
etfs, eps := elemSchemas(tfs, ps)
for k, e := range v.ObjectValue() {
var elementPath string
if strings.ContainsAny(string(k), `."[]`) {
Expand All @@ -150,8 +161,7 @@ func visitPropertyValue(
elementPath = fmt.Sprintf("%s.%s", path, k)
}

en, etf, eps := getInfoFromPulumiName(k, tfflds, psflds, rawElementNames)
visitPropertyValue(ctx, name+"."+en, elementPath, e, etf, eps, rawElementNames, visitor)
visitPropertyValue(ctx, name+"."+string(k), elementPath, e, etfs, eps, visitor)
}
}
}
Expand All @@ -166,7 +176,7 @@ func makePropertyDiff(
forceDiff *bool,
tfs shim.Schema,
ps *SchemaInfo,
finalize, rawNames bool,
finalize bool,
) {

visitor := func(name, path string, v resource.PropertyValue) bool {
Expand Down Expand Up @@ -257,7 +267,7 @@ func makePropertyDiff(
return false
}

visitPropertyValue(ctx, name, path, v, tfs, ps, rawNames, visitor)
visitPropertyValue(ctx, name, path, v, tfs, ps, visitor)
}

func newIgnoreChanges(
Expand Down Expand Up @@ -295,12 +305,12 @@ func computeIgnoreChanges(
return true
}
for k, v := range olds {
en, etf, eps := getInfoFromPulumiName(k, tfs, ps, false)
visitPropertyValue(ctx, en, string(k), v, etf, eps, shimutil.IsOfTypeMap(etf), visitor)
en, etf, eps := getInfoFromPulumiName(k, tfs, ps)
visitPropertyValue(ctx, en, string(k), v, etf, eps, visitor)
}
for k, v := range news {
en, etf, eps := getInfoFromPulumiName(k, tfs, ps, false)
visitPropertyValue(ctx, en, string(k), v, etf, eps, shimutil.IsOfTypeMap(etf), visitor)
en, etf, eps := getInfoFromPulumiName(k, tfs, ps)
visitPropertyValue(ctx, en, string(k), v, etf, eps, visitor)
}
return ignoredKeySet
}
Expand Down Expand Up @@ -351,19 +361,19 @@ func makeDetailedDiffExtra(
diff := map[string]*pulumirpc.PropertyDiff{}
collectionDiffs := map[string]*pulumirpc.PropertyDiff{}
for k, v := range olds {
en, etf, eps := getInfoFromPulumiName(k, tfs, ps, false)
en, etf, eps := getInfoFromPulumiName(k, tfs, ps)
makePropertyDiff(ctx, en, string(k), v, tfDiff, diff, collectionDiffs, forceDiff,
etf, eps, false, shimutil.IsOfTypeMap(etf))
etf, eps, false)
}
for k, v := range news {
en, etf, eps := getInfoFromPulumiName(k, tfs, ps, false)
en, etf, eps := getInfoFromPulumiName(k, tfs, ps)
makePropertyDiff(ctx, en, string(k), v, tfDiff, diff, collectionDiffs, forceDiff,
etf, eps, false, shimutil.IsOfTypeMap(etf))
etf, eps, false)
}
for k, v := range olds {
en, etf, eps := getInfoFromPulumiName(k, tfs, ps, false)
en, etf, eps := getInfoFromPulumiName(k, tfs, ps)
makePropertyDiff(ctx, en, string(k), v, tfDiff, diff, collectionDiffs, forceDiff,
etf, eps, true, shimutil.IsOfTypeMap(etf))
etf, eps, true)
}

changes := pulumirpc.DiffResponse_DIFF_NONE
Expand Down
2 changes: 1 addition & 1 deletion pkg/tfbridge/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -810,7 +810,7 @@ func (p *Provider) Check(ctx context.Context, req *pulumirpc.CheckRequest) (*pul

// After all is said and done, we need to go back and return only what got populated as a diff from the origin.
pinputs := MakeTerraformOutputs(
ctx, p.tf, inputs, res.TF.Schema(), res.Schema.Fields, assets, false, p.supportsSecrets,
ctx, p.tf, inputs, res.TF.Schema(), res.Schema.Fields, assets, p.supportsSecrets,
)

pinputsWithSecrets := MarkSchemaSecrets(ctx, res.TF.Schema(), res.Schema.Fields,
Expand Down
Loading