Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: PoC - Model generation - Supporting nested schema (objects - Map, MapNested, SingleNested Attributes) #2704

94 changes: 82 additions & 12 deletions tools/codegen/codespec/api_to_provider_spec_mapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,6 @@ func TestConvertToProviderSpec_nested(t *testing.T) {
String: &codespec.StringAttribute{},
Description: conversion.StringPtr(testPathParamDesc),
},
{
Copy link
Collaborator Author

@maastha maastha Oct 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cleaning up some attrs so the test focuses on nested attrs only

Name: "id",
ComputedOptionalRequired: codespec.Computed,
String: &codespec.StringAttribute{},
Description: conversion.StringPtr(testFieldDesc),
},
{
Name: "list_primitive_string_attr",
ComputedOptionalRequired: codespec.Computed,
Expand All @@ -144,7 +138,7 @@ func TestConvertToProviderSpec_nested(t *testing.T) {
Description: conversion.StringPtr(testFieldDesc),
},
{
Name: "nested_object_array_attr",
Name: "nested_list_array_attr",
ComputedOptionalRequired: codespec.Required,
ListNested: &codespec.ListNestedAttribute{
NestedObject: codespec.NestedAttributeObject{
Expand All @@ -156,9 +150,43 @@ func TestConvertToProviderSpec_nested(t *testing.T) {
Description: conversion.StringPtr(testFieldDesc),
},
{
Name: "inner_str_attr",
ComputedOptionalRequired: codespec.Required,
Name: "list_primitive_string_attr",
ComputedOptionalRequired: codespec.Optional,
List: &codespec.ListAttribute{
ElementType: codespec.String,
},
Description: conversion.StringPtr(testFieldDesc),
},
},
},
},
Description: conversion.StringPtr(testFieldDesc),
},
{
Name: "nested_map_object_attr",
ComputedOptionalRequired: codespec.Computed,
MapNested: &codespec.MapNestedAttribute{
NestedObject: codespec.NestedAttributeObject{
Attributes: codespec.Attributes{
{
Name: "attr",
ComputedOptionalRequired: codespec.Optional,
String: &codespec.StringAttribute{},
},
},
},
},
},
{
Name: "nested_set_array_attr",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added nested set attr

ComputedOptionalRequired: codespec.Computed,
SetNested: &codespec.SetNestedAttribute{
NestedObject: codespec.NestedAttributeObject{
Attributes: codespec.Attributes{
{
Name: "inner_num_attr",
ComputedOptionalRequired: codespec.Required,
Int64: &codespec.Int64Attribute{},
Description: conversion.StringPtr(testFieldDesc),
},
{
Expand All @@ -183,10 +211,52 @@ func TestConvertToProviderSpec_nested(t *testing.T) {
Description: conversion.StringPtr(testFieldDesc),
},
{
Name: "str_computed_attr",
Name: "single_nested_attr",
ComputedOptionalRequired: codespec.Computed,
String: &codespec.StringAttribute{},
Description: conversion.StringPtr(testFieldDesc),
SingleNested: &codespec.SingleNestedAttribute{
NestedObject: codespec.NestedAttributeObject{
Attributes: codespec.Attributes{
{
Name: "inner_int_attr",
ComputedOptionalRequired: codespec.Required,
Int64: &codespec.Int64Attribute{},
Description: conversion.StringPtr(testFieldDesc),
},
{
Name: "inner_str_attr",
ComputedOptionalRequired: codespec.Required,
String: &codespec.StringAttribute{},
Description: conversion.StringPtr(testFieldDesc),
},
},
},
},
Description: conversion.StringPtr(testFieldDesc),
},
{
Name: "single_nested_attr_with_nested_maps",
ComputedOptionalRequired: codespec.Computed,
SingleNested: &codespec.SingleNestedAttribute{
NestedObject: codespec.NestedAttributeObject{
Attributes: codespec.Attributes{
{
Name: "map_attr1",
ComputedOptionalRequired: codespec.Optional,
Map: &codespec.MapAttribute{
ElementType: codespec.String,
},
},
{
Name: "map_attr2",
ComputedOptionalRequired: codespec.Optional,
Map: &codespec.MapAttribute{
ElementType: codespec.String,
},
},
},
},
},
Description: conversion.StringPtr(testFieldDesc),
},
},
},
Expand Down
62 changes: 61 additions & 1 deletion tools/codegen/codespec/attribute.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ func (s *APISpecSchema) buildResourceAttr(name string, computability ComputedOpt
case OASTypeArray:
return s.buildArrayAttr(name, computability)
case OASTypeObject:
return nil, nil // TODO: add support for SingleNestedObject and MapAttribute
if s.Schema.AdditionalProperties != nil && s.Schema.AdditionalProperties.IsA() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: what does IsA() imply?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

libopenapi stores additionalProperties as Dynamic values (AdditionalProperties *DynamicValue[*SchemaProxy, bool] ) defined here https://github.com/pb33f/libopenapi/blob/eff4ce83e7814228bc50e34e5e60c4312a041e98/datamodel/high/base/dynamic_value.go#L20 "The left value (A) is a 3.0 schema property value"

return s.buildMapAttr(name, computability)
}
return s.buildSingleNestedAttr(name, computability)
default:
return nil, fmt.Errorf("invalid schema type '%s'", s.Type)
}
Expand Down Expand Up @@ -210,3 +213,60 @@ func (s *APISpecSchema) buildArrayAttr(name string, computability ComputedOption

return result, nil
}

func (s *APISpecSchema) buildMapAttr(name string, computability ComputedOptionalRequired) (*Attribute, error) {
mapSchema, err := BuildSchema(s.Schema.AdditionalProperties.A)
if err != nil {
return nil, err
}

result := &Attribute{
Name: terraformAttrName(name),
ComputedOptionalRequired: computability,
DeprecationMessage: s.GetDeprecationMessage(),
Description: s.GetDescription(),
}

if mapSchema.Type == OASTypeObject {
mapAttributes, err := buildResourceAttrs(mapSchema)
if err != nil {
return nil, err
}

result.MapNested = &MapNestedAttribute{
NestedObject: NestedAttributeObject{
Attributes: mapAttributes,
},
}
} else {
elemType, err := mapSchema.buildElementType()
if err != nil {
return nil, err
}

result.Map = &MapAttribute{
ElementType: elemType,
}
}

return result, nil
}

func (s *APISpecSchema) buildSingleNestedAttr(name string, computability ComputedOptionalRequired) (*Attribute, error) {
objectAttributes, err := buildResourceAttrs(s)
if err != nil {
return nil, err
}

return &Attribute{
Name: terraformAttrName(name),
ComputedOptionalRequired: computability,
DeprecationMessage: s.GetDeprecationMessage(),
Description: s.GetDescription(),
SingleNested: &SingleNestedAttribute{
NestedObject: NestedAttributeObject{
Attributes: objectAttributes,
},
},
}, nil
}
70 changes: 54 additions & 16 deletions tools/codegen/codespec/testdata/api-spec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -328,20 +328,19 @@ components:
minLength: 24
pattern: ^([a-f0-9]{24})$
readOnly: true
id:
type: string
nestedListArrayAttr:
type: array
description: Test field description
example: 32b6e34b3d91647abb20e7b8
maxLength: 24
minLength: 24
pattern: ^([a-f0-9]{24})$
items:
$ref: "#/components/schemas/NestedObjectAttr"
readOnly: true
nestedObjectArrayAttr:
nestedSetArrayAttr:
type: array
description: Test field description
items:
$ref: "#/components/schemas/NestedObjectAttr"
readOnly: true
uniqueItems: true
setPrimitiveStringAttr:
type: array
description: Test field description
Expand All @@ -353,16 +352,45 @@ components:
description: Test field description
items:
type: string
strComputedAttr:
type: string
description: Test field description
singleNestedAttrWithNestedMaps:
$ref: "#/components/schemas/SingleNestedAttrWithNestedMaps"
singleNestedAttr:
$ref: "#/components/schemas/SingleNestedAttr"
nestedMapObjectAttr:
$ref: "#/components/schemas/NestedMapObjectAttr"
SingleNestedAttrWithNestedMaps:
type: object
description: Test field description
properties:
mapAttr1:
type: object
additionalProperties:
type: string
readOnly: true
readOnly: true
NestedObjectAttr:
mapAttr2:
type: object
additionalProperties:
type: string
readOnly: true
readOnly: true
readOnly: true
title: Outbound Control Plane IP Addresses By Cloud Provider
SingleNestedAttr:
type: object
description: Test field description
properties:
innerStrAttr:
type: string
innerIntAttr:
type: integer
description: Test field description
innerStrAttr:
$ref: "#/components/schemas/SimpleStringRefObject"
required:
- innerIntAttr
- innerStrAttr
NestedObjectAttr:
type: object
properties:
innerNumAttr:
type: integer
format: int32
Expand All @@ -376,20 +404,30 @@ components:
items:
type: string
required:
- innerStrAttr
- innerNumAttr
NestedTestResourceRequest:
type: object
properties:
nestedObjectArrayAttr:
nestedListArrayAttr:
type: array
description: Test field description
items:
$ref: "#/components/schemas/NestedObjectAttr"
maxItems: 1
minItems: 1
required:
- nestedObjectArrayAttr
- nestedListArrayAttr
SimpleStringRefObject:
type: string
description: Test field description
NoBody:
type: object
description: Endpoint does not return a response body.
NestedMapObjectAttr:
type: object
additionalProperties:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: curious if in our api spec there is use of additionalProperties for representing generic maps.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great 👍

type: object
properties:
attr:
type: string