diff --git a/internal/mapper/attrmapper/dynamic.go b/internal/mapper/attrmapper/dynamic.go new file mode 100644 index 00000000..b2d8e7d9 --- /dev/null +++ b/internal/mapper/attrmapper/dynamic.go @@ -0,0 +1,91 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package attrmapper + +import ( + "github.com/starburstdata/terraform-plugin-codegen-openapi/internal/explorer" + "github.com/starburstdata/terraform-plugin-codegen-openapi/internal/mapper/util" + "github.com/hashicorp/terraform-plugin-codegen-spec/datasource" + "github.com/hashicorp/terraform-plugin-codegen-spec/provider" + "github.com/hashicorp/terraform-plugin-codegen-spec/resource" +) + +type ResourceDynamicAttribute struct { + resource.DynamicAttribute + + Name string +} + +func (a *ResourceDynamicAttribute) GetName() string { + return a.Name +} + +func (a *ResourceDynamicAttribute) Merge(mergeAttribute ResourceAttribute) (ResourceAttribute, error) { + dynamicAttribute, ok := mergeAttribute.(*ResourceDynamicAttribute) + // TODO: return error if types don't match? + if ok && (a.Description == nil || *a.Description == "") { + a.Description = dynamicAttribute.Description + } + + return a, nil +} + +func (a *ResourceDynamicAttribute) ApplyOverride(override explorer.Override) (ResourceAttribute, error) { + a.Description = &override.Description + + return a, nil +} + +func (a *ResourceDynamicAttribute) ToSpec() resource.Attribute { + return resource.Attribute{ + Name: util.TerraformIdentifier(a.Name), + Dynamic: &a.DynamicAttribute, + } +} + +type DataSourceDynamicAttribute struct { + datasource.DynamicAttribute + + Name string +} + +func (a *DataSourceDynamicAttribute) GetName() string { + return a.Name +} + +func (a *DataSourceDynamicAttribute) Merge(mergeAttribute DataSourceAttribute) (DataSourceAttribute, error) { + dynamicAttribute, ok := mergeAttribute.(*DataSourceDynamicAttribute) + // TODO: return error if types don't match? + if ok && (a.Description == nil || *a.Description == "") { + a.Description = dynamicAttribute.Description + } + + return a, nil +} + +func (a *DataSourceDynamicAttribute) ApplyOverride(override explorer.Override) (DataSourceAttribute, error) { + a.Description = &override.Description + + return a, nil +} + +func (a *DataSourceDynamicAttribute) ToSpec() datasource.Attribute { + return datasource.Attribute{ + Name: util.TerraformIdentifier(a.Name), + Dynamic: &a.DynamicAttribute, + } +} + +type ProviderDynamicAttribute struct { + provider.DynamicAttribute + + Name string +} + +func (a *ProviderDynamicAttribute) ToSpec() provider.Attribute { + return provider.Attribute{ + Name: util.TerraformIdentifier(a.Name), + Dynamic: &a.DynamicAttribute, + } +} diff --git a/internal/mapper/datasource_mapper_test.go b/internal/mapper/datasource_mapper_test.go index c5de984d..5219bae3 100644 --- a/internal/mapper/datasource_mapper_test.go +++ b/internal/mapper/datasource_mapper_test.go @@ -823,7 +823,7 @@ func TestDataSourceMapper_basic_merges(t *testing.T) { ReadOp: createTestReadOp(testCase.readResponseSchema, testCase.readParams), SchemaOptions: testCase.schemaOptions, }, - }, config.Config{}) + }, nil, config.Config{}) got, err := mapper.MapToIR(slog.Default()) if err != nil { t.Fatalf("unexpected error: %s", err) @@ -993,7 +993,7 @@ func TestDataSourceMapper_collections(t *testing.T) { "test_datasources": { ReadOp: createTestReadOp(testCase.readResponseSchema, nil), }, - }, config.Config{}) + }, nil, config.Config{}) got, err := mapper.MapToIR(slog.Default()) if err != nil { t.Fatalf("unexpected error: %s", err) diff --git a/internal/mapper/oas/attribute.go b/internal/mapper/oas/attribute.go index 2a1f6a44..c69dfee7 100644 --- a/internal/mapper/oas/attribute.go +++ b/internal/mapper/oas/attribute.go @@ -34,6 +34,19 @@ func (s *OASSchema) BuildResourceAttributes() (attrmapper.ResourceAttributes, *S return nil, s.NestSchemaError(err, name) } + // If this property references a schema already open on the descent path, it is a + // self-referential (recursive) schema. Degrade to a dynamic attribute instead of + // recursing forever (which stack overflows). + if s.GlobalSchemaOpts.IsVisitedRef(pProxy) { + attribute, err := pSchema.BuildDynamicResource(name, s.GetComputability(name)) + if err != nil { + return nil, err + } + + objectAttributes = append(objectAttributes, attribute) + continue + } + attribute, err := pSchema.BuildResourceAttribute(name, s.GetComputability(name)) if err != nil { return nil, err @@ -92,6 +105,19 @@ func (s *OASSchema) BuildDataSourceAttributes() (attrmapper.DataSourceAttributes return nil, s.NestSchemaError(err, name) } + // If this property references a schema already open on the descent path, it is a + // self-referential (recursive) schema. Degrade to a dynamic attribute instead of + // recursing forever (which stack overflows). + if s.GlobalSchemaOpts.IsVisitedRef(pProxy) { + attribute, err := pSchema.BuildDynamicDataSource(name, s.GetComputability(name)) + if err != nil { + return nil, err + } + + objectAttributes = append(objectAttributes, attribute) + continue + } + attribute, err := pSchema.BuildDataSourceAttribute(name, s.GetComputability(name)) if err != nil { return nil, err @@ -150,6 +176,19 @@ func (s *OASSchema) BuildProviderAttributes() (attrmapper.ProviderAttributes, *S return nil, s.NestSchemaError(err, name) } + // If this property references a schema already open on the descent path, it is a + // self-referential (recursive) schema. Degrade to a dynamic attribute instead of + // recursing forever (which stack overflows). + if s.GlobalSchemaOpts.IsVisitedRef(pProxy) { + attribute, err := pSchema.BuildDynamicProvider(name, s.GetOptionalOrRequired(name)) + if err != nil { + return nil, err + } + + objectAttributes = append(objectAttributes, attribute) + continue + } + attribute, err := pSchema.BuildProviderAttribute(name, s.GetOptionalOrRequired(name)) if err != nil { return nil, err diff --git a/internal/mapper/oas/build.go b/internal/mapper/oas/build.go index c077217d..ab411050 100644 --- a/internal/mapper/oas/build.go +++ b/internal/mapper/oas/build.go @@ -98,6 +98,14 @@ func getSchemaFromMediaType(mediaTypes *orderedmap.Map[string, *high.MediaType], func BuildSchema(proxy *base.SchemaProxy, schemaOpts SchemaOpts, globalOpts GlobalSchemaOpts) (*OASSchema, *SchemaError) { resp := OASSchema{} + // If the incoming schema is a reference ($ref), record it in the visited set as we + // descend through it. This is what lets deeper builders detect a reference cycle + // (a schema that transitively references itself) and degrade the recursive edge to + // a dynamic attribute instead of recursing forever. + if proxy != nil && proxy.IsReference() { + globalOpts = globalOpts.WithVisitedRef(proxy.GetReference()) + } + s, err := buildSchemaProxy(proxy, globalOpts) if err != nil { return nil, err diff --git a/internal/mapper/oas/build_test.go b/internal/mapper/oas/build_test.go index 095e6f4d..90a0dd73 100644 --- a/internal/mapper/oas/build_test.go +++ b/internal/mapper/oas/build_test.go @@ -1065,6 +1065,46 @@ func TestBuildSchema_AllOfSchemaComposition(t *testing.T) { }, }, }, + "allOf with multiple elements - merge properties from each subschema": { + schemaProxy: base.CreateSchemaProxy(&base.Schema{ + AllOf: []*base.SchemaProxy{ + base.CreateSchemaProxy(&base.Schema{ + Type: []string{"object"}, + Properties: orderedmap.ToOrderedMap(map[string]*base.SchemaProxy{ + "bool_prop": base.CreateSchemaProxy(&base.Schema{ + Type: []string{"boolean"}, + Description: "hey there! I'm a bool type.", + }), + }), + }), + base.CreateSchemaProxy(&base.Schema{ + Type: []string{"object"}, + Properties: orderedmap.ToOrderedMap(map[string]*base.SchemaProxy{ + "string_prop": base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + Description: "hey there! I'm a string type.", + }), + }), + }), + }, + }), + expectedAttributes: attrmapper.ResourceAttributes{ + &attrmapper.ResourceBoolAttribute{ + Name: "bool_prop", + BoolAttribute: resource.BoolAttribute{ + ComputedOptionalRequired: schema.ComputedOptional, + Description: pointer("hey there! I'm a bool type."), + }, + }, + &attrmapper.ResourceStringAttribute{ + Name: "string_prop", + StringAttribute: resource.StringAttribute{ + ComputedOptionalRequired: schema.ComputedOptional, + Description: pointer("hey there! I'm a string type."), + }, + }, + }, + }, } for name, testCase := range testCases { @@ -1134,19 +1174,6 @@ func TestBuildSchema_Errors(t *testing.T) { }), expectedErrRegex: `\[object string\] - unsupported multi-type, attribute cannot be created`, }, - "too many allOf": { - schemaProxy: base.CreateSchemaProxy(&base.Schema{ - AllOf: []*base.SchemaProxy{ - base.CreateSchemaProxy(&base.Schema{ - Type: []string{"null"}, - }), - base.CreateSchemaProxy(&base.Schema{ - Type: []string{"string"}, - }), - }, - }), - expectedErrRegex: `found 2 allOf subschema\(s\), schema composition is currently not supported`, - }, "too many anyOf": { schemaProxy: base.CreateSchemaProxy(&base.Schema{ AnyOf: []*base.SchemaProxy{ diff --git a/internal/mapper/oas/collection.go b/internal/mapper/oas/collection.go index bf0dc5e2..d2d33c9a 100644 --- a/internal/mapper/oas/collection.go +++ b/internal/mapper/oas/collection.go @@ -20,6 +20,13 @@ func (s *OASSchema) BuildCollectionResource(name string, computability schema.Co return nil, s.SchemaErrorFromProperty(errors.New("invalid array items property, doesn't have a schema"), name) } + // If the array item references a schema that is already open on the descent path, + // this is a self-referential (recursive) schema. Terraform has no recursive nested + // type, so degrade this edge to a dynamic attribute instead of recursing forever. + if s.GlobalSchemaOpts.IsVisitedRef(s.Schema.Items.A) { + return s.BuildDynamicResource(name, computability) + } + schemaOpts := SchemaOpts{ Ignores: s.SchemaOpts.Ignores, } @@ -119,6 +126,13 @@ func (s *OASSchema) BuildCollectionDataSource(name string, computability schema. return nil, s.SchemaErrorFromProperty(errors.New("invalid array items property, doesn't have a schema"), name) } + // If the array item references a schema that is already open on the descent path, + // this is a self-referential (recursive) schema. Terraform has no recursive nested + // type, so degrade this edge to a dynamic attribute instead of recursing forever. + if s.GlobalSchemaOpts.IsVisitedRef(s.Schema.Items.A) { + return s.BuildDynamicDataSource(name, computability) + } + schemaOpts := SchemaOpts{ Ignores: s.SchemaOpts.Ignores, } @@ -220,6 +234,13 @@ func (s *OASSchema) BuildCollectionProvider(name string, optionalOrRequired sche return nil, s.SchemaErrorFromProperty(errors.New("invalid array items property, doesn't have a schema"), name) } + // If the array item references a schema that is already open on the descent path, + // this is a self-referential (recursive) schema. Terraform has no recursive nested + // type, so degrade this edge to a dynamic attribute instead of recursing forever. + if s.GlobalSchemaOpts.IsVisitedRef(s.Schema.Items.A) { + return s.BuildDynamicProvider(name, optionalOrRequired) + } + schemaOpts := SchemaOpts{ Ignores: s.SchemaOpts.Ignores, } diff --git a/internal/mapper/oas/collection_test.go b/internal/mapper/oas/collection_test.go index e2f4d82a..bb48e1ee 100644 --- a/internal/mapper/oas/collection_test.go +++ b/internal/mapper/oas/collection_test.go @@ -2215,3 +2215,76 @@ func TestGetSetValidators(t *testing.T) { }) } } + +// TestBuildCollectionResource_RecursiveSchemaBecomesDynamic is a regression test for a +// stack overflow on self-referential OpenAPI schemas. A schema with an array property +// whose items $ref back to the enclosing schema (e.g. IngestColumn.nestedColumns) +// previously recursed forever because the attribute builder had no cycle tracking. +// +// With visited-ref tracking, once the referenced schema is already open on the descent +// path (recorded in GlobalSchemaOpts.VisitedRefs), the recursive array edge is degraded +// to a dynamic attribute instead of recursing. The build must return (not overflow) and +// the recursive property must be a *attrmapper.ResourceDynamicAttribute. +func TestBuildCollectionResource_RecursiveSchemaBecomesDynamic(t *testing.T) { + t.Parallel() + + // ref simulates the enclosing schema's own $ref location. It is seeded into + // VisitedRefs to model the fact that we have already descended through it once + // (as BuildSchema does when it resolves a reference), so re-entering it via the + // array items is a cycle. + ref := "#/components/schemas/IngestColumn" + + oasSchema := oas.OASSchema{ + Schema: &base.Schema{ + Type: []string{"object"}, + Properties: orderedmap.ToOrderedMap(map[string]*base.SchemaProxy{ + "name": base.CreateSchemaProxy(&base.Schema{ + Type: []string{"string"}, + Description: "hey there! I'm a string type.", + }), + "nested_columns": base.CreateSchemaProxy(&base.Schema{ + Type: []string{"array"}, + Description: "recursive nested columns", + // Items reference the enclosing schema, forming a cycle. + Items: &base.DynamicValue[*base.SchemaProxy, bool]{ + A: base.CreateSchemaProxyRef(ref), + }, + }), + }), + }, + GlobalSchemaOpts: oas.GlobalSchemaOpts{ + VisitedRefs: map[string]bool{ref: true}, + }, + } + + attributes, err := oasSchema.BuildResourceAttributes() + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + expectedAttributes := attrmapper.ResourceAttributes{ + &attrmapper.ResourceStringAttribute{ + Name: "name", + StringAttribute: resource.StringAttribute{ + ComputedOptionalRequired: schema.ComputedOptional, + Description: pointer("hey there! I'm a string type."), + }, + }, + &attrmapper.ResourceDynamicAttribute{ + Name: "nested_columns", + DynamicAttribute: resource.DynamicAttribute{ + ComputedOptionalRequired: schema.ComputedOptional, + Description: pointer("recursive nested columns"), + }, + }, + } + + if diff := cmp.Diff(attributes, expectedAttributes); diff != "" { + t.Errorf("unexpected difference: %s", diff) + } + + // The recursive edge must specifically be a dynamic attribute. + if _, ok := attributes[1].(*attrmapper.ResourceDynamicAttribute); !ok { + t.Errorf("expected nested_columns to be a *attrmapper.ResourceDynamicAttribute, got %T", attributes[1]) + } +} diff --git a/internal/mapper/oas/dynamic.go b/internal/mapper/oas/dynamic.go new file mode 100644 index 00000000..fc2bbe2c --- /dev/null +++ b/internal/mapper/oas/dynamic.go @@ -0,0 +1,59 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package oas + +import ( + "github.com/starburstdata/terraform-plugin-codegen-openapi/internal/mapper/attrmapper" + "github.com/hashicorp/terraform-plugin-codegen-spec/datasource" + "github.com/hashicorp/terraform-plugin-codegen-spec/provider" + "github.com/hashicorp/terraform-plugin-codegen-spec/resource" + "github.com/hashicorp/terraform-plugin-codegen-spec/schema" +) + +// BuildDynamicResource builds a dynamic attribute. It is used to represent a schema +// edge that cannot be expressed as a typed Terraform attribute - in particular a +// self-referential (recursive) schema, which Terraform's plugin framework has no +// recursive nested type for. Degrading such an edge to `dynamic` keeps the surrounding +// resource generatable and the recursive structure authorable. +func (s *OASSchema) BuildDynamicResource(name string, computability schema.ComputedOptionalRequired) (attrmapper.ResourceAttribute, *SchemaError) { + result := &attrmapper.ResourceDynamicAttribute{ + Name: name, + DynamicAttribute: resource.DynamicAttribute{ + ComputedOptionalRequired: computability, + DeprecationMessage: s.GetDeprecationMessage(), + Description: s.GetDescription(), + Sensitive: s.IsSensitive(), + }, + } + + return result, nil +} + +func (s *OASSchema) BuildDynamicDataSource(name string, computability schema.ComputedOptionalRequired) (attrmapper.DataSourceAttribute, *SchemaError) { + result := &attrmapper.DataSourceDynamicAttribute{ + Name: name, + DynamicAttribute: datasource.DynamicAttribute{ + ComputedOptionalRequired: computability, + DeprecationMessage: s.GetDeprecationMessage(), + Description: s.GetDescription(), + Sensitive: s.IsSensitive(), + }, + } + + return result, nil +} + +func (s *OASSchema) BuildDynamicProvider(name string, optionalOrRequired schema.OptionalRequired) (attrmapper.ProviderAttribute, *SchemaError) { + result := &attrmapper.ProviderDynamicAttribute{ + Name: name, + DynamicAttribute: provider.DynamicAttribute{ + OptionalRequired: optionalOrRequired, + DeprecationMessage: s.GetDeprecationMessage(), + Description: s.GetDescription(), + Sensitive: s.IsSensitive(), + }, + } + + return result, nil +} diff --git a/internal/mapper/oas/oas_schema.go b/internal/mapper/oas/oas_schema.go index a81e0113..e63f0132 100644 --- a/internal/mapper/oas/oas_schema.go +++ b/internal/mapper/oas/oas_schema.go @@ -39,6 +39,40 @@ type GlobalSchemaOpts struct { // DiscriminatorDepth tracks recursion depth to prevent infinite loops in discriminator resolution DiscriminatorDepth int + + // VisitedRefs tracks the set of schema $ref locations currently open on the + // descent path (keyed by the reference string, e.g. "#/components/schemas/IngestColumn"). + // It is used to detect self-referential (recursive) schemas: if a schema being + // expanded references a location that is already on the path, we have a cycle. + // Terraform's plugin framework has no recursive nested type, so such an edge is + // degraded to a `dynamic` attribute rather than recursing forever (which stack overflows). + VisitedRefs map[string]bool +} + +// WithVisitedRef returns a copy of the GlobalSchemaOpts with the given reference +// added to the VisitedRefs set. It allocates a new map so that sibling branches of +// the schema tree do not share mutation - i.e. adding a ref on one branch must not +// leak into the parent's map or into unrelated sibling branches. +func (o GlobalSchemaOpts) WithVisitedRef(ref string) GlobalSchemaOpts { + newVisited := make(map[string]bool, len(o.VisitedRefs)+1) + for k, v := range o.VisitedRefs { + newVisited[k] = v + } + newVisited[ref] = true + + o.VisitedRefs = newVisited + return o +} + +// IsVisitedRef reports whether the given schema proxy is a reference ($ref) to a +// location that has already been visited on the current descent path. When true, the +// proxy re-enters a type already being expanded, which is a reference cycle. +func (o GlobalSchemaOpts) IsVisitedRef(proxy *base.SchemaProxy) bool { + if proxy == nil || !proxy.IsReference() { + return false + } + + return o.VisitedRefs[proxy.GetReference()] } // SchemaOpts is NOT passed recursively through built OASSchema structs, and will only be available to the top level schema. This is used diff --git a/internal/mapper/provider_mapper_test.go b/internal/mapper/provider_mapper_test.go index 267ffa6e..4147fad9 100644 --- a/internal/mapper/provider_mapper_test.go +++ b/internal/mapper/provider_mapper_test.go @@ -250,7 +250,7 @@ func TestProviderMapper_basic(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - mapper := mapper.NewProviderMapper(testCase.exploredProvider, config.Config{}) + mapper := mapper.NewProviderMapper(testCase.exploredProvider, nil, config.Config{}) got, err := mapper.MapToIR(slog.Default()) if err != nil { t.Fatalf("unexpected error: %s", err) diff --git a/internal/mapper/resource_mapper_test.go b/internal/mapper/resource_mapper_test.go index 951dd80a..e49113df 100644 --- a/internal/mapper/resource_mapper_test.go +++ b/internal/mapper/resource_mapper_test.go @@ -1048,7 +1048,7 @@ func TestResourceMapper_basic_merges(t *testing.T) { ReadOp: createTestReadOp(testCase.readResponseSchema, testCase.readParams), SchemaOptions: testCase.schemaOptions, }, - }, config.Config{}) + }, nil, config.Config{}) got, err := mapper.MapToIR(slog.Default()) if err != nil { t.Fatalf("unexpected error: %s", err)