diff --git a/internal/audit/observe/bodies.go b/internal/audit/observe/bodies.go new file mode 100644 index 0000000..286b256 --- /dev/null +++ b/internal/audit/observe/bodies.go @@ -0,0 +1,141 @@ +// The request bodies a run got the API to accept, and what it answered with. +// +// An observation says something about one property. These say what a whole +// create looked like when it worked, which is the one thing a generated +// acceptance test cannot derive: the document describes what should be +// accepted, and only a run knows what was. + +package observe + +import ( + "bytes" + "encoding/json" + "fmt" + "os" + "path/filepath" + "sort" +) + +// encodeBodies renders one entity's record deterministically, matching the +// observations beside it: sorted map keys, no HTML escaping, two-space indent. +func encodeBodies(b Bodies) ([]byte, error) { + var buf bytes.Buffer + enc := json.NewEncoder(&buf) + enc.SetEscapeHTML(false) + enc.SetIndent("", " ") + if err := enc.Encode(b); err != nil { + return nil, fmt.Errorf("encoding bodies for %s: %w", b.Entity, err) + } + return buf.Bytes(), nil +} + +// BodiesSuffix is the committed file naming, one file per entity, matching +// the observations beside it. +const BodiesSuffix = ".bodies.json" + +// Bodies is what one entity's creates looked like when the API accepted them. +type Bodies struct { + Entity string `json:"entity"` + // Minimal is the smallest create the run got accepted, and Maximal the + // fullest. Either may be absent when no create of that shape succeeded. + Minimal *AcceptedBody `json:"minimal,omitempty"` + Maximal *AcceptedBody `json:"maximal,omitempty"` +} + +// AcceptedBody is one create the API answered 2xx to. +type AcceptedBody struct { + // Status is the code the API answered, kept so a reader can see that this + // was an acceptance rather than an assumption. + Status int `json:"status"` + // Request is the body as sent, with the run's own placeholders already + // resolved — what a repeat of this create would have to carry. + Request map[string]any `json:"request"` + // Response is the object the API answered with. A property the request + // carried and this does not is one the API accepts and never returns, + // which terraform cannot hold in state without losing it on the next read. + Response map[string]any `json:"response,omitempty"` +} + +// Echoed reports whether the response carried the named wire property. +// +// A field the API accepts and never echoes cannot appear in a generated +// configuration: terraform compares what it planned against what the provider +// answers, and a value that never comes back reads as the provider losing it. +func (b *AcceptedBody) Echoed(wire string) bool { + if b == nil || b.Response == nil { + return false + } + _, ok := b.Response[wire] + return ok +} + +// WriteBodies commits one .bodies.json per entity under dir. +// Encoding matches the observations: sorted keys, stable bytes, so a re-run +// that learned nothing new rewrites nothing. +func WriteBodies(dir string, bodies []Bodies) error { + if len(bodies) == 0 { + return nil + } + byEntity := map[string]Bodies{} + for _, b := range bodies { + if b.Entity == "" || (b.Minimal == nil && b.Maximal == nil) { + continue + } + byEntity[b.Entity] = b + } + entities := make([]string, 0, len(byEntity)) + for e := range byEntity { + entities = append(entities, e) + } + sort.Strings(entities) + + encoded := make(map[string][]byte, len(entities)) + for _, entity := range entities { + raw, err := encodeBodies(byEntity[entity]) + if err != nil { + return err + } + encoded[entity] = raw + } + if err := os.MkdirAll(dir, 0o755); err != nil { + return fmt.Errorf("creating %s: %w", dir, err) + } + for _, entity := range entities { + path := filepath.Join(dir, entity+BodiesSuffix) + if err := os.WriteFile(path, encoded[entity], 0o644); err != nil { + return fmt.Errorf("writing %s: %w", path, err) + } + } + return nil +} + +// ReadBodies loads every recorded body under dir, keyed by entity. A missing +// directory is not an error: an entity the probe never cleared has none, and +// generation falls back to deriving values from the document. +func ReadBodies(dir string) (map[string]Bodies, error) { + out := map[string]Bodies{} + entries, err := os.ReadDir(dir) + if err != nil { + if os.IsNotExist(err) { + return out, nil + } + return nil, fmt.Errorf("reading %s: %w", dir, err) + } + for _, e := range entries { + if e.IsDir() || filepath.Ext(e.Name()) != ".json" { + continue + } + raw, err := os.ReadFile(filepath.Join(dir, e.Name())) + if err != nil { + return nil, fmt.Errorf("reading %s: %w", e.Name(), err) + } + var b Bodies + if err := json.Unmarshal(raw, &b); err != nil { + return nil, fmt.Errorf("reading %s: %w", e.Name(), err) + } + if b.Entity != "" { + out[b.Entity] = b + } + } + return out, nil +} diff --git a/internal/audit/observe/bodies_test.go b/internal/audit/observe/bodies_test.go new file mode 100644 index 0000000..b6397a4 --- /dev/null +++ b/internal/audit/observe/bodies_test.go @@ -0,0 +1,88 @@ +package observe + +import ( + "path/filepath" + "testing" +) + +// TestUnit_Observe_RecordedBodiesRoundTrip proves a recorded body survives the +// trip to disk unchanged. A generated configuration is built from these, so a +// value that shifts in the file is a configuration that no longer matches the +// request the API accepted. +func TestUnit_Observe_RecordedBodiesRoundTrip(t *testing.T) { + dir := t.TempDir() + in := []Bodies{ + { + Entity: "tag", + Minimal: &AcceptedBody{ + Status: 201, + Request: map[string]any{"key": "branch", "value": "sfo"}, + Response: map[string]any{"key": "branch", "value": "sfo", "id": "7"}, + }, + }, + { + Entity: "role", + Maximal: &AcceptedBody{Status: 200, Request: map[string]any{"name": "n"}}, + }, + // Neither shape accepted: nothing to record, and nothing written. + {Entity: "user"}, + } + if err := WriteBodies(dir, in); err != nil { + t.Fatalf("WriteBodies: %v", err) + } + + out, err := ReadBodies(dir) + if err != nil { + t.Fatalf("ReadBodies: %v", err) + } + if len(out) != 2 { + t.Fatalf("read %d entities, want the two that had an accepted create", len(out)) + } + tag := out["tag"] + if tag.Minimal == nil || tag.Minimal.Status != 201 { + t.Fatalf("tag minimal = %#v", tag.Minimal) + } + if tag.Minimal.Request["value"] != "sfo" { + t.Errorf("request value = %#v, want the value that was sent", tag.Minimal.Request["value"]) + } + if _, recorded := out["user"]; recorded { + t.Error("an entity with no accepted create was written") + } + if got := filepath.Base(dir); got == "" { + t.Fatal("temp dir vanished") + } +} + +// TestUnit_Observe_EchoedReadsTheResponse pins the check a configuration +// depends on: a property the API took and never returned cannot be held in +// terraform state. +func TestUnit_Observe_EchoedReadsTheResponse(t *testing.T) { + b := &AcceptedBody{ + Request: map[string]any{"name": "n", "matchType": "and"}, + Response: map[string]any{"name": "n"}, + } + if !b.Echoed("name") { + t.Error("a property the response carried reads as not echoed") + } + if b.Echoed("matchType") { + t.Error("a property the response omitted reads as echoed") + } + // No response recorded says nothing about any property. + var none *AcceptedBody + if none.Echoed("name") { + t.Error("an absent record claimed an echo") + } +} + +// TestUnit_Observe_ReadBodiesToleratesNoRun proves a tree the probe has never +// run against reads as empty rather than as an error: generation falls back to +// deriving values from the document. +func TestUnit_Observe_ReadBodiesToleratesNoRun(t *testing.T) { + out, err := ReadBodies(filepath.Join(t.TempDir(), "never-written")) + if err != nil { + t.Fatalf("a missing directory is a normal state: %v", err) + } + if len(out) != 0 { + t.Errorf("read %d entities from nothing", len(out)) + } +} diff --git a/internal/audit/run/adjust_unit_test.go b/internal/audit/run/adjust_unit_test.go index fecd3a8..3a5b369 100644 --- a/internal/audit/run/adjust_unit_test.go +++ b/internal/audit/run/adjust_unit_test.go @@ -322,3 +322,81 @@ func TestUnit_Evidence_TheIdentifyingPropertyIsFoundByValue(t *testing.T) { }) } } + +// TestUnit_Search_CandidatesAreOrderedCheapestSignalFirst pins the order the +// additive search adds fields in. The order decides how many live creates a +// blocked entity costs, and it must be the same on a re-run. +func TestUnit_Search_CandidatesAreOrderedCheapestSignalFirst(t *testing.T) { + t.Parallel() + + r := &runner{hints: map[string]map[string]strategy.SynthHint{ + "widget": { + "alreadySent": {Field: "alreadySent", Type: "string"}, + "zNamed": {Field: "zNamed", Type: "string"}, + "aPlain": {Field: "aPlain", Type: "string"}, + "bPlain": {Field: "bPlain", Type: "string"}, + "withEnum": {Field: "withEnum", Type: "string", Enum: []any{"x"}}, + "nested": {Field: "nested", Type: "object"}, + }, + }} + ent := &entityState{plan: &plan.EntityPlan{Entity: "widget"}} + body := map[string]any{"alreadySent": "v"} + refusal := &httpResult{body: []byte(`{"detail":"zNamed is wrong somehow"}`)} + + got := r.searchCandidates(ent, body, refusal) + want := []string{"zNamed", "withEnum", "aPlain", "bPlain", "nested"} + if len(got) != len(want) { + t.Fatalf("candidates = %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("candidates = %v, want %v", got, want) + } + } + // A field the body already carries is never a candidate. + for _, f := range got { + if f == "alreadySent" { + t.Error("a field already in the body was offered as a candidate") + } + } +} + +// TestUnit_Search_AllowanceIsBounded pins the ceiling on how many live creates +// one entity's search may spend. +func TestUnit_Search_AllowanceIsBounded(t *testing.T) { + t.Parallel() + if got := searchAllowance(3); got != 3 { + t.Errorf("searchAllowance(3) = %d, want 3", got) + } + if got := searchAllowance(500); got != 24 { + t.Errorf("searchAllowance(500) = %d, want the cap", got) + } +} + +// TestUnit_Search_MaximalCulpritPrefersTheNamedField pins which field the +// reduction drops next. A refusal that names one is believed; otherwise the +// choice is the last in order, so a re-run reduces the same way. +func TestUnit_Search_MaximalCulpritPrefersTheNamedField(t *testing.T) { + t.Parallel() + + r := &runner{} + body := map[string]any{"name": "n", "colour": "c", "shape": "s"} + minimal := map[string]any{"name": "n"} + + named := &httpResult{body: []byte(`{"detail":"colour is not valid here"}`)} + if got := r.maximalCulprit(body, minimal, named); got != "colour" { + t.Errorf("culprit = %q, want the field the refusal named", got) + } + + // Nothing named: the last optional field in order, never a field the + // minimal create needs. + silent := &httpResult{body: []byte(`{"detail":"bad request"}`)} + if got := r.maximalCulprit(body, minimal, silent); got != "shape" { + t.Errorf("culprit = %q, want the last optional field", got) + } + + // Only the minimal body left: there is nothing safe to drop. + if got := r.maximalCulprit(minimal, minimal, silent); got != "" { + t.Errorf("culprit = %q, want none", got) + } +} diff --git a/internal/audit/run/entity.go b/internal/audit/run/entity.go index b1cb94d..dd1eef1 100644 --- a/internal/audit/run/entity.go +++ b/internal/audit/run/entity.go @@ -72,12 +72,34 @@ func (r *runner) runEntity(ctx context.Context, ep *plan.EntityPlan) { ConditionalValues: ent.ev.conditionalValues, IdentifierProperty: ent.ev.idField, } + r.summary.Bodies = append(r.summary.Bodies, recordedBodies(ep.Entity, ent)) r.summary.Entities = append(r.summary.Entities, EntityResult{ Entity: ep.Entity, Status: ent.status, Reason: ent.reason, }) r.log.Info().Str("entity", ep.Entity).Str("status", ent.status).Str("reason", ent.reason).Int("requests", ent.requests).Msg("entity finished") } +// recordedBodies is what this entity's accepted creates looked like, for the +// generated acceptance tests to be built from. +// +// A create the API refused is not here: the point of the record is that these +// are requests it took, so a configuration replaying one is a configuration +// known to apply. +func recordedBodies(entity string, ent *entityState) observe.Bodies { + out := observe.Bodies{Entity: entity} + if ent.ev.sent != nil { + out.Minimal = &observe.AcceptedBody{ + Status: ent.ev.sentStatus, Request: ent.ev.sent, Response: ent.ev.got, + } + } + if ent.ev.maximalSent != nil { + out.Maximal = &observe.AcceptedBody{ + Status: ent.ev.maximalStatus, Request: ent.ev.maximalSent, Response: ent.ev.maximalGot, + } + } + return out +} + // halt classifies a step failure onto the entity. func (r *runner) halt(ent *entityState, err error) { var blocked blockedError diff --git a/internal/audit/run/evidence.go b/internal/audit/run/evidence.go index 228d1b0..7972734 100644 --- a/internal/audit/run/evidence.go +++ b/internal/audit/run/evidence.go @@ -24,6 +24,10 @@ type evidence struct { // optional fields a minimal create never sends. maximalSent map[string]any maximalGot map[string]any + // The status each accepted create answered, kept so the recorded body + // shows it was an acceptance rather than an assumption. + sentStatus int + maximalStatus int // volatile marks fields the consecutive read saw change. volatile map[string]bool // omitted collects, per field, the value each created object answered diff --git a/internal/audit/run/run.go b/internal/audit/run/run.go index 10dd431..234abc0 100644 --- a/internal/audit/run/run.go +++ b/internal/audit/run/run.go @@ -155,6 +155,10 @@ type Summary struct { Blocked int `json:"blocked"` TimedOut int `json:"timedOut"` Skipped int `json:"skipped"` + // Bodies is what each entity's accepted creates looked like. A generated + // acceptance test is built from these rather than from values derived + // again from the document, because only these were actually accepted. + Bodies []observe.Bodies `json:"-"` // SkippedEntities is every entity the plan left out, with its reason. // Carried because the count alone cannot be acted on: it does not // distinguish a run that covered the API from one that skipped most of it. diff --git a/internal/audit/run/steps_create.go b/internal/audit/run/steps_create.go index 46c3e0f..63bd982 100644 --- a/internal/audit/run/steps_create.go +++ b/internal/audit/run/steps_create.go @@ -23,6 +23,17 @@ func (r *runner) runCreateMinimal(ctx context.Context, ent *entityState, step *p if err != nil { return err } + // The grammar heals a refusal that names its field. A refusal that says + // only that the request was bad names nothing to act on, and the document + // that produced this body is the same document that understated it — so + // ask the API instead, one field at a time. + if rr.obj == nil && rr.res != nil && rr.res.refused() { + if searched, serr := r.searchMinimal(ctx, ent, ent.recipe, rr.body, rr.res); serr != nil { + return serr + } else if searched.obj != nil { + rr = searched + } + } if rr.obj != nil { sent, err := r.resolveBody(ctx, ent, rr.body) if err != nil { @@ -31,6 +42,7 @@ func (r *runner) runCreateMinimal(ctx context.Context, ent *entityState, step *p r.registry[ent.plan.Entity] = rr.obj ent.createdAt = time.Now() ent.ev.sent = sent + ent.ev.sentStatus = rr.res.status ent.ev.createProof = &rr.res.excerpt ent.ev.acceptedBodies = append(ent.ev.acceptedBodies, cloneAnyMap(rr.body)) return nil @@ -73,6 +85,7 @@ func (r *runner) runCreateMaximal(ctx context.Context, ent *entityState, step *p } ent.ev.maximalSent = sent ent.ev.maximalGot = rr.res.object() + ent.ev.maximalStatus = rr.res.status ent.ev.acceptedBodies = append(ent.ev.acceptedBodies, cloneAnyMap(rr.body)) _, _ = r.deleteObject(ctx, ent, ent.recipe, rr.obj) return nil @@ -80,7 +93,12 @@ func (r *runner) runCreateMaximal(ctx context.Context, ent *entityState, step *p if rr.res == nil || !rr.res.refused() { return nil } - return r.bisectMaximal(ctx, ent, step, rr.res) + // Narrow first, so a single culprit is named as rejected evidence, then + // drop what the API will not take until it takes the rest. + if err := r.bisectMaximal(ctx, ent, step, rr.res); err != nil { + return err + } + return r.reduceMaximal(ctx, ent, step, rr.body, rr.res) } // bisectMaximal narrows a refused maximal create to the optional field @@ -300,3 +318,179 @@ func appendProof(proof []observe.Excerpt, e observe.Excerpt) []observe.Excerpt { } return append(proof, e) } + +// searchAllowance bounds the additive minimal search: how many extra create +// attempts one entity may spend looking for a body the API accepts. +// +// Sized from the candidate count the way bisectionAllowance is sized from the +// optional count, and capped, because a wide entity would otherwise spend the +// whole run's requests on one search. A refused create makes no object, so the +// cost is requests and wall clock, never debris. +func searchAllowance(candidates int) int { + const cap = 24 + if candidates > cap { + return cap + } + return candidates +} + +// searchMinimal looks for a create body the API accepts by adding one field at +// a time to a body it refused. +// +// The document is only a hypothesis about what a create needs, and an API that +// declares nothing required leaves the derivation with an empty body that +// cannot make anything. The refusal grammar handles a refusal that names its +// field; this handles the rest, which is every API whose 400 says only that +// the request was bad. +// +// Additive rather than combinatorial: each field that does not provoke a +// refusal naming it stays, so a body needing several fields is found in as +// many attempts rather than exponentially many. What it finds is a viable +// minimal body, not a proof that every field in it is individually necessary. +func (r *runner) searchMinimal(ctx context.Context, ent *entityState, rec *entityRecipe, body map[string]any, refusal *httpResult) (adjustResult, error) { + candidates := r.searchCandidates(ent, body, refusal) + allowance := searchAllowance(len(candidates)) + last := refusal + + for i := 0; i < allowance; i++ { + field := candidates[i] + body[field] = r.synthField(ent, field) + + obj, res, err := r.createObject(ctx, ent, rec, body) + if err != nil { + return adjustResult{}, err + } + if obj != nil { + // Every field the search added is part of the smallest body this + // run could get accepted, which is what the fixture must carry. + for _, added := range candidates[:i+1] { + if _, kept := body[added]; kept { + r.recordAdjustAdd(ent, added, "", "", res.excerpt) + } + } + return adjustResult{obj: obj, res: res, body: body, adjusted: true}, nil + } + if res == nil || !res.refused() { + return adjustResult{res: res, body: body, adjusted: true, gaveUp: true}, nil + } + // The API now objects to the field just added, so it is not one this + // create wants; the ones before it stay. + if res.mentions(field) { + delete(body, field) + } + last = res + } + return adjustResult{res: last, body: body, adjusted: true, gaveUp: true}, nil +} + +// searchCandidates orders the fields the search may add, cheapest-signal +// first, so the common case ends in a handful of attempts and a re-run repeats +// the same order. +func (r *runner) searchCandidates(ent *entityState, body map[string]any, refusal *httpResult) []string { + hints := r.hints[ent.plan.Entity] + type candidate struct { + field string + rank int + } + var out []candidate + for field, h := range hints { + if _, present := body[field]; present { + continue + } + rank := 3 + switch { + case refusal != nil && refusal.mentions(field): + // The API has already said this field's name out loud. + rank = 0 + case len(h.Enum) > 0 || h.Example != nil || h.Default != nil: + // The document states a value the API is known to accept. + rank = 1 + case h.Type != "" && h.Type != "object" && h.Type != "array": + rank = 2 + } + out = append(out, candidate{field: field, rank: rank}) + } + sort.Slice(out, func(i, j int) bool { + if out[i].rank != out[j].rank { + return out[i].rank < out[j].rank + } + return out[i].field < out[j].field + }) + fields := make([]string, 0, len(out)) + for _, c := range out { + fields = append(fields, c.field) + } + return fields +} + +// reduceMaximal drops the fields the API objects to until it accepts the +// create, and records the body it accepted. +// +// The counterpart to searchMinimal: that one adds until a create works, this +// one removes. What survives is the fullest create this run could get taken, +// which is what a generated maximal configuration has to be — every field in +// it is one the API demonstrably tolerates alongside the others. +func (r *runner) reduceMaximal(ctx context.Context, ent *entityState, step *plan.Step, body map[string]any, refusal *httpResult) error { + minimal := ent.recipe.minimalBody + allowance := searchAllowance(len(body)) + last := refusal + + for i := 0; i < allowance; i++ { + culprit := r.maximalCulprit(body, minimal, last) + if culprit == "" { + return nil + } + // The evidence for a refusal is bisectMaximal's to record; this only + // shapes the body, so a field dropped here is not claimed twice. + delete(body, culprit) + + obj, res, err := r.createObject(ctx, ent, ent.recipe, body) + if err != nil { + return err + } + if obj != nil { + sent, err := r.resolveBody(ctx, ent, body) + if err != nil { + return err + } + ent.ev.maximalSent = sent + ent.ev.maximalGot = res.object() + ent.ev.maximalStatus = res.status + ent.ev.acceptedBodies = append(ent.ev.acceptedBodies, cloneAnyMap(body)) + _, _ = r.deleteObject(ctx, ent, ent.recipe, obj) + return nil + } + if res == nil || !res.refused() { + return nil + } + last = res + } + return nil +} + +// maximalCulprit names the optional field to drop next: the one the refusal +// mentions, else the last in document order, which is deterministic and so +// repeats the same reduction on a re-run. +// +// A field the minimal create needs is never a candidate — removing it would +// trade a refused maximal for a refused minimal. +func (r *runner) maximalCulprit(body, minimal map[string]any, refusal *httpResult) string { + var optional []string + for k := range body { + if _, needed := minimal[k]; !needed { + optional = append(optional, k) + } + } + if len(optional) == 0 { + return "" + } + sort.Strings(optional) + if refusal != nil { + for _, k := range optional { + if refusal.mentions(k) { + return k + } + } + } + return optional[len(optional)-1] +} diff --git a/internal/cli/audit.go b/internal/cli/audit.go index ca4a836..ce38544 100644 --- a/internal/cli/audit.go +++ b/internal/cli/audit.go @@ -100,6 +100,18 @@ func newAuditRunCommand() *cobra.Command { return writeErr } } + // The accepted bodies sit beside the observations: an observation + // says something about a property, these say what a whole create + // looked like when the API took it. + if len(sum.Bodies) > 0 { + bodiesDir := filepath.Join(filepath.Dir(out), "bodies") + if writeErr := observe.WriteBodies(bodiesDir, sum.Bodies); writeErr != nil { + if runErr != nil { + return fmt.Errorf("%v; additionally %w", runErr, writeErr) + } + return writeErr + } + } printSummary(cmd.OutOrStdout(), out, len(obs), sum) return runErr }, diff --git a/internal/emit/provider_core.go b/internal/emit/provider_core.go index 8e871c2..e25f7be 100644 --- a/internal/emit/provider_core.go +++ b/internal/emit/provider_core.go @@ -13,6 +13,7 @@ package emit import ( "fmt" + "github.com/deploymenttheory/terraform-plugin-framework-codegen/internal/audit/observe" "strings" "github.com/deploymenttheory/terraform-plugin-framework-codegen/internal/config" @@ -26,6 +27,13 @@ const DefaultGoVersion = "1.25" // template assembles a value — and the backend and auth booleans exist so // templates branch on presence, never on meaning. type ProviderCore struct { + // AcceptedBodies is what the probe recorded of each entity's accepted + // creates, keyed by entity. An acceptance fixture is built from these + // rather than from values derived again from the document: the document + // says what should be accepted, these are what was. Empty for an entity + // no run has cleared, which falls back to the derivation. + AcceptedBodies map[string]observe.Bodies + // Module is the provider repo's Go module path, e.g. // "github.com/exampleco/terraform-provider-petstore". Module string diff --git a/internal/emit/render_action.go b/internal/emit/render_action.go index 2d91692..8978e02 100644 --- a/internal/emit/render_action.go +++ b/internal/emit/render_action.go @@ -177,7 +177,7 @@ func (e *serviceRenderer) action(a *ir.Action, ab *sdkbind.ActionBinding) ([]Fil exampleHeader := fmt.Sprintf("action %q %q", a.Names.TerraformType, "example") exampleBody := " config {\n" + reindent(spec.HCL(fixtures.ConfigMaximal), " ") + " }\n" - example, err := hclBlock(a.Names.Key, exampleHeader, exampleBody, nil) + example, err := hclBlock(a.Names.Key, exampleHeader, "", exampleBody, nil) if err != nil { return nil, err } diff --git a/internal/emit/render_datasource.go b/internal/emit/render_datasource.go index c6771c3..920e5df 100644 --- a/internal/emit/render_datasource.go +++ b/internal/emit/render_datasource.go @@ -545,7 +545,7 @@ func (e *serviceRenderer) datasourceFixtures(ds *ir.Datasource, spec fixtures.Fi // whole collection matched it. body = addressingHCL } - fixture, err := hclBlock(source, blockHeader, body, nil) + fixture, err := hclBlock(source, blockHeader, "", body, nil) if err != nil { return nil, err } @@ -563,7 +563,7 @@ func (e *serviceRenderer) datasourceFixtures(ds *ir.Datasource, spec fixtures.Fi } exampleHeader := fmt.Sprintf("data %q %q", ds.Names.TerraformType, "example") - example, err := hclBlock(source, exampleHeader, body, nil) + example, err := hclBlock(source, exampleHeader, "", body, nil) if err != nil { return nil, err } diff --git a/internal/emit/render_fixtures.go b/internal/emit/render_fixtures.go index 611ee27..17dd3c1 100644 --- a/internal/emit/render_fixtures.go +++ b/internal/emit/render_fixtures.go @@ -32,13 +32,16 @@ func hashHeader(source string) (string, error) { // hclBlock renders one terraform block around a fixture body, with the // shared header above it. -func hclBlock(source, blockHeader, body string, skips []fixtures.Omission) ([]byte, error) { +func hclBlock(source, blockHeader, preamble, body string, skips []fixtures.Omission) ([]byte, error) { header, err := hashHeader(source) if err != nil { return nil, err } var b bytes.Buffer b.WriteString(header + "\n\n") + if preamble != "" { + b.WriteString(preamble + "\n\n") + } b.WriteString(blockHeader + " {\n") b.WriteString(body) for _, s := range skips { @@ -48,6 +51,20 @@ func hclBlock(source, blockHeader, body string, skips []fixtures.Omission) ([]by return b.Bytes(), nil } +// requiredWireNames is the wire names the document marks required, which stay +// in a replayed configuration even where the API does not echo them: a create +// omitting one is refused, so the risk of holding it belongs to the API rather +// than to the generator. +func requiredWireNames(spec fixtures.Fixture) map[string]bool { + out := map[string]bool{} + for _, e := range spec.Entries { + if e.ComputedOptionalRequired == ir.Required && e.Wire != "" { + out[e.Wire] = true + } + } + return out +} + // resourceFixtures emits a resource's terraform fixtures, response // fixtures and examples. func (e *serviceRenderer) resourceFixtures(r *ir.Resource, spec fixtures.Fixture, dir string) ([]File, error) { @@ -55,20 +72,68 @@ func (e *serviceRenderer) resourceFixtures(r *ir.Resource, spec fixtures.Fixture source := key blockHeader := fmt.Sprintf("resource %q %q", r.Names.TerraformType, "test") - minimal, err := hclBlock(source, blockHeader, spec.HCL(fixtures.ConfigMinimal), nil) - if err != nil { - return nil, err + // The unit suite meets a mock built from these same values, so its + // configuration carries them verbatim. The acceptance suite meets a live + // API, where a name that is a constant collides with whatever the last + // run left behind. + // What the probe got the API to accept, where it cleared this entity. + // The acceptance suite replays those; the unit suite keeps the derivation, + // because its mock is built from the same derived values. + accMinimal, accMaximal := spec, spec + replayed := false + if rec, ok := e.pc.AcceptedBodies[r.Names.Key]; ok { + required := requiredWireNames(spec) + if rec.Minimal != nil { + accMinimal = spec.FromAcceptedBody(rec.Minimal.Request, rec.Minimal.Response, required) + replayed = true + } + switch { + case rec.Maximal != nil: + accMaximal = spec.FromAcceptedBody(rec.Maximal.Request, rec.Maximal.Response, required) + replayed = true + case rec.Minimal != nil: + // No larger create was ever accepted, so the fullest known + // configuration is the smallest one. Emitting the document's + // derived maximal instead would put back the invented values the + // record exists to replace. + accMaximal = accMinimal + accMaximal.Omissions = append(accMaximal.Omissions, fixtures.Omission{ + Name: "(every optional attribute)", + Reason: "no create larger than the minimal one was accepted, so this configuration is the minimal one", + }) + } } - maximal, err := hclBlock(source, blockHeader, spec.HCL(fixtures.ConfigMaximal), spec.Omissions) - if err != nil { - return nil, err + liveMinimal := accMinimal.WithRunSuffix() + liveMaximal := accMaximal.WithRunSuffix() + suites := []struct { + name string + minimal fixtures.Fixture + maximal fixtures.Fixture + preamble string + }{ + {name: "unit", minimal: spec, maximal: spec}, + {name: "acceptance", minimal: liveMinimal, maximal: liveMaximal, preamble: fixtures.RunSuffixBlock}, } var files []File - for _, suite := range []string{"unit", "acceptance"} { + for _, suite := range suites { + // A replayed body is already the smallest and fullest accepted create, + // so it renders whole; a derived fixture still selects by presence. + minForm, maxForm := fixtures.ConfigMinimal, fixtures.ConfigMaximal + if suite.name == "acceptance" && replayed { + minForm, maxForm = fixtures.ConfigMaximal, fixtures.ConfigMaximal + } + minimal, err := hclBlock(source, blockHeader, suite.preamble, suite.minimal.HCL(minForm), nil) + if err != nil { + return nil, err + } + maximal, err := hclBlock(source, blockHeader, suite.preamble, suite.maximal.HCL(maxForm), suite.maximal.Omissions) + if err != nil { + return nil, err + } files = append(files, - rawFile(path.Join(dir, "tests/terraform", suite, "resource_minimal.tf"), source, minimal), - rawFile(path.Join(dir, "tests/terraform", suite, "resource_maximal.tf"), source, maximal), + rawFile(path.Join(dir, "tests/terraform", suite.name, "resource_minimal.tf"), source, minimal), + rawFile(path.Join(dir, "tests/terraform", suite.name, "resource_maximal.tf"), source, maximal), ) } files = append(files, @@ -77,7 +142,7 @@ func (e *serviceRenderer) resourceFixtures(r *ir.Resource, spec fixtures.Fixture ) exampleHeader := fmt.Sprintf("resource %q %q", r.Names.TerraformType, "example") - example, err := hclBlock(source, exampleHeader, spec.HCL(fixtures.ConfigMaximal), nil) + example, err := hclBlock(source, exampleHeader, "", spec.HCL(fixtures.ConfigMaximal), nil) if err != nil { return nil, err } diff --git a/internal/emit/render_resource.go b/internal/emit/render_resource.go index 0d2ec8b..1607493 100644 --- a/internal/emit/render_resource.go +++ b/internal/emit/render_resource.go @@ -49,6 +49,10 @@ type resourceData struct { // refuses a create or read that declares an identity schema and leaves // the identity unset, so the two are emitted together or not at all. IdentitySets string + // IdentitySetsRead is IdentitySets indented for the guard Read wraps it + // in: the retrying read is driven by a response of the toolkit's own + // making, which carries no identity schema to write into. + IdentitySetsRead string SchemaDescription string SchemaAttributes string @@ -256,6 +260,7 @@ func (e *serviceRenderer) resourceCode(d *resourceData, r *ir.Resource, rb *sdkb e.identities[r.Names.Key] = identity d.IdentityAttributes = identitySchemaDecls(identity, 3) d.IdentitySets = identitySetLines(identity, "data", 1) + d.IdentitySetsRead = identitySetLines(identity, "data", 2) imports.add("identityschema", "github.com/hashicorp/terraform-plugin-framework/resource/identityschema") } } diff --git a/internal/fixtures/fixtures.go b/internal/fixtures/fixtures.go index 2f0ef76..68523d9 100644 --- a/internal/fixtures/fixtures.go +++ b/internal/fixtures/fixtures.go @@ -594,3 +594,137 @@ func (s Fixture) topLevel(a Form) []Entry { } return out } + +// RunSuffixExpr is the terraform expression an acceptance configuration +// suffixes its synthesised names with, and the block that supplies it. +// +// A live API that requires a name to be unique refuses the second run of a +// test whose name is a constant, and an object a failed run leaves behind +// holds that name for good. The committed configuration stays byte-identical +// because the expression, not a value, is what it carries. +const ( + RunSuffixExpr = "${random_string.tfpfgen_run.result}" + RunSuffixBlock = `resource "random_string" "tfpfgen_run" { + length = 10 + special = false + upper = false +}` +) + +// WithRunSuffix answers a copy whose synthesised names carry the run suffix. +// +// Only the names this package invented are suffixed: a value the document +// supplied is one the API is known to accept, and appending to it could make +// it invalid — a URL, an enum member, a formatted identifier. +func (s Fixture) WithRunSuffix() Fixture { + out := s + out.Entries = suffixedEntries(s.Entries) + return out +} + +// suffixedEntries copies a level, suffixing the synthesised names in it. +func suffixedEntries(values []Entry) []Entry { + if values == nil { + return nil + } + out := make([]Entry, len(values)) + copy(out, values) + for i := range out { + if text, ok := out[i].Scalar.(string); ok && strings.HasPrefix(text, NamePrefix) { + out[i].Scalar = text + "-" + RunSuffixExpr + } + out[i].Nested = suffixedEntries(out[i].Nested) + } + return out +} + +// FromAcceptedBody answers the entries a recorded create actually carried, +// with the values it carried them as. +// +// This is the difference between a configuration that looks like one the API +// would take and one it demonstrably did. A value derived from the document is +// a guess about what is acceptable; these were accepted. +// +// A property the request carried and the response did not is dropped, with the +// reason recorded: terraform compares what it planned against what the +// provider answers, so a value the API never echoes reads as the provider +// losing it, and no configuration can hold one. +func (s Fixture) FromAcceptedBody(request, response map[string]any, requiredWire map[string]bool) Fixture { + out := s + out.Entries, out.Omissions = overlayEntries(s.Entries, request, response, requiredWire, nil) + out.Omissions = append(out.Omissions, s.Omissions...) + return out +} + +// overlayEntries keeps the entries the body carried, taking their values from +// it, and reports the ones it dropped. +func overlayEntries(values []Entry, request, response map[string]any, requiredWire map[string]bool, path []string) ([]Entry, []Omission) { + var kept []Entry + var dropped []Omission + for _, v := range values { + at := append(append([]string{}, path...), v.Name) + carried, inRequest := request[v.Wire] + if !inRequest { + continue + } + // A field the API takes and never returns cannot live in a + // configuration; a required one has to be sent anyway, so it stays + // and the risk is the API's rather than the generator's. + if response != nil && !requiredWire[v.Wire] { + if _, echoed := response[v.Wire]; !echoed { + dropped = append(dropped, Omission{ + Name: strings.Join(at, "."), + Reason: "the API accepted this property and did not return it, so terraform cannot hold it in state", + }) + continue + } + } + kept = append(kept, overlayOne(v, carried, response, requiredWire, at, &dropped)) + } + return kept, dropped +} + +// overlayOne sets one entry from the value the body carried, recursing into +// the nested shapes a body spells as objects and arrays of objects. +func overlayOne(v Entry, carried any, response map[string]any, requiredWire map[string]bool, at []string, dropped *[]Omission) Entry { + switch nested := carried.(type) { + case map[string]any: + if v.Nested != nil { + var inner []Omission + v.Nested, inner = overlayEntries(v.Nested, nested, nestedResponse(response, v.Wire), requiredWire, at) + *dropped = append(*dropped, inner...) + return v + } + case []any: + if v.Nested != nil && len(nested) > 0 { + if first, ok := nested[0].(map[string]any); ok { + var inner []Omission + v.Nested, inner = overlayEntries(v.Nested, first, nil, requiredWire, at) + *dropped = append(*dropped, inner...) + return v + } + } + // A list of scalars: the fixture carries one element. + if len(nested) > 0 { + v.Scalar = nested[0] + } + return v + } + if v.Nested == nil { + v.Scalar = carried + } + return v +} + +// nestedResponse is the object the response carried under one property, or nil +// when it carried none — in which case the level below is not echo-checked, +// because absence of the parent says nothing about its children. +func nestedResponse(response map[string]any, wire string) map[string]any { + if response == nil { + return nil + } + if inner, ok := response[wire].(map[string]any); ok { + return inner + } + return nil +} diff --git a/internal/fixtures/fixtures_test.go b/internal/fixtures/fixtures_test.go index 7df6919..99a082a 100644 --- a/internal/fixtures/fixtures_test.go +++ b/internal/fixtures/fixtures_test.go @@ -502,3 +502,106 @@ func TestUnit_Fixturespec_APrefixedStringSuppressesTheRestore(t *testing.T) { t.Errorf("name = %#v, want the declared example kept", got) } } + +// acceptedTree is one entity whose shape covers what a replayed body has to +// carry: scalars, a list of scalars, and a nested object. +func acceptedTree() *ir.AttributeTree { + return &ir.AttributeTree{ + Attributes: []ir.Attribute{ + {Name: "name", WireName: "name", Kind: ir.TypeString, ComputedOptionalRequired: ir.Required}, + {Name: "match_type", WireName: "matchType", Kind: ir.TypeString, ComputedOptionalRequired: ir.Optional}, + {Name: "colour", WireName: "colour", Kind: ir.TypeString, ComputedOptionalRequired: ir.Optional}, + {Name: "labels", WireName: "labels", Kind: ir.TypeList, ElementType: ir.TypeString, ComputedOptionalRequired: ir.Optional}, + {Name: "settings", WireName: "settings", Kind: ir.TypeObject, ComputedOptionalRequired: ir.Optional, + Nested: &ir.AttributeTree{Attributes: []ir.Attribute{ + {Name: "retries", WireName: "retries", Kind: ir.TypeInt64, ComputedOptionalRequired: ir.Optional}, + }}}, + }, + } +} + +func TestUnit_Fixturespec_AReplayedBodyCarriesWhatTheAPITook(t *testing.T) { + spec := Derive(acceptedTree()) + request := map[string]any{ + "name": "the-name-it-took", + "colour": "#FF0000", + "labels": []any{"first", "second"}, + "settings": map[string]any{"retries": int64(3)}, + } + response := map[string]any{ + "name": "the-name-it-took", "colour": "#FF0000", + "labels": []any{"first"}, "settings": map[string]any{"retries": int64(3)}, + } + + got := spec.FromAcceptedBody(request, response, map[string]bool{"name": true}) + + // The values are the ones the request carried, not the ones derived. + if v := valueByName(t, got, "name").Scalar; v != "the-name-it-took" { + t.Errorf("name = %#v, want the value the API took", v) + } + if v := valueByName(t, got, "colour").Scalar; v != "#FF0000" { + t.Errorf("colour = %#v, want the value the API took", v) + } + // A list carries its first element, which is what one fixture renders. + if v := valueByName(t, got, "labels").Scalar; v != "first" { + t.Errorf("labels = %#v, want the first element sent", v) + } + // A nested object recurses. + settings := valueByName(t, got, "settings") + if len(settings.Nested) != 1 || settings.Nested[0].Scalar != int64(3) { + t.Errorf("settings = %#v, want the nested value the API took", settings.Nested) + } + // An attribute the request never carried is not in a replay of it. + for _, e := range got.Entries { + if e.Name == "match_type" { + t.Error("an attribute the accepted body did not carry was replayed") + } + } +} + +func TestUnit_Fixturespec_AReplayDropsWhatTheAPINeverReturns(t *testing.T) { + spec := Derive(acceptedTree()) + request := map[string]any{"name": "n", "match_type": "and", "matchType": "and", "colour": "#FF0000"} + // The API took matchType and answered without it. + response := map[string]any{"name": "n", "colour": "#FF0000"} + + got := spec.FromAcceptedBody(request, response, map[string]bool{"name": true}) + + for _, e := range got.Entries { + if e.Name == "match_type" { + t.Fatal("a property the API never returns was left in a configuration") + } + } + var explained bool + for _, o := range got.Omissions { + if strings.Contains(o.Name, "match_type") && strings.Contains(o.Reason, "did not return it") { + explained = true + } + } + if !explained { + t.Errorf("the dropped property was not explained: %#v", got.Omissions) + } + // A required property stays even unreturned: the create needs it. + requiredUnreturned := spec.FromAcceptedBody( + map[string]any{"name": "n"}, map[string]any{}, map[string]bool{"name": true}) + if len(requiredUnreturned.Entries) != 1 { + t.Errorf("a required property was dropped for not being echoed: %#v", requiredUnreturned.Entries) + } +} + +func TestUnit_Fixturespec_TheRunSuffixOnlyTouchesInventedNames(t *testing.T) { + spec := Derive(acceptedTree()) + spec.Entries[1].Scalar = NamePrefix + "invented" + spec.Entries[2].Scalar = "#FF0000" + + got := spec.WithRunSuffix() + + if v := got.Entries[1].Scalar; v != NamePrefix+"invented-"+RunSuffixExpr { + t.Errorf("an invented name = %#v, want the run suffix appended", v) + } + // A value the document supplied is one the API is known to accept; + // appending to it could make it invalid. + if v := got.Entries[2].Scalar; v != "#FF0000" { + t.Errorf("a document value = %#v, want it left alone", v) + } +} diff --git a/internal/intermediate_representation/attributes.go b/internal/intermediate_representation/attributes.go index 549e2e7..6ad17f0 100644 --- a/internal/intermediate_representation/attributes.go +++ b/internal/intermediate_representation/attributes.go @@ -442,7 +442,7 @@ func buildAttribute(wire string, attributeSite site) (Attribute, attributeEdges) serverForced, _ := extensions.ServerForced() volatile, _ := extensions.Volatile() createOnly, _ := extensions.CreateOnly() - _, serverFills := extensions.ServerDefault() + serverDefault, serverFills := extensions.ServerDefault() attribute.SilentlyIgnoredOnUpdate, _ = extensions.SilentlyIgnoredOnUpdate() // The document's prose, taken from whichever side declares any. A @@ -464,6 +464,10 @@ func buildAttribute(wire string, attributeSite site) (Attribute, attributeEdges) // it, so a response-only attribute could never carry it anyway. attribute.Format = flatPrimary.format attribute.Example = flatPrimary.example + // What the API itself answered for this property, where a run has read + // one. It outranks every other source of a fixture value: a document says + // what should be accepted, this is what was. + attribute.ServerDefault = serverDefault attribute.WriteOnly = flatCreate.writeOnly attribute.Deprecated = flatCreate.deprecated || flatRead.deprecated attribute.UniqueItems = flatPrimary.uniqueItems diff --git a/internal/intermediate_representation/model.go b/internal/intermediate_representation/model.go index a053321..2e001c4 100644 --- a/internal/intermediate_representation/model.go +++ b/internal/intermediate_representation/model.go @@ -324,6 +324,10 @@ type Attribute struct { // Format is the document's declared format, which says what a string // carries beyond being a string: "password", "date-time", "uuid". Format string `json:"format,omitempty"` + // ServerDefault is the value a run read back for this property when a + // create omitted it, from x-tfpfgen-server-default; nil when no run has + // measured one. It is a fact about the API rather than the document. + ServerDefault any `json:"server_default,omitempty"` // Example is the document's declared example value. Fixture derivation // prefers it to an invented value: a document that declares no format // often still states, through an example, that the value has a shape the diff --git a/internal/providergen/providergen.go b/internal/providergen/providergen.go index 89c9a37..1c40971 100644 --- a/internal/providergen/providergen.go +++ b/internal/providergen/providergen.go @@ -16,6 +16,7 @@ import ( "context" "encoding/json" "fmt" + "github.com/deploymenttheory/terraform-plugin-framework-codegen/internal/audit/observe" "os" "path/filepath" "sort" @@ -251,6 +252,13 @@ func generate(opts Options) (*generation, error) { if err != nil { return nil, err } + // What a probe run recorded of the API's accepted creates. Absent before + // any run, which leaves every acceptance fixture derived from the + // document as it was. + pc.AcceptedBodies, err = observe.ReadBodies(filepath.Join(opts.Root, "audit", "bodies")) + if err != nil { + return nil, err + } info, err := sdkbind.InfoFor(opts.Config, pc.SDKImport) if err != nil { diff --git a/internal/templates/services/resource/crud.go.tmpl b/internal/templates/services/resource/crud.go.tmpl index 7178c5b..af7f12c 100644 --- a/internal/templates/services/resource/crud.go.tmpl +++ b/internal/templates/services/resource/crud.go.tmpl @@ -108,8 +108,13 @@ func (r *{{ .Type }}) Read(ctx context.Context, req resource.ReadRequest, resp * data.ID = types.StringValue({{ .SingletonID | printf "%q" }}) {{- end }} {{ if .IdentitySets }} -{{ .IdentitySets }} if resp.Diagnostics.HasError() { - return + // The read-after-write loop calls Read with a response it makes itself, + // which carries no identity schema; the create that drove it has already + // written the identity. + if resp.Identity != nil { +{{ .IdentitySetsRead }} if resp.Diagnostics.HasError() { + return + } } {{ end }} resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) diff --git a/internal/templates/services/resource/resource_acceptance_test.go.tmpl b/internal/templates/services/resource/resource_acceptance_test.go.tmpl index 314c73b..77a4ffb 100644 --- a/internal/templates/services/resource/resource_acceptance_test.go.tmpl +++ b/internal/templates/services/resource/resource_acceptance_test.go.tmpl @@ -17,6 +17,12 @@ func TestAcc{{ .Pascal }}Resource_Lifecycle(t *testing.T) { resource.Test(t, resource.TestCase{ PreCheck: func() { acceptance.PreCheck(t) }, ProtoV6ProviderFactories: acceptance.ProtoV6ProviderFactories, + // The configuration suffixes its synthesised names with a random + // string, so a re-run never collides with what the last one left + // behind. The provider supplying it is external to this one. + ExternalProviders: map[string]resource.ExternalProvider{ + "random": {Source: "hashicorp/random"}, + }, Steps: []resource.TestStep{ { Config: accConfigMinimal,