diff --git a/README.md b/README.md index e377dfb..bd99bc3 100644 --- a/README.md +++ b/README.md @@ -1,26 +1,40 @@ # terraform-plugin-framework-codegen -A community CLI tool `tfpfgen` that turns an OpenAPI 3 document into a complete +A community CLI tool `tfpfgen` that turns an OpenAPI 3 document into a complete, opinionated, [terraform-plugin-framework](https://github.com/hashicorp/terraform-plugin-framework) -provider, zero touch. An operator supplies a spec URL and API credentials; -everything else cascades through a GitHub Actions pipeline: +terraform provider, zero touch. An operator supplies a spec source URL and API credentials for discovery; +everything else is cascaded through a GitHub Actions pipeline: ``` config validate → spec import → audit run → spec revise → sdk generate → provider generate → verify → PR ``` -An OpenAPI document tells you what an API's fields are *called*. It does not -tell you what makes a Terraform provider actually work. So the pipeline -**audits** the live API, minimum and maximum valid configuration, field -dependencies, value-conditional rules and records **observations**. Those -observations become proposed **corrections** to the spec (RFC-6902 operations +It's highly recconmended that you use this tool against a lab instance as the +generation workflow creates, updates and delete real resources within your +instance by design as part of the generation workflow. + +## Why + +Building terraform providers is a valuable but time consuming exercise. From +experience there are lots of challenges with building them. Including: a lack +of provider building experience, a lack of golang experience, api behaviours +rarely match documentation with frequent inconsistencies to varying degrees +of impact, api's frequently change and so too then does a terraform provider. + +So how do we solve for this ? Realistically the only way to have a fighting +chance is if the vendor provides and maintains an OpenApi3 specification +of their api. However while an OpenAPI document tells you what an API's fields are *called*. It does not +tell you what makes a Terraform provider actually work. So this tools workflow pipeline +**audits** the live API, attempts to identify minimum and maximum valid configuration, inter-field +dependencies, value-conditional rules and records them as **observations**. Those +observations then become proposed **corrections** to the Open Api spec if needed (RFC-6902 operations with a justification and a pointer to the observation that proves them). The -**revised spec** is the single source of truth from which both the SDK and the +**revised spec** is then used as a single source of truth from which both the SDK and the provider are generated. Every generated file is exactly that, generated; -human judgment enters only as data: `tfpfgen.yaml`, accepted corrections, and +human judgment enters only as configuratioin options within: `tfpfgen.yaml`, accepted corrections, and audit inputs. -Generation happens twice, and the second half is what makes the output real. +Generation happens in two phases, and the second half is what makes the output usable. An SDK is generated first, by the configured backend. The provider is then generated against that SDK and resolved onto it with `go/types`: every call expression, accessor and model type is checked against the SDK that was diff --git a/internal/audit/plan/derive_test.go b/internal/audit/plan/derive_test.go index e857201..ed436f6 100644 --- a/internal/audit/plan/derive_test.go +++ b/internal/audit/plan/derive_test.go @@ -575,3 +575,41 @@ func TestUnit_Plan_RunBudget(t *testing.T) { t.Errorf("empty-plan duration = %q, want the floor", got.Duration) } } + +// TestUnit_Plan_ASingletonIsRefusedNotPanicked pins the shape that crashed a +// live run: a resource whose create slot is empty because it is written +// through its update call. One entity is refused; the rest of the run stands. +func TestUnit_Plan_ASingletonIsRefusedNotPanicked(t *testing.T) { + doc := loadDoc(t, ` +openapi: 3.0.1 +info: {title: t, version: "1"} +paths: + /settings: + get: + responses: + "200": + content: + application/json: + schema: {type: object, properties: {name: {type: string}}} + put: + requestBody: + content: + application/json: + schema: {type: object, properties: {name: {type: string}}} + responses: + "200": + content: + application/json: + schema: {type: object, properties: {name: {type: string}}} +`) + p, err := Derive(doc, testConfig(), &Inputs{}) + if err != nil { + t.Fatalf("Derive: %v", err) + } + for _, s := range p.Skipped { + if strings.Contains(s.Reason, "no create operation") { + return + } + } + t.Fatalf("the singleton was not refused with a reason; skipped: %+v", p.Skipped) +} diff --git a/internal/audit/plan/steps.go b/internal/audit/plan/steps.go index 97efaeb..fd77899 100644 --- a/internal/audit/plan/steps.go +++ b/internal/audit/plan/steps.go @@ -37,6 +37,17 @@ func (d *deriver) resourcePlan(c specmodel.Classification) (EntityPlan, *Skipped return EntityPlan{}, &Skipped{Entity: c.Key, Reason: reason} } + // A singleton is a resource with no create operation: it is written + // through its update call, and the classification leaves the create slot + // empty. Auditing that shape is not derived yet, so the entity is + // refused rather than the run. + if createOp == nil { + return EntityPlan{}, &Skipped{ + Entity: c.Key, + Reason: "the entity has no create operation to exercise: a singleton is written through its update call, which the audit does not derive", + } + } + createSchema := createOp.RequestBody minimal, reason := sy.minimalBody(createSchema) if reason != "" { diff --git a/internal/emit/render_action.go b/internal/emit/render_action.go index 2948ed3..2d91692 100644 --- a/internal/emit/render_action.go +++ b/internal/emit/render_action.go @@ -90,6 +90,9 @@ func (e *serviceRenderer) action(a *ir.Action, ab *sdkbind.ActionBinding) ([]Fil if strings.Contains(d.Models, "types.") { modelImports.add("", "github.com/hashicorp/terraform-plugin-framework/types") } + if strings.Contains(d.Models, "attr.") { + modelImports.add("", "github.com/hashicorp/terraform-plugin-framework/attr") + } d.ModelImports = modelImports.render() invokeImports := newImportSet(e.pc.Module) @@ -100,13 +103,16 @@ func (e *serviceRenderer) action(a *ir.Action, ab *sdkbind.ActionBinding) ([]Fil if d.HasBody { d.ConstructReturnType = "*" + ab.WriteModel d.WriteConstructor = ab.WriteConstructor - body, usesFmt, cerr := constructLines(bodyNodes, "data", "body", "", 1, false) + body, usesFmt, cerr := constructLinesFor(bodyNodes, d.Pascal, "data", "body", "", 1, false) if cerr != nil { return nil, cerr } d.ConstructBody = body if usesFmt { invokeImports.add("", "fmt") + if strings.Contains(body, "basetypes.") { + invokeImports.add("", "github.com/hashicorp/terraform-plugin-framework/types/basetypes") + } } if strings.Contains(body, "convert.") { invokeImports.add("", e.pc.Module+"/internal/services/common/convert") diff --git a/internal/emit/render_datasource.go b/internal/emit/render_datasource.go index a0e754c..cbd840d 100644 --- a/internal/emit/render_datasource.go +++ b/internal/emit/render_datasource.go @@ -218,6 +218,10 @@ func (e *serviceRenderer) lookupDatasource(d *datasourceData, ds *ir.Datasource, d.StateBody = stateBody stateImports := newImportSet(e.pc.Module) stateImports.add("", "context") + stateImports.add("", "github.com/hashicorp/terraform-plugin-framework/diag") + if strings.Contains(stateBody, "types.") { + stateImports.add("", "github.com/hashicorp/terraform-plugin-framework/types") + } if strings.Contains(stateBody, "convert.") { stateImports.add("", e.pc.Module+"/internal/services/common/convert") } @@ -379,6 +383,10 @@ func (e *serviceRenderer) companionDatasource(d *datasourceData, ds *ir.Datasour d.MapItemBody = mapBody stateImports := newImportSet(e.pc.Module) stateImports.add("", "context") + stateImports.add("", "github.com/hashicorp/terraform-plugin-framework/diag") + if strings.Contains(mapBody, "types.") { + stateImports.add("", "github.com/hashicorp/terraform-plugin-framework/types") + } if strings.Contains(mapBody, "convert.") { stateImports.add("", e.pc.Module+"/internal/services/common/convert") } @@ -442,6 +450,9 @@ func (e *serviceRenderer) datasourceModelImports(models string) string { if strings.Contains(models, "types.") { imports.add("", "github.com/hashicorp/terraform-plugin-framework/types") } + if strings.Contains(models, "attr.") { + imports.add("", "github.com/hashicorp/terraform-plugin-framework/attr") + } return imports.render() } diff --git a/internal/emit/render_mapping.go b/internal/emit/render_mapping.go index 8621e14..034d631 100644 --- a/internal/emit/render_mapping.go +++ b/internal/emit/render_mapping.go @@ -25,12 +25,25 @@ func errReturn(attrPath string) string { return fmt.Sprintf("return nil, fmt.Errorf(\"the %s attribute: %%w\", err)", attrPath) } +// diagReturn is errReturn for a failure reported as diagnostics rather than +// an error: decoding a plan object into its generated struct. +func diagReturn(attrPath string) string { + return fmt.Sprintf("return nil, fmt.Errorf(\"the %s attribute: %%v\", diags)", attrPath) +} + // constructLines renders the body statements mapping one level of plan // fields onto the SDK write model. src is the model expression // ("data", "data.Settings"), dst the settable SDK value ("body"), // gateUpdates wraps attributes updates silently discard in an isCreate // guard. -func constructLines(nodes []node, src, dst, attrPrefix string, depth int, gateUpdates bool) (string, bool, error) { +// constructLinesFor is constructLines' entry point, resolving the entity's +// nested model names once so a decoded plan object is spelled the same here +// as in the model declaration. +func constructLinesFor(nodes []node, modelPrefix, src, dst, attrPrefix string, depth int, gateUpdates bool) (string, bool, error) { + return constructLines(newModelNamer(modelPrefix, nodes), "", nodes, src, dst, attrPrefix, depth, gateUpdates) +} + +func constructLines(namer *modelNamer, path string, nodes []node, src, dst, attrPrefix string, depth int, gateUpdates bool) (string, bool, error) { var b strings.Builder usesFmt := false indent := strings.Repeat("\t", depth) @@ -48,7 +61,7 @@ func constructLines(nodes []node, src, dst, attrPrefix string, depth int, gateUp var err error var nestedUsesFmt bool if n.attr.Nested != nil { - lines, nestedUsesFmt, err = constructNested(n, src, dst, attrPath, depth) + lines, nestedUsesFmt, err = constructNested(namer, childPath(path, n), n, src, dst, attrPath, depth) usesFmt = usesFmt || nestedUsesFmt } else if strings.HasSuffix(n.fb.Access.ConvertSet, "MapAdditionalData") { lines, err = constructAdditionalDataMap(n, src, dst, attrPath, indent) @@ -112,7 +125,7 @@ func constructScalar(n node, src, dst, attrPath, indent string) (string, bool, e } // constructNested renders a nested object or list-of-objects write. -func constructNested(n node, src, dst, attrPath string, depth int) (string, bool, error) { +func constructNested(namer *modelNamer, path string, n node, src, dst, attrPath string, depth int) (string, bool, error) { indent := strings.Repeat("\t", depth) field := src + "." + ir.GoName(n.attr.Name) // Construction builds the type the setter takes. That is usually the @@ -135,7 +148,8 @@ func constructNested(n node, src, dst, attrPath string, depth int) (string, bool indexVar := "index" + depthSuffix(depth) elemVar := lowerCamel(n.attr.Name) + "Element" + depthSuffix(depth) - inner, usesFmt, err := constructLines(n.children, field+"["+indexVar+"]", elemVar, attrPath, depth+2, false) + modelsVar := lowerCamel(n.attr.Name) + "Models" + depthSuffix(depth) + inner, _, err := constructLines(namer, path, n.children, modelsVar+"["+indexVar+"]", elemVar, attrPath, depth+2, false) if err != nil { return "", false, err } @@ -147,16 +161,22 @@ func constructNested(n node, src, dst, attrPath string, depth int) (string, bool } var b strings.Builder - fmt.Fprintf(&b, "%sif %s != nil {\n", indent, field) - fmt.Fprintf(&b, "%s\t%s := make([]%s, 0, len(%s))\n", indent, listVar, elemType, field) - fmt.Fprintf(&b, "%s\tfor %s := range %s {\n", indent, indexVar, field) + // A null or unknown list writes nothing: the plan has no elements to + // build from, and unknown is what a computed list carries before the + // API has answered. + fmt.Fprintf(&b, "%sif !%s.IsNull() && !%s.IsUnknown() {\n", indent, field, field) + fmt.Fprintf(&b, "%s\tvar %s []%s\n", indent, modelsVar, namer.name(path)) + fmt.Fprintf(&b, "%s\tif diags := %s.ElementsAs(ctx, &%s, false); diags.HasError() {\n", indent, field, modelsVar) + fmt.Fprintf(&b, "%s\t\t%s\n%s\t}\n", indent, diagReturn(attrPath), indent) + fmt.Fprintf(&b, "%s\t%s := make([]%s, 0, len(%s))\n", indent, listVar, elemType, modelsVar) + fmt.Fprintf(&b, "%s\tfor %s := range %s {\n", indent, indexVar, modelsVar) fmt.Fprintf(&b, "%s\t\t%s := %s\n", indent, elemVar, n.fb.NestedConstructor) b.WriteString(inner) fmt.Fprintf(&b, "%s\t\t%s = append(%s, %s%s)\n", indent, listVar, listVar, deref, elemVar) fmt.Fprintf(&b, "%s\t}\n", indent) fmt.Fprintf(&b, "%s\t%s.%s(%s)\n", indent, dst, n.fb.Access.Set, listVar) fmt.Fprintf(&b, "%s}\n", indent) - return b.String(), usesFmt, nil + return b.String(), true, nil } singleDeref := "" @@ -164,7 +184,8 @@ func constructNested(n node, src, dst, attrPath string, depth int) (string, bool singleDeref = "*" } nestedVar := lowerCamel(n.attr.Name) + "Body" + depthSuffix(depth) - inner, usesFmt, err := constructLines(n.children, field, nestedVar, attrPath, depth+1, false) + modelVar := lowerCamel(n.attr.Name) + "Model" + depthSuffix(depth) + inner, _, err := constructLines(namer, path, n.children, modelVar, nestedVar, attrPath, depth+1, false) if err != nil { return "", false, err } @@ -173,12 +194,18 @@ func constructNested(n node, src, dst, attrPath string, depth int) (string, bool } var b strings.Builder - fmt.Fprintf(&b, "%sif %s != nil {\n", indent, field) + // A null or unknown object writes nothing: unknown is what a computed + // object carries before the API has answered, and neither state has + // fields to build from. + fmt.Fprintf(&b, "%sif !%s.IsNull() && !%s.IsUnknown() {\n", indent, field, field) + fmt.Fprintf(&b, "%s\tvar %s %s\n", indent, modelVar, namer.name(path)) + fmt.Fprintf(&b, "%s\tif diags := %s.As(ctx, &%s, basetypes.ObjectAsOptions{}); diags.HasError() {\n", indent, field, modelVar) + fmt.Fprintf(&b, "%s\t\t%s\n%s\t}\n", indent, diagReturn(attrPath), indent) fmt.Fprintf(&b, "%s\t%s := %s\n", indent, nestedVar, n.fb.NestedConstructor) b.WriteString(inner) fmt.Fprintf(&b, "%s\t%s.%s(%s%s)\n", indent, dst, n.fb.Access.Set, singleDeref, nestedVar) fmt.Fprintf(&b, "%s}\n", indent) - return b.String(), usesFmt, nil + return b.String(), true, nil } // stateLines renders the body statements mapping one level of SDK fields @@ -244,8 +271,15 @@ func stateNested(namer *modelNamer, path string, n node, src, dst string, depth b.WriteString(inner) fmt.Fprintf(&b, "%s\t\t%s = append(%s, %s)\n", indent, listVar, listVar, elemVar) fmt.Fprintf(&b, "%s\t}\n", indent) - fmt.Fprintf(&b, "%s\t%s = %s\n", indent, field, listVar) - fmt.Fprintf(&b, "%s} else {\n%s\t%s = nil\n%s}\n", indent, indent, field, indent) + valueName := lowerCamel(n.attr.Name) + "Value" + depthSuffix(depth) + diagsName := lowerCamel(n.attr.Name) + "Diags" + depthSuffix(depth) + elemType := nestedObjectType(namer, path) + fmt.Fprintf(&b, "%s\t%s, %s := types.ListValueFrom(ctx, %s, %s)\n", + indent, valueName, diagsName, elemType, listVar) + fmt.Fprintf(&b, "%s\tdiags.Append(%s...)\n", indent, diagsName) + fmt.Fprintf(&b, "%s\t%s = %s\n", indent, field, valueName) + fmt.Fprintf(&b, "%s} else {\n%s\t%s = types.ListNull(%s)\n%s}\n", + indent, indent, field, elemType, indent) return b.String(), nil } @@ -269,11 +303,17 @@ func stateNested(namer *modelNamer, path string, n node, src, dst string, depth } else { fmt.Fprintf(&b, "%s{\n%s\t%s := %s.%s()\n", indent, indent, valueVar, src, n.fb.Access.Get) } - fmt.Fprintf(&b, "%s\t%s := &%s{}\n", indent, nestedVar, modelType) + valueName := lowerCamel(n.attr.Name) + "Value" + depthSuffix(depth) + diagsName := lowerCamel(n.attr.Name) + "Diags" + depthSuffix(depth) + fmt.Fprintf(&b, "%s\t%s := %s{}\n", indent, nestedVar, modelType) b.WriteString(inner) - fmt.Fprintf(&b, "%s\t%s = %s\n", indent, field, nestedVar) + fmt.Fprintf(&b, "%s\t%s, %s := types.ObjectValueFrom(ctx, %s(), %s)\n", + indent, valueName, diagsName, attrTypesFuncName(modelType), nestedVar) + fmt.Fprintf(&b, "%s\tdiags.Append(%s...)\n", indent, diagsName) + fmt.Fprintf(&b, "%s\t%s = %s\n", indent, field, valueName) if nilable { - fmt.Fprintf(&b, "%s} else {\n%s\t%s = nil\n%s}\n", indent, indent, field, indent) + fmt.Fprintf(&b, "%s} else {\n%s\t%s = types.ObjectNull(%s())\n%s}\n", + indent, indent, field, attrTypesFuncName(modelType), indent) } else { fmt.Fprintf(&b, "%s}\n", indent) } diff --git a/internal/emit/render_resource.go b/internal/emit/render_resource.go index 87c0e98..1eabd23 100644 --- a/internal/emit/render_resource.go +++ b/internal/emit/render_resource.go @@ -273,12 +273,15 @@ func (e *serviceRenderer) resourceCode(d *resourceData, r *ir.Resource, rb *sdkb if strings.Contains(d.Models, "types.") { modelImports.add("", "github.com/hashicorp/terraform-plugin-framework/types") } + if strings.Contains(d.Models, "attr.") { + modelImports.add("", "github.com/hashicorp/terraform-plugin-framework/attr") + } d.ModelImports = modelImports.render() // Construct. d.ConstructReturnType = "*" + rb.WriteModel d.WriteConstructor = rb.WriteConstructor - body, usesFmt, err := constructLines(nodes, "data", "body", "", 1, true) + body, usesFmt, err := constructLinesFor(nodes, d.Pascal, "data", "body", "", 1, true) if err != nil { return err } @@ -292,7 +295,7 @@ func (e *serviceRenderer) resourceCode(d *resourceData, r *ir.Resource, rb *sdkb if rb.UpdateWriteModel != "" { updateNodes := e.joinTree(bindingKindResource, r.Names.Key, r.Schema, rb.UpdateFields, addressingNames( r.Operations.Read, r.Operations.Create, r.Operations.Update, r.Operations.Delete)) - updateBody, updateUsesFmt, err = constructLines(updateNodes, "data", "body", "", 1, false) + updateBody, updateUsesFmt, err = constructLinesFor(updateNodes, d.Pascal, "data", "body", "", 1, false) if err != nil { return err } @@ -304,6 +307,9 @@ func (e *serviceRenderer) resourceCode(d *resourceData, r *ir.Resource, rb *sdkb constructImports := newImportSet(e.pc.Module) constructImports.add("", "context") + if strings.Contains(body+updateBody, "basetypes.") { + constructImports.add("", "github.com/hashicorp/terraform-plugin-framework/types/basetypes") + } if usesFmt || updateUsesFmt { constructImports.add("", "fmt") } @@ -322,6 +328,10 @@ func (e *serviceRenderer) resourceCode(d *resourceData, r *ir.Resource, rb *sdkb d.StateBody = stateBody stateImports := newImportSet(e.pc.Module) stateImports.add("", "context") + stateImports.add("", "github.com/hashicorp/terraform-plugin-framework/diag") + if strings.Contains(stateBody, "types.") { + stateImports.add("", "github.com/hashicorp/terraform-plugin-framework/types") + } if strings.Contains(stateBody, "convert.") { stateImports.add("", e.pc.Module+"/internal/services/common/convert") } diff --git a/internal/emit/render_schema.go b/internal/emit/render_schema.go index 33b3beb..7d9397b 100644 --- a/internal/emit/render_schema.go +++ b/internal/emit/render_schema.go @@ -419,11 +419,27 @@ func buildModels(rootName, typePrefix string, nodes []node, extraFields []string } for _, n := range nodes { fmt.Fprintf(&b, "\t%s %s `tfsdk:%q`\n", - ir.GoName(n.attr.Name), fieldType(namer, childPath(path, n), n), n.attr.Name) + ir.GoName(n.attr.Name), fieldType(n), n.attr.Name) } b.WriteString("}") decls = append(decls, modelDecl{name: name, body: b.String()}) + // The root model is read and written whole through Get and Set, so + // only nested shapes need an object type to be built from. + if path != "" { + var t strings.Builder + fmt.Fprintf(&t, "// %s is the object type %s maps onto.\n", + attrTypesFuncName(name), name) + fmt.Fprintf(&t, "func %s() map[string]attr.Type {\n", attrTypesFuncName(name)) + t.WriteString("\treturn map[string]attr.Type{\n") + for _, n := range nodes { + fmt.Fprintf(&t, "\t\t%q: %s,\n", + n.attr.Name, attrTypeExpr(namer, childPath(path, n), n)) + } + t.WriteString("\t}\n}") + decls = append(decls, modelDecl{name: attrTypesFuncName(name), body: t.String()}) + } + for _, n := range nodes { if n.attr.Nested == nil { continue @@ -438,19 +454,53 @@ func buildModels(rootName, typePrefix string, nodes []node, extraFields []string } // fieldType is the Go type one model field carries. -// A nested attribute's model field is a generated struct, so only the -// nesting shape is decided here; everything else is the record's ValueType. -func fieldType(namer *modelNamer, path string, n node) string { +// +// A nested attribute is held as types.Object or types.List rather than as +// the generated struct, because a Computed attribute arrives unknown in the +// plan and neither a struct pointer nor a slice can represent unknown. The +// struct is still generated: it is what the object is built from and read +// back into, through the AttrTypes function beside it. +func fieldType(n node) string { switch { case n.attr.Nested != nil && n.attr.Kind == ir.TypeList: - return "[]" + namer.name(path) + return "types.List" case n.attr.Nested != nil: - return "*" + namer.name(path) + return "types.Object" default: return schemaTypeOf(n).ValueType } } +// attrTypeExpr is the attr.Type one model field is described by, which an +// object or list value must be given to be built or nulled. +func attrTypeExpr(namer *modelNamer, path string, n node) string { + switch { + case n.attr.Nested != nil && n.attr.Kind == ir.TypeList: + return "types.ListType{ElemType: " + nestedObjectType(namer, path) + "}" + case n.attr.Nested != nil: + return nestedObjectType(namer, path) + default: + resolved := schemaTypeOf(n) + if resolved.ElementType != "" { + return resolved.ValueType + "Type{ElemType: " + resolved.ElementType + "}" + } + return resolved.ValueType + "Type" + } +} + +// nestedObjectType is the object type one generated nested struct maps onto. +func nestedObjectType(namer *modelNamer, path string) string { + return "types.ObjectType{AttrTypes: " + attrTypesFuncName(namer.name(path)) + "()}" +} + +// attrTypesFuncName is the function beside a nested model that answers its +// attribute types. Generated rather than assembled at run time: the shape is +// known here, and a mismatch between the struct and its types is then a +// compile-time fact rather than a runtime diagnostic. +func attrTypesFuncName(modelName string) string { + return modelName + "AttrTypes" +} + // renderModelDecls joins model declarations into one finished block. func renderModelDecls(decls []modelDecl) string { parts := make([]string, len(decls)) diff --git a/internal/emit/render_validators.go b/internal/emit/render_validators.go index e4b6b38..a2417d8 100644 --- a/internal/emit/render_validators.go +++ b/internal/emit/render_validators.go @@ -280,21 +280,15 @@ func writeError(b *strings.Builder, indent, attr, summary, detail string) { } // nullCheck is the absent-value test for one attribute's model field. +// Every field is a framework value, nested ones included, so one test +// serves them all. func nullCheck(n node) string { - field := "data." + ir.GoName(n.attr.Name) - if n.attr.Nested != nil { - return field + " == nil" - } - return field + ".IsNull()" + return "data." + ir.GoName(n.attr.Name) + ".IsNull()" } // notNull is the present-value test, the negation of nullCheck. func notNull(n node) string { - field := "data." + ir.GoName(n.attr.Name) - if n.attr.Nested != nil { - return field + " != nil" - } - return "!" + field + ".IsNull()" + return "!data." + ir.GoName(n.attr.Name) + ".IsNull()" } // orList renders a set of values as human prose: "a", "a or b", diff --git a/internal/emit/render_validators_test.go b/internal/emit/render_validators_test.go index 314286b..659a52b 100644 --- a/internal/emit/render_validators_test.go +++ b/internal/emit/render_validators_test.go @@ -25,7 +25,7 @@ func TestUnit_ConstructNested_SkipsAnUnwritableBlock(t *testing.T) { Access: sdkbind.FieldAccess{Get: "GetRoles", Set: "SetRoles", SDKType: "[]models.Roleable"}}, children: []node{readOnlyChild}, } - lines, _, err := constructNested(n, "data", "body", "roles", 1) + lines, _, err := constructNested(newModelNamer("Entity", []node{n}), "roles", n, "data", "body", "roles", 1) if err != nil { t.Fatalf("%s: constructNested: %v", kind, err) } @@ -53,7 +53,7 @@ func TestUnit_ConstructNested_BuildsTheWriteType(t *testing.T) { children: []node{child}, } - lines, _, err := constructNested(n, "data", "body", "services", 1) + lines, _, err := constructNested(newModelNamer("Entity", []node{n}), "services", n, "data", "body", "services", 1) if err != nil { t.Fatalf("constructNested: %v", err) } diff --git a/internal/emit/services_test.go b/internal/emit/services_test.go index f27991c..17fda73 100644 --- a/internal/emit/services_test.go +++ b/internal/emit/services_test.go @@ -224,12 +224,12 @@ func TestUnit_RenderServices_TheRenderedCodeCarriesTheDecisions(t *testing.T) { "func (v kindRequiredWhenValidator) ValidateResource(ctx context.Context, req resource.ValidateConfigRequest, resp *resource.ValidateConfigResponse) {", // required-when: settings required when kind == advanced `data.Kind.ValueString() == "advanced"`, - "data.Settings == nil", + "data.Settings.IsNull()", // valid-when: rules valid only under kind == advanced `data.Kind.ValueString() != "advanced"`, - "if data.Rules != nil {", + "if !data.Rules.IsNull() {", // valid-configuration: port only under basic, settings only under advanced - `if data.Settings != nil && data.Kind.ValueString() != "advanced" {`, + `if !data.Settings.IsNull() && data.Kind.ValueString() != "advanced" {`, `if !data.Port.IsNull() && data.Kind.ValueString() != "basic" {`, } { if !strings.Contains(validators, want) { diff --git a/internal/templates/services/datasource/read.go.tmpl b/internal/templates/services/datasource/read.go.tmpl index 5724dae..f3d4e64 100644 --- a/internal/templates/services/datasource/read.go.tmpl +++ b/internal/templates/services/datasource/read.go.tmpl @@ -27,7 +27,10 @@ func (d *{{ .Type }}) Read(ctx context.Context, req datasource.ReadRequest, resp return } - MapRemoteStateToDatasource(ctx, &data, {{ .ReadPlan.Payload }}) + resp.Diagnostics.Append(MapRemoteStateToDatasource(ctx, &data, {{ .ReadPlan.Payload }})...) + if resp.Diagnostics.HasError() { + return + } resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) } {{ else }} @@ -65,7 +68,8 @@ func (d *{{ .Type }}) Read(ctx context.Context, req datasource.ReadRequest, resp errors.HandleDatasourceReadError(ctx, err, resp, nil) return } - item := mapItem(ctx, {{ .ReadItem }}) + item, itemDiags := mapItem(ctx, {{ .ReadItem }}) + resp.Diagnostics.Append(itemDiags...) if matches(&data, &item) { items = append(items, item) } @@ -80,7 +84,8 @@ func (d *{{ .Type }}) Read(ctx context.Context, req datasource.ReadRequest, resp return } for _, element := range {{ .Collection }} { - item := mapItem(ctx, element) + item, itemDiags := mapItem(ctx, element) + resp.Diagnostics.Append(itemDiags...) if !matches(&data, &item) { continue } diff --git a/internal/templates/services/datasource/state.go.tmpl b/internal/templates/services/datasource/state.go.tmpl index 74fff57..9b39d78 100644 --- a/internal/templates/services/datasource/state.go.tmpl +++ b/internal/templates/services/datasource/state.go.tmpl @@ -7,17 +7,20 @@ package {{ .Package }} {{ if .LookupByKey }} // MapRemoteStateToDatasource maps the API's answer onto the framework // model. A nil field in the answer becomes a null attribute. -func MapRemoteStateToDatasource(ctx context.Context, data *{{ .Type }}Model, remote {{ .ReadModel }}) { +func MapRemoteStateToDatasource(ctx context.Context, data *{{ .Type }}Model, remote {{ .ReadModel }}) diag.Diagnostics { + var diags diag.Diagnostics _ = ctx if remote == nil { - return + return diags } -{{ .StateBody }}} +{{ .StateBody }} return diags +} {{ else }} // mapItem maps one listed element onto the item model. -func mapItem(ctx context.Context, remote {{ .ElementType }}) {{ .ItemModel }} { +func mapItem(ctx context.Context, remote {{ .ElementType }}) ({{ .ItemModel }}, diag.Diagnostics) { + var diags diag.Diagnostics _ = ctx item := {{ .ItemModel }}{} -{{ .MapItemBody }} return item +{{ .MapItemBody }} return item, diags } {{ end }} diff --git a/internal/templates/services/resource/crud.go.tmpl b/internal/templates/services/resource/crud.go.tmpl index 01d2706..d461f07 100644 --- a/internal/templates/services/resource/crud.go.tmpl +++ b/internal/templates/services/resource/crud.go.tmpl @@ -46,7 +46,10 @@ func (r *{{ .Type }}) Create(ctx context.Context, req resource.CreateRequest, re return } {{ if .CreateMapsResponse }} - MapRemoteStateToTerraform(ctx, &data, {{ .CreatePlan.Payload }}) + resp.Diagnostics.Append(MapRemoteStateToTerraform(ctx, &data, {{ .CreatePlan.Payload }})...) + if resp.Diagnostics.HasError() { + return + } {{ end }} {{- if .Singleton }} data.ID = types.StringValue({{ .SingletonID | printf "%q" }}) @@ -89,7 +92,10 @@ func (r *{{ .Type }}) Read(ctx context.Context, req resource.ReadRequest, resp * return } - MapRemoteStateToTerraform(ctx, &data, {{ .ReadPlan.Payload }}) + resp.Diagnostics.Append(MapRemoteStateToTerraform(ctx, &data, {{ .ReadPlan.Payload }})...) + if resp.Diagnostics.HasError() { + return + } {{- if .Singleton }} data.ID = types.StringValue({{ .SingletonID | printf "%q" }}) {{- end }} @@ -141,7 +147,10 @@ func (r *{{ .Type }}) Update(ctx context.Context, req resource.UpdateRequest, re return } {{ if .UpdateMapsResponse }} - MapRemoteStateToTerraform(ctx, &data, {{ .UpdatePlan.Payload }}) + resp.Diagnostics.Append(MapRemoteStateToTerraform(ctx, &data, {{ .UpdatePlan.Payload }})...) + if resp.Diagnostics.HasError() { + return + } {{ else if .UpdateParamCopies }} {{ .UpdateParamCopies }} {{ end }} diff --git a/internal/templates/services/resource/state.go.tmpl b/internal/templates/services/resource/state.go.tmpl index 67fa048..0654bca 100644 --- a/internal/templates/services/resource/state.go.tmpl +++ b/internal/templates/services/resource/state.go.tmpl @@ -6,9 +6,15 @@ package {{ .Package }} // MapRemoteStateToTerraform maps the API's answer onto the framework // model. A nil field in the answer becomes a null attribute. -func MapRemoteStateToTerraform(ctx context.Context, data *{{ .Type }}Model, remote {{ .ReadModel }}) { +// +// Returns diagnostics because a nested attribute is built as an object or a +// list value, and building one can report a mismatch against its declared +// attribute types. +func MapRemoteStateToTerraform(ctx context.Context, data *{{ .Type }}Model, remote {{ .ReadModel }}) diag.Diagnostics { + var diags diag.Diagnostics _ = ctx if remote == nil { - return + return diags } -{{ .StateBody }}} +{{ .StateBody }} return diags +}