From 98788a9d628e9bd3eb5325834debac9ce12c422e Mon Sep 17 00:00:00 2001 From: Eddie Knight Date: Tue, 25 Aug 2026 04:24:00 -0500 Subject: [PATCH 1/2] fix(cmd): emit enum values and filter hidden definitions in cue2openapi - Disjunctions of string literals now emit OpenAPI enum values (handles nested chains and *"default" markers). - Hidden definitions (#_Name) are omitted from output; refs to them are rewritten to the visible definition they constrain. - Hidden struct fields (_name) no longer leak as properties. - make test now also runs the cmd/ test suite in CI. Signed-off-by: Eddie Knight --- Makefile | 1 + cmd/internal/cmd/converter.go | 107 +++++++++++++++++++++++++++-- cmd/internal/cmd/converter_test.go | 57 +++++++++++++++ 3 files changed, 160 insertions(+), 5 deletions(-) create mode 100644 cmd/internal/cmd/converter_test.go diff --git a/Makefile b/Makefile index e5ba828c..c7672742 100644 --- a/Makefile +++ b/Makefile @@ -9,6 +9,7 @@ all: tidy cuefmtcheck lintcue lintinsights test test: @echo " > Running schema validation tests ..." @cd test && go test -v ./... + @cd cmd && go test ./... @echo " > Schema validation tests complete." diff --git a/cmd/internal/cmd/converter.go b/cmd/internal/cmd/converter.go index fadd9db5..001b999f 100644 --- a/cmd/internal/cmd/converter.go +++ b/cmd/internal/cmd/converter.go @@ -40,6 +40,11 @@ type OpenAPIComponents struct { Schemas map[string]interface{} `yaml:"schemas" json:"schemas"` } +// hiddenBase maps a hidden definition name (e.g. "_MappingStrict") to the +// visible definition it constrains (e.g. "Mapping"), so refs can be rewritten +// and the hidden definition omitted from output. Populated before parsing. +var hiddenBase map[string]string + type SchemaInfo struct { Type string `yaml:"type,omitempty" json:"type,omitempty"` Description string `yaml:"description,omitempty" json:"description,omitempty"` @@ -48,6 +53,7 @@ type SchemaInfo struct { Pattern string `yaml:"pattern,omitempty" json:"pattern,omitempty"` Format string `yaml:"format,omitempty" json:"format,omitempty"` Items interface{} `yaml:"items,omitempty" json:"items,omitempty"` + Enum []string `yaml:"enum,omitempty" json:"enum,omitempty"` Ref string `yaml:"$ref,omitempty" json:"$ref,omitempty"` XStatus string `yaml:"x-status,omitempty" json:"x-status,omitempty"` } @@ -105,6 +111,26 @@ func convertCUEToOpenAPI(schemaDir, outputPath string, opts ConvertOpts) error { seen := make(map[string]bool) manifest := make(map[string][]string) // filename → schema names files := insts[0].Files + + // Pre-pass: map hidden definitions (#_Name) to the visible definition + // they constrain, so they can be filtered and their refs rewritten. + hiddenBase = make(map[string]string) + for _, f := range files { + for _, decl := range f.Decls { + field, ok := decl.(*ast.Field) + if !ok { + continue + } + ident, ok := field.Label.(*ast.Ident) + if !ok || !strings.HasPrefix(ident.Name, "#_") { + continue + } + if base := findBaseIdent(field.Value); base != "" { + hiddenBase[strings.TrimPrefix(ident.Name, "#")] = base + } + } + } + names := make([]string, 0, len(files)) byName := make(map[string]*ast.File) for _, f := range files { @@ -205,6 +231,9 @@ func parseFile(file *ast.File, spec *OpenAPISpec, seen map[string]bool, rootName continue } typeName := strings.TrimPrefix(ident.Name, "#") + if _, hidden := hiddenBase[typeName]; hidden { + continue // internal validation helper; refs are rewritten to its base + } if rootName != "" && ident.Name == "#"+rootName { if field.Comments() != nil { for _, cg := range field.Comments() { @@ -339,6 +368,73 @@ func parseDefinitionField(field *ast.Field, spec *OpenAPISpec, fileStatus string } } +// findBaseIdent walks a conjunction like `{...} & #Base & {...}` and returns +// the first visible definition name it embeds or unifies with. +func findBaseIdent(expr ast.Expr) string { + switch x := expr.(type) { + case *ast.Ident: + if strings.HasPrefix(x.Name, "#") && !strings.HasPrefix(x.Name, "#_") { + return strings.TrimPrefix(x.Name, "#") + } + case *ast.BinaryExpr: + if x.Op == token.AND { + if n := findBaseIdent(x.X); n != "" { + return n + } + return findBaseIdent(x.Y) + } + case *ast.StructLit: + for _, elt := range x.Elts { + if ed, ok := elt.(*ast.EmbedDecl); ok { + if n := findBaseIdent(ed.Expr); n != "" { + return n + } + } + } + } + return "" +} + +// refTarget resolves a definition reference name, substituting hidden +// definitions with the visible definition they constrain. +func refTarget(name string) string { + n := strings.TrimPrefix(name, "#") + if base, ok := hiddenBase[n]; ok { + return base + } + return n +} + +// collectEnumStrings flattens a disjunction of string literals ("a" | *"b" | "c") +// into its values. Returns ok=false if any branch is not a string literal. +func collectEnumStrings(expr ast.Expr) ([]string, bool) { + switch x := expr.(type) { + case *ast.BinaryExpr: + if x.Op != token.OR { + return nil, false + } + left, ok := collectEnumStrings(x.X) + if !ok { + return nil, false + } + right, ok := collectEnumStrings(x.Y) + if !ok { + return nil, false + } + return append(left, right...), true + case *ast.UnaryExpr: + // Default marker: *"value" + if x.Op == token.MUL { + return collectEnumStrings(x.X) + } + case *ast.BasicLit: + if x.Kind == token.STRING { + return []string{strings.Trim(x.Value, "\"")}, true + } + } + return nil, false +} + func convertStructToSchema(st *ast.StructLit, spec *OpenAPISpec, description string) *SchemaInfo { schema := &SchemaInfo{ Type: "object", @@ -355,7 +451,7 @@ func convertStructToSchema(st *ast.StructLit, spec *OpenAPISpec, description str fieldSchema := convertFieldToSchema(x, spec, pendingComment) if fieldSchema != nil { fieldName := getFieldName(x) - if fieldName != "" { + if fieldName != "" && !strings.HasPrefix(fieldName, "_") { schema.Properties[fieldName] = fieldSchema // Check if field is required if x.Optional == token.NoPos { @@ -427,8 +523,7 @@ func convertIdentToSchema(ident *ast.Ident, spec *OpenAPISpec, description strin return &SchemaInfo{Type: "boolean", Description: description} } else if strings.HasPrefix(name, "#") { // Type reference - refType := strings.TrimPrefix(name, "#") - return &SchemaInfo{Ref: fmt.Sprintf("#/components/schemas/%s", refType), Description: description} + return &SchemaInfo{Ref: fmt.Sprintf("#/components/schemas/%s", refTarget(name)), Description: description} } return &SchemaInfo{Type: "string", Description: description} @@ -461,6 +556,9 @@ func convertBinaryExprToSchema(expr *ast.BinaryExpr, spec *OpenAPISpec, descript // Handle union types (disjunctions) if expr.Op == token.OR { + if values, ok := collectEnumStrings(expr); ok { + return &SchemaInfo{Type: "string", Description: description, Enum: values} + } return &SchemaInfo{Type: "string", Description: description} } @@ -483,12 +581,11 @@ func convertListLitToSchema(list *ast.ListLit, spec *OpenAPISpec, description st // Also check for [#Contact, ...] pattern if ident, ok := elt.(*ast.Ident); ok { if strings.HasPrefix(ident.Name, "#") { - refType := strings.TrimPrefix(ident.Name, "#") return &SchemaInfo{ Type: "array", Description: description, Items: &SchemaInfo{ - Ref: fmt.Sprintf("#/components/schemas/%s", refType), + Ref: fmt.Sprintf("#/components/schemas/%s", refTarget(ident.Name)), }, } } diff --git a/cmd/internal/cmd/converter_test.go b/cmd/internal/cmd/converter_test.go new file mode 100644 index 00000000..a5f57c08 --- /dev/null +++ b/cmd/internal/cmd/converter_test.go @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: Apache-2.0 + +package cmd + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/goccy/go-yaml" +) + +func TestConvertCUEToOpenAPI(t *testing.T) { + out := filepath.Join(t.TempDir(), "openapi.yaml") + if err := convertCUEToOpenAPI("../../..", out, ConvertOpts{}); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(out) + if err != nil { + t.Fatal(err) + } + var spec struct { + Components struct { + Schemas map[string]SchemaInfo `yaml:"schemas"` + } `yaml:"components"` + } + if err := yaml.Unmarshal(data, &spec); err != nil { + t.Fatal(err) + } + schemas := spec.Components.Schemas + + // Disjunctions of string literals emit enum values. + if got := schemas["MethodType"].Enum; len(got) != 4 { + t.Errorf("MethodType enum = %v, want 4 values", got) + } + if got := schemas["Lifecycle"].Enum; len(got) != 4 { // includes the *"Active" default + t.Errorf("Lifecycle enum = %v, want 4 values", got) + } + + // Hidden definitions and hidden fields don't leak into the output. + for name, s := range schemas { + if strings.HasPrefix(name, "_") { + t.Errorf("hidden definition %s leaked into schemas", name) + } + for prop := range s.Properties { + if strings.HasPrefix(prop, "_") { + t.Errorf("hidden field %s.%s leaked into properties", name, prop) + } + } + } + + // Refs to hidden definitions resolve to their visible base type. + if !strings.Contains(string(data), "schemas/Mapping") || strings.Contains(string(data), "schemas/_") { + t.Errorf("refs to hidden definitions not rewritten to base types") + } +} From 16fbcd18aa0efcd13ba39f0ba280dc2ea47934fd Mon Sep 17 00:00:00 2001 From: Eddie Knight Date: Tue, 25 Aug 2026 04:40:26 -0500 Subject: [PATCH 2/2] fix(cmd): fail on unresolvable hidden definitions and tighten converter tests - Error out in the pre-pass when a #_ definition does not unify with a visible definition, instead of silently leaving it in the output where it only surfaced as a confusing leak-check test failure. - Document that ref rewriting intentionally drops the hidden definition's extra constraints, that findBaseIdent ignores attribute-only structs, and that CUE default markers are deliberately not emitted as OpenAPI defaults. - Treat only unquoted _name labels as hidden fields; a quoted "_name" is an exported CUE field. - Assert exact enum values (citing their CUE sources), assert the rewritten $ref structurally instead of via a loose substring match, and add a table test for collectEnumStrings' non-enum fallback. - Run the test suites with -count=1: they read *.cue schemas at runtime, which Go's test cache cannot track. Signed-off-by: Eddie Knight --- Makefile | 6 ++-- cmd/internal/cmd/converter.go | 24 ++++++++++--- cmd/internal/cmd/converter_test.go | 58 +++++++++++++++++++++++++----- 3 files changed, 73 insertions(+), 15 deletions(-) diff --git a/Makefile b/Makefile index c7672742..137b94d0 100644 --- a/Makefile +++ b/Makefile @@ -6,10 +6,12 @@ all: tidy cuefmtcheck lintcue lintinsights test # SCHEMA VALIDATION TESTS # +# -count=1: the tests read the *.cue schemas at runtime, which Go's test cache +# cannot track, so cached passes can go stale against edited schemas. test: @echo " > Running schema validation tests ..." - @cd test && go test -v ./... - @cd cmd && go test ./... + @cd test && go test -v -count=1 ./... + @cd cmd && go test -v -count=1 ./... @echo " > Schema validation tests complete." diff --git a/cmd/internal/cmd/converter.go b/cmd/internal/cmd/converter.go index 001b999f..18eda35c 100644 --- a/cmd/internal/cmd/converter.go +++ b/cmd/internal/cmd/converter.go @@ -43,6 +43,9 @@ type OpenAPIComponents struct { // hiddenBase maps a hidden definition name (e.g. "_MappingStrict") to the // visible definition it constrains (e.g. "Mapping"), so refs can be rewritten // and the hidden definition omitted from output. Populated before parsing. +// Rewriting a ref intentionally drops the hidden definition's extra +// constraints (e.g. conditional requiredness), which OpenAPI cannot express; +// the ref collapses to the visible base type. var hiddenBase map[string]string type SchemaInfo struct { @@ -125,9 +128,11 @@ func convertCUEToOpenAPI(schemaDir, outputPath string, opts ConvertOpts) error { if !ok || !strings.HasPrefix(ident.Name, "#_") { continue } - if base := findBaseIdent(field.Value); base != "" { - hiddenBase[strings.TrimPrefix(ident.Name, "#")] = base + base := findBaseIdent(field.Value) + if base == "" { + return fmt.Errorf("hidden definition %s does not unify with a visible definition; cue2openapi cannot rewrite refs to it", ident.Name) } + hiddenBase[strings.TrimPrefix(ident.Name, "#")] = base } } @@ -369,7 +374,9 @@ func parseDefinitionField(field *ast.Field, spec *OpenAPISpec, fileStatus string } // findBaseIdent walks a conjunction like `{...} & #Base & {...}` and returns -// the first visible definition name it embeds or unifies with. +// the first visible definition name it embeds or unifies with. Inside struct +// literals only embed declarations are inspected, so an attribute-only struct +// like `{@go(-)}` never counts as a base. func findBaseIdent(expr ast.Expr) string { switch x := expr.(type) { case *ast.Ident: @@ -423,7 +430,8 @@ func collectEnumStrings(expr ast.Expr) ([]string, bool) { } return append(left, right...), true case *ast.UnaryExpr: - // Default marker: *"value" + // Default marker: *"value". The default itself is deliberately not + // emitted as an OpenAPI `default`; only the enum value is kept. if x.Op == token.MUL { return collectEnumStrings(x.X) } @@ -451,7 +459,13 @@ func convertStructToSchema(st *ast.StructLit, spec *OpenAPISpec, description str fieldSchema := convertFieldToSchema(x, spec, pendingComment) if fieldSchema != nil { fieldName := getFieldName(x) - if fieldName != "" && !strings.HasPrefix(fieldName, "_") { + // Only an unquoted _name label is hidden in CUE; a quoted + // "_name" is a regular exported field. + hidden := false + if id, ok := x.Label.(*ast.Ident); ok { + hidden = strings.HasPrefix(id.Name, "_") + } + if fieldName != "" && !hidden { schema.Properties[fieldName] = fieldSchema // Check if field is required if x.Optional == token.NoPos { diff --git a/cmd/internal/cmd/converter_test.go b/cmd/internal/cmd/converter_test.go index a5f57c08..4aeb6598 100644 --- a/cmd/internal/cmd/converter_test.go +++ b/cmd/internal/cmd/converter_test.go @@ -5,9 +5,11 @@ package cmd import ( "os" "path/filepath" + "slices" "strings" "testing" + "cuelang.org/go/cue/parser" "github.com/goccy/go-yaml" ) @@ -30,12 +32,15 @@ func TestConvertCUEToOpenAPI(t *testing.T) { } schemas := spec.Components.Schemas - // Disjunctions of string literals emit enum values. - if got := schemas["MethodType"].Enum; len(got) != 4 { - t.Errorf("MethodType enum = %v, want 4 values", got) + // Disjunctions of string literals emit enum values. Expected values come + // from #MethodType (policy.cue) and #Lifecycle (collections.cue); update + // here when those CUE enums change. + if got := schemas["MethodType"].Enum; !slices.Equal(got, []string{"Behavioral", "Intent", "Remediation", "Gate"}) { + t.Errorf("MethodType enum = %v", got) } - if got := schemas["Lifecycle"].Enum; len(got) != 4 { // includes the *"Active" default - t.Errorf("Lifecycle enum = %v, want 4 values", got) + // "Active" is the *default in CUE; it appears here as a plain enum value. + if got := schemas["Lifecycle"].Enum; !slices.Equal(got, []string{"Active", "Draft", "Deprecated", "Retired"}) { + t.Errorf("Lifecycle enum = %v", got) } // Hidden definitions and hidden fields don't leak into the output. @@ -50,8 +55,45 @@ func TestConvertCUEToOpenAPI(t *testing.T) { } } - // Refs to hidden definitions resolve to their visible base type. - if !strings.Contains(string(data), "schemas/Mapping") || strings.Contains(string(data), "schemas/_") { - t.Errorf("refs to hidden definitions not rewritten to base types") + // Refs to hidden definitions resolve to their visible base type + // (#_MappingStrict -> Mapping, mappingdocument.cue). + mappings, ok := schemas["MappingDocument"].Properties["mappings"].(map[string]interface{}) + if !ok { + t.Fatalf("MappingDocument.mappings missing or not a map: %v", schemas["MappingDocument"].Properties["mappings"]) + } + items, _ := mappings["items"].(map[string]interface{}) + if ref, _ := items["$ref"].(string); ref != "#/components/schemas/Mapping" { + t.Errorf("MappingDocument.mappings items $ref = %q, want #/components/schemas/Mapping", ref) + } + if strings.Contains(string(data), "schemas/_") { + t.Errorf("ref to a hidden definition leaked into output") + } +} + +func TestCollectEnumStrings(t *testing.T) { + cases := []struct { + expr string + want []string // nil: expect ok=false + }{ + {`"a" | "b" | "c"`, []string{"a", "b", "c"}}, + {`*"a" | "b"`, []string{"a", "b"}}, + {`"a" | *"b" | "c"`, []string{"a", "b", "c"}}, + {`"a" | #Other`, nil}, + {`int | string`, nil}, + {`"a" & "b"`, nil}, + } + for _, tc := range cases { + expr, err := parser.ParseExpr("test", tc.expr) + if err != nil { + t.Fatalf("%s: %v", tc.expr, err) + } + got, ok := collectEnumStrings(expr) + if tc.want == nil { + if ok { + t.Errorf("%s: expected ok=false, got %v", tc.expr, got) + } + } else if !ok || !slices.Equal(got, tc.want) { + t.Errorf("%s: got %v (ok=%v), want %v", tc.expr, got, ok, tc.want) + } } }