From 3dd9c26ba5b6b4fd98cf23ab80a80d930b021877 Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Tue, 10 Sep 2024 14:46:23 -0400 Subject: [PATCH 01/13] Implement type sharing support --- pkg/tfbridge/info.go | 5 + pkg/tfbridge/info/info.go | 14 +- pkg/tfgen/generate.go | 23 +++ pkg/tfgen/generate_schema.go | 4 + pkg/tfgen/generate_schema_test.go | 158 +++++++++++++++++++++ pkg/tfgen/testdata/TestTypeSharing.golden | 164 ++++++++++++++++++++++ 6 files changed, 367 insertions(+), 1 deletion(-) create mode 100644 pkg/tfgen/testdata/TestTypeSharing.golden diff --git a/pkg/tfbridge/info.go b/pkg/tfbridge/info.go index fbb966254..2d0042585 100644 --- a/pkg/tfbridge/info.go +++ b/pkg/tfbridge/info.go @@ -323,6 +323,11 @@ func BoolRef(b bool) *bool { return &b } +// String returns a reference to the string argument. +func StringRef(s string) *string { + return &s +} + // StringValue gets a string value from a property map if present, else "" func StringValue(vars resource.PropertyMap, prop resource.PropertyKey) string { val, ok := vars[prop] diff --git a/pkg/tfbridge/info/info.go b/pkg/tfbridge/info/info.go index 94ac1f071..e5bc76a7b 100644 --- a/pkg/tfbridge/info/info.go +++ b/pkg/tfbridge/info/info.go @@ -441,9 +441,15 @@ type Schema struct { // a name to override the default when targeting C#; "" uses the default. CSharpName string - // a type to override the default; "" uses the default. + // An optional Pulumi type token to use for the Pulumi type projection of the current property. When unset, the + // default behavior is to generate fresh named Pulumi types as needed to represent the schema. To force the use + // of a known type and avoid generating unnecessary types, use both [Type] and [OmitType]. Type tokens.Type + // Used together with [Type] to omit generating any Pulumi types whatsoever for the current property, and + // instead use the object type identified by the token setup in [Type]. + OmitType bool + // alternative types that can be used instead of the override. AltTypes []tokens.Type @@ -502,6 +508,12 @@ type Schema struct { // whether or not to treat this property as secret Secret *bool + + // If specified, resets the type name prefix for any types that Pulumi needs to generate to represent the schema + // of the current property. Normally the names for helper object types are built from concatenating fragments + // representing the path to the type in the schema. The default strategy can lead to unacceptably long type + // names, reducing the SDK usability. Using TypePrefixOverride allows the maintainer to get shorter type names. + TypePrefixOverride *string } // Config represents a synthetic configuration variable that is Pulumi-only, and not passed to Terraform. diff --git a/pkg/tfgen/generate.go b/pkg/tfgen/generate.go index a81e1e410..ad14e64ad 100644 --- a/pkg/tfgen/generate.go +++ b/pkg/tfgen/generate.go @@ -342,6 +342,8 @@ type propertyType struct { nestedType tokens.Type altTypes []tokens.Type asset *tfbridge.AssetTranslation + + typePrefixOverride *string } func (g *Generator) Sink() diag.Sink { @@ -353,6 +355,9 @@ func (g *Generator) makePropertyType(typePath paths.TypePath, entityDocs entityDocs) (*propertyType, error) { t := &propertyType{} + if info != nil { + t.typePrefixOverride = info.TypePrefixOverride + } var elemInfo *tfbridge.SchemaInfo if info != nil { @@ -456,10 +461,20 @@ func getDocsFromSchemaMap(key string, schemaMap shim.SchemaMap) string { func (g *Generator) makeObjectPropertyType(typePath paths.TypePath, res shim.Resource, info *tfbridge.SchemaInfo, out bool, entityDocs entityDocs) (*propertyType, error) { + + // If the user supplied an explicit Type token override, omit generating types and short-circuit. + if info != nil && info.OmitType && info.Type != "" { + return &propertyType{typ: info.Type}, nil + } + t := &propertyType{ kind: kindObject, } + if info != nil { + t.typePrefixOverride = info.TypePrefixOverride + } + if info != nil { t.typ = info.Type t.nestedType = info.NestedType @@ -549,6 +564,14 @@ func (t *propertyType) equals(other *propertyType) bool { if len(t.properties) != len(other.properties) { return false } + switch { + case t.typePrefixOverride != nil && other.typePrefixOverride == nil: + return false + case t.typePrefixOverride == nil && other.typePrefixOverride != nil: + return false + case t.typePrefixOverride != nil && other.typePrefixOverride != nil && *t.typePrefixOverride != *other.typePrefixOverride: + return false + } for i, p := range t.properties { o := other.properties[i] if p.name != o.name { diff --git a/pkg/tfgen/generate_schema.go b/pkg/tfgen/generate_schema.go index 4b05095c9..5f944b77f 100644 --- a/pkg/tfgen/generate_schema.go +++ b/pkg/tfgen/generate_schema.go @@ -119,6 +119,10 @@ type declarer interface { func (nt *schemaNestedTypes) declareType(typePath paths.TypePath, declarer declarer, namePrefix, name string, typ *propertyType, isInput bool) string { + if typ.typePrefixOverride != nil { + namePrefix = *typ.typePrefixOverride + } + // Generate a name for this nested type. typeName := namePrefix + cases.Title(language.Und, cases.NoLower).String(name) diff --git a/pkg/tfgen/generate_schema_test.go b/pkg/tfgen/generate_schema_test.go index 27616255e..85257c84d 100644 --- a/pkg/tfgen/generate_schema_test.go +++ b/pkg/tfgen/generate_schema_test.go @@ -19,6 +19,8 @@ import ( "encoding/json" "fmt" "io" + "os" + "sort" "testing" "text/template" @@ -33,9 +35,12 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" bridgetesting "github.com/pulumi/pulumi-terraform-bridge/v3/internal/testing" "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/info" "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfgen/internal/testprovider" + sdkv2 "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2" "github.com/pulumi/pulumi-terraform-bridge/v3/unstable/metadata" "github.com/pulumi/pulumi-terraform-bridge/x/muxer" ) @@ -116,6 +121,159 @@ func TestCSharpMiniRandom(t *testing.T) { bridgetesting.AssertEqualsJSONFile(t, "test_data/minirandom-schema-csharp.json", schema) } +// Test the ability to force type sharing. Some of the upstream providers generate very large concrete schemata by in +// Go, with TF not being materially affected. The example is inspired by QuickSight types in AWS. In Pulumi the default +// projection is going to generate named types for every instance of the shared schema. This may lead to SDK bloat. Test +// the ability of the provider author to curb the bloat and force an explit sharing. +func TestTypeSharing(t *testing.T) { + tmpdir := t.TempDir() + barCharVisualSchema := func() *schema.Schema { + return &schema.Schema{ + Type: schema.TypeList, + Optional: true, + MinItems: 1, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "nest": { + Type: schema.TypeList, + MaxItems: 1, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "nested_prop": { + Type: schema.TypeBool, + Optional: true, + }, + }, + }, + }, + }, + }, + } + } + visualsSchema := func() *schema.Schema { + return &schema.Schema{ + Type: schema.TypeList, + MinItems: 1, + MaxItems: 50, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "bar_chart_visual": barCharVisualSchema(), + "box_plot_visual": barCharVisualSchema(), + }, + }, + } + } + provider := info.Provider{ + Name: "testprov", + P: sdkv2.NewProvider(&schema.Provider{ + ResourcesMap: map[string]*schema.Resource{ + "testprov_r1": { + Schema: map[string]*schema.Schema{ + "sheets": { + Type: schema.TypeList, + MinItems: 1, + MaxItems: 20, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "visuals": visualsSchema(), + }, + }, + }, + }, + }, + "testprov_r2": { + Schema: map[string]*schema.Schema{ + "x": { + Type: schema.TypeInt, + Optional: true, + }, + "sheets": { + Type: schema.TypeList, + MinItems: 1, + MaxItems: 20, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "y": { + Type: schema.TypeBool, + Optional: true, + }, + "visuals": visualsSchema(), + }, + }, + }, + }, + }, + }, + }), + UpstreamRepoPath: tmpdir, + Resources: map[string]*info.Resource{ + "testprov_r1": { + Tok: "testprov:index:R1", + Fields: map[string]*info.Schema{ + "sheets": { + Elem: &info.Schema{ + Fields: map[string]*info.Schema{ + "visuals": { + Elem: &info.Schema{ + TypePrefixOverride: tfbridge.StringRef(""), + }, + }, + }, + }, + }, + }, + }, + "testprov_r2": { + Tok: "testprov:index:R2", + Fields: map[string]*info.Schema{ + "sheets": { + Elem: &info.Schema{ + Fields: map[string]*info.Schema{ + "visuals": { + Elem: &info.Schema{ + Type: "testprov:index/Visual:Visual", + OmitType: true, + }, + }, + }, + }, + }, + }, + }, + }, + } + schema, err := GenerateSchema(provider, diag.DefaultSink(os.Stdout, os.Stdout, diag.FormatOptions{ + Color: colors.Never, + })) + require.NoError(t, err) + + keys := []string{} + for k := range schema.Types { + keys = append(keys, string(k)) + } + sort.Strings(keys) + + // Note that there is only one set of helper types, and they are not prefixed by any of the resource names. + autogold.Expect([]string{ + "testprov:index/R1Sheet:R1Sheet", "testprov:index/R2Sheet:R2Sheet", + "testprov:index/Visual:Visual", + "testprov:index/VisualBarChartVisual:VisualBarChartVisual", + "testprov:index/VisualBarChartVisualNest:VisualBarChartVisualNest", + "testprov:index/VisualBoxPlotVisual:VisualBoxPlotVisual", + "testprov:index/VisualBoxPlotVisualNest:VisualBoxPlotVisualNest", + }).Equal(t, keys) + + bytes, err := json.MarshalIndent(schema, "", " ") + require.NoError(t, err) + + autogold.ExpectFile(t, autogold.Raw(string(bytes))) +} + // TestPropertyDocumentationEdits tests that documentation edits are applied to // individual properties. This includes both the property description and // deprecation message. This tests the following workflow diff --git a/pkg/tfgen/testdata/TestTypeSharing.golden b/pkg/tfgen/testdata/TestTypeSharing.golden new file mode 100644 index 000000000..a846f64e7 --- /dev/null +++ b/pkg/tfgen/testdata/TestTypeSharing.golden @@ -0,0 +1,164 @@ +{ + "name": "testprov", + "attribution": "This Pulumi package is based on the [`testprov` Terraform Provider](https://github.com/terraform-providers/terraform-provider-testprov).", + "meta": { + "moduleFormat": "(.*)(?:/[^/]*)" + }, + "language": { + "nodejs": { + "readme": "\u003e This provider is a derived work of the [Terraform Provider](https://github.com/terraform-providers/terraform-provider-testprov)\n\u003e distributed under [MPL 2.0](https://www.mozilla.org/en-US/MPL/2.0/). If you encounter a bug or missing feature,\n\u003e first check the [`pulumi-testprov` repo](/issues); however, if that doesn't turn up anything,\n\u003e please consult the source [`terraform-provider-testprov` repo](https://github.com/terraform-providers/terraform-provider-testprov/issues).", + "compatibility": "tfbridge20", + "disableUnionOutputTypes": true + }, + "python": { + "readme": "\u003e This provider is a derived work of the [Terraform Provider](https://github.com/terraform-providers/terraform-provider-testprov)\n\u003e distributed under [MPL 2.0](https://www.mozilla.org/en-US/MPL/2.0/). If you encounter a bug or missing feature,\n\u003e first check the [`pulumi-testprov` repo](/issues); however, if that doesn't turn up anything,\n\u003e please consult the source [`terraform-provider-testprov` repo](https://github.com/terraform-providers/terraform-provider-testprov/issues).", + "compatibility": "tfbridge20", + "pyproject": {} + } + }, + "config": {}, + "types": { + "testprov:index/R1Sheet:R1Sheet": { + "properties": { + "visuals": { + "type": "array", + "items": { + "$ref": "#/types/testprov:index/Visual:Visual" + } + } + }, + "type": "object" + }, + "testprov:index/R2Sheet:R2Sheet": { + "properties": { + "visuals": { + "type": "array", + "items": { + "$ref": "#/types/testprov:index/Visual:Visual" + } + }, + "y": { + "type": "boolean" + } + }, + "type": "object" + }, + "testprov:index/Visual:Visual": { + "properties": { + "barChartVisual": { + "$ref": "#/types/testprov:index/VisualBarChartVisual:VisualBarChartVisual" + }, + "boxPlotVisual": { + "$ref": "#/types/testprov:index/VisualBoxPlotVisual:VisualBoxPlotVisual" + } + }, + "type": "object" + }, + "testprov:index/VisualBarChartVisual:VisualBarChartVisual": { + "properties": { + "nest": { + "$ref": "#/types/testprov:index/VisualBarChartVisualNest:VisualBarChartVisualNest" + } + }, + "type": "object" + }, + "testprov:index/VisualBarChartVisualNest:VisualBarChartVisualNest": { + "properties": { + "nestedProp": { + "type": "boolean" + } + }, + "type": "object" + }, + "testprov:index/VisualBoxPlotVisual:VisualBoxPlotVisual": { + "properties": { + "nest": { + "$ref": "#/types/testprov:index/VisualBoxPlotVisualNest:VisualBoxPlotVisualNest" + } + }, + "type": "object" + }, + "testprov:index/VisualBoxPlotVisualNest:VisualBoxPlotVisualNest": { + "properties": { + "nestedProp": { + "type": "boolean" + } + }, + "type": "object" + } + }, + "provider": { + "description": "The provider type for the testprov package. By default, resources use package-wide configuration\nsettings, however an explicit `Provider` instance may be created and passed during resource\nconstruction to achieve fine-grained programmatic control over provider settings. See the\n[documentation](https://www.pulumi.com/docs/reference/programming-model/#providers) for more information.\n" + }, + "resources": { + "testprov:index:R1": { + "properties": { + "sheets": { + "type": "array", + "items": { + "$ref": "#/types/testprov:index/R1Sheet:R1Sheet" + } + } + }, + "inputProperties": { + "sheets": { + "type": "array", + "items": { + "$ref": "#/types/testprov:index/R1Sheet:R1Sheet" + } + } + }, + "stateInputs": { + "description": "Input properties used for looking up and filtering R1 resources.\n", + "properties": { + "sheets": { + "type": "array", + "items": { + "$ref": "#/types/testprov:index/R1Sheet:R1Sheet" + } + } + }, + "type": "object" + } + }, + "testprov:index:R2": { + "properties": { + "sheets": { + "type": "array", + "items": { + "$ref": "#/types/testprov:index/R2Sheet:R2Sheet" + } + }, + "x": { + "type": "integer" + } + }, + "inputProperties": { + "sheets": { + "type": "array", + "items": { + "$ref": "#/types/testprov:index/R2Sheet:R2Sheet" + } + }, + "x": { + "type": "integer" + } + }, + "stateInputs": { + "description": "Input properties used for looking up and filtering R2 resources.\n", + "properties": { + "sheets": { + "type": "array", + "items": { + "$ref": "#/types/testprov:index/R2Sheet:R2Sheet" + } + }, + "x": { + "type": "integer" + } + }, + "type": "object" + } + } + } +} \ No newline at end of file From 1a24b64d78e386e94352ed98427f09b4a1af3024 Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Tue, 10 Sep 2024 14:49:50 -0400 Subject: [PATCH 02/13] Fix chattiness of the test --- pkg/tfgen/generate_schema_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/tfgen/generate_schema_test.go b/pkg/tfgen/generate_schema_test.go index 85257c84d..4904fb972 100644 --- a/pkg/tfgen/generate_schema_test.go +++ b/pkg/tfgen/generate_schema_test.go @@ -19,7 +19,6 @@ import ( "encoding/json" "fmt" "io" - "os" "sort" "testing" "text/template" @@ -247,11 +246,15 @@ func TestTypeSharing(t *testing.T) { }, }, } - schema, err := GenerateSchema(provider, diag.DefaultSink(os.Stdout, os.Stdout, diag.FormatOptions{ + + var buf bytes.Buffer + schema, err := GenerateSchema(provider, diag.DefaultSink(&buf, &buf, diag.FormatOptions{ Color: colors.Never, })) require.NoError(t, err) + t.Logf("%s", buf.String()) + keys := []string{} for k := range schema.Types { keys = append(keys, string(k)) From 87c08e2687c846cec7255d5d605d3bb5deedaa6b Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Tue, 10 Sep 2024 15:08:12 -0400 Subject: [PATCH 03/13] Lint --- pkg/tfgen/generate.go | 3 ++- pkg/tfgen/generate_schema_test.go | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/tfgen/generate.go b/pkg/tfgen/generate.go index ad14e64ad..c72539d63 100644 --- a/pkg/tfgen/generate.go +++ b/pkg/tfgen/generate.go @@ -569,7 +569,8 @@ func (t *propertyType) equals(other *propertyType) bool { return false case t.typePrefixOverride == nil && other.typePrefixOverride != nil: return false - case t.typePrefixOverride != nil && other.typePrefixOverride != nil && *t.typePrefixOverride != *other.typePrefixOverride: + case t.typePrefixOverride != nil && other.typePrefixOverride != nil && + *t.typePrefixOverride != *other.typePrefixOverride: return false } for i, p := range t.properties { diff --git a/pkg/tfgen/generate_schema_test.go b/pkg/tfgen/generate_schema_test.go index 4904fb972..b65dee3ce 100644 --- a/pkg/tfgen/generate_schema_test.go +++ b/pkg/tfgen/generate_schema_test.go @@ -24,6 +24,7 @@ import ( "text/template" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hexops/autogold/v2" csgen "github.com/pulumi/pulumi/pkg/v3/codegen/dotnet" gogen "github.com/pulumi/pulumi/pkg/v3/codegen/go" @@ -34,7 +35,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" bridgetesting "github.com/pulumi/pulumi-terraform-bridge/v3/internal/testing" "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge" "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/info" @@ -257,7 +257,7 @@ func TestTypeSharing(t *testing.T) { keys := []string{} for k := range schema.Types { - keys = append(keys, string(k)) + keys = append(keys, k) } sort.Strings(keys) From 07308468aa8c7a42eaf34abfd173e283904fdc77 Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Tue, 10 Sep 2024 15:27:31 -0400 Subject: [PATCH 04/13] Skip new test on Windows --- pkg/tfgen/generate_schema_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/tfgen/generate_schema_test.go b/pkg/tfgen/generate_schema_test.go index b65dee3ce..4abe2d592 100644 --- a/pkg/tfgen/generate_schema_test.go +++ b/pkg/tfgen/generate_schema_test.go @@ -19,6 +19,7 @@ import ( "encoding/json" "fmt" "io" + "runtime" "sort" "testing" "text/template" @@ -125,6 +126,10 @@ func TestCSharpMiniRandom(t *testing.T) { // projection is going to generate named types for every instance of the shared schema. This may lead to SDK bloat. Test // the ability of the provider author to curb the bloat and force an explit sharing. func TestTypeSharing(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skipf("Skipping on Windows due to a test setup issue") + } + tmpdir := t.TempDir() barCharVisualSchema := func() *schema.Schema { return &schema.Schema{ From 11d4115f209f6f490339ce77d932078da011355d Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Wed, 11 Sep 2024 14:17:14 -0400 Subject: [PATCH 05/13] Control the exact type name and not prefix --- pkg/tfbridge/info/info.go | 12 +++++++----- pkg/tfgen/generate.go | 14 +++++++------- pkg/tfgen/generate_schema.go | 14 +++++++++----- pkg/tfgen/generate_schema_test.go | 4 ++-- 4 files changed, 25 insertions(+), 19 deletions(-) diff --git a/pkg/tfbridge/info/info.go b/pkg/tfbridge/info/info.go index e5bc76a7b..f48d014a3 100644 --- a/pkg/tfbridge/info/info.go +++ b/pkg/tfbridge/info/info.go @@ -509,11 +509,13 @@ type Schema struct { // whether or not to treat this property as secret Secret *bool - // If specified, resets the type name prefix for any types that Pulumi needs to generate to represent the schema - // of the current property. Normally the names for helper object types are built from concatenating fragments - // representing the path to the type in the schema. The default strategy can lead to unacceptably long type - // names, reducing the SDK usability. Using TypePrefixOverride allows the maintainer to get shorter type names. - TypePrefixOverride *string + // Specifies the exact name to use for the generated type. + // + // When generating types for properties, by default Pulumi picks reasonable names based on the property path + // prefix and the name of the property. Use [TypeName] to override this decision when the default names for + // nested properties are too long or otherwise undesirable. The choice will further affect the automatically + // generated names for any properties nested under the current one. + TypeName *string } // Config represents a synthetic configuration variable that is Pulumi-only, and not passed to Terraform. diff --git a/pkg/tfgen/generate.go b/pkg/tfgen/generate.go index c72539d63..5fd6e5784 100644 --- a/pkg/tfgen/generate.go +++ b/pkg/tfgen/generate.go @@ -343,7 +343,7 @@ type propertyType struct { altTypes []tokens.Type asset *tfbridge.AssetTranslation - typePrefixOverride *string + typeName *string } func (g *Generator) Sink() diag.Sink { @@ -356,7 +356,7 @@ func (g *Generator) makePropertyType(typePath paths.TypePath, t := &propertyType{} if info != nil { - t.typePrefixOverride = info.TypePrefixOverride + t.typeName = info.TypeName } var elemInfo *tfbridge.SchemaInfo @@ -472,7 +472,7 @@ func (g *Generator) makeObjectPropertyType(typePath paths.TypePath, } if info != nil { - t.typePrefixOverride = info.TypePrefixOverride + t.typeName = info.TypeName } if info != nil { @@ -565,12 +565,12 @@ func (t *propertyType) equals(other *propertyType) bool { return false } switch { - case t.typePrefixOverride != nil && other.typePrefixOverride == nil: + case t.typeName != nil && other.typeName == nil: return false - case t.typePrefixOverride == nil && other.typePrefixOverride != nil: + case t.typeName == nil && other.typeName != nil: return false - case t.typePrefixOverride != nil && other.typePrefixOverride != nil && - *t.typePrefixOverride != *other.typePrefixOverride: + case t.typeName != nil && other.typeName != nil && + *t.typeName != *other.typeName: return false } for i, p := range t.properties { diff --git a/pkg/tfgen/generate_schema.go b/pkg/tfgen/generate_schema.go index 5f944b77f..c699096c2 100644 --- a/pkg/tfgen/generate_schema.go +++ b/pkg/tfgen/generate_schema.go @@ -119,12 +119,16 @@ type declarer interface { func (nt *schemaNestedTypes) declareType(typePath paths.TypePath, declarer declarer, namePrefix, name string, typ *propertyType, isInput bool) string { - if typ.typePrefixOverride != nil { - namePrefix = *typ.typePrefixOverride - } - // Generate a name for this nested type. - typeName := namePrefix + cases.Title(language.Und, cases.NoLower).String(name) + var typeName string + + if typ.typeName != nil { + // Use an explicit name if provided. + typeName = *typ.typeName + } else { + // Otherwise build one based on the current property name and prefix. + typeName = namePrefix + cases.Title(language.Und, cases.NoLower).String(name) + } // Override the nested type name, if necessary. if typ.nestedType.Name().String() != "" { diff --git a/pkg/tfgen/generate_schema_test.go b/pkg/tfgen/generate_schema_test.go index 4abe2d592..a6c6e7aee 100644 --- a/pkg/tfgen/generate_schema_test.go +++ b/pkg/tfgen/generate_schema_test.go @@ -124,7 +124,7 @@ func TestCSharpMiniRandom(t *testing.T) { // Test the ability to force type sharing. Some of the upstream providers generate very large concrete schemata by in // Go, with TF not being materially affected. The example is inspired by QuickSight types in AWS. In Pulumi the default // projection is going to generate named types for every instance of the shared schema. This may lead to SDK bloat. Test -// the ability of the provider author to curb the bloat and force an explit sharing. +// the ability of the provider author to curb the bloat and force an explicit sharing. func TestTypeSharing(t *testing.T) { if runtime.GOOS == "windows" { t.Skipf("Skipping on Windows due to a test setup issue") @@ -224,7 +224,7 @@ func TestTypeSharing(t *testing.T) { Fields: map[string]*info.Schema{ "visuals": { Elem: &info.Schema{ - TypePrefixOverride: tfbridge.StringRef(""), + TypeName: tfbridge.StringRef("Visual"), }, }, }, From 7231894ac46293db2d08d1c6c5e87d2146e62a2c Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Wed, 11 Sep 2024 14:20:20 -0400 Subject: [PATCH 06/13] Switch to Ref --- pkg/tfbridge/info.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/tfbridge/info.go b/pkg/tfbridge/info.go index 2d0042585..1f4ff0fda 100644 --- a/pkg/tfbridge/info.go +++ b/pkg/tfbridge/info.go @@ -318,14 +318,16 @@ func MakeResource(pkg string, mod string, res string) tokens.Type { return tokens.NewTypeToken(modT, tokens.TypeName(res)) } -// BoolRef returns a reference to the bool argument. +// BoolRef returns a reference to the bool argument. Retained for backwards compatibility. Prefer [Ref] for new usage +// and other types like strings. func BoolRef(b bool) *bool { return &b } -// String returns a reference to the string argument. -func StringRef(s string) *string { - return &s +// Fluently construct a reference to the argument. This utility function is needed to ease configuring the bridge where +// references are expected instead of plain boolean or string literals. +func Ref[T any](x T) *T { + return &x } // StringValue gets a string value from a property map if present, else "" From e238d074653e5ea403e1d2cc7e916e120deb1082 Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Wed, 11 Sep 2024 14:20:49 -0400 Subject: [PATCH 07/13] Fix the test --- pkg/tfgen/generate_schema_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/tfgen/generate_schema_test.go b/pkg/tfgen/generate_schema_test.go index a6c6e7aee..29b691332 100644 --- a/pkg/tfgen/generate_schema_test.go +++ b/pkg/tfgen/generate_schema_test.go @@ -224,7 +224,7 @@ func TestTypeSharing(t *testing.T) { Fields: map[string]*info.Schema{ "visuals": { Elem: &info.Schema{ - TypeName: tfbridge.StringRef("Visual"), + TypeName: tfbridge.Ref("Visual"), }, }, }, From 68dffbe592be4d425abf946fdd13779cc2dadd2a Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Wed, 11 Sep 2024 14:23:43 -0400 Subject: [PATCH 08/13] PR feedback --- pkg/tfbridge/info/info.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/tfbridge/info/info.go b/pkg/tfbridge/info/info.go index f48d014a3..599ad93b2 100644 --- a/pkg/tfbridge/info/info.go +++ b/pkg/tfbridge/info/info.go @@ -448,6 +448,8 @@ type Schema struct { // Used together with [Type] to omit generating any Pulumi types whatsoever for the current property, and // instead use the object type identified by the token setup in [Type]. + // + // It is an error to set [OmitType] to true without specifying [Type]. OmitType bool // alternative types that can be used instead of the override. From ac1be903bb33db163c6bdf1baea5b3546deb2a0e Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Wed, 11 Sep 2024 14:25:15 -0400 Subject: [PATCH 09/13] PR Feedback 2 --- pkg/tfgen/generate.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/tfgen/generate.go b/pkg/tfgen/generate.go index 5fd6e5784..11ffeb536 100644 --- a/pkg/tfgen/generate.go +++ b/pkg/tfgen/generate.go @@ -463,7 +463,10 @@ func (g *Generator) makeObjectPropertyType(typePath paths.TypePath, out bool, entityDocs entityDocs) (*propertyType, error) { // If the user supplied an explicit Type token override, omit generating types and short-circuit. - if info != nil && info.OmitType && info.Type != "" { + if info != nil && info.OmitType { + if info.Type != "" { + return nil, fmt.Errorf("Cannot set info.OmitType without also setting info.Type") + } return &propertyType{typ: info.Type}, nil } From 51091c3eb8d065f5646c5f5f82005f01ee577bcf Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Wed, 11 Sep 2024 14:27:01 -0400 Subject: [PATCH 10/13] Fix typo --- pkg/tfgen/generate_schema_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/tfgen/generate_schema_test.go b/pkg/tfgen/generate_schema_test.go index 29b691332..e36fd0401 100644 --- a/pkg/tfgen/generate_schema_test.go +++ b/pkg/tfgen/generate_schema_test.go @@ -121,8 +121,8 @@ func TestCSharpMiniRandom(t *testing.T) { bridgetesting.AssertEqualsJSONFile(t, "test_data/minirandom-schema-csharp.json", schema) } -// Test the ability to force type sharing. Some of the upstream providers generate very large concrete schemata by in -// Go, with TF not being materially affected. The example is inspired by QuickSight types in AWS. In Pulumi the default +// Test the ability to force type sharing. Some of the upstream providers generate very large concrete schemata in Go, +// with TF not being materially affected. The example is inspired by QuickSight types in AWS. In Pulumi the default // projection is going to generate named types for every instance of the shared schema. This may lead to SDK bloat. Test // the ability of the provider author to curb the bloat and force an explicit sharing. func TestTypeSharing(t *testing.T) { From d76371d5c7f5d17e126377401631759024c55c72 Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Wed, 11 Sep 2024 14:35:22 -0400 Subject: [PATCH 11/13] Fix typo --- pkg/tfgen/generate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/tfgen/generate.go b/pkg/tfgen/generate.go index 11ffeb536..85e256a67 100644 --- a/pkg/tfgen/generate.go +++ b/pkg/tfgen/generate.go @@ -464,7 +464,7 @@ func (g *Generator) makeObjectPropertyType(typePath paths.TypePath, // If the user supplied an explicit Type token override, omit generating types and short-circuit. if info != nil && info.OmitType { - if info.Type != "" { + if info.Type == "" { return nil, fmt.Errorf("Cannot set info.OmitType without also setting info.Type") } return &propertyType{typ: info.Type}, nil From 0a27d5911835daa9496b96fd1624908a1a0dcf33 Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Wed, 11 Sep 2024 14:39:27 -0400 Subject: [PATCH 12/13] Experimental markers in the comments --- pkg/tfbridge/info/info.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/tfbridge/info/info.go b/pkg/tfbridge/info/info.go index 599ad93b2..231d1e3b4 100644 --- a/pkg/tfbridge/info/info.go +++ b/pkg/tfbridge/info/info.go @@ -450,6 +450,8 @@ type Schema struct { // instead use the object type identified by the token setup in [Type]. // // It is an error to set [OmitType] to true without specifying [Type]. + // + // Experimental. OmitType bool // alternative types that can be used instead of the override. @@ -517,6 +519,8 @@ type Schema struct { // prefix and the name of the property. Use [TypeName] to override this decision when the default names for // nested properties are too long or otherwise undesirable. The choice will further affect the automatically // generated names for any properties nested under the current one. + // + // Experimental. TypeName *string } From 1370dde757b35b877f8e7f699091fab3bda51195 Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Thu, 12 Sep 2024 10:59:29 -0400 Subject: [PATCH 13/13] Add example use --- pkg/tfbridge/info/info.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/tfbridge/info/info.go b/pkg/tfbridge/info/info.go index 231d1e3b4..aeb8689ab 100644 --- a/pkg/tfbridge/info/info.go +++ b/pkg/tfbridge/info/info.go @@ -520,6 +520,14 @@ type Schema struct { // nested properties are too long or otherwise undesirable. The choice will further affect the automatically // generated names for any properties nested under the current one. // + // Example use: + // + // TypeName: tfbridge.Ref("Visual") + // + // Note that the type name, and not the full token like "aws:quicksight/Visual:Visual" is specified. The token + // will be picked based on the current module ("quicksight" in the above example) where the parent resource or + // data source is found. + // // Experimental. TypeName *string }