From c70a8f0f609f955893b614b9cc185ecf75c48c94 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 20:28:10 +0000 Subject: [PATCH 1/2] fix: emit dynamic attribute for self-referential schemas to prevent stack overflow Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_011drBEhNrtGfnXQNsedmJLa --- internal/mapper/attrmapper/dynamic.go | 91 ++++++++++++++++++++++++++ internal/mapper/oas/attribute.go | 39 +++++++++++ internal/mapper/oas/build.go | 8 +++ internal/mapper/oas/collection.go | 21 ++++++ internal/mapper/oas/collection_test.go | 73 +++++++++++++++++++++ internal/mapper/oas/dynamic.go | 59 +++++++++++++++++ internal/mapper/oas/oas_schema.go | 34 ++++++++++ 7 files changed, 325 insertions(+) create mode 100644 internal/mapper/attrmapper/dynamic.go create mode 100644 internal/mapper/oas/dynamic.go 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/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/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 From 74800ef2c3be5ff521e619c1f9ca929c80762d60 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 21:13:10 +0000 Subject: [PATCH 2/2] fix: repair pre-existing test failures unrelated to this branch Update NewResourceMapper/NewDataSourceMapper/NewProviderMapper test call sites to pass the *high.Document parameter added by the discriminator support commit (0dffe9d) but never updated in the mapper test files, which broke compilation of the internal/mapper test package. Also fix the stale TestBuildSchema_Errors/too_many_allOf case: that same commit added composeAllOfSchemas to legitimately support composing multiple allOf subschemas (for inheritance patterns), so allOf with more than one subschema is no longer an error. Remove the obsolete error-path fixture and add a passing coverage case to TestBuildSchema_AllOfSchemaComposition verifying multi-schema property merging. --- internal/mapper/datasource_mapper_test.go | 4 +- internal/mapper/oas/build_test.go | 53 +++++++++++++++++------ internal/mapper/provider_mapper_test.go | 2 +- internal/mapper/resource_mapper_test.go | 2 +- 4 files changed, 44 insertions(+), 17 deletions(-) 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/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/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)