fix: a nested attribute is held as a value that can be unknown

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

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)
+}

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 != "" {

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")

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()
 }
 

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,24 +161,31 @@ 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 := ""
 	if writeType == n.fb.NestedWriteModel {
 		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)
 	}

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")
 	}