Skip to content

Commit

Permalink
Omit omitted types from the schema (#1548)
Browse files Browse the repository at this point in the history
`tfbridge.SchemaInfo` has an `Omit bool` field, which will omit a
property from the schema. Unfortunately, omitted fields don't omit their
associated types from the schema, leading to unnecessary bloat. This
leads to code such as this (from aws):

```go
// We removed the `definition` property from quicksights.Template, see
// #1118
// But the types are still present in the schema, which pollutes the Go SDK
// specifically. This function removes those types from the schema.
func removeUnusedQuicksightTypes(pulumiPackageSpec *schema.PackageSpec) {
        var elidedTypes []string
        for tok := range pulumiPackageSpec.Types {
                if strings.Contains(tok, ":quicksight/AnalysisDefinition") ||
                        strings.Contains(tok, ":quicksight/DashboardDefinition") ||
                        strings.Contains(tok, ":quicksight/TemplateDefinition") {
                        elidedTypes = append(elidedTypes, tok)
                }
        }
        for _, tok := range elidedTypes {
                delete(pulumiPackageSpec.Types, tok)
        }
}
```

`Omit: true` is only used in 2 providers: `pulumi-aws` and
`pulumi-alicloud`
([source](https://github.com/search?q=org%3Apulumi%20Omit%3A%20true&type=code)).

Datadog has recenlty added some hefty (+900k lines) types, and I am
considering using `Omit: true` to trim unwanted types. This is a
necessary pre-cursor for that usage. It also has the advantage of making
more intuitive sense to future users of the bridge.
  • Loading branch information
iwahbe authored Nov 30, 2023
1 parent 00d9e39 commit 6385710
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 38 deletions.
59 changes: 38 additions & 21 deletions pkg/tfgen/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -565,24 +565,23 @@ func (v *variable) forceNew() bool {
}

// optional checks whether the given property is optional, either due to Terraform or an overlay.
func (v *variable) optional() bool {
if v.opt {
return true
}
func (v *variable) optional() bool { return v.opt || isOptional(v.info, v.schema, v.out, v.config) }

func isOptional(info *tfbridge.SchemaInfo, schema shim.Schema, out bool, config bool) bool {
//if we have an explicit marked as optional then let's return that
if v.info != nil && v.info.MarkAsOptional != nil {
return *v.info.MarkAsOptional
if info != nil && info.MarkAsOptional != nil {
return *info.MarkAsOptional
}

// If we're checking a property used in an output position, it isn't optional if it's computed.
//
// Note that config values with custom defaults are _not_ considered optional unless they are marked as such.
customDefault := !v.config && v.info != nil && v.info.HasDefault()
if v.out {
return v.schema != nil && v.schema.Optional() && !v.schema.Computed() && !customDefault
customDefault := !config && info != nil && info.HasDefault()
if out {
return schema != nil && schema.Optional() && !schema.Computed() && !customDefault
}
return (v.schema != nil && v.schema.Optional() || v.schema.Computed()) || customDefault
return (schema != nil && schema.Optional() || schema.Computed()) || customDefault

}

// resourceType is a generated resource type that represents a Pulumi CustomResource definition.
Expand Down Expand Up @@ -1279,8 +1278,10 @@ func (g *Generator) gatherResource(rawname string,
// Make a state variable. This is always optional and simply lets callers perform lookups.
stateVar := g.propertyVariable(resourcePath.State(), key, schema.Schema(), info.Fields,
doc, rawdoc, false /*out*/, entityDocs)
stateVar.opt = true
stateVars = append(stateVars, stateVar)
if stateVar != nil {
stateVar.opt = true
stateVars = append(stateVars, stateVar)
}
}

className := res.name
Expand Down Expand Up @@ -1445,17 +1446,20 @@ func (g *Generator) gatherDataSource(rawname string,

argvar := g.propertyVariable(dataSourcePath.Args(),
arg, ds.Schema(), info.Fields, doc, "", false /*out*/, entityDocs)
fun.args = append(fun.args, argvar)
if !argvar.optional() {
fun.reqargs[argvar.name] = true
if argvar != nil {
fun.args = append(fun.args, argvar)
if !argvar.optional() {
fun.reqargs[argvar.name] = true
}
}
}

// Also remember properties for the resulting return data structure.
// Emit documentation for the property if available
fun.rets = append(fun.rets,
g.propertyVariable(dataSourcePath.Results(),
arg, ds.Schema(), info.Fields, entityDocs.Attributes[arg], "", true /*out*/, entityDocs))
if p := g.propertyVariable(dataSourcePath.Results(), arg, ds.Schema(), info.Fields,
entityDocs.Attributes[arg], "", true /*out*/, entityDocs); p != nil {
fun.rets = append(fun.rets, p)
}
}

// If the data source's schema doesn't expose an id property, make one up since we'd like to expose it for data
Expand All @@ -1466,9 +1470,10 @@ func (g *Generator) gatherDataSource(rawname string,
idSchema := schema.SchemaMap(map[string]shim.Schema{
"id": (&schema.Schema{Type: shim.TypeString, Computed: true}).Shim(),
})
fun.rets = append(fun.rets,
g.propertyVariable(dataSourcePath.Results(),
"id", idSchema, cust, "", rawdoc, true /*out*/, entityDocs))
if p := g.propertyVariable(dataSourcePath.Results(), "id", idSchema, cust, "",
rawdoc, true /*out*/, entityDocs); p != nil {
fun.rets = append(fun.rets, p)
}
}

// Produce the args/return types, if needed.
Expand Down Expand Up @@ -1646,6 +1651,18 @@ func (g *Generator) propertyVariable(parentPath paths.TypePath, key string,
varInfo = info[key]
}

// If a variable is marked as omitted, omit it.
//
// Because the recursive traversal into the fields used by this type are
// from g.makePropertyType below, this has the effect of omitting all
// types generated by the omitted type, which is what we want.
if varInfo != nil && varInfo.Omit {
contract.Assertf(isOptional(varInfo, schema, false, false /* config */),
"required property %q (@ %s) may not be omitted from binding generation",
propName, typePath)
return nil
}

return &variable{
name: name,
out: out,
Expand Down
18 changes: 2 additions & 16 deletions pkg/tfgen/generate_schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -659,14 +659,6 @@ func (g *schemaGenerator) genResourceType(mod tokens.Module, res *resourceType)
spec.Properties = map[string]pschema.PropertySpec{}

for _, prop := range res.outprops {
// The property will be dropped from the schema
if prop.info != nil && prop.info.Omit {
if prop.schema.Required() {
contract.Failf("required property %q may not be omitted from binding generation", prop.name)
} else {
continue
}
}
// let's check that we are not trying to add a duplicate computed id property
if prop.name == "id" {
continue
Expand Down Expand Up @@ -793,14 +785,8 @@ func (g *schemaGenerator) genObjectType(typInfo *schemaNestedType, isTopLevel bo

spec.Properties = map[string]pschema.PropertySpec{}
for _, prop := range typ.properties {
if prop.info != nil && prop.info.Omit {
if prop.schema.Required() {
contract.Failf("required object property %q may not be omitted from binding generation", prop.name)
} else {
continue
}
}
// let's not build any additional ID properties - we don't want to exclude any required id properties
// let's not build any additional ID properties - we don't want to exclude
// any required id properties
if isTopLevel && prop.name == "id" {
continue
}
Expand Down
99 changes: 98 additions & 1 deletion pkg/tfgen/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

pschema "github.com/pulumi/pulumi/pkg/v3/codegen/schema"
"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"

bridgetesting "github.com/pulumi/pulumi-terraform-bridge/v3/internal/testing"
"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tf2pulumi/il"
Expand All @@ -36,7 +38,6 @@ import (
shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim"
shimschema "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/schema"
shimv1 "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v1"
"github.com/pulumi/pulumi/sdk/v3/go/common/tokens"
)

func Test_DeprecationFromTFSchema(t *testing.T) {
Expand Down Expand Up @@ -284,6 +285,102 @@ func Test_ProviderWithObjectTypesInConfigCanGenerateRenames(t *testing.T) {
assert.Equal(t, "foo_bar", r.Renames.RenamedProperties["test:index/ProviderProp:ProviderProp"]["fooBar"])
}

func Test_ProviderWithOmittedTypes(t *testing.T) {

gen := func(t *testing.T, f func(*tfbridge.ResourceInfo)) pschema.PackageSpec {
strType := (&shimschema.Schema{Type: shim.TypeString}).Shim()
nestedObj := (&shimschema.Schema{
Type: shim.TypeMap,
Optional: true,
Elem: (&shimschema.Resource{
Schema: shimschema.SchemaMap{
"fizz_buzz": strType,
},
}).Shim(),
}).Shim()
objType := (&shimschema.Schema{
Type: shim.TypeMap,
Optional: true,
Elem: (&shimschema.Resource{
Schema: shimschema.SchemaMap{
"foo_bar": strType,
"nested": nestedObj,
},
}).Shim(),
}).Shim()

p := (&shimschema.Provider{
ResourcesMap: shimschema.ResourceMap{
"test_res": (&shimschema.Resource{
Schema: shimschema.SchemaMap{
"obj": objType,
},
}).Shim(),
},
}).Shim()

nilSink := diag.DefaultSink(io.Discard, io.Discard, diag.FormatOptions{
Color: colors.Never,
})

res := &tfbridge.ResourceInfo{
Tok: "test:index:Bar",
}
if f != nil {
f(res)
}

r, err := GenerateSchemaWithOptions(GenerateSchemaOptions{
DiagnosticsSink: nilSink,
ProviderInfo: tfbridge.ProviderInfo{
Name: "test",
P: p,
Resources: map[string]*tfbridge.ResourceInfo{
"test_res": res,
},
},
})
require.NoError(t, err)
return r.PackageSpec
}

t.Run("no-omit", func(t *testing.T) {
spec := gen(t, nil)
assert.Len(t, spec.Resources, 1)
assert.Len(t, spec.Resources["test:index:Bar"].InputProperties, 1)
assert.Len(t, spec.Types, 2)
})

t.Run("omit-top-level-prop", func(t *testing.T) {
spec := gen(t, func(info *tfbridge.ResourceInfo) {
info.Fields = map[string]*tfbridge.SchemaInfo{
"obj": {Omit: true},
}
})
assert.Len(t, spec.Resources, 1)
assert.Len(t, spec.Resources["test:index:Bar"].InputProperties, 0)
assert.Len(t, spec.Types, 0)
})

t.Run("omit-nested-prop", func(t *testing.T) {
spec := gen(t, func(info *tfbridge.ResourceInfo) {
info.Fields = map[string]*tfbridge.SchemaInfo{
"obj": {
Elem: &tfbridge.SchemaInfo{
Fields: map[string]*tfbridge.SchemaInfo{
"nested": {Omit: true},
},
},
},
}
})
assert.Len(t, spec.Resources, 1)
assert.Len(t, spec.Resources["test:index:Bar"].InputProperties, 1)
assert.Len(t, spec.Types, 1)
})

}

func TestModulePlacementForType(t *testing.T) {
t.Parallel()

Expand Down

0 comments on commit 6385710

Please sign in to comment.