Skip to content

Commit bef196a

Browse files
austinvalleaustinvalle
authored
Refactor internal building of schemas and improve error messaging (#73)
* initial refactoring of build schema + errors * don't shadow errors * fixed bug where nested schema composition wasn't being resolved properly * added better errors for invalid terraform identifiers * protect from GoLow * add tests and adjust some interfaces * upgrade to v0.1.0 release
1 parent c41858d commit bef196a

22 files changed

+606
-350
lines changed

‎go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ module github.com/hashicorp/terraform-plugin-codegen-openapi
33
go 1.21
44

55
require (
6-
github.com/google/go-cmp v0.5.9
7-
github.com/hashicorp/terraform-plugin-codegen-spec v0.0.0-20230927170433-b779baa44f04
6+
github.com/google/go-cmp v0.6.0
7+
github.com/hashicorp/terraform-plugin-codegen-spec v0.1.0
88
github.com/mattn/go-colorable v0.1.13
99
github.com/mitchellh/cli v1.1.5
1010
github.com/pb33f/libopenapi v0.11.0

‎go.sum

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMyw
4242
github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
4343
github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
4444
github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
45-
github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
46-
github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
45+
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
46+
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
4747
github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE=
4848
github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
4949
github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
@@ -55,8 +55,8 @@ github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brv
5555
github.com/hashicorp/go-multierror v1.0.0/go.mod h1:dHtQlpGsu+cZNNAkkCN/P3hoUDHhCYQXV3UM06sGGrk=
5656
github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo=
5757
github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM=
58-
github.com/hashicorp/terraform-plugin-codegen-spec v0.0.0-20230927170433-b779baa44f04 h1:WrMQeSAMqgVEqqBvqL0hSBYy2uk2qn9YtwlFgn1vRjk=
59-
github.com/hashicorp/terraform-plugin-codegen-spec v0.0.0-20230927170433-b779baa44f04/go.mod h1:4aZFKiOI2xiQFOpeQMMyDL8vRmKAZXHUY4kWol7Fdco=
58+
github.com/hashicorp/terraform-plugin-codegen-spec v0.1.0 h1:flL5dprli2h54RxewQi6po02am0zXDRq6nsV6c4WQ/I=
59+
github.com/hashicorp/terraform-plugin-codegen-spec v0.1.0/go.mod h1:PQn6bDD8UWoAVJoHXqFk2i/RmLbeQBjbiP38i+E+YIw=
6060
github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU=
6161
github.com/huandu/xstrings v1.3.1/go.mod h1:y5/lhBue+AyNmUVz9RLU9xbLR0o4KIIExikq4ovT0aE=
6262
github.com/huandu/xstrings v1.3.2/go.mod h1:y5/lhBue+AyNmUVz9RLU9xbLR0o4KIIExikq4ovT0aE=

‎internal/cmd/testdata/kubernetes/provider_code_spec.json

Lines changed: 76 additions & 38 deletions
Large diffs are not rendered by default.

‎internal/log/warn.go

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,14 @@ func WarnLogOnError(logger *slog.Logger, err error, message string) {
1616
return
1717
}
1818

19-
var propErr *oas.PropertyError
20-
21-
if errors.As(err, &propErr) {
22-
logger.Warn(
23-
message,
24-
"err", err,
25-
"oas_property", propErr.Path(),
26-
"oas_line_number", propErr.LineNumber(),
27-
)
28-
29-
return
19+
var schemaErr *oas.SchemaError
20+
if errors.As(err, &schemaErr) {
21+
if schemaErr.Path() != "" {
22+
logger = logger.With("oas_path", schemaErr.Path())
23+
}
24+
if schemaErr.LineNumber() != 0 {
25+
logger = logger.With("oas_line_number", schemaErr.LineNumber())
26+
}
3027
}
3128

3229
logger.Warn(message, "err", err)

‎internal/mapper/datasource_mapper.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,9 @@ func generateDataSourceSchema(logger *slog.Logger, dataSource explorer.DataSourc
7272
if err != nil {
7373
return nil, err
7474
}
75-
readResponseAttributes, propErr := readResponseSchema.BuildDataSourceAttributes()
76-
if propErr != nil {
77-
return nil, propErr
75+
readResponseAttributes, schemaErr := readResponseSchema.BuildDataSourceAttributes()
76+
if schemaErr != nil {
77+
return nil, schemaErr
7878
}
7979

8080
// ****************
@@ -90,9 +90,9 @@ func generateDataSourceSchema(logger *slog.Logger, dataSource explorer.DataSourc
9090
pLogger := logger.With("param", param.Name)
9191
schemaOpts := oas.SchemaOpts{OverrideDescription: param.Description}
9292

93-
s, err := oas.BuildSchema(param.Schema, schemaOpts, oas.GlobalSchemaOpts{})
94-
if err != nil {
95-
log.WarnLogOnError(pLogger, err, "skipping mapping of read operation parameter")
93+
s, schemaErr := oas.BuildSchema(param.Schema, schemaOpts, oas.GlobalSchemaOpts{})
94+
if schemaErr != nil {
95+
log.WarnLogOnError(pLogger, schemaErr, "skipping mapping of read operation parameter")
9696
continue
9797
}
9898

@@ -108,9 +108,9 @@ func generateDataSourceSchema(logger *slog.Logger, dataSource explorer.DataSourc
108108
paramName = aliasedName
109109
}
110110

111-
parameterAttribute, propErr := s.BuildDataSourceAttribute(paramName, computability)
112-
if propErr != nil {
113-
log.WarnLogOnError(pLogger, propErr, "skipping mapping of read operation parameter")
111+
parameterAttribute, schemaErr := s.BuildDataSourceAttribute(paramName, computability)
112+
if schemaErr != nil {
113+
log.WarnLogOnError(pLogger, schemaErr, "skipping mapping of read operation parameter")
114114
continue
115115
}
116116

‎internal/mapper/oas/attribute.go

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111
"github.com/hashicorp/terraform-plugin-codegen-spec/schema"
1212
)
1313

14-
func (s *OASSchema) BuildResourceAttributes() (attrmapper.ResourceAttributes, *PropertyError) {
14+
func (s *OASSchema) BuildResourceAttributes() (attrmapper.ResourceAttributes, *SchemaError) {
1515
objectAttributes := attrmapper.ResourceAttributes{}
1616

1717
// TODO: throw error if it's not an object?
@@ -23,12 +23,12 @@ func (s *OASSchema) BuildResourceAttributes() (attrmapper.ResourceAttributes, *P
2323
pProxy := s.Schema.Properties[name]
2424
pSchema, err := BuildSchema(pProxy, SchemaOpts{}, s.GlobalSchemaOpts)
2525
if err != nil {
26-
return nil, s.NewPropertyError(err, name)
26+
return nil, s.NestSchemaError(err, name)
2727
}
2828

29-
attribute, propErr := pSchema.BuildResourceAttribute(name, s.GetComputability(name))
30-
if propErr != nil {
31-
return nil, propErr
29+
attribute, err := pSchema.BuildResourceAttribute(name, s.GetComputability(name))
30+
if err != nil {
31+
return nil, err
3232
}
3333

3434
objectAttributes = append(objectAttributes, attribute)
@@ -37,7 +37,11 @@ func (s *OASSchema) BuildResourceAttributes() (attrmapper.ResourceAttributes, *P
3737
return objectAttributes, nil
3838
}
3939

40-
func (s *OASSchema) BuildResourceAttribute(name string, computability schema.ComputedOptionalRequired) (attrmapper.ResourceAttribute, *PropertyError) {
40+
func (s *OASSchema) BuildResourceAttribute(name string, computability schema.ComputedOptionalRequired) (attrmapper.ResourceAttribute, *SchemaError) {
41+
if util.TerraformIdentifier(name) == "" {
42+
return nil, s.SchemaErrorFromProperty(fmt.Errorf("'%s' cannot be converted to a valid Terraform identifier", name), name)
43+
}
44+
4145
switch s.Type {
4246
case util.OAS_type_string:
4347
return s.BuildStringResource(name, computability)
@@ -55,11 +59,11 @@ func (s *OASSchema) BuildResourceAttribute(name string, computability schema.Com
5559
}
5660
return s.BuildSingleNestedResource(name, computability)
5761
default:
58-
return nil, s.NewPropertyError(fmt.Errorf("invalid schema type '%s'", s.Type), name)
62+
return nil, s.SchemaErrorFromProperty(fmt.Errorf("invalid schema type '%s'", s.Type), name)
5963
}
6064
}
6165

62-
func (s *OASSchema) BuildDataSourceAttributes() (attrmapper.DataSourceAttributes, *PropertyError) {
66+
func (s *OASSchema) BuildDataSourceAttributes() (attrmapper.DataSourceAttributes, *SchemaError) {
6367
objectAttributes := attrmapper.DataSourceAttributes{}
6468

6569
// TODO: throw error if it's not an object?
@@ -71,12 +75,12 @@ func (s *OASSchema) BuildDataSourceAttributes() (attrmapper.DataSourceAttributes
7175
pProxy := s.Schema.Properties[name]
7276
pSchema, err := BuildSchema(pProxy, SchemaOpts{}, s.GlobalSchemaOpts)
7377
if err != nil {
74-
return nil, s.NewPropertyError(err, name)
78+
return nil, s.NestSchemaError(err, name)
7579
}
7680

77-
attribute, propErr := pSchema.BuildDataSourceAttribute(name, s.GetComputability(name))
81+
attribute, err := pSchema.BuildDataSourceAttribute(name, s.GetComputability(name))
7882
if err != nil {
79-
return nil, propErr
83+
return nil, err
8084
}
8185

8286
objectAttributes = append(objectAttributes, attribute)
@@ -85,7 +89,11 @@ func (s *OASSchema) BuildDataSourceAttributes() (attrmapper.DataSourceAttributes
8589
return objectAttributes, nil
8690
}
8791

88-
func (s *OASSchema) BuildDataSourceAttribute(name string, computability schema.ComputedOptionalRequired) (attrmapper.DataSourceAttribute, *PropertyError) {
92+
func (s *OASSchema) BuildDataSourceAttribute(name string, computability schema.ComputedOptionalRequired) (attrmapper.DataSourceAttribute, *SchemaError) {
93+
if util.TerraformIdentifier(name) == "" {
94+
return nil, s.SchemaErrorFromProperty(fmt.Errorf("'%s' cannot be converted to a valid Terraform identifier", name), name)
95+
}
96+
8997
switch s.Type {
9098
case util.OAS_type_string:
9199
return s.BuildStringDataSource(name, computability)
@@ -103,11 +111,11 @@ func (s *OASSchema) BuildDataSourceAttribute(name string, computability schema.C
103111
}
104112
return s.BuildSingleNestedDataSource(name, computability)
105113
default:
106-
return nil, s.NewPropertyError(fmt.Errorf("invalid schema type '%s'", s.Type), name)
114+
return nil, s.SchemaErrorFromProperty(fmt.Errorf("invalid schema type '%s'", s.Type), name)
107115
}
108116
}
109117

110-
func (s *OASSchema) BuildProviderAttributes() (attrmapper.ProviderAttributes, *PropertyError) {
118+
func (s *OASSchema) BuildProviderAttributes() (attrmapper.ProviderAttributes, *SchemaError) {
111119
objectAttributes := attrmapper.ProviderAttributes{}
112120

113121
// TODO: throw error if it's not an object?
@@ -119,12 +127,12 @@ func (s *OASSchema) BuildProviderAttributes() (attrmapper.ProviderAttributes, *P
119127
pProxy := s.Schema.Properties[name]
120128
pSchema, err := BuildSchema(pProxy, SchemaOpts{}, s.GlobalSchemaOpts)
121129
if err != nil {
122-
return nil, s.NewPropertyError(err, name)
130+
return nil, s.NestSchemaError(err, name)
123131
}
124132

125-
attribute, propErr := pSchema.BuildProviderAttribute(name, s.GetOptionalOrRequired(name))
133+
attribute, err := pSchema.BuildProviderAttribute(name, s.GetOptionalOrRequired(name))
126134
if err != nil {
127-
return nil, propErr
135+
return nil, err
128136
}
129137

130138
objectAttributes = append(objectAttributes, attribute)
@@ -133,7 +141,11 @@ func (s *OASSchema) BuildProviderAttributes() (attrmapper.ProviderAttributes, *P
133141
return objectAttributes, nil
134142
}
135143

136-
func (s *OASSchema) BuildProviderAttribute(name string, optionalOrRequired schema.OptionalRequired) (attrmapper.ProviderAttribute, *PropertyError) {
144+
func (s *OASSchema) BuildProviderAttribute(name string, optionalOrRequired schema.OptionalRequired) (attrmapper.ProviderAttribute, *SchemaError) {
145+
if util.TerraformIdentifier(name) == "" {
146+
return nil, s.SchemaErrorFromProperty(fmt.Errorf("'%s' cannot be converted to a valid Terraform identifier", name), name)
147+
}
148+
137149
switch s.Type {
138150
case util.OAS_type_string:
139151
return s.BuildStringProvider(name, optionalOrRequired)
@@ -151,6 +163,6 @@ func (s *OASSchema) BuildProviderAttribute(name string, optionalOrRequired schem
151163
}
152164
return s.BuildSingleNestedProvider(name, optionalOrRequired)
153165
default:
154-
return nil, s.NewPropertyError(fmt.Errorf("invalid schema type '%s'", s.Type), name)
166+
return nil, s.SchemaErrorFromProperty(fmt.Errorf("invalid schema type '%s'", s.Type), name)
155167
}
156168
}

‎internal/mapper/oas/bool.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111
"github.com/hashicorp/terraform-plugin-codegen-spec/schema"
1212
)
1313

14-
func (s *OASSchema) BuildBoolResource(name string, computability schema.ComputedOptionalRequired) (attrmapper.ResourceAttribute, *PropertyError) {
14+
func (s *OASSchema) BuildBoolResource(name string, computability schema.ComputedOptionalRequired) (attrmapper.ResourceAttribute, *SchemaError) {
1515
result := &attrmapper.ResourceBoolAttribute{
1616
Name: name,
1717
BoolAttribute: resource.BoolAttribute{
@@ -38,7 +38,7 @@ func (s *OASSchema) BuildBoolResource(name string, computability schema.Computed
3838
return result, nil
3939
}
4040

41-
func (s *OASSchema) BuildBoolDataSource(name string, computability schema.ComputedOptionalRequired) (attrmapper.DataSourceAttribute, *PropertyError) {
41+
func (s *OASSchema) BuildBoolDataSource(name string, computability schema.ComputedOptionalRequired) (attrmapper.DataSourceAttribute, *SchemaError) {
4242
result := &attrmapper.DataSourceBoolAttribute{
4343
Name: name,
4444
BoolAttribute: datasource.BoolAttribute{
@@ -51,7 +51,7 @@ func (s *OASSchema) BuildBoolDataSource(name string, computability schema.Comput
5151
return result, nil
5252
}
5353

54-
func (s *OASSchema) BuildBoolProvider(name string, optionalOrRequired schema.OptionalRequired) (attrmapper.ProviderAttribute, *PropertyError) {
54+
func (s *OASSchema) BuildBoolProvider(name string, optionalOrRequired schema.OptionalRequired) (attrmapper.ProviderAttribute, *SchemaError) {
5555
return &attrmapper.ProviderBoolAttribute{
5656
Name: name,
5757
BoolAttribute: provider.BoolAttribute{
@@ -62,7 +62,7 @@ func (s *OASSchema) BuildBoolProvider(name string, optionalOrRequired schema.Opt
6262
}, nil
6363
}
6464

65-
func (s *OASSchema) BuildBoolElementType() (schema.ElementType, *PropertyError) {
65+
func (s *OASSchema) BuildBoolElementType() (schema.ElementType, *SchemaError) {
6666
return schema.ElementType{
6767
Bool: &schema.BoolType{},
6868
}, nil

0 commit comments

Comments
 (0)