From 3daafe70de9d99ca5eb12530aeb999cdd826af86 Mon Sep 17 00:00:00 2001 From: Ian Wahbe Date: Thu, 30 Nov 2023 16:23:52 -0800 Subject: [PATCH 1/6] Remove type prefixing when derived from type overrides --- pkg/tfgen/generate_schema.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/pkg/tfgen/generate_schema.go b/pkg/tfgen/generate_schema.go index 75fd980f4..c9cec6805 100644 --- a/pkg/tfgen/generate_schema.go +++ b/pkg/tfgen/generate_schema.go @@ -180,8 +180,12 @@ func (nt *schemaNestedTypes) gatherFromProperties(pathContext paths.TypePath, name = inflector.Singularize(name) } - nt.gatherFromPropertyType(paths.NewProperyPath(pathContext, p.propertyName), - declarer, namePrefix, name, p.typ, isInput) + var path paths.TypePath = paths.NewProperyPath(pathContext, p.propertyName) + if t := p.typ.typ; t != "" && p.typ.kind == kindObject { + namePrefix = "" + } + + nt.gatherFromPropertyType(path, declarer, namePrefix, name, p.typ, isInput) } } @@ -195,6 +199,9 @@ func (nt *schemaNestedTypes) gatherFromPropertyType(typePath paths.TypePath, dec declarer, namePrefix, name, typ.element, isInput) } case kindObject: + if typ.typ != "" { + namePrefix = "" + } baseName := nt.declareType(typePath, declarer, namePrefix, name, typ, isInput) nt.gatherFromProperties(typePath, declarer, baseName, typ.properties, isInput) } From 48dc0fd40d86c6a0a3fa14971d4851503b5cfc5c Mon Sep 17 00:00:00 2001 From: Ian Wahbe Date: Thu, 30 Nov 2023 17:01:29 -0800 Subject: [PATCH 2/6] Error when declaring conflicting types --- pkg/tfgen/generate_schema.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/pkg/tfgen/generate_schema.go b/pkg/tfgen/generate_schema.go index c9cec6805..c9cb9e6c2 100644 --- a/pkg/tfgen/generate_schema.go +++ b/pkg/tfgen/generate_schema.go @@ -254,13 +254,23 @@ func (g *schemaGenerator) genPackageSpec(pack *pkg) (pschema.PackageSpec, error) spec.Attribution = fmt.Sprintf(attributionFormatString, g.info.Name, g.info.GetGitHubOrg(), g.info.GetGitHubHost()) var config []*variable + declaredTypes := map[string]*schemaNestedType{} for _, mod := range pack.modules.values() { // Generate nested types. for _, t := range gatherSchemaNestedTypesForModule(mod) { tok := g.genObjectTypeToken(t) ts := g.genObjectType(t, false) - spec.Types[tok] = pschema.ComplexTypeSpec{ - ObjectTypeSpec: ts, + + if existing, ok := declaredTypes[tok]; ok { + if !t.typ.equals(existing.typ) { + return pschema.PackageSpec{}, + fmt.Errorf("%s: conflicting type definitions", tok) + } + } else { + declaredTypes[tok] = t + spec.Types[tok] = pschema.ComplexTypeSpec{ + ObjectTypeSpec: ts, + } } } From caca3d23aafd7b702963df21cc6cffc4097508da Mon Sep 17 00:00:00 2001 From: Ian Wahbe Date: Thu, 30 Nov 2023 17:45:03 -0800 Subject: [PATCH 3/6] WIP Add test --- pkg/tfbridge/info.go | 4 + pkg/tfgen/generate_test.go | 163 +++++++++++++++++++++++++------------ 2 files changed, 114 insertions(+), 53 deletions(-) diff --git a/pkg/tfbridge/info.go b/pkg/tfbridge/info.go index 4b90b28e7..ceaba66f7 100644 --- a/pkg/tfbridge/info.go +++ b/pkg/tfbridge/info.go @@ -416,6 +416,10 @@ type SchemaInfo struct { CSharpName string // a type to override the default; "" uses the default. + // + // If the type overriden is an object type, `Type: newName` is interpreted as a + // move operation, pointing the property to a type called `newName` and creating + // `newName` with the schema described by this type. Type tokens.Type // alternative types that can be used instead of the override. diff --git a/pkg/tfgen/generate_test.go b/pkg/tfgen/generate_test.go index d75b24483..f13cc0277 100644 --- a/pkg/tfgen/generate_test.go +++ b/pkg/tfgen/generate_test.go @@ -285,74 +285,77 @@ 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) { +func generateNestedSchema(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() - 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{ + p := (&shimschema.Provider{ + ResourcesMap: shimschema.ResourceMap{ + "test_res": (&shimschema.Resource{ Schema: shimschema.SchemaMap{ - "foo_bar": strType, - "nested": nestedObj, + "obj": objType, }, }).Shim(), - }).Shim() - - p := (&shimschema.Provider{ - ResourcesMap: shimschema.ResourceMap{ - "test_res": (&shimschema.Resource{ - Schema: shimschema.SchemaMap{ - "obj": objType, - }, - }).Shim(), - }, - }).Shim() + }, + }).Shim() - nilSink := diag.DefaultSink(io.Discard, io.Discard, diag.FormatOptions{ - Color: colors.Never, - }) + nilSink := diag.DefaultSink(io.Discard, io.Discard, diag.FormatOptions{ + Color: colors.Never, + }) - res := &tfbridge.ResourceInfo{ - Tok: "test:index:Bar", - } - if f != nil { - f(res) - } + 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, - }, + 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 - } + }, + }) + require.NoError(t, err) + return r.PackageSpec +} + +func Test_ProviderWithOmittedTypes(t *testing.T) { + t.Parallel() t.Run("no-omit", func(t *testing.T) { - spec := gen(t, nil) + t.Parallel() + spec := generateNestedSchema(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) { + t.Parallel() + spec := generateNestedSchema(t, func(info *tfbridge.ResourceInfo) { info.Fields = map[string]*tfbridge.SchemaInfo{ "obj": {Omit: true}, } @@ -363,7 +366,8 @@ func Test_ProviderWithOmittedTypes(t *testing.T) { }) t.Run("omit-nested-prop", func(t *testing.T) { - spec := gen(t, func(info *tfbridge.ResourceInfo) { + t.Parallel() + spec := generateNestedSchema(t, func(info *tfbridge.ResourceInfo) { info.Fields = map[string]*tfbridge.SchemaInfo{ "obj": { Elem: &tfbridge.SchemaInfo{ @@ -381,6 +385,59 @@ func Test_ProviderWithOmittedTypes(t *testing.T) { } +func Test_ProviderWithMovedTypes(t *testing.T) { + t.Parallel() + + t.Run("none", func(t *testing.T) { + t.Parallel() + spec := generateNestedSchema(t, nil) + assert.Len(t, spec.Resources, 1) + assert.Len(t, spec.Resources["test:index:Bar"].InputProperties, 1) + assert.Len(t, spec.Types, 2) + assert.Contains(t, spec.Types, "test:index/BarObj:BarObj") + assert.Contains(t, spec.Types, "test:index/BarObjNested:BarObjNested") + }) + + t.Run("top-level", func(t *testing.T) { + t.Parallel() + spec := generateNestedSchema(t, func(info *tfbridge.ResourceInfo) { + info.Fields = map[string]*tfbridge.SchemaInfo{ + "obj": {Type: "test:moved:Top"}, + } + }) + assert.Len(t, spec.Resources, 1) + assert.Len(t, spec.Types, 2) + if assert.Contains(t, spec.Types, "test:moved:Top") { + assert.Contains(t, spec.Types, "test:moved:TopNested") + } + }) + + t.Run("nested-prop", func(t *testing.T) { + t.Parallel() + spec := generateNestedSchema(t, func(info *tfbridge.ResourceInfo) { + info.Fields = map[string]*tfbridge.SchemaInfo{ + "obj": { + Elem: &tfbridge.SchemaInfo{ + Fields: map[string]*tfbridge.SchemaInfo{ + "nested": {Type: "test:moved:Nested"}, + }, + }, + }, + } + }) + assert.Len(t, spec.Resources, 1) + assert.Len(t, spec.Types, 2) + assert.Contains(t, spec.Types, "test:index/BarObj:BarObj") + assert.Contains(t, spec.Types, "test:moved:Nested") + }) + + t.Run("conflict", func(t *testing.T) { + t.Parallel() + t.Fatalf("TODO Test that the provider errors when multiple types are directed to the same token.") + }) + +} + func TestModulePlacementForType(t *testing.T) { t.Parallel() From ccf288d5dd8de5f45bd16b2c7cb791ea44231fd3 Mon Sep 17 00:00:00 2001 From: Ian Wahbe Date: Fri, 1 Dec 2023 10:11:21 -0800 Subject: [PATCH 4/6] Progress towards correct type tokens --- pkg/tfgen/generate_schema.go | 19 +++++++++---- pkg/tfgen/generate_test.go | 47 +++++++++++++++++-------------- pkg/tfgen/internal/paths/paths.go | 17 +++++++++++ 3 files changed, 57 insertions(+), 26 deletions(-) diff --git a/pkg/tfgen/generate_schema.go b/pkg/tfgen/generate_schema.go index c9cb9e6c2..b57a2e667 100644 --- a/pkg/tfgen/generate_schema.go +++ b/pkg/tfgen/generate_schema.go @@ -123,6 +123,10 @@ func (nt *schemaNestedTypes) declareType(typePath paths.TypePath, declarer decla typeName = typ.nestedType.Name().String() } + if typ.typ != "" && typ.kind == kindObject { + typeName = typ.typ.Name().String() + } + typ.name = typeName required := codegen.StringSet{} @@ -781,11 +785,14 @@ func (g *schemaGenerator) genObjectTypeToken(typInfo *schemaNestedType) string { } mod := modulePlacementForTypeSet(g.pkg, typInfo.typePaths) - token := fmt.Sprintf("%s/%s:%s", mod.String(), name, name) - - g.renamesBuilder.registerNamedObjectType(typInfo.typePaths, tokens.Type(token)) + token := tokens.Type(fmt.Sprintf("%s/%s:%s", mod, name, name)) + if typ.typ != "" { + token = typ.typ + } - return token + g.renamesBuilder.registerNamedObjectType(typInfo.typePaths, token) + fmt.Printf("Generated token %s (mod = %s, name = %s)\n", token, mod, name) + return token.String() } func (g *schemaGenerator) genObjectType(typInfo *schemaNestedType, isTopLevel bool) pschema.ObjectTypeSpec { @@ -922,7 +929,7 @@ func (g *schemaGenerator) schemaType(path paths.TypePath, typ *propertyType, out return pschema.TypeSpec{Type: "object", AdditionalProperties: &additionalProperties} case kindObject: mod := modulePlacementForType(g.pkg, path) - ref := fmt.Sprintf("#/types/%s/%s:%s", mod.String(), typ.name, typ.name) + ref := fmt.Sprintf("#/types/%s/%s:%s", mod, typ.name, typ.name) return pschema.TypeSpec{Ref: ref} default: contract.Failf("Unrecognized type kind: %v", typ.kind) @@ -1145,6 +1152,8 @@ func modulePlacementForType(pkg tokens.Package, path paths.TypePath) tokens.Modu return parentModuleOrSelf(m) case *paths.ConfigPath: return tokens.NewModuleToken(pkg, configMod) + case *paths.RawTypePath: + return pp.Raw().Module() default: contract.Assertf(false, "invalid ParentKind") return "" diff --git a/pkg/tfgen/generate_test.go b/pkg/tfgen/generate_test.go index f13cc0277..4403bc0d9 100644 --- a/pkg/tfgen/generate_test.go +++ b/pkg/tfgen/generate_test.go @@ -287,25 +287,21 @@ func Test_ProviderWithObjectTypesInConfigCanGenerateRenames(t *testing.T) { func generateNestedSchema(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() + mkObj := func(m shim.SchemaMap) shim.Schema { + return (&shimschema.Schema{ + Type: shim.TypeMap, + Optional: true, + Elem: (&shimschema.Resource{Schema: m}).Shim(), + }).Shim() + } + + nestedObj := mkObj(shimschema.SchemaMap{ + "fizz_buzz": strType, + }) + objType := mkObj(shimschema.SchemaMap{ + "foo_bar": strType, + "nested": nestedObj, + }) p := (&shimschema.Provider{ ResourcesMap: shimschema.ResourceMap{ @@ -402,9 +398,14 @@ func Test_ProviderWithMovedTypes(t *testing.T) { t.Parallel() spec := generateNestedSchema(t, func(info *tfbridge.ResourceInfo) { info.Fields = map[string]*tfbridge.SchemaInfo{ - "obj": {Type: "test:moved:Top"}, + "obj": { + Elem: &tfbridge.SchemaInfo{ + Type: "test:moved:Top", + }, + }, } }) + assert.Len(t, spec.Resources, 1) assert.Len(t, spec.Types, 2) if assert.Contains(t, spec.Types, "test:moved:Top") { @@ -419,7 +420,11 @@ func Test_ProviderWithMovedTypes(t *testing.T) { "obj": { Elem: &tfbridge.SchemaInfo{ Fields: map[string]*tfbridge.SchemaInfo{ - "nested": {Type: "test:moved:Nested"}, + "nested": { + Elem: &tfbridge.SchemaInfo{ + Type: "test:moved:Nested", + }, + }, }, }, }, diff --git a/pkg/tfgen/internal/paths/paths.go b/pkg/tfgen/internal/paths/paths.go index 4b2c0ae0c..395ddefa5 100644 --- a/pkg/tfgen/internal/paths/paths.go +++ b/pkg/tfgen/internal/paths/paths.go @@ -336,3 +336,20 @@ func (s TypePathSet) Paths() []TypePath { }) return res } + +// RawTypePath represents a type anchored from an opaque user provided type. +type RawTypePath struct{ t tokens.Type } + +func NewRawPath(typ tokens.Type) *RawTypePath { return &RawTypePath{typ} } + +var _ TypePath = (*RawTypePath)(nil) + +func (p *RawTypePath) Parent() TypePath { return nil } + +// Useful for comparing paths. +func (p *RawTypePath) UniqueKey() string { return p.t.String() } + +// Human friendly representation. +func (p *RawTypePath) String() string { return p.t.Name().String() } + +func (p *RawTypePath) Raw() tokens.Type { return tokens.Type(p.t) } From 36c657fb5b9c487af0f76397f90edf329bd19b0c Mon Sep 17 00:00:00 2001 From: Ian Wahbe Date: Fri, 1 Dec 2023 11:10:26 -0800 Subject: [PATCH 5/6] Correct type tokens --- pkg/tfgen/generate_schema.go | 31 ++++++++++++++++++++++++++----- pkg/tfgen/generate_test.go | 2 +- pkg/tfgen/internal/paths/paths.go | 11 +++++++++-- pkg/tfgen/namecheck.go | 2 ++ pkg/tfgen/renames.go | 27 +++++++++++++++++++++++---- 5 files changed, 61 insertions(+), 12 deletions(-) diff --git a/pkg/tfgen/generate_schema.go b/pkg/tfgen/generate_schema.go index b57a2e667..6aba7ce9f 100644 --- a/pkg/tfgen/generate_schema.go +++ b/pkg/tfgen/generate_schema.go @@ -67,17 +67,19 @@ type schemaNestedType struct { } type schemaNestedTypes struct { - nameToType map[string]*schemaNestedType + nameToType map[string]*schemaNestedType + pathCorrections map[string]paths.TypePath } -func gatherSchemaNestedTypesForModule(mod *module) map[string]*schemaNestedType { +func gatherSchemaNestedTypesForModule(mod *module) (map[string]*schemaNestedType, map[string]paths.TypePath) { nt := &schemaNestedTypes{ - nameToType: make(map[string]*schemaNestedType), + nameToType: make(map[string]*schemaNestedType), + pathCorrections: make(map[string]paths.TypePath), } for _, member := range mod.members { nt.gatherFromMember(member) } - return nt.nameToType + return nt.nameToType, nt.pathCorrections } func gatherSchemaNestedTypesForMember(member moduleMember) map[string]*schemaNestedType { @@ -187,12 +189,20 @@ func (nt *schemaNestedTypes) gatherFromProperties(pathContext paths.TypePath, var path paths.TypePath = paths.NewProperyPath(pathContext, p.propertyName) if t := p.typ.typ; t != "" && p.typ.kind == kindObject { namePrefix = "" + newPath := paths.NewRawPath(t, path) + nt.correctPath(path, newPath) + path = newPath } nt.gatherFromPropertyType(path, declarer, namePrefix, name, p.typ, isInput) } } +// Insert a type path redirect, setting src to point to dst. +func (nt *schemaNestedTypes) correctPath(src, dst paths.TypePath) { + nt.pathCorrections[src.UniqueKey()] = dst +} + func (nt *schemaNestedTypes) gatherFromPropertyType(typePath paths.TypePath, declarer declarer, namePrefix, name string, typ *propertyType, isInput bool) { @@ -205,6 +215,7 @@ func (nt *schemaNestedTypes) gatherFromPropertyType(typePath paths.TypePath, dec case kindObject: if typ.typ != "" { namePrefix = "" + typePath = paths.NewRawPath(typ.typ, typePath) } baseName := nt.declareType(typePath, declarer, namePrefix, name, typ, isInput) nt.gatherFromProperties(typePath, declarer, baseName, typ.properties, isInput) @@ -261,7 +272,17 @@ func (g *schemaGenerator) genPackageSpec(pack *pkg) (pschema.PackageSpec, error) declaredTypes := map[string]*schemaNestedType{} for _, mod := range pack.modules.values() { // Generate nested types. - for _, t := range gatherSchemaNestedTypesForModule(mod) { + nestedTypes, pathCorrections := gatherSchemaNestedTypesForModule(mod) + if b := g.renamesBuilder; b != nil { + if b.pathCorrections == nil { + b.pathCorrections = pathCorrections + } else { + for k, v := range pathCorrections { + b.pathCorrections[k] = v + } + } + } + for _, t := range nestedTypes { tok := g.genObjectTypeToken(t) ts := g.genObjectType(t, false) diff --git a/pkg/tfgen/generate_test.go b/pkg/tfgen/generate_test.go index 4403bc0d9..4de06efab 100644 --- a/pkg/tfgen/generate_test.go +++ b/pkg/tfgen/generate_test.go @@ -409,7 +409,7 @@ func Test_ProviderWithMovedTypes(t *testing.T) { assert.Len(t, spec.Resources, 1) assert.Len(t, spec.Types, 2) if assert.Contains(t, spec.Types, "test:moved:Top") { - assert.Contains(t, spec.Types, "test:moved:TopNested") + assert.Contains(t, spec.Types, "test:moved/TopNested:TopNested") } }) diff --git a/pkg/tfgen/internal/paths/paths.go b/pkg/tfgen/internal/paths/paths.go index 395ddefa5..95c89a04e 100644 --- a/pkg/tfgen/internal/paths/paths.go +++ b/pkg/tfgen/internal/paths/paths.go @@ -338,9 +338,14 @@ func (s TypePathSet) Paths() []TypePath { } // RawTypePath represents a type anchored from an opaque user provided type. -type RawTypePath struct{ t tokens.Type } +type RawTypePath struct { + t tokens.Type + structuralPath TypePath +} -func NewRawPath(typ tokens.Type) *RawTypePath { return &RawTypePath{typ} } +func NewRawPath(typ tokens.Type, structuralPath TypePath) *RawTypePath { + return &RawTypePath{typ, structuralPath} +} var _ TypePath = (*RawTypePath)(nil) @@ -353,3 +358,5 @@ func (p *RawTypePath) UniqueKey() string { return p.t.String() } func (p *RawTypePath) String() string { return p.t.Name().String() } func (p *RawTypePath) Raw() tokens.Type { return tokens.Type(p.t) } + +func (p *RawTypePath) StructuralPath() TypePath { return p.structuralPath } diff --git a/pkg/tfgen/namecheck.go b/pkg/tfgen/namecheck.go index 5368dc136..4b98fc4b7 100644 --- a/pkg/tfgen/namecheck.go +++ b/pkg/tfgen/namecheck.go @@ -383,6 +383,8 @@ func locateTypByTypePath(prov tfbridge.ProviderInfo, path paths.TypePath) (typ, return nil, err } return t.Element() + case *paths.RawTypePath: + return locateTypByTypePath(prov, p.StructuralPath()) default: panic("impossible match in locateTypByTypePath") } diff --git a/pkg/tfgen/renames.go b/pkg/tfgen/renames.go index 1fbffade7..15e80dd08 100644 --- a/pkg/tfgen/renames.go +++ b/pkg/tfgen/renames.go @@ -70,6 +70,12 @@ type renamesBuilder struct { functions map[tokens.ModuleMember]string objectTypes map[string]tokens.Type objectTypePaths map[tokens.Type]paths.TypePathSet + + // pathCorrections accounts for the case where a user has explicitly specified a + // path, so it no longer matches it's expected location. + // + // This maps the unique key of a structural path to the user supplied path. + pathCorrections map[string]paths.TypePath } func newRenamesBuilder(pkg tokens.Package, resourcePrefix string) *renamesBuilder { @@ -215,6 +221,8 @@ func (r *renamesBuilder) BuildRenames() (Renames, error) { } func (r renamesBuilder) findObjectTypeToken(path paths.TypePath) (tokens.Type, error) { + path = r.correctPath(path) + if p, ok := r.objectTypes[path.String()]; ok { return p, nil } @@ -227,7 +235,20 @@ func (r renamesBuilder) findObjectTypeToken(path paths.TypePath) (tokens.Type, e if p, ok := r.objectTypes[path.String()]; ok { return p, nil } - return "", fmt.Errorf("expected registerNamedObjectType to be called for %s", path.String()) + return "", fmt.Errorf("expected registerNamedObjectType to be called for %s (%[1]T)", path) +} + +func (r renamesBuilder) correctPath(path paths.TypePath) paths.TypePath { + if v, ok := r.pathCorrections[path.UniqueKey()]; ok { + return v + } + + switch path := path.(type) { + case *paths.PropertyPath: + return paths.NewProperyPath(r.correctPath(path.Parent()), path.PropertyName) + default: + return path + } } func (r renamesBuilder) normalizeProviderStateToProviderInputs(p paths.TypePath) paths.TypePath { @@ -237,9 +258,7 @@ func (r renamesBuilder) normalizeProviderStateToProviderInputs(p paths.TypePath) return pp.ResourcePath.Inputs() } return p - case *paths.DataSourceMemberPath: - return p - case *paths.ConfigPath: + case *paths.DataSourceMemberPath, *paths.ConfigPath, *paths.RawTypePath: return p case *paths.ElementPath: return paths.NewElementPath(r.normalizeProviderStateToProviderInputs(pp.Parent())) From a0f16392a4c265766fc4de152155019d81a15212 Mon Sep 17 00:00:00 2001 From: Ian Wahbe Date: Fri, 1 Dec 2023 11:55:21 -0800 Subject: [PATCH 6/6] Fix type merger It looks like type merger was broken at the equality (.name) comparison, but since all nested objects had a blank name at the time of comparison, this was ok. It was necessary to fix this to allow equal types to compare equal. --- pkg/tfbridge/info.go | 4 +- pkg/tfgen/generate_schema.go | 34 +++++--- pkg/tfgen/generate_test.go | 129 +++++++++++++++++++++++++++++- pkg/tfgen/internal/paths/paths.go | 2 +- 4 files changed, 155 insertions(+), 14 deletions(-) diff --git a/pkg/tfbridge/info.go b/pkg/tfbridge/info.go index ceaba66f7..fe67a8b10 100644 --- a/pkg/tfbridge/info.go +++ b/pkg/tfbridge/info.go @@ -417,7 +417,7 @@ type SchemaInfo struct { // a type to override the default; "" uses the default. // - // If the type overriden is an object type, `Type: newName` is interpreted as a + // If the type overridden is an object type, `Type: newName` is interpreted as a // move operation, pointing the property to a type called `newName` and creating // `newName` with the schema described by this type. Type tokens.Type @@ -428,7 +428,7 @@ type SchemaInfo struct { // a type to override when the property is a nested structure. NestedType tokens.Type - // an optional idemponent transformation, applied before passing to TF. + // an optional idempotent transformation, applied before passing to TF. Transform Transformer // a schema override for elements for arrays, maps, and sets. diff --git a/pkg/tfgen/generate_schema.go b/pkg/tfgen/generate_schema.go index 6aba7ce9f..df0c851d0 100644 --- a/pkg/tfgen/generate_schema.go +++ b/pkg/tfgen/generate_schema.go @@ -114,9 +114,7 @@ type declarer interface { Name() string } -func (nt *schemaNestedTypes) declareType(typePath paths.TypePath, declarer declarer, namePrefix, name string, - typ *propertyType, isInput bool) string { - +func (nt *schemaNestedTypes) typeName(namePrefix, name string, typ *propertyType) string { // Generate a name for this nested type. typeName := namePrefix + cases.Title(language.Und, cases.NoLower).String(name) @@ -130,6 +128,16 @@ func (nt *schemaNestedTypes) declareType(typePath paths.TypePath, declarer decla } typ.name = typeName + return typ.name +} + +// declareType emits the declared type into nt. +// +// declareType requires that: +// 1. nt.typeName(..., typ) has been called previously on this type. +// 2. All nested types have already been declare. +func (nt *schemaNestedTypes) declareType(typePath paths.TypePath, declarer declarer, + typ *propertyType, isInput bool) { required := codegen.StringSet{} for _, p := range typ.properties { @@ -146,8 +154,9 @@ func (nt *schemaNestedTypes) declareType(typePath paths.TypePath, declarer decla } // Merging makes sure that structurally identical types are shared and not generated more than once. - if existing, ok := nt.nameToType[typeName]; ok { - contract.Assertf(existing.declarer == declarer || existing.typ.equals(typ), "duplicate type %v", typeName) + if existing, ok := nt.nameToType[typ.name]; ok { + contract.Assertf(existing.declarer == declarer || existing.typ.equals(typ), + "%s declared twice with different values", typ.name) // Remember that existing type is now also seen at the current typePath. existing.typePaths.Add(typePath) @@ -162,10 +171,10 @@ func (nt *schemaNestedTypes) declareType(typePath paths.TypePath, declarer decla } existing.typ, existing.required = typ, required - return typeName + return } - nt.nameToType[typeName] = &schemaNestedType{ + nt.nameToType[typ.name] = &schemaNestedType{ typ: typ, declarer: declarer, required: required, @@ -173,7 +182,6 @@ func (nt *schemaNestedTypes) declareType(typePath paths.TypePath, declarer decla requiredOutputs: requiredOutputs, typePaths: paths.SingletonTypePathSet(typePath), } - return typeName } func (nt *schemaNestedTypes) gatherFromProperties(pathContext paths.TypePath, @@ -217,8 +225,15 @@ func (nt *schemaNestedTypes) gatherFromPropertyType(typePath paths.TypePath, dec namePrefix = "" typePath = paths.NewRawPath(typ.typ, typePath) } - baseName := nt.declareType(typePath, declarer, namePrefix, name, typ, isInput) + + // Because nt.declareType expects the declared type to be fully formed (to + // allow an equality comparison against other fully formed types), all + // nested types must themselves be fully formed. + // + // gatherFromProperties must be called before declareType. + baseName := nt.typeName(namePrefix, name, typ) nt.gatherFromProperties(typePath, declarer, baseName, typ.properties, isInput) + nt.declareType(typePath, declarer, typ, isInput) } } @@ -812,7 +827,6 @@ func (g *schemaGenerator) genObjectTypeToken(typInfo *schemaNestedType) string { } g.renamesBuilder.registerNamedObjectType(typInfo.typePaths, token) - fmt.Printf("Generated token %s (mod = %s, name = %s)\n", token, mod, name) return token.String() } diff --git a/pkg/tfgen/generate_test.go b/pkg/tfgen/generate_test.go index 4de06efab..14a92993b 100644 --- a/pkg/tfgen/generate_test.go +++ b/pkg/tfgen/generate_test.go @@ -436,9 +436,136 @@ func Test_ProviderWithMovedTypes(t *testing.T) { assert.Contains(t, spec.Types, "test:moved:Nested") }) + strType := (&shimschema.Schema{Type: shim.TypeString}).Shim() + mkObj := func(m shim.SchemaMap) shim.Schema { + return (&shimschema.Schema{ + Type: shim.TypeMap, + Optional: true, + Elem: (&shimschema.Resource{Schema: m}).Shim(), + }).Shim() + } + nilSink := diag.DefaultSink(io.Discard, io.Discard, diag.FormatOptions{ + Color: colors.Never, + }) + t.Run("conflict", func(t *testing.T) { t.Parallel() - t.Fatalf("TODO Test that the provider errors when multiple types are directed to the same token.") + + nestedObj := mkObj(shimschema.SchemaMap{ + "fizz_buzz": strType, + }) + objType := mkObj(shimschema.SchemaMap{ + "foo_bar": strType, + "nested": nestedObj, + }) + + p := (&shimschema.Provider{ + ResourcesMap: shimschema.ResourceMap{ + "test_res": (&shimschema.Resource{ + Schema: shimschema.SchemaMap{ + "obj": objType, + }, + }).Shim(), + "test_other": (&shimschema.Resource{ + Schema: shimschema.SchemaMap{ + "obj": nestedObj, + }, + }).Shim(), + }, + }).Shim() + + nameObj := map[string]*tfbridge.SchemaInfo{ + "obj": { + Elem: &tfbridge.SchemaInfo{ + Type: "test:index:Obj", + }, + }, + } + + // GenerateSchemaWithOptions should return an error when we force distinct + // types to have the same type token. Instead, it panics. We capture this + // behavior for now, since we want to assert that it doesn't return + // garbage. + err := "fatal: An assertion has failed: Obj declared twice with different values" + assert.PanicsWithValue(t, err, func() { + _, _ = GenerateSchemaWithOptions(GenerateSchemaOptions{ + DiagnosticsSink: nilSink, + ProviderInfo: tfbridge.ProviderInfo{ + Name: "test", + P: p, + Resources: map[string]*tfbridge.ResourceInfo{ + "test_res": { + Tok: "test:index:Res", + Fields: nameObj, + }, + "test_other": { + Tok: "test:index:Other", + Fields: nameObj, + }, + }, + }, + }) + }) + }) + + t.Run("no-conflict", func(t *testing.T) { + t.Parallel() + + nestedObj := mkObj(shimschema.SchemaMap{ + "fizz_buzz": strType, + }) + objType := mkObj(shimschema.SchemaMap{ + "foo_bar": strType, + "nested": nestedObj, + }) + + p := (&shimschema.Provider{ + ResourcesMap: shimschema.ResourceMap{ + "test_res": (&shimschema.Resource{ + Schema: shimschema.SchemaMap{ + "field1": objType, + }, + }).Shim(), + "test_other": (&shimschema.Resource{ + Schema: shimschema.SchemaMap{ + "field2": objType, + }, + }).Shim(), + }, + }).Shim() + + nameObj := func(i int) map[string]*tfbridge.SchemaInfo { + return map[string]*tfbridge.SchemaInfo{ + fmt.Sprintf("field%d", i): { + Elem: &tfbridge.SchemaInfo{ + Type: "test:index:Obj", + }, + }, + } + } + + r, err := GenerateSchemaWithOptions(GenerateSchemaOptions{ + DiagnosticsSink: nilSink, + ProviderInfo: tfbridge.ProviderInfo{ + Name: "test", + P: p, + Resources: map[string]*tfbridge.ResourceInfo{ + "test_res": { + Tok: "test:index:Res", + Fields: nameObj(1), + }, + "test_other": { + Tok: "test:index:Other", + Fields: nameObj(2), + }, + }, + }, + }) + require.NoError(t, err) + + assert.Len(t, r.PackageSpec.Resources, 2) + assert.Len(t, r.PackageSpec.Types, 2, + "Since nested types were declared to have the same name, they should only show up once.") }) } diff --git a/pkg/tfgen/internal/paths/paths.go b/pkg/tfgen/internal/paths/paths.go index 95c89a04e..55d725390 100644 --- a/pkg/tfgen/internal/paths/paths.go +++ b/pkg/tfgen/internal/paths/paths.go @@ -357,6 +357,6 @@ func (p *RawTypePath) UniqueKey() string { return p.t.String() } // Human friendly representation. func (p *RawTypePath) String() string { return p.t.Name().String() } -func (p *RawTypePath) Raw() tokens.Type { return tokens.Type(p.t) } +func (p *RawTypePath) Raw() tokens.Type { return p.t } func (p *RawTypePath) StructuralPath() TypePath { return p.structuralPath }