From c81b14130235451d607e175f9fa011fbe6bfb8a3 Mon Sep 17 00:00:00 2001 From: Jerrin Francis Date: Mon, 4 May 2026 06:40:26 +0530 Subject: [PATCH] Fix: handle optional collections in CUE strict mode in defkit (#7102) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(defkit): handle optional collections in CUE strict mode Applying a defkit-generated ComponentDefinition that referenced optional Array/Map params via SetIf guards failed at template render with errors like: output.metadata: cannot reference optional field: labels output.spec.template.spec.containers.0: cannot reference optional field: args parameter: cannot reference optional field: volume The cuegen was emitting `parameter.X` (dot syntax), `len(parameter.X)`, and OneOf-with-default discriminator blocks that all violate CUE strict mode. Update condition rendering to match the bracket-existence pattern used by KubeVela's built-in components (cron-task.cue, daemon.cue): - LenCondition / ArrayContainsCondition / MapHasKeyCondition on collection params now emit `parameter["X"] != _|_` (or `== _|_` for IsEmpty). Trade-off: IsNotEmpty / LenGt(0) / Contains() collapse to existence checks; for exact-length predicates use Validators(...) on the parameter schema. - LenCondition on String params is unchanged (raw `len(...)`) since string length checks are typically used in Validators against required/defaulted strings where strict mode does not fire. - OneOfParam with HasDefault() drops the `?` marker so the sibling- scope `if name == "..."` blocks can reference the discriminator without strict-mode errors. Mirrors how Bool with Default() behaves. Two related fixes bundled in: - StringKeyMapParam gains HasKey, IsEmpty, IsNotEmpty, LenEq, LenGt for parity with MapParam — they generate identical CUE today. - ArrayParam.RequiredImports() reports the "list" stdlib import when MinItems/MaxItems is set, and the import-detection walker now visits SetIfOp/SpreadIfOp/IfBlock condition operands. Signed-off-by: Jerrin Francis * Fixing go lint issues Signed-off-by: Jerrin Francis * fix: Reverting ArrayContainsCondition Signed-off-by: Jerrin Francis * fix(defkit): chained-if guards + AbsentOrEmpty for collection conditions Extends PR #7102 to fix several semantic and structural bugs in how conditions on optional collection parameters render to CUE. LenCondition: drop the unused `fallback` field and render uniformly as `parameter["X"] != _|_ if len(parameter["X"]) op N`. Restores exact-length semantics that were previously collapsed to bare existence checks; works for required strings too (the outer guard always passes). AbsentOrEmptyCondition (new): returned by IsEmpty() and LenEq(0) on Array/Map/StringKeyMap params. Expands at render time into TWO if blocks (absent + set-and-empty) since CUE cannot express "absent OR empty" as a single boolean — `||` is strict in both operands and `len(_|_)` propagates bottom. Fires on both nil and empty inputs, symmetric with IsNotEmpty(). ArrayContainsCondition: render as `parameter["X"] != _|_ if list.Contains(parameter["X"], val)` instead of the `&&`-joined form. CUE does not short-circuit `&&`, so list.Contains was evaluated against `_|_` when the field was absent. Compound joiners (AndCondition, LogicalExpr AND mode, AllConditionsCondition): detect chained-guard operands via a new `usesChainedGuard` helper and join with ` if ` instead of ` && ` — chained-if expressions are invalid inside `(...) && (...)`. writeValidator: refactored through a new `writeIfBlocksForCond` helper so both FailWhen and OnlyWhen correctly expand AbsentOrEmptyCondition into two if blocks (the validator struct duplicates under each guard). writeFieldNode bracket-access leaf: previously dropped node.cond and condValues entirely, emitting bracket-access fields unconditionally. Now mirrors the regular-field rendering so SetIf(cond, "data[hyphen-key]", value) emits the expected if-block wrapper. Tests: 1198 specs pass. Removes 4 obsolete Fallback() tests; adds regression coverage for FailWhen/OnlyWhen with collection IsEmpty(), bracket-access conditional rendering, and the IsEmpty two-if-block form. Signed-off-by: Jerrin Francis * Fixing the lint errors Signed-off-by: Jerrin Francis * Adding test case for handling hyphenated fields in cue Signed-off-by: Jerrin Francis --------- Signed-off-by: Jerrin Francis --- pkg/definition/defkit/cuegen.go | 324 +++++++++++--- pkg/definition/defkit/cuegen_test.go | 411 ++++++++++++++++++ pkg/definition/defkit/expr.go | 43 +- pkg/definition/defkit/param.go | 112 ++++- .../defkit/param_constraints_test.go | 69 ++- pkg/definition/defkit/param_test.go | 38 ++ pkg/definition/defkit/validator_test.go | 45 ++ 7 files changed, 928 insertions(+), 114 deletions(-) diff --git a/pkg/definition/defkit/cuegen.go b/pkg/definition/defkit/cuegen.go index a6ac8b13d..2c48eeac1 100644 --- a/pkg/definition/defkit/cuegen.go +++ b/pkg/definition/defkit/cuegen.go @@ -269,15 +269,19 @@ func (g *CUEGenerator) collectImportsFromResource(res *Resource) { g.collectImportsFromValue(o.Value()) case *SetIfOp: g.collectImportsFromValue(o.Value()) + g.collectImportsFromValue(o.Cond()) case *SpreadIfOp: g.collectImportsFromValue(o.Value()) + g.collectImportsFromValue(o.Cond()) case *IfBlock: + g.collectImportsFromValue(o.Cond()) for _, innerOp := range o.Ops() { switch inner := innerOp.(type) { case *SetOp: g.collectImportsFromValue(inner.Value()) case *SetIfOp: g.collectImportsFromValue(inner.Value()) + g.collectImportsFromValue(inner.Cond()) } } } @@ -526,34 +530,48 @@ func (g *CUEGenerator) writeValidator(sb *strings.Builder, v *Validator, depth i name = "_validate" } + writeBody := func(bodyIndent, innerBodyIndent string) { + sb.WriteString(fmt.Sprintf("%s%s: {\n", bodyIndent, name)) + sb.WriteString(fmt.Sprintf("%s%q: true\n", innerBodyIndent, v.Message())) + if v.FailCondition() != nil { + g.writeIfBlocksForCond(sb, v.FailCondition(), innerBodyIndent, func() { + sb.WriteString(fmt.Sprintf("%s\t%q: false\n", innerBodyIndent, v.Message())) + }) + } + sb.WriteString(fmt.Sprintf("%s}\n", bodyIndent)) + } + if v.GuardCondition() != nil { // Guarded validator: wrap in if guard { ... } - guardCUE := g.conditionToCUE(v.GuardCondition()) - sb.WriteString(fmt.Sprintf("%sif %s {\n", indent, guardCUE)) - sb.WriteString(fmt.Sprintf("%s%s: {\n", inner, name)) - sb.WriteString(fmt.Sprintf("%s%q: true\n", inner2, v.Message())) - if v.FailCondition() != nil { - failCUE := g.conditionToCUE(v.FailCondition()) - sb.WriteString(fmt.Sprintf("%sif %s {\n", inner2, failCUE)) - sb.WriteString(fmt.Sprintf("%s\t%q: false\n", inner2, v.Message())) - sb.WriteString(fmt.Sprintf("%s}\n", inner2)) - } - sb.WriteString(fmt.Sprintf("%s}\n", inner)) - sb.WriteString(fmt.Sprintf("%s}\n", indent)) + g.writeIfBlocksForCond(sb, v.GuardCondition(), indent, func() { + writeBody(inner, inner2) + }) } else { - // Unguarded validator - sb.WriteString(fmt.Sprintf("%s%s: {\n", indent, name)) - sb.WriteString(fmt.Sprintf("%s%q: true\n", inner, v.Message())) - if v.FailCondition() != nil { - failCUE := g.conditionToCUE(v.FailCondition()) - sb.WriteString(fmt.Sprintf("%sif %s {\n", inner, failCUE)) - sb.WriteString(fmt.Sprintf("%s\t%q: false\n", inner, v.Message())) - sb.WriteString(fmt.Sprintf("%s}\n", inner)) - } - sb.WriteString(fmt.Sprintf("%s}\n", indent)) + writeBody(indent, inner) } } +// writeIfBlocksForCond writes one or more `if cond { body }` blocks. For +// AbsentOrEmptyCondition the body is duplicated into two blocks (one for +// each branch) — CUE's `||` cannot express "absent OR empty" safely on +// optional fields, so the structural duplication is required. For all other +// conditions a single if block is emitted. +func (g *CUEGenerator) writeIfBlocksForCond(sb *strings.Builder, cond Condition, indent string, writeBody func()) { + if aoe, ok := cond.(*AbsentOrEmptyCondition); ok { + for _, branch := range aoe.Branches() { + condCUE := g.conditionToCUE(branch) + sb.WriteString(fmt.Sprintf("%sif %s {\n", indent, condCUE)) + writeBody() + sb.WriteString(fmt.Sprintf("%s}\n", indent)) + } + return + } + condCUE := g.conditionToCUE(cond) + sb.WriteString(fmt.Sprintf("%sif %s {\n", indent, condCUE)) + writeBody() + sb.WriteString(fmt.Sprintf("%s}\n", indent)) +} + // writeConditionalParamBlock writes conditional parameter branches. // Example output: // @@ -1350,10 +1368,25 @@ func (g *CUEGenerator) buildFieldTree(ops []ResourceOp) *fieldNode { case *SetOp: g.insertIntoTree(root, o.Path(), o.Value(), nil) case *SetIfOp: - g.insertIntoTree(root, o.Path(), o.Value(), o.Cond()) + // Expand AbsentOrEmptyCondition into branches: each branch + // produces its own conditional value at the same path. The tree + // renderer emits one if block per condValue. + if aoe, ok := o.Cond().(*AbsentOrEmptyCondition); ok { + for _, branch := range aoe.Branches() { + g.insertIntoTree(root, o.Path(), o.Value(), branch) + } + } else { + g.insertIntoTree(root, o.Path(), o.Value(), o.Cond()) + } case *SpreadIfOp: // SpreadIfOp adds a spread entry to the target node - g.insertSpreadIntoTree(root, o.Path(), o.Value(), o.Cond()) + if aoe, ok := o.Cond().(*AbsentOrEmptyCondition); ok { + for _, branch := range aoe.Branches() { + g.insertSpreadIntoTree(root, o.Path(), o.Value(), branch) + } + } else { + g.insertSpreadIntoTree(root, o.Path(), o.Value(), o.Cond()) + } case *ForEachOp: // ForEachOp creates a for-each iteration at the target path g.insertForEachIntoTree(root, o, nil) @@ -1374,13 +1407,28 @@ func (g *CUEGenerator) buildFieldTree(ops []ResourceOp) *fieldNode { case *SetOp: g.insertIntoTree(root, inner.Path(), inner.Value(), o.Cond()) case *SetIfOp: - // Combine conditions - combinedCond := &AndCondition{left: o.Cond(), right: inner.Cond()} - g.insertIntoTree(root, inner.Path(), inner.Value(), combinedCond) + // Combine conditions. If inner is AbsentOrEmpty, expand + // each branch and combine the outer block's cond with each. + if aoe, ok := inner.Cond().(*AbsentOrEmptyCondition); ok { + for _, branch := range aoe.Branches() { + combined := &AndCondition{left: o.Cond(), right: branch} + g.insertIntoTree(root, inner.Path(), inner.Value(), combined) + } + } else { + combinedCond := &AndCondition{left: o.Cond(), right: inner.Cond()} + g.insertIntoTree(root, inner.Path(), inner.Value(), combinedCond) + } case *SpreadIfOp: // Combine conditions for spread - combinedCond := &AndCondition{left: o.Cond(), right: inner.Cond()} - g.insertSpreadIntoTree(root, inner.Path(), inner.Value(), combinedCond) + if aoe, ok := inner.Cond().(*AbsentOrEmptyCondition); ok { + for _, branch := range aoe.Branches() { + combined := &AndCondition{left: o.Cond(), right: branch} + g.insertSpreadIntoTree(root, inner.Path(), inner.Value(), combined) + } + } else { + combinedCond := &AndCondition{left: o.Cond(), right: inner.Cond()} + g.insertSpreadIntoTree(root, inner.Path(), inner.Value(), combinedCond) + } case *ForEachOp: // ForEach inside an if block - pass the block's condition g.insertForEachIntoTree(root, inner, o.Cond()) @@ -1866,16 +1914,7 @@ func (g *CUEGenerator) writeFieldNode(sb *strings.Builder, name string, node *fi // Handle bracket notation in name (like [app.oam.dev/name]) if strings.HasPrefix(name, "[") && !strings.HasPrefix(name, "[0]") { - // This is a map key access - extract the key - key := strings.Trim(name, "[]") - if node.value != nil { - valStr := g.valueToCUE(node.value) - sb.WriteString(fmt.Sprintf("%s%q: %s\n", indent, key, valStr)) - } else if len(node.children) > 0 { - sb.WriteString(fmt.Sprintf("%s%q: {\n", indent, key)) - g.writeFieldTree(sb, node, depth+1) - sb.WriteString(fmt.Sprintf("%s}\n", indent)) - } + g.writeBracketKeyNode(sb, name, node, indent, depth) return } @@ -1935,6 +1974,51 @@ func (g *CUEGenerator) writeFieldNode(sb *strings.Builder, name string, node *fi } } +// writeBracketKeyNode renders a map-key access leaf (`[key]`) honoring the +// node's primary condition and any additional condValues. Split out from +// writeFieldNode so the dispatcher stays under gocritic's ifElseChain check. +func (g *CUEGenerator) writeBracketKeyNode(sb *strings.Builder, name string, node *fieldNode, indent string, depth int) { + key := strings.Trim(name, "[]") + quoted := fmt.Sprintf("%q", key) + + // Subtree: render as a nested struct (no condition handling — children + // carry their own conditions). + if node.value == nil { + if len(node.children) > 0 { + sb.WriteString(fmt.Sprintf("%s%s: {\n", indent, quoted)) + g.writeFieldTree(sb, node, depth+1) + sb.WriteString(fmt.Sprintf("%s}\n", indent)) + } + return + } + + valStr := g.valueToCUE(node.value) + writeGuarded := func(cond Condition, v string) { + condStr := g.conditionToCUE(cond) + sb.WriteString(fmt.Sprintf("%sif %s {\n", indent, condStr)) + sb.WriteString(fmt.Sprintf("%s\t%s: %s\n", indent, quoted, v)) + sb.WriteString(fmt.Sprintf("%s}\n", indent)) + } + + switch { + case len(node.condValues) > 0: + // Multiple conditional values at the same bracket-access path — + // render each inside its own if block. + if node.cond != nil { + writeGuarded(node.cond, valStr) + } else { + sb.WriteString(fmt.Sprintf("%s%s: %s\n", indent, quoted, valStr)) + } + for _, cv := range node.condValues { + writeGuarded(cv.cond, g.valueToCUE(cv.value)) + } + case node.cond != nil: + writeGuarded(node.cond, valStr) + default: + sb.WriteString(fmt.Sprintf("%s%s: %s\n", indent, quoted, valStr)) + } +} + // canDecomposeByCondition checks if a struct node's children can be split into // separate conditional blocks. This is possible when every child subtree has // the same uniform set of leaf conditions (e.g., every leaf is guarded by either @@ -2101,6 +2185,10 @@ func (g *CUEGenerator) valueToCUE(v Value) string { // Return reference to the helper by name return val.Name() case *StringParam, *IntParam, *BoolParam, *FloatParam, *ArrayParam, *MapParam, *StringKeyMapParam, *EnumParam, *OneOfParam: + // Dot syntax is safe here: the call sites that wrap value refs in a + // guarded if-block (e.g. `if len(parameter.X | []) > 0 { foo: parameter.X }`) + // have already established the field is concrete before the body + // evaluates. return "parameter." + v.(Param).Name() case *DynamicMapParam: // Dynamic map parameters reference just "parameter" @@ -2907,11 +2995,17 @@ func (g *CUEGenerator) conditionToCUE(cond Condition) string { case *StringEndsWithCondition: return fmt.Sprintf(`strings.HasSuffix(parameter.%s, %q)`, c.ParamName(), c.Suffix()) case *LenCondition: - return fmt.Sprintf("len(parameter.%s) %s %d", c.ParamName(), c.Op(), c.Length()) + return g.lenConditionToCUE(c) + case *AbsentOrEmptyCondition: + return g.absentOrEmptyConditionToCUE(c) case *ArrayContainsCondition: - return fmt.Sprintf("list.Contains(parameter.%s, %s)", c.ParamName(), formatCUEValue(c.Value())) + return g.arrayContainsConditionToCUE(c) case *MapHasKeyCondition: - return fmt.Sprintf("parameter.%s.%s != _|_", c.ParamName(), c.Key()) + // daemon.cue's idiom for nested optional access: bracket on the + // outer (optional) map, dot on the inner key (concrete after the + // outer guard). + return fmt.Sprintf(`parameter[%q] != _|_ && parameter[%q].%s != _|_`, + c.ParamName(), c.ParamName(), c.Key()) case *ParamCompareCondition: // Parameter comparison: parameter.name op value return fmt.Sprintf("parameter.%s %s %s", c.ParamName(), c.Op(), formatCUEValue(c.CompareValue())) @@ -2920,29 +3014,11 @@ func (g *CUEGenerator) conditionToCUE(cond Condition) string { right := g.exprToCUE(c.Right()) return fmt.Sprintf("%s %s %s", left, c.Op(), right) case *AndCondition: - left := g.conditionToCUE(c.left) - right := g.conditionToCUE(c.right) - return fmt.Sprintf("(%s) && (%s)", left, right) + return g.andConditionToCUE(c) case *LogicalExpr: - parts := make([]string, len(c.Conditions())) - for i, sub := range c.Conditions() { - parts[i] = g.conditionToCUE(sub) - } - op := " && " - if c.Op() == OpOr { - op = " || " - } - return strings.Join(parts, op) + return g.logicalExprToCUE(c) case *NotExpr: - // Special case: Not(IsSet("x")) -> parameter["x"] == _|_ - if isSet, ok := c.Cond().(*IsSetCondition); ok { - return fmt.Sprintf("parameter[%q] == _|_", isSet.ParamName()) - } - // Special case: Not(PathExists("x")) -> x == _|_ - if pe, ok := c.Cond().(*PathExistsCondition); ok { - return fmt.Sprintf("%s == _|_", pe.Path()) - } - return fmt.Sprintf("!(%s)", g.conditionToCUE(c.Cond())) + return g.notExprToCUE(c) case *HasExposedPortsCondition: // Check if any port has expose=true portsStr := g.valueToCUE(c.Ports()) @@ -2971,14 +3047,7 @@ func (g *CUEGenerator) conditionToCUE(cond Condition) string { // Check if a context.output path exists return fmt.Sprintf("context.output.%s != _|_", c.Path()) case *AllConditionsCondition: - // Generate compound condition: if cond1 if cond2 ... - var parts []string - for _, cond := range c.Conditions() { - parts = append(parts, g.conditionToCUE(cond)) - } - // For CUE, we generate: cond1 && cond2 && cond3 - // which will be used in a single if statement - return strings.Join(parts, " && ") + return g.allConditionsConditionToCUE(c) case *RegexMatchCondition: // General-purpose regex match: =~ "pattern" return fmt.Sprintf(`%s =~ %q`, g.valueToCUE(c.Source()), c.Pattern()) @@ -2990,6 +3059,114 @@ func (g *CUEGenerator) conditionToCUE(cond Condition) string { } } +// usesChainedGuard returns true for condition types whose conditionToCUE output +// uses CUE chained-if syntax (e.g. `guard if inner`). These conditions cannot +// be placed inside `(…) && (…)` — compound conditions must join with ` if ` +// instead of ` && ` when any operand uses chained guards. +func usesChainedGuard(c Condition) bool { + switch c.(type) { + case *ArrayContainsCondition, *LenCondition: + return true + } + return false +} + +// lenConditionToCUE renders a LenCondition using CUE's chained-if guard +// pattern: `parameter["X"] != _|_ if len(parameter["X"]) op N`. +// +// The bracket-existence guard handles CUE strict mode on optional fields +// (dot syntax `parameter.X` errors on `_|_`). The second `if` only evaluates +// when the first passes, so `len()` never references an absent value. +// For required fields the outer guard always passes — a few characters +// longer than `len(parameter.X) op N` but correctness is uniform across +// required and optional fields. Pattern matches the chained-if form already +// used at cuegen.go:1145 for compound optional access. +func (g *CUEGenerator) lenConditionToCUE(c *LenCondition) string { + return fmt.Sprintf(`parameter[%q] != _|_ if len(parameter[%q]) %s %d`, + c.ParamName(), c.ParamName(), c.Op(), c.Length()) +} + +// absentOrEmptyConditionToCUE is the conditionToCUE fallback for paths that +// haven't been updated to expand AbsentOrEmpty branches into separate if +// blocks. It renders only the "set and empty" branch (the "absent" branch is +// lost). Top-level SetIfOp / SpreadIfOp in buildFieldTree DO expand and +// render both branches correctly via the field tree's condValues. +func (g *CUEGenerator) absentOrEmptyConditionToCUE(c *AbsentOrEmptyCondition) string { + return fmt.Sprintf(`parameter[%q] != _|_ if len(parameter[%q]) == 0`, + c.ParamName(), c.ParamName()) +} + +// arrayContainsConditionToCUE renders an ArrayContainsCondition as the +// chained-if guard `parameter["X"] != _|_ if list.Contains(parameter["X"], val)`. +// CUE does not short-circuit `&&`, so the inner list.Contains would otherwise +// be evaluated against `_|_` when the field is absent. +func (g *CUEGenerator) arrayContainsConditionToCUE(c *ArrayContainsCondition) string { + return fmt.Sprintf(`parameter[%q] != _|_ if list.Contains(parameter[%q], %s)`, + c.ParamName(), c.ParamName(), formatCUEValue(c.Value())) +} + +// andConditionToCUE renders an AndCondition. If either operand uses chained-if +// guard syntax (e.g. ArrayContainsCondition), join with ` if ` instead of +// ` && ` because chained-if expressions are invalid inside `(...) && (...)`. +func (g *CUEGenerator) andConditionToCUE(c *AndCondition) string { + left := g.conditionToCUE(c.left) + right := g.conditionToCUE(c.right) + if usesChainedGuard(c.left) || usesChainedGuard(c.right) { + return fmt.Sprintf("%s if %s", left, right) + } + return fmt.Sprintf("(%s) && (%s)", left, right) +} + +// logicalExprToCUE renders a LogicalExpr (AND/OR over N conditions). For +// AND mode with any chained-guard operand, joins with ` if `; otherwise +// ` && `. OR mode always joins with ` || `. +func (g *CUEGenerator) logicalExprToCUE(c *LogicalExpr) string { + parts := make([]string, len(c.Conditions())) + anyChained := false + for i, sub := range c.Conditions() { + parts[i] = g.conditionToCUE(sub) + if usesChainedGuard(sub) { + anyChained = true + } + } + if c.Op() == OpOr { + return strings.Join(parts, " || ") + } + if anyChained { + return strings.Join(parts, " if ") + } + return strings.Join(parts, " && ") +} + +// allConditionsConditionToCUE renders an AllConditionsCondition (AND over N +// conditions). Joins with ` if ` when any operand uses chained-guard syntax. +func (g *CUEGenerator) allConditionsConditionToCUE(c *AllConditionsCondition) string { + parts := make([]string, 0, len(c.Conditions())) + anyChained := false + for _, cond := range c.Conditions() { + parts = append(parts, g.conditionToCUE(cond)) + if usesChainedGuard(cond) { + anyChained = true + } + } + if anyChained { + return strings.Join(parts, " if ") + } + return strings.Join(parts, " && ") +} + +// notExprToCUE renders a NotExpr, special-casing Not(IsSet) and +// Not(PathExists) to the canonical `X == _|_` form. +func (g *CUEGenerator) notExprToCUE(c *NotExpr) string { + if isSet, ok := c.Cond().(*IsSetCondition); ok { + return fmt.Sprintf("parameter[%q] == _|_", isSet.ParamName()) + } + if pe, ok := c.Cond().(*PathExistsCondition); ok { + return fmt.Sprintf("%s == _|_", pe.Path()) + } + return fmt.Sprintf("!(%s)", g.conditionToCUE(c.Cond())) +} + // inConditionToCUE converts an InCondition to CUE syntax. // Generates: parameter.name == val1 || parameter.name == val2 || ... func (g *CUEGenerator) inConditionToCUE(c *InCondition) string { @@ -3586,6 +3763,11 @@ func (g *CUEGenerator) writeOneOfParam(sb *strings.Builder, p *OneOfParam, inden enumParts = append(enumParts, fmt.Sprintf("%q", v.Name())) } } + // A default makes the field effectively non-optional in CUE — the + // downstream `if name == "..."` blocks reference the field by name + // from sibling scope, which fails CUE strict mode when marked + // optional. Drop the "?" marker so the field is concrete. + optional = "" } else { for _, v := range variants { enumParts = append(enumParts, fmt.Sprintf("%q", v.Name())) diff --git a/pkg/definition/defkit/cuegen_test.go b/pkg/definition/defkit/cuegen_test.go index 8a94f21b8..0d94da8c8 100644 --- a/pkg/definition/defkit/cuegen_test.go +++ b/pkg/definition/defkit/cuegen_test.go @@ -2137,4 +2137,415 @@ var _ = Describe("CUEGenerator", func() { Expect(cue).To(ContainSubstring("$params:")) }) }) + + // --- OneOf with Default -------------------------------------------------- + // + // Background: when a OneOfParam has both Optional() and Default(), the + // generated discriminator block uses sibling-scope `if name == "..."` + // references that fail CUE strict mode if the field is marked `?`. + // Default makes the value concrete; the `?` marker must be dropped. + Context("OneOf with Default", func() { + It("should drop the ? marker when a default is set", func() { + vol := defkit.OneOf("volume").Optional().Default("emptyDir").Variants( + defkit.Variant("emptyDir").WithFields( + defkit.Field("medium", defkit.ParamTypeString).Optional(), + ), + defkit.Variant("configMap").WithFields( + defkit.Field("name", defkit.ParamTypeString).Required(), + ), + ) + schema := defkit.NewCUEGenerator().GenerateParameterSchema( + defkit.NewComponent("c").Params(vol)) + Expect(schema).To(ContainSubstring(`volume: *"emptyDir" | "configMap"`)) + Expect(schema).NotTo(ContainSubstring(`volume?:`)) + }) + + It("should keep the ? marker when no default is set", func() { + vol := defkit.OneOf("volume").Optional().Variants( + defkit.Variant("a").WithFields(defkit.Field("x", defkit.ParamTypeString).Required()), + defkit.Variant("b").WithFields(defkit.Field("y", defkit.ParamTypeString).Required()), + ) + schema := defkit.NewCUEGenerator().GenerateParameterSchema( + defkit.NewComponent("c").Params(vol)) + Expect(schema).To(ContainSubstring(`volume?:`)) + }) + }) + + // --- Auto-import for ArrayParam list constraints ------------------------ + // + // Background: ArrayParam.MinItems/MaxItems emit list.MinItems(N) / + // list.MaxItems(N), which require the CUE "list" stdlib import. The + // auto-import scanner picks this up via ArrayParam.RequiredImports. + Context("Auto-import for Array list constraints", func() { + It("should add the list import when Array.MinItems is set", func() { + ports := defkit.IntList("ports").Optional().MinItems(1) + cue := defkit.NewComponent("c"). + Workload("v1", "ConfigMap"). + Params(ports). + Template(func(tpl *defkit.Template) {}). + ToCue() + Expect(cue).To(ContainSubstring(`"list"`)) + Expect(cue).To(ContainSubstring(`list.MinItems(1)`)) + }) + + It("should add the list import when Array.MaxItems is set", func() { + ports := defkit.IntList("ports").Optional().MaxItems(10) + cue := defkit.NewComponent("c"). + Workload("v1", "ConfigMap"). + Params(ports). + Template(func(tpl *defkit.Template) {}). + ToCue() + Expect(cue).To(ContainSubstring(`"list"`)) + Expect(cue).To(ContainSubstring(`list.MaxItems(10)`)) + }) + + It("should NOT add the list import for a plain Array param", func() { + args := defkit.StringList("args").Optional() + cue := defkit.NewComponent("c"). + Workload("v1", "ConfigMap"). + Params(args). + Template(func(tpl *defkit.Template) {}). + ToCue() + Expect(cue).NotTo(ContainSubstring(`"list"`)) + }) + + It("should emit the list import only once when both MinItems and MaxItems are set", func() { + ports := defkit.IntList("ports").Optional().MinItems(1).MaxItems(10) + cue := defkit.NewComponent("c"). + Workload("v1", "ConfigMap"). + Params(ports). + Template(func(tpl *defkit.Template) {}). + ToCue() + Expect(strings.Count(cue, `"list"`)).To(Equal(1)) + }) + + It("should add the list import when Array.Contains() is used", func() { + tags := defkit.StringList("tags").Optional() + cue := defkit.NewComponent("c"). + Workload("v1", "ConfigMap"). + Params(tags). + Template(func(tpl *defkit.Template) { + tpl.Output(defkit.NewResource("v1", "ConfigMap"). + SetIf(tags.Contains("gpu"), "data.gpu", defkit.Lit("true"))) + }). + ToCue() + Expect(cue).To(ContainSubstring(`"list"`)) + Expect(cue).To(ContainSubstring(`list.Contains`)) + }) + }) + + // --- Optional collection rendering (end-to-end) ------------------------- + // + // Background: in CUE strict mode (and the KubeVela template pipeline + // specifically), references to optional fields like `parameter.X` (dot) + // or `len(parameter.X)` trip "cannot reference optional field". The + // rendering must use the bracket-existence pattern `parameter["X"] != _|_` + // — the same form every built-in KubeVela component (cron-task.cue, + // daemon.cue, helmchart.cue) uses. + // --- Bracket-access conditional rendering -------------------------------- + // + // Regression: when SetIf targets a bracket-access path (e.g. + // `data[args-empty]`), the bracket-leaf rendering previously dropped the + // node's condition and condValues, emitting the field unconditionally. + // Both single conditions and AbsentOrEmpty's two-branch expansion must + // produce wrapping if blocks for keys with hyphens / dots / etc. + Context("Bracket-access conditional rendering", func() { + It("should wrap a bracket-access SetIf in an if block (single condition)", func() { + args := defkit.StringList("args").Optional() + cue := defkit.NewComponent("c"). + Workload("v1", "ConfigMap"). + Params(args). + Template(func(tpl *defkit.Template) { + tpl.Output(defkit.NewResource("v1", "ConfigMap"). + SetIf(args.IsNotEmpty(), "data[has-args]", defkit.Lit("yes"))) + }). + ToCue() + Expect(cue).To(ContainSubstring(`if parameter["args"] != _|_ if len(parameter["args"]) > 0`)) + // The bracket key must be quoted (CUE requires quoting for + // non-identifier field names) and live inside the if block. + Expect(cue).To(ContainSubstring(`"has-args": "yes"`)) + }) + + It("should wrap a bracket-access SetIf with AbsentOrEmpty in TWO if blocks", func() { + args := defkit.StringList("args").Optional() + cue := defkit.NewComponent("c"). + Workload("v1", "ConfigMap"). + Params(args). + Template(func(tpl *defkit.Template) { + tpl.Output(defkit.NewResource("v1", "ConfigMap"). + SetIf(args.IsEmpty(), "data[args-empty]", defkit.Lit("yes"))) + }). + ToCue() + // Two branches: absent + set-and-empty, both wrapping the same key. + Expect(cue).To(ContainSubstring(`if parameter["args"] == _|_`)) + Expect(cue).To(ContainSubstring(`if parameter["args"] != _|_ if len(parameter["args"]) == 0`)) + Expect(strings.Count(cue, `"args-empty": "yes"`)).To(Equal(2)) + }) + + It("should render an unconditional bracket-key Set without an if block", func() { + cue := defkit.NewComponent("c"). + Workload("v1", "ConfigMap"). + Template(func(tpl *defkit.Template) { + tpl.Output(defkit.NewResource("v1", "ConfigMap"). + Set("data[my-key]", defkit.Lit("v"))) + }). + ToCue() + Expect(cue).To(ContainSubstring(`"my-key": "v"`)) + Expect(cue).NotTo(ContainSubstring(`if `)) + }) + + It("should render a bracket-key parent with a nested child as a struct", func() { + cue := defkit.NewComponent("c"). + Workload("v1", "ConfigMap"). + Template(func(tpl *defkit.Template) { + tpl.Output(defkit.NewResource("v1", "ConfigMap"). + Set("metadata.annotations[my-key].nested", defkit.Lit("v"))) + }). + ToCue() + // Bracket key with a child renders as `"my-key": { nested: ... }`. + Expect(cue).To(ContainSubstring(`"my-key": {`)) + Expect(cue).To(ContainSubstring(`nested: "v"`)) + }) + + It("should keep per-bracket-key conditions when sibling keys have different conds", func() { + args := defkit.StringList("args").Optional() + tags := defkit.StringList("tags").Optional() + cue := defkit.NewComponent("c"). + Workload("v1", "ConfigMap"). + Params(args, tags). + Template(func(tpl *defkit.Template) { + // Two bracket keys with DIFFERENT conditions — liftChildConditions + // can't merge them, so each bracket leaf keeps its own cond and + // writeBracketKeyNode hits the per-leaf if-block emission path. + tpl.Output(defkit.NewResource("v1", "ConfigMap"). + SetIf(args.IsNotEmpty(), "data[has-args]", defkit.Lit("yes")). + SetIf(tags.IsNotEmpty(), "data[has-tags]", defkit.Lit("yes"))) + }). + ToCue() + Expect(cue).To(ContainSubstring(`"has-args": "yes"`)) + Expect(cue).To(ContainSubstring(`"has-tags": "yes"`)) + Expect(cue).To(ContainSubstring(`len(parameter["args"]) > 0`)) + Expect(cue).To(ContainSubstring(`len(parameter["tags"]) > 0`)) + }) + + It("should keep per-bracket-key condValues when AbsentOrEmpty mixes with other conds", func() { + args := defkit.StringList("args").Optional() + tags := defkit.StringList("tags").Optional() + // Mix AbsentOrEmpty (two condValues) with another condition on a + // sibling bracket key — prevents liftChildConditions from sharing, + // so the bracket leaf keeps its condValues and writeBracketKeyNode + // hits `case len(node.condValues) > 0`. + cue := defkit.NewComponent("c"). + Workload("v1", "ConfigMap"). + Params(args, tags). + Template(func(tpl *defkit.Template) { + tpl.Output(defkit.NewResource("v1", "ConfigMap"). + SetIf(args.IsEmpty(), "data[args-empty]", defkit.Lit("yes")). + SetIf(tags.IsNotEmpty(), "data[has-tags]", defkit.Lit("yes"))) + }). + ToCue() + Expect(cue).To(ContainSubstring(`"args-empty": "yes"`)) + Expect(cue).To(ContainSubstring(`"has-tags": "yes"`)) + }) + }) + + // --- Compound condition rendering --------------------------------------- + // + // Conditions that use CUE chained-if syntax (LenCondition, ArrayContains) + // cannot live inside `(...) && (...)` — compound joiners must use ` if ` + // instead. Cover the chained-guard branches of And / LogicalExpr and the + // LogicalExpr OR pass-through. + Context("Compound condition rendering", func() { + It("should join AND with chained-guard LenCondition operand using ` if `", func() { + args := defkit.StringList("args").Optional() + flag := defkit.Bool("flag").Default(false) + cue := defkit.NewComponent("c"). + Workload("v1", "ConfigMap"). + Params(args, flag). + Template(func(tpl *defkit.Template) { + tpl.Output(defkit.NewResource("v1", "ConfigMap"). + SetIf(defkit.And(args.LenGt(0), flag.IsTrue()), + "data[both]", defkit.Lit("y"))) + }). + ToCue() + Expect(cue).To(ContainSubstring(`len(parameter["args"]) > 0`)) + // The parenthesized && form must NOT wrap a chained-guard operand. + Expect(cue).NotTo(MatchRegexp(`\([^)]*len\(parameter\["args"\]\)[^)]*\) && `)) + }) + + It("should join AND with chained-guard ArrayContains operands using ` if `", func() { + tags := defkit.StringList("tags").Optional() + flag := defkit.Bool("flag").Default(false) + cue := defkit.NewComponent("c"). + Workload("v1", "ConfigMap"). + Params(tags, flag). + Template(func(tpl *defkit.Template) { + tpl.Output(defkit.NewResource("v1", "ConfigMap"). + SetIf(defkit.And(tags.Contains("gpu"), flag.IsTrue(), tags.LenGt(0)), + "data[ok]", defkit.Lit("y"))) + }). + ToCue() + Expect(cue).To(ContainSubstring(`list.Contains(parameter["tags"], "gpu")`)) + Expect(cue).To(ContainSubstring(`len(parameter["tags"]) > 0`)) + }) + + It("should join LogicalExpr OR with ` || ` regardless of chained guards", func() { + flag := defkit.Bool("flag").Default(false) + cue := defkit.NewComponent("c"). + Workload("v1", "ConfigMap"). + Params(flag). + Template(func(tpl *defkit.Template) { + tpl.Output(defkit.NewResource("v1", "ConfigMap"). + SetIf(defkit.Or(flag.IsTrue(), flag.IsFalse()), + "data[either]", defkit.Lit("y"))) + }). + ToCue() + Expect(cue).To(ContainSubstring(` || `)) + }) + }) + + Context("Optional collection rendering", func() { + It("should render IsNotEmpty() on optional Array as chained-if guard with len() > 0", func() { + args := defkit.StringList("args").Optional() + cue := defkit.NewComponent("c"). + Workload("v1", "ConfigMap"). + Params(args). + Template(func(tpl *defkit.Template) { + tpl.Output(defkit.NewResource("v1", "ConfigMap"). + SetIf(args.IsNotEmpty(), "data.x", defkit.Lit("y"))) + }). + ToCue() + Expect(cue).To(ContainSubstring(`if parameter["args"] != _|_`)) + Expect(cue).To(ContainSubstring(`len(parameter["args"]) > 0`)) + Expect(cue).NotTo(ContainSubstring(`len(parameter.args)`)) + Expect(cue).NotTo(ContainSubstring(`parameter.args | []`)) + }) + + It("should render IsEmpty() on optional Array as two if blocks (absent OR set-and-empty)", func() { + args := defkit.StringList("args").Optional() + cue := defkit.NewComponent("c"). + Workload("v1", "ConfigMap"). + Params(args). + Template(func(tpl *defkit.Template) { + tpl.Output(defkit.NewResource("v1", "ConfigMap"). + SetIf(args.IsEmpty(), "data.empty", defkit.Lit("yes"))) + }). + ToCue() + // Branch 1: field absent + Expect(cue).To(ContainSubstring(`if parameter["args"] == _|_`)) + // Branch 2: field set and empty + Expect(cue).To(ContainSubstring(`if parameter["args"] != _|_ if len(parameter["args"]) == 0`)) + }) + + It("should expand AbsentOrEmpty in SpreadIf into two spread blocks", func() { + extra := defkit.Map("extra").Of(defkit.ParamTypeString).Optional() + // SpreadIf renders only when its target node also has at least one + // regular child (otherwise the leaf-with-only-spreads case is a + // pre-existing no-op in writeFieldNode). Add a sibling Set under + // metadata.labels so the spread is exercised. + cue := defkit.NewComponent("c"). + Workload("v1", "ConfigMap"). + Params(extra). + Template(func(tpl *defkit.Template) { + tpl.Output(defkit.NewResource("v1", "ConfigMap"). + Set("metadata.labels.fixed", defkit.Lit("y")). + SpreadIf(extra.IsEmpty(), "metadata.labels", defkit.Reference("parameter.extra"))) + }). + ToCue() + // Both AbsentOrEmpty branches must wrap the spread. + Expect(cue).To(ContainSubstring(`if parameter["extra"] == _|_`)) + Expect(cue).To(ContainSubstring(`if parameter["extra"] != _|_ if len(parameter["extra"]) == 0`)) + }) + + It("should expand AbsentOrEmpty SetIf inside an IfBlock with combined guards", func() { + args := defkit.StringList("args").Optional() + flag := defkit.Bool("flag").Default(false) + cue := defkit.NewComponent("c"). + Workload("v1", "ConfigMap"). + Params(args, flag). + Template(func(tpl *defkit.Template) { + tpl.Output(defkit.NewResource("v1", "ConfigMap"). + If(flag.IsTrue()). + SetIf(args.IsEmpty(), "data[when-flag]", defkit.Lit("y")). + EndIf()) + }). + ToCue() + // Outer flag guard combined with each inner AbsentOrEmpty branch. + Expect(cue).To(ContainSubstring(`parameter["args"] == _|_`)) + Expect(cue).To(ContainSubstring(`len(parameter["args"]) == 0`)) + Expect(cue).To(ContainSubstring(`parameter.flag`)) + }) + + It("should render Map.HasKey() with the daemon.cue two-clause guard", func() { + cfg := defkit.Map("config").Of(defkit.ParamTypeString).Optional() + schema := defkit.NewCUEGenerator().GenerateParameterSchema( + defkit.NewComponent("c").Params(cfg). + Validators( + defkit.Validate("debug must not be set"). + WithName("_v"). + FailWhen(cfg.HasKey("debug")), + )) + Expect(schema).To(ContainSubstring(`parameter["config"] != _|_`)) + Expect(schema).To(ContainSubstring(`parameter["config"].debug != _|_`)) + Expect(schema).NotTo(ContainSubstring(`parameter.config.debug != _|_`)) + }) + + It("should render a multi-collection ComponentDefinition without dot-references to optional fields", func() { + image := defkit.String("image").Required() + args := defkit.StringList("args").Optional() + ports := defkit.IntList("ports").Optional().MinItems(1).MaxItems(10) + labels := defkit.StringKeyMap("labels").Optional() + anns := defkit.Map("annotations").Of(defkit.ParamTypeString).Optional() + vol := defkit.OneOf("volume").Optional().Default("emptyDir").Variants( + defkit.Variant("emptyDir").WithFields( + defkit.Field("medium", defkit.ParamTypeString).Optional(), + ), + defkit.Variant("configMap").WithFields( + defkit.Field("name", defkit.ParamTypeString).Required(), + ), + ) + + c := defkit.NewComponent("collection-showcase"). + Workload("apps/v1", "Deployment"). + PodSpecPath("spec.template.spec"). + Params(image, args, ports, labels, anns, vol). + Template(func(tpl *defkit.Template) { + vela := defkit.VelaCtx() + tpl.Output(defkit.NewResource("apps/v1", "Deployment"). + Set("metadata.name", vela.Name()). + Set("spec.template.spec.containers[0].image", image). + SetIf(labels.IsNotEmpty(), "metadata.labels", labels). + SetIf(anns.IsNotEmpty(), "metadata.annotations", anns). + SetIf(args.IsNotEmpty(), "spec.template.spec.containers[0].args", args). + SetIf(ports.IsNotEmpty(), "metadata.annotations[showcase/ports-set]", defkit.Lit("true"))) + }) + + cue := c.ToCue() + + // 1. Schema-level: list import present, MinItems/MaxItems intact. + Expect(cue).To(ContainSubstring(`"list"`)) + Expect(cue).To(ContainSubstring(`list.MinItems(1) & list.MaxItems(10)`)) + + // 2. Every optional-collection guard uses bracket existence. + Expect(cue).To(ContainSubstring(`if parameter["labels"] != _|_`)) + Expect(cue).To(ContainSubstring(`if parameter["annotations"] != _|_`)) + Expect(cue).To(ContainSubstring(`if parameter["args"] != _|_`)) + Expect(cue).To(ContainSubstring(`if parameter["ports"] != _|_`)) + + // 3. None of the strict-mode-failing forms appear. + Expect(cue).NotTo(ContainSubstring(`len(parameter.labels)`)) + Expect(cue).NotTo(ContainSubstring(`len(parameter.args)`)) + Expect(cue).NotTo(ContainSubstring(`parameter.labels | {}`)) + Expect(cue).NotTo(ContainSubstring(`parameter.args | []`)) + + // 4. OneOf with Default: discriminator field is concrete, not optional. + Expect(cue).To(ContainSubstring(`volume: *"emptyDir" | "configMap"`)) + Expect(cue).NotTo(ContainSubstring(`volume?:`)) + + // 5. Inside a guarded if-block, dot syntax for the value reference + // is still emitted (safe because the guard establishes existence). + Expect(cue).To(ContainSubstring(`labels: parameter.labels`)) + Expect(cue).To(ContainSubstring(`args: parameter.args`)) + }) + }) }) diff --git a/pkg/definition/defkit/expr.go b/pkg/definition/defkit/expr.go index 21201cbd1..2baf788e8 100644 --- a/pkg/definition/defkit/expr.go +++ b/pkg/definition/defkit/expr.go @@ -267,7 +267,13 @@ func (c *StringEndsWithCondition) ParamName() string { return c.paramName } func (c *StringEndsWithCondition) Suffix() string { return c.suffix } // LenCondition checks the length of a parameter (string, array, or map). -// Generates: len(parameter.name) op n +// Generates: parameter["name"] != _|_ if len(parameter["name"]) op n +// +// CUE chained-if guard form. The bracket-existence guard handles strict +// mode on optional fields (dot syntax `parameter.X` errors on `_|_`). The +// second `if` only evaluates when the first passes, so `len()` never +// references an absent (`_|_`) value. For required fields the outer guard +// always passes. type LenCondition struct { baseCondition paramName string @@ -284,6 +290,35 @@ func (c *LenCondition) Op() string { return c.op } // Length returns the length to compare against. func (c *LenCondition) Length() int { return c.length } +// AbsentOrEmptyCondition fires when a collection parameter is either absent +// (parameter["X"] == _|_) or set and empty (len(parameter["X"]) == 0). +// +// CUE cannot express "absent OR empty" as a single boolean expression: `||` +// is strict in both operands, and `len(_|_)` propagates bottom. The condition +// is therefore expanded at render time into two separate if blocks, each +// emitting the same body — CUE unifies same-path/same-value writes. +type AbsentOrEmptyCondition struct { + baseCondition + paramName string +} + +// ParamName returns the parameter name being checked. +func (c *AbsentOrEmptyCondition) ParamName() string { return c.paramName } + +// Branches returns the two simpler conditions equivalent to this OR: +// 1. field absent (Not(IsSet)) +// 2. field set and empty (LenCondition == 0) +// +// Render paths that handle compound emission expand into both branches; paths +// that don't (e.g. unknown contexts) fall back to conditionToCUE which renders +// only the "set and empty" branch. +func (c *AbsentOrEmptyCondition) Branches() []Condition { + return []Condition{ + &NotExpr{cond: &IsSetCondition{paramName: c.paramName}}, + &LenCondition{paramName: c.paramName, op: "==", length: 0}, + } +} + // ArrayContainsCondition checks if an array parameter contains a specific value. // Generates: list.Contains(parameter.name, value) type ArrayContainsCondition struct { @@ -298,6 +333,12 @@ func (c *ArrayContainsCondition) ParamName() string { return c.paramName } // Value returns the value to check for. func (c *ArrayContainsCondition) Value() any { return c.value } +// RequiredImports returns the CUE imports required by ArrayContainsCondition. +// list.Contains requires the "list" package. +func (c *ArrayContainsCondition) RequiredImports() []string { + return []string{"list"} +} + // MapHasKeyCondition checks if a map parameter has a specific key. // Generates: parameter.name.key != _|_ type MapHasKeyCondition struct { diff --git a/pkg/definition/defkit/param.go b/pkg/definition/defkit/param.go index a896712f3..334fa0edf 100644 --- a/pkg/definition/defkit/param.go +++ b/pkg/definition/defkit/param.go @@ -293,31 +293,31 @@ func (p *StringParam) EndsWith(suffix string) Condition { } // LenEq creates a condition that checks if this string parameter has exactly n characters. -// Example: name.LenEq(5) generates: len(parameter.name) == 5 +// Example: name.LenEq(5) generates: parameter["name"] != _|_ if len(parameter["name"]) == 5 func (p *StringParam) LenEq(n int) Condition { return &LenCondition{paramName: p.name, op: "==", length: n} } // LenGt creates a condition that checks if this string parameter has more than n characters. -// Example: name.LenGt(5) generates: len(parameter.name) > 5 +// Example: name.LenGt(5) generates: parameter["name"] != _|_ if len(parameter["name"]) > 5 func (p *StringParam) LenGt(n int) Condition { return &LenCondition{paramName: p.name, op: ">", length: n} } // LenGte creates a condition that checks if this string parameter has n or more characters. -// Example: name.LenGte(5) generates: len(parameter.name) >= 5 +// Example: name.LenGte(5) generates: parameter["name"] != _|_ if len(parameter["name"]) >= 5 func (p *StringParam) LenGte(n int) Condition { return &LenCondition{paramName: p.name, op: ">=", length: n} } // LenLt creates a condition that checks if this string parameter has fewer than n characters. -// Example: name.LenLt(5) generates: len(parameter.name) < 5 +// Example: name.LenLt(5) generates: parameter["name"] != _|_ if len(parameter["name"]) < 5 func (p *StringParam) LenLt(n int) Condition { return &LenCondition{paramName: p.name, op: "<", length: n} } // LenLte creates a condition that checks if this string parameter has n or fewer characters. -// Example: name.LenLte(5) generates: len(parameter.name) <= 5 +// Example: name.LenLte(5) generates: parameter["name"] != _|_ if len(parameter["name"]) <= 5 func (p *StringParam) LenLte(n int) Condition { return &LenCondition{paramName: p.name, op: "<=", length: n} } @@ -764,52 +764,70 @@ func (p *ArrayParam) GetMaxItems() *int { return p.maxItems } +// RequiredImports returns the CUE imports needed by this parameter's constraints. +// MinItems/MaxItems generate list.MinItems()/list.MaxItems() which require "list". +func (p *ArrayParam) RequiredImports() []string { + if p.minItems != nil || p.maxItems != nil { + return []string{"list"} + } + return nil +} + // --- ArrayParam Runtime Condition Methods --- // LenEq creates a condition that checks if this array has exactly n elements. -// Example: tags.LenEq(5) generates: len(parameter.tags) == 5 +// Example: tags.LenEq(5) generates: parameter["tags"] != _|_ if len(parameter["tags"]) == 5 +// +// LenEq(0) is treated as "absent OR empty" — equivalent to IsEmpty(). It +// returns an AbsentOrEmptyCondition that the renderer expands into two if +// blocks (one for absent, one for set-and-empty). func (p *ArrayParam) LenEq(n int) Condition { + if n == 0 { + return &AbsentOrEmptyCondition{paramName: p.name} + } return &LenCondition{paramName: p.name, op: "==", length: n} } // LenGt creates a condition that checks if this array has more than n elements. -// Example: tags.LenGt(0) generates: len(parameter.tags) > 0 +// Example: tags.LenGt(0) generates: parameter["tags"] != _|_ if len(parameter["tags"]) > 0 func (p *ArrayParam) LenGt(n int) Condition { return &LenCondition{paramName: p.name, op: ">", length: n} } // LenGte creates a condition that checks if this array has n or more elements. -// Example: tags.LenGte(1) generates: len(parameter.tags) >= 1 +// Example: tags.LenGte(1) generates: parameter["tags"] != _|_ if len(parameter["tags"]) >= 1 func (p *ArrayParam) LenGte(n int) Condition { return &LenCondition{paramName: p.name, op: ">=", length: n} } // LenLt creates a condition that checks if this array has fewer than n elements. -// Example: tags.LenLt(10) generates: len(parameter.tags) < 10 +// Example: tags.LenLt(10) generates: parameter["tags"] != _|_ if len(parameter["tags"]) < 10 func (p *ArrayParam) LenLt(n int) Condition { return &LenCondition{paramName: p.name, op: "<", length: n} } // LenLte creates a condition that checks if this array has n or fewer elements. -// Example: tags.LenLte(10) generates: len(parameter.tags) <= 10 +// Example: tags.LenLte(10) generates: parameter["tags"] != _|_ if len(parameter["tags"]) <= 10 func (p *ArrayParam) LenLte(n int) Condition { return &LenCondition{paramName: p.name, op: "<=", length: n} } // Contains creates a condition that checks if this array contains a specific value. -// Example: tags.Contains("gpu") generates: list.Contains(parameter.tags, "gpu") +// Example: tags.Contains("gpu") generates: parameter["tags"] != _|_ if list.Contains(parameter["tags"], "gpu") func (p *ArrayParam) Contains(val any) Condition { return &ArrayContainsCondition{paramName: p.name, value: val} } -// IsEmpty creates a condition that checks if this array is empty. -// Example: tags.IsEmpty() generates: len(parameter.tags) == 0 +// IsEmpty creates a condition that checks if this array is absent or empty. +// Renders as two separate if blocks: one for `parameter["X"] == _|_` (absent) +// and one for `parameter["X"] != _|_ if len(parameter["X"]) == 0` (set and +// empty). Both blocks emit the same body; CUE unifies same-path writes. func (p *ArrayParam) IsEmpty() Condition { - return &LenCondition{paramName: p.name, op: "==", length: 0} + return &AbsentOrEmptyCondition{paramName: p.name} } -// IsNotEmpty creates a condition that checks if this array is not empty. -// Example: tags.IsNotEmpty() generates: len(parameter.tags) > 0 +// IsNotEmpty creates a condition that checks if this array is set and non-empty. +// Example: tags.IsNotEmpty() generates: parameter["tags"] != _|_ if len(parameter["tags"]) > 0 func (p *ArrayParam) IsNotEmpty() Condition { return &LenCondition{paramName: p.name, op: ">", length: 0} } @@ -963,25 +981,30 @@ func (p *MapParam) HasKey(key string) Condition { } // LenEq creates a condition that checks if this map has exactly n entries. -// Example: config.LenEq(5) generates: len(parameter.config) == 5 +// Example: config.LenEq(5) generates: parameter["config"] != _|_ if len(parameter["config"]) == 5 +// +// LenEq(0) is treated as "absent OR empty" — equivalent to IsEmpty(). func (p *MapParam) LenEq(n int) Condition { + if n == 0 { + return &AbsentOrEmptyCondition{paramName: p.name} + } return &LenCondition{paramName: p.name, op: "==", length: n} } // LenGt creates a condition that checks if this map has more than n entries. -// Example: config.LenGt(0) generates: len(parameter.config) > 0 +// Example: config.LenGt(0) generates: parameter["config"] != _|_ if len(parameter["config"]) > 0 func (p *MapParam) LenGt(n int) Condition { return &LenCondition{paramName: p.name, op: ">", length: n} } -// IsEmpty creates a condition that checks if this map is empty. -// Example: config.IsEmpty() generates: len(parameter.config) == 0 +// IsEmpty creates a condition that checks if this map is absent or empty. +// Renders as two if blocks (absent + set-and-empty). See AbsentOrEmptyCondition. func (p *MapParam) IsEmpty() Condition { - return &LenCondition{paramName: p.name, op: "==", length: 0} + return &AbsentOrEmptyCondition{paramName: p.name} } -// IsNotEmpty creates a condition that checks if this map is not empty. -// Example: config.IsNotEmpty() generates: len(parameter.config) > 0 +// IsNotEmpty creates a condition that checks if this map is set and non-empty. +// Example: config.IsNotEmpty() generates: parameter["config"] != _|_ if len(parameter["config"]) > 0 func (p *MapParam) IsNotEmpty() Condition { return &LenCondition{paramName: p.name, op: ">", length: 0} } @@ -1493,6 +1516,49 @@ func (p *StringKeyMapParam) Description(desc string) *StringKeyMapParam { // GetType returns the parameter type. func (p *StringKeyMapParam) GetType() ParamType { return p.paramType } +// --- StringKeyMapParam Runtime Condition Methods --- +// +// These mirror MapParam's runtime conditions. StringKeyMap and Map.Of(ParamTypeString) +// generate the same CUE schema ([string]: string), so they should expose the same +// runtime predicates. Without these, callers writing validators or SetIf guards +// against a StringKeyMap have to fall back to Map.Of(ParamTypeString) just to +// recover HasKey / IsNotEmpty. + +// HasKey creates a condition that checks if this map has a specific key. +// Example: labels.HasKey("app") generates: parameter.labels.app != _|_ +func (p *StringKeyMapParam) HasKey(key string) Condition { + return &MapHasKeyCondition{paramName: p.name, key: key} +} + +// LenEq creates a condition that checks if this map has exactly n entries. +// Example: labels.LenEq(3) generates: parameter["labels"] != _|_ if len(parameter["labels"]) == 3 +// +// LenEq(0) is treated as "absent OR empty" — equivalent to IsEmpty(). +func (p *StringKeyMapParam) LenEq(n int) Condition { + if n == 0 { + return &AbsentOrEmptyCondition{paramName: p.name} + } + return &LenCondition{paramName: p.name, op: "==", length: n} +} + +// LenGt creates a condition that checks if this map has more than n entries. +// Example: labels.LenGt(0) generates: parameter["labels"] != _|_ if len(parameter["labels"]) > 0 +func (p *StringKeyMapParam) LenGt(n int) Condition { + return &LenCondition{paramName: p.name, op: ">", length: n} +} + +// IsEmpty creates a condition that checks if this map is absent or empty. +// Renders as two if blocks (absent + set-and-empty). See AbsentOrEmptyCondition. +func (p *StringKeyMapParam) IsEmpty() Condition { + return &AbsentOrEmptyCondition{paramName: p.name} +} + +// IsNotEmpty creates a condition that checks if this map is set and non-empty. +// Example: labels.IsNotEmpty() generates: parameter["labels"] != _|_ if len(parameter["labels"]) > 0 +func (p *StringKeyMapParam) IsNotEmpty() Condition { + return &LenCondition{paramName: p.name, op: ">", length: 0} +} + // DynamicMapParam represents a parameter where the parameter itself is a dynamic map. // In CUE: parameter: [string]: T (where T is the value type) // This is used for traits like labels where all user values become map keys. diff --git a/pkg/definition/defkit/param_constraints_test.go b/pkg/definition/defkit/param_constraints_test.go index a61f33e65..49de58ea2 100644 --- a/pkg/definition/defkit/param_constraints_test.go +++ b/pkg/definition/defkit/param_constraints_test.go @@ -196,11 +196,11 @@ var _ = ginkgo.Describe("Parameter Constraints", func() { cueStr := gen.conditionToCUE(condFn(p)) gomega.Expect(cueStr).To(gomega.Equal(expected)) }, - ginkgo.Entry("LenEq", func(p *StringParam) Condition { return p.LenEq(5) }, "len(parameter.name) == 5"), - ginkgo.Entry("LenGt", func(p *StringParam) Condition { return p.LenGt(5) }, "len(parameter.name) > 5"), - ginkgo.Entry("LenGte", func(p *StringParam) Condition { return p.LenGte(5) }, "len(parameter.name) >= 5"), - ginkgo.Entry("LenLt", func(p *StringParam) Condition { return p.LenLt(5) }, "len(parameter.name) < 5"), - ginkgo.Entry("LenLte", func(p *StringParam) Condition { return p.LenLte(5) }, "len(parameter.name) <= 5"), + ginkgo.Entry("LenEq", func(p *StringParam) Condition { return p.LenEq(5) }, `parameter["name"] != _|_ if len(parameter["name"]) == 5`), + ginkgo.Entry("LenGt", func(p *StringParam) Condition { return p.LenGt(5) }, `parameter["name"] != _|_ if len(parameter["name"]) > 5`), + ginkgo.Entry("LenGte", func(p *StringParam) Condition { return p.LenGte(5) }, `parameter["name"] != _|_ if len(parameter["name"]) >= 5`), + ginkgo.Entry("LenLt", func(p *StringParam) Condition { return p.LenLt(5) }, `parameter["name"] != _|_ if len(parameter["name"]) < 5`), + ginkgo.Entry("LenLte", func(p *StringParam) Condition { return p.LenLte(5) }, `parameter["name"] != _|_ if len(parameter["name"]) <= 5`), ) }) @@ -211,19 +211,27 @@ var _ = ginkgo.Describe("Parameter Constraints", func() { cueStr := gen.conditionToCUE(condFn(p)) gomega.Expect(cueStr).To(gomega.Equal(expected)) }, - ginkgo.Entry("LenEq", func(p *ArrayParam) Condition { return p.LenEq(5) }, "len(parameter.tags) == 5"), - ginkgo.Entry("LenGt", func(p *ArrayParam) Condition { return p.LenGt(0) }, "len(parameter.tags) > 0"), - ginkgo.Entry("IsEmpty", func(p *ArrayParam) Condition { return p.IsEmpty() }, "len(parameter.tags) == 0"), - ginkgo.Entry("IsNotEmpty", func(p *ArrayParam) Condition { return p.IsNotEmpty() }, "len(parameter.tags) > 0"), - ginkgo.Entry("Contains", func(p *ArrayParam) Condition { return p.Contains("gpu") }, `list.Contains(parameter.tags, "gpu")`), + // All length predicates use CUE chained-if guard syntax. See + // `lenConditionToCUE` in cuegen.go. + // + // IsEmpty() (and LenEq(0)) returns *AbsentOrEmptyCondition, which + // expands into TWO if blocks at SetIf rendering. The string here + // is the fallback (set-and-empty branch only) used when the + // condition appears in non-expanding contexts. + ginkgo.Entry("LenEq", func(p *ArrayParam) Condition { return p.LenEq(5) }, `parameter["tags"] != _|_ if len(parameter["tags"]) == 5`), + ginkgo.Entry("LenGt", func(p *ArrayParam) Condition { return p.LenGt(0) }, `parameter["tags"] != _|_ if len(parameter["tags"]) > 0`), + ginkgo.Entry("IsEmpty", func(p *ArrayParam) Condition { return p.IsEmpty() }, `parameter["tags"] != _|_ if len(parameter["tags"]) == 0`), + ginkgo.Entry("LenEq(0)", func(p *ArrayParam) Condition { return p.LenEq(0) }, `parameter["tags"] != _|_ if len(parameter["tags"]) == 0`), + ginkgo.Entry("IsNotEmpty", func(p *ArrayParam) Condition { return p.IsNotEmpty() }, `parameter["tags"] != _|_ if len(parameter["tags"]) > 0`), + ginkgo.Entry("Contains", func(p *ArrayParam) Condition { return p.Contains("gpu") }, `parameter["tags"] != _|_ if list.Contains(parameter["tags"], "gpu")`), ) ginkgo.It("should generate array Contains with different element types", func() { intArray := Array("ports").Of(ParamTypeInt) - gomega.Expect(gen.conditionToCUE(intArray.Contains(8080))).To(gomega.Equal(`list.Contains(parameter.ports, 8080)`)) + gomega.Expect(gen.conditionToCUE(intArray.Contains(8080))).To(gomega.Equal(`parameter["ports"] != _|_ if list.Contains(parameter["ports"], 8080)`)) boolArray := Array("flags").Of(ParamTypeBool) - gomega.Expect(gen.conditionToCUE(boolArray.Contains(true))).To(gomega.Equal(`list.Contains(parameter.flags, true)`)) + gomega.Expect(gen.conditionToCUE(boolArray.Contains(true))).To(gomega.Equal(`parameter["flags"] != _|_ if list.Contains(parameter["flags"], true)`)) }) }) @@ -234,14 +242,37 @@ var _ = ginkgo.Describe("Parameter Constraints", func() { cueStr := gen.conditionToCUE(condFn(p)) gomega.Expect(cueStr).To(gomega.Equal(expected)) }, - ginkgo.Entry("HasKey", func(p *MapParam) Condition { return p.HasKey("debug") }, "parameter.config.debug != _|_"), - ginkgo.Entry("LenEq", func(p *MapParam) Condition { return p.LenEq(5) }, "len(parameter.config) == 5"), - ginkgo.Entry("LenGt", func(p *MapParam) Condition { return p.LenGt(0) }, "len(parameter.config) > 0"), - ginkgo.Entry("IsEmpty", func(p *MapParam) Condition { return p.IsEmpty() }, "len(parameter.config) == 0"), - ginkgo.Entry("IsNotEmpty", func(p *MapParam) Condition { return p.IsNotEmpty() }, "len(parameter.config) > 0"), + ginkgo.Entry("HasKey", func(p *MapParam) Condition { return p.HasKey("debug") }, `parameter["config"] != _|_ && parameter["config"].debug != _|_`), + ginkgo.Entry("LenEq", func(p *MapParam) Condition { return p.LenEq(5) }, `parameter["config"] != _|_ if len(parameter["config"]) == 5`), + ginkgo.Entry("LenGt", func(p *MapParam) Condition { return p.LenGt(0) }, `parameter["config"] != _|_ if len(parameter["config"]) > 0`), + ginkgo.Entry("IsEmpty", func(p *MapParam) Condition { return p.IsEmpty() }, `parameter["config"] != _|_ if len(parameter["config"]) == 0`), + ginkgo.Entry("IsNotEmpty", func(p *MapParam) Condition { return p.IsNotEmpty() }, `parameter["config"] != _|_ if len(parameter["config"]) > 0`), ) }) + // --- AllConditions Rendering --- + // + // AllConditions(...) builds an AllConditionsCondition over N sub-conditions. + // Joiner switches between ` && ` (default) and ` if ` (when any sub uses + // chained-guard syntax — e.g. ArrayContainsCondition / LenCondition with + // non-empty Fallback). + ginkgo.Context("AllConditions rendering", func() { + ginkgo.It("joins non-chained conditions with ` && `", func() { + flag := Bool("flag").Default(false) + replicas := Int("replicas").Default(1) + cond := AllConditions(flag.IsTrue(), replicas.Gt(0)) + gomega.Expect(gen.conditionToCUE(cond)).To(gomega.Equal(`parameter.flag && parameter.replicas > 0`)) + }) + + ginkgo.It("joins with ` if ` when any sub-condition is chained-guard", func() { + tags := StringList("tags").Optional() + flag := Bool("flag").Default(false) + out := gen.conditionToCUE(AllConditions(flag.IsTrue(), tags.Contains("gpu"))) + gomega.Expect(out).To(gomega.ContainSubstring(` if `)) + gomega.Expect(out).To(gomega.ContainSubstring(`list.Contains(parameter["tags"], "gpu")`)) + }) + }) + // --- Chaining Tests --- ginkgo.Context("Constraint Chaining", func() { @@ -346,8 +377,8 @@ var _ = ginkgo.Describe("Parameter Constraints", func() { gomega.Expect(cue).To(gomega.ContainSubstring(`strings.HasPrefix(parameter.name, "prod-")`)) gomega.Expect(cue).To(gomega.ContainSubstring(`strings.Contains(parameter.name, "canary")`)) gomega.Expect(cue).To(gomega.ContainSubstring(`parameter.replicas > 5`)) - gomega.Expect(cue).To(gomega.ContainSubstring(`len(parameter.tags) > 0`)) - gomega.Expect(cue).To(gomega.ContainSubstring(`list.Contains(parameter.tags, "gpu")`)) + gomega.Expect(cue).To(gomega.ContainSubstring(`parameter["tags"] != _|_`)) + gomega.Expect(cue).To(gomega.ContainSubstring(`list.Contains(parameter["tags"], "gpu")`)) }) }) diff --git a/pkg/definition/defkit/param_test.go b/pkg/definition/defkit/param_test.go index b0482e689..f33d5c8d6 100644 --- a/pkg/definition/defkit/param_test.go +++ b/pkg/definition/defkit/param_test.go @@ -663,6 +663,20 @@ var _ = Describe("Parameters", func() { Expect(lenCond.Length()).To(Equal(1)) }) + It("should require the list import only when MinItems or MaxItems is set", func() { + plain := defkit.StringList("tags") + Expect(plain.RequiredImports()).To(BeNil()) + + withMin := defkit.StringList("tags").MinItems(1) + Expect(withMin.RequiredImports()).To(Equal([]string{"list"})) + + withMax := defkit.StringList("tags").MaxItems(10) + Expect(withMax.RequiredImports()).To(Equal([]string{"list"})) + + withBoth := defkit.StringList("tags").MinItems(1).MaxItems(10) + Expect(withBoth.RequiredImports()).To(Equal([]string{"list"})) + }) + It("should set WithFields for array items", func() { p := defkit.List("ports").WithFields( defkit.Int("port").Required(), @@ -672,6 +686,30 @@ var _ = Describe("Parameters", func() { }) }) + Context("StringKeyMapParam conditions", func() { + // Until this fix, StringKeyMapParam (the convenience constructor for + // [string]: string maps) did not expose any of the runtime predicate + // helpers that MapParam offered. Callers had to fall back to + // Map(...).Of(ParamTypeString) just to get HasKey or IsNotEmpty. + It("should support HasKey", func() { + labels := defkit.StringKeyMap("labels") + cond := labels.HasKey("app") + Expect(cond).NotTo(BeNil()) + hasKey, ok := cond.(*defkit.MapHasKeyCondition) + Expect(ok).To(BeTrue(), "expected *MapHasKeyCondition") + Expect(hasKey.ParamName()).To(Equal("labels")) + Expect(hasKey.Key()).To(Equal("app")) + }) + + It("should support IsEmpty / IsNotEmpty / LenEq / LenGt", func() { + labels := defkit.StringKeyMap("labels") + Expect(labels.IsEmpty()).NotTo(BeNil()) + Expect(labels.IsNotEmpty()).NotTo(BeNil()) + Expect(labels.LenEq(3)).NotTo(BeNil()) + Expect(labels.LenGt(0)).NotTo(BeNil()) + }) + }) + Context("MapParam Optional method", func() { It("should set map as optional", func() { p := defkit.Map("labels").Optional() diff --git a/pkg/definition/defkit/validator_test.go b/pkg/definition/defkit/validator_test.go index 785a041f2..e5b8a0799 100644 --- a/pkg/definition/defkit/validator_test.go +++ b/pkg/definition/defkit/validator_test.go @@ -17,6 +17,8 @@ limitations under the License. package defkit_test import ( + "strings" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -136,6 +138,49 @@ var _ = Describe("Validator", func() { }) }) + Context("FailWhen / OnlyWhen with collection IsEmpty()", func() { + // Regression: ParamRef.IsEmpty() returns *AbsentOrEmptyCondition. + // In CUE, "absent OR empty" cannot be expressed as a single boolean + // (`||` is strict, len(_|_) propagates bottom). The renderer must + // duplicate the body into two if blocks — one per branch — so the + // validator fires on both unset and empty-collection inputs. + + It("FailWhen(args.IsEmpty()) emits two false-clauses (absent + set-and-empty)", func() { + args := defkit.StringList("args").Optional() + v := defkit.Validate("args must be non-empty"). + WithName("_v"). + FailWhen(args.IsEmpty()) + + comp := defkit.NewComponent("test").Params(args).Validators(v) + cue := gen.GenerateParameterSchema(comp) + + Expect(cue).To(ContainSubstring(`if parameter["args"] == _|_`)) + Expect(cue).To(ContainSubstring(`if parameter["args"] != _|_ if len(parameter["args"]) == 0`)) + // Both branches set the message to false (tripping validation). + falseCount := strings.Count(cue, `"args must be non-empty": false`) + Expect(falseCount).To(Equal(2)) + }) + + It("OnlyWhen(labels.IsEmpty()) wraps the validator in two guard blocks", func() { + labels := defkit.StringKeyMap("labels").Optional() + args := defkit.StringList("args").Optional() + v := defkit.Validate("at least one arg required when no labels"). + WithName("_v"). + OnlyWhen(labels.IsEmpty()). + FailWhen(args.IsEmpty()) + + comp := defkit.NewComponent("test").Params(labels, args).Validators(v) + cue := gen.GenerateParameterSchema(comp) + + // Two outer guards: labels-absent + labels-set-and-empty. + Expect(cue).To(ContainSubstring(`if parameter["labels"] == _|_`)) + Expect(cue).To(ContainSubstring(`if parameter["labels"] != _|_ if len(parameter["labels"]) == 0`)) + // Validator definition is duplicated under each guard. + validatorCount := strings.Count(cue, `_v: {`) + Expect(validatorCount).To(Equal(2)) + }) + }) + Context("Validator inside MapParam", func() { It("should emit validator inside struct", func() { v := defkit.Validate("name is required").