From 03709b62670b15e13674a7cb669dfa62ecce3cf4 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Fri, 14 Aug 2026 15:29:41 +0100 Subject: [PATCH] feat: carry the document's declared constraints into the derivation A document states more about a property than the derivation could see. format reached only the audit's value synthesis, and writeOnly, deprecated, uniqueItems and the length and size bounds were not parsed at all, so nothing downstream could act on them. specmodel reads them and the attribute carries them. Bounds fold first-wins, as the description and the enum do: a later branch stating the same bound states nothing new. Constraints are taken from the write side, because a constraint is a rule about what may be sent. Nothing reads them yet: this is what the sensitivity, the deprecation notice and the plan-time validators are decided from. Co-Authored-By: Claude Opus 5 (1M context) --- .../intermediate_representation/attributes.go | 64 ++++++++- .../constraints_test.go | 127 ++++++++++++++++++ internal/intermediate_representation/model.go | 21 +++ internal/specmodel/constraints_test.go | 122 +++++++++++++++++ internal/specmodel/load.go | 42 +++++- internal/specmodel/load_test.go | 18 +++ internal/specmodel/model.go | 16 +++ 7 files changed, 405 insertions(+), 5 deletions(-) create mode 100644 internal/intermediate_representation/constraints_test.go create mode 100644 internal/specmodel/constraints_test.go diff --git a/internal/intermediate_representation/attributes.go b/internal/intermediate_representation/attributes.go index 3e78c50..2a33d00 100644 --- a/internal/intermediate_representation/attributes.go +++ b/internal/intermediate_representation/attributes.go @@ -13,7 +13,20 @@ import ( type flat struct { empty bool declaredType string - readOnly bool + // format is the document's declared format, which says what a string + // carries beyond being a string: a password to hide, a timestamp to + // spell as one. + format string + readOnly bool + writeOnly bool + deprecated bool + uniqueItems bool + // The declared constraints, nil when the document states none. They + // become plan-time validators. + pattern string + minimum, maximum *float64 + minLength, maxLength *int64 + minItems, maxItems *int64 // description is the document's own prose for the schema, folded from // the first branch that states any. It is the only human-written text in // the whole derivation; everything else the generated schema says about @@ -73,6 +86,30 @@ func flatten(schema *specmodel.Schema) flat { if schema.ReadOnly { flattened.readOnly = true } + if schema.WriteOnly { + flattened.writeOnly = true + } + if schema.Deprecated { + flattened.deprecated = true + } + if schema.UniqueItems { + flattened.uniqueItems = true + } + // The declared facts about the value, folded first-wins like the + // description: a branch that states one is more specific than a + // branch that states nothing. + if flattened.format == "" { + flattened.format = schema.Format + } + if flattened.pattern == "" { + flattened.pattern = schema.Pattern + } + foldBound(&flattened.minimum, schema.Minimum) + foldBound(&flattened.maximum, schema.Maximum) + foldBound(&flattened.minLength, schema.MinLength) + foldBound(&flattened.maxLength, schema.MaxLength) + foldBound(&flattened.minItems, schema.MinItems) + foldBound(&flattened.maxItems, schema.MaxItems) if flattened.description == "" { flattened.description = strings.TrimSpace(schema.Description) } @@ -112,6 +149,17 @@ func flatten(schema *specmodel.Schema) flat { return flattened } +// foldBound takes a declared bound the fold has not seen yet. First +// declaration wins, matching how the description and the enum fold: a later +// branch that states the same bound states nothing new, and one that states a +// different bound is describing a different use of the same type. +func foldBound[T int64 | float64](dst **T, declared *T) { + if *dst == nil && declared != nil { + value := *declared + *dst = &value + } +} + // scalarTypes are the declared types a single terraform attribute holds // directly. var scalarTypes = map[string]bool{"string": true, "integer": true, "number": true, "boolean": true} @@ -393,6 +441,20 @@ func buildAttribute(wire string, attributeSite site) (Attribute, attributeEdges) } } + // What the document declares about the value itself. These are taken + // from the write side when there is one: a constraint is a rule about + // what may be sent, and a response schema restating it says nothing + // extra. writeOnly is the exception — only a request schema can declare + // it, so a response-only attribute could never carry it anyway. + attribute.Format = flatPrimary.format + attribute.WriteOnly = flatCreate.writeOnly + attribute.Deprecated = flatCreate.deprecated || flatRead.deprecated + attribute.UniqueItems = flatPrimary.uniqueItems + attribute.Pattern = flatPrimary.pattern + attribute.Minimum, attribute.Maximum = flatPrimary.minimum, flatPrimary.maximum + attribute.MinLength, attribute.MaxLength = flatPrimary.minLength, flatPrimary.maxLength + attribute.MinItems, attribute.MaxItems = flatPrimary.minItems, flatPrimary.maxItems + // Every attribute lands in exactly one of five outcomes, and this is where // four of them are chosen (the fifth, omitted entirely, is decided by // deriveType marking the attribute unsupported). diff --git a/internal/intermediate_representation/constraints_test.go b/internal/intermediate_representation/constraints_test.go new file mode 100644 index 0000000..7ea4f99 --- /dev/null +++ b/internal/intermediate_representation/constraints_test.go @@ -0,0 +1,127 @@ +package intermediate_representation + +import "testing" + +// constrainedSpec is one resource whose create body declares every +// constraint keyword, so one derivation proves the whole set reaches the +// attribute. +const constrainedSpec = `openapi: 3.0.3 +info: {title: T, version: "1"} +paths: + /keys: + post: + requestBody: + content: + application/json: + schema: {$ref: '#/components/schemas/Key'} + responses: + "201": + content: + application/json: + schema: {$ref: '#/components/schemas/Key'} + /keys/{keyId}: + get: + responses: + "200": + content: + application/json: + schema: {$ref: '#/components/schemas/Key'} + patch: + requestBody: + content: + application/json: + schema: {$ref: '#/components/schemas/Key'} + responses: + "200": + content: + application/json: + schema: {$ref: '#/components/schemas/Key'} + delete: + responses: + "204": {description: gone} +components: + schemas: + Key: + type: object + properties: + secret: + type: string + format: password + writeOnly: true + minLength: 8 + maxLength: 64 + pattern: "^[a-z]+$" + legacy: + type: string + deprecated: true + weight: + type: integer + minimum: 1 + maximum: 10 + tags: + type: array + uniqueItems: true + minItems: 1 + maxItems: 5 + items: {type: string} + plain: + type: string +` + +// TestUnit_Attribute_CarriesTheDeclaredConstraints proves the derivation +// carries every declared constraint onto the attribute. Nothing reads them +// yet; they are what the emitted validators, the sensitivity and the +// deprecation notice are decided from. +func TestUnit_Attribute_CarriesTheDeclaredConstraints(t *testing.T) { + r := resourceByKey(t, mustDerive(t, constrainedSpec, testConfig()), "key") + + secret := attribute(t, r.Schema, "secret") + if !secret.WriteOnly { + t.Error("WriteOnly not carried") + } + if secret.Format != "password" { + t.Errorf("Format = %q", secret.Format) + } + if secret.Pattern != "^[a-z]+$" { + t.Errorf("Pattern = %q", secret.Pattern) + } + assertBound(t, "MinLength", secret.MinLength, 8) + assertBound(t, "MaxLength", secret.MaxLength, 64) + + if !attribute(t, r.Schema, "legacy").Deprecated { + t.Error("Deprecated not carried") + } + + weight := attribute(t, r.Schema, "weight") + if weight.Minimum == nil || *weight.Minimum != 1 { + t.Errorf("Minimum = %v", weight.Minimum) + } + if weight.Maximum == nil || *weight.Maximum != 10 { + t.Errorf("Maximum = %v", weight.Maximum) + } + + tags := attribute(t, r.Schema, "tags") + if !tags.UniqueItems { + t.Error("UniqueItems not carried") + } + assertBound(t, "MinItems", tags.MinItems, 1) + assertBound(t, "MaxItems", tags.MaxItems, 5) + + plain := attribute(t, r.Schema, "plain") + if plain.WriteOnly || plain.Deprecated || plain.UniqueItems || + plain.Format != "" || plain.Pattern != "" || + plain.MinLength != nil || plain.MaxLength != nil { + t.Errorf("a property declaring nothing carries something: %+v", plain) + } +} + +func assertBound(t *testing.T, name string, got *int64, want int64) { + t.Helper() + if got == nil { + t.Errorf("%s not carried", name) + return + } + if *got != want { + t.Errorf("%s = %d, want %d", name, *got, want) + } +} diff --git a/internal/intermediate_representation/model.go b/internal/intermediate_representation/model.go index 9abc4af..102792b 100644 --- a/internal/intermediate_representation/model.go +++ b/internal/intermediate_representation/model.go @@ -322,6 +322,27 @@ type Attribute struct { // re-creation: x-tfpfgen-create-only, or every writable attribute of // a resource with no update operation. RequiresReplace bool `json:"requires_replace,omitempty"` + // 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"` + // WriteOnly marks a property the API accepts on write and never + // returns. + WriteOnly bool `json:"write_only,omitempty"` + // Deprecated marks a property the document declares deprecated. + Deprecated bool `json:"deprecated,omitempty"` + // UniqueItems marks a collection whose members are a set, so the order + // they are returned in carries no meaning. + UniqueItems bool `json:"unique_items,omitempty"` + // The constraints the document declares, nil or empty when it declares + // none. Each becomes a plan-time validator, so a configuration the API + // would refuse or silently clamp fails before it is sent. + Pattern string `json:"pattern,omitempty"` + Minimum *float64 `json:"minimum,omitempty"` + Maximum *float64 `json:"maximum,omitempty"` + MinLength *int64 `json:"min_length,omitempty"` + MaxLength *int64 `json:"max_length,omitempty"` + MinItems *int64 `json:"min_items,omitempty"` + MaxItems *int64 `json:"max_items,omitempty"` // OneOf lists a closed enum's values for a validator. OneOf []string `json:"one_of,omitempty"` // AdvisoryValues lists an open enum's known values diff --git a/internal/specmodel/constraints_test.go b/internal/specmodel/constraints_test.go new file mode 100644 index 0000000..1b62ad7 --- /dev/null +++ b/internal/specmodel/constraints_test.go @@ -0,0 +1,122 @@ +package specmodel + +import "testing" + +// constrainedSpec declares every constraint keyword the model reads, so one +// load proves the whole set arrives. +const constrainedSpec = `openapi: 3.0.3 +info: {title: T, version: "1"} +components: + schemas: + A: + type: object + properties: + secret: + type: string + format: password + writeOnly: true + minLength: 8 + maxLength: 64 + pattern: "^[a-z]+$" + legacy: + type: string + deprecated: true + weight: + type: integer + minimum: 1 + maximum: 10 + tags: + type: array + uniqueItems: true + minItems: 1 + maxItems: 5 + items: {type: string} + plain: + type: string +` + +// property fetches one property of the constrained schema, failing the test +// when the document does not declare it. +func property(t *testing.T, schema *Schema, name string) *Schema { + t.Helper() + got, ok := schema.Property(name) + if !ok { + t.Fatalf("no %s property", name) + } + return got +} + +// TestUnit_Specmodel_ReadsTheDeclaredConstraints proves every keyword the +// emitted schema needs survives the load, and that a schema declaring none +// carries none rather than a zero that reads as a bound. +func TestUnit_Specmodel_ReadsTheDeclaredConstraints(t *testing.T) { + doc, err := Load([]byte(constrainedSpec)) + if err != nil { + t.Fatalf("Load: %v", err) + } + a := doc.Schemas["A"] + if a == nil { + t.Fatal("no A schema") + } + + secret := property(t, a, "secret") + if !secret.WriteOnly { + t.Error("writeOnly not read") + } + if secret.Format != "password" { + t.Errorf("format = %q", secret.Format) + } + if secret.Pattern != "^[a-z]+$" { + t.Errorf("pattern = %q", secret.Pattern) + } + assertInt64(t, "minLength", secret.MinLength, 8) + assertInt64(t, "maxLength", secret.MaxLength, 64) + + if !property(t, a, "legacy").Deprecated { + t.Error("deprecated not read") + } + + weight := property(t, a, "weight") + assertFloat64(t, "minimum", weight.Minimum, 1) + assertFloat64(t, "maximum", weight.Maximum, 10) + + tags := property(t, a, "tags") + if !tags.UniqueItems { + t.Error("uniqueItems not read") + } + assertInt64(t, "minItems", tags.MinItems, 1) + assertInt64(t, "maxItems", tags.MaxItems, 5) + + // A property declaring nothing carries nothing: a nil bound and a zero + // bound mean different things, and only nil means the document is silent. + plain := property(t, a, "plain") + if plain.WriteOnly || plain.Deprecated || plain.UniqueItems { + t.Errorf("undeclared flags set: %+v", plain) + } + if plain.MinLength != nil || plain.MaxLength != nil || + plain.MinItems != nil || plain.MaxItems != nil { + t.Errorf("undeclared bounds set: %+v", plain) + } +} + +func assertInt64(t *testing.T, name string, got *int64, want int64) { + t.Helper() + if got == nil { + t.Errorf("%s not read", name) + return + } + if *got != want { + t.Errorf("%s = %d, want %d", name, *got, want) + } +} + +func assertFloat64(t *testing.T, name string, got *float64, want float64) { + t.Helper() + if got == nil { + t.Errorf("%s not read", name) + return + } + if *got != want { + t.Errorf("%s = %v, want %v", name, *got, want) + } +} diff --git a/internal/specmodel/load.go b/internal/specmodel/load.go index 86c1d42..e8d1c73 100644 --- a/internal/specmodel/load.go +++ b/internal/specmodel/load.go @@ -414,9 +414,24 @@ func (l *loader) schema(node *yaml.Node, at string) (*Schema, error) { if desc := lookup(node, "description"); desc != nil { s.Description = desc.Value } - if ro := deref(lookup(node, "readOnly")); ro != nil { - if err := ro.Decode(&s.ReadOnly); err != nil { - return nil, fmt.Errorf("%s.readOnly: must be true or false, got %q", at, ro.Value) + // The declared flags. readOnly and writeOnly decide whether an attribute + // is settable at all; deprecated and uniqueItems decide what the emitted + // schema says about it. + for _, field := range []struct { + key string + dst *bool + }{ + {"readOnly", &s.ReadOnly}, + {"writeOnly", &s.WriteOnly}, + {"deprecated", &s.Deprecated}, + {"uniqueItems", &s.UniqueItems}, + } { + n := deref(lookup(node, field.key)) + if n == nil { + continue + } + if err := n.Decode(field.dst); err != nil { + return nil, fmt.Errorf("%s.%s: must be true or false, got %q", at, field.key, n.Value) } } if enum := deref(lookup(node, "enum")); enum != nil { @@ -449,7 +464,8 @@ func (l *loader) schema(node *yaml.Node, at string) (*Schema, error) { // Numeric bounds are read because the audit sends values: a probe outside // the declared range tests the API's validation, not the field, and a // clamp or refusal read as behaviour is a finding the document already - // predicted. + // predicted. They also become plan-time validators, so a configuration + // the API would silently clamp fails before it is sent. for _, field := range []struct { key string dst **float64 @@ -465,6 +481,24 @@ func (l *loader) schema(node *yaml.Node, at string) (*Schema, error) { *field.dst = &v } } + // Length and size bounds, read for the same reason as the numeric ones. + for _, field := range []struct { + key string + dst **int64 + }{ + {"minLength", &s.MinLength}, + {"maxLength", &s.MaxLength}, + {"minItems", &s.MinItems}, + {"maxItems", &s.MaxItems}, + } { + if n := deref(lookup(node, field.key)); n != nil { + var v int64 + if err := n.Decode(&v); err != nil { + return nil, fmt.Errorf("%s.%s: %w", at, field.key, err) + } + *field.dst = &v + } + } if req := deref(lookup(node, "required")); req != nil { for _, rn := range req.Content { s.Required = append(s.Required, deref(rn).Value) diff --git a/internal/specmodel/load_test.go b/internal/specmodel/load_test.go index dec3a46..c82bcb4 100644 --- a/internal/specmodel/load_test.go +++ b/internal/specmodel/load_test.go @@ -459,6 +459,24 @@ func TestUnit_Specmodel_LoadRefusals(t *testing.T) { type: object readOnly: mostly `), "readOnly: must be true or false"}, + {"writeOnly is not a bool", minimal(`components: + schemas: + A: + type: object + writeOnly: sometimes +`), "writeOnly: must be true or false"}, + {"uniqueItems is not a bool", minimal(`components: + schemas: + A: + type: array + uniqueItems: occasionally +`), "uniqueItems: must be true or false"}, + {"maxLength is not a number", minimal(`components: + schemas: + A: + type: string + maxLength: plenty +`), "maxLength"}, {"schema is a sequence", minimal(`components: schemas: A: diff --git a/internal/specmodel/model.go b/internal/specmodel/model.go index 7b89c8a..56be3f2 100644 --- a/internal/specmodel/model.go +++ b/internal/specmodel/model.go @@ -113,6 +113,22 @@ type Schema struct { Description string // ReadOnly is the declared readOnly flag. ReadOnly bool + // WriteOnly is the declared writeOnly flag: the property is accepted on + // write and never returned. + WriteOnly bool + // Deprecated is the declared deprecated flag. + Deprecated bool + // UniqueItems is the declared uniqueItems flag on an array: its members + // are a set, so the order they come back in means nothing. + UniqueItems bool + // MinLength and MaxLength are the declared length bounds on a string; + // nil when absent. + MinLength *int64 + MaxLength *int64 + // MinItems and MaxItems are the declared size bounds on an array; nil + // when absent. + MinItems *int64 + MaxItems *int64 // Enum lists the declared enum values as decoded scalars. Enum []any // Default is the declared default value, decoded; nil when absent.