From 356ecffe74fb2319f7ba2328892c67bf7fce6b9e Mon Sep 17 00:00:00 2001 From: Vishal Kumar Date: Tue, 3 Mar 2026 16:45:58 +0530 Subject: [PATCH] Fix: Update duration format handling and enhance StringParam with enum string support (#7053) * Fix: update duration format handling and enhance StringParam with enum string support Signed-off-by: Anaswara Suresh M K Signed-off-by: vishal210893 * date build-push-image workflow and improve image handlinggp Signed-off-by: Anaswara Suresh M K * chore: commit to re run pipeline Signed-off-by: Anaswara Suresh M K * Feat: add deduplication and field order tracking in collections and array elements Signed-off-by: Vaibhav Agrawal * Fix: update test expectation for parameter data count in workflow step Signed-off-by: Vaibhav Agrawal * Feat: add tests for label handling and template body in workflow steps Signed-off-by: Vaibhav Agrawal * Feat: add tests for label handling and template body in workflow steps Signed-off-by: Vaibhav Agrawal * Refactor: rename AllowString to OpenEnum for clarity in enum handling Signed-off-by: Vaibhav Agrawal * Chore: format StringParam struct for improved readability Signed-off-by: Vaibhav Agrawal --------- Signed-off-by: Anaswara Suresh M K Signed-off-by: vishal210893 Signed-off-by: Vaibhav Agrawal --- .../defwithtemplate/check-metrics.yaml | 1 - .../defwithtemplate/notification.yaml | 14 +- .../defwithtemplate/restart-workflow.yaml | 10 +- .../templates/defwithtemplate/webhook.yaml | 10 +- pkg/definition/definition_test.go | 2 +- pkg/definition/defkit/collections.go | 6 + pkg/definition/defkit/collections_test.go | 6 + pkg/definition/defkit/cuegen.go | 3 + pkg/definition/defkit/cuegen_test.go | 11 ++ pkg/definition/defkit/expr.go | 12 +- pkg/definition/defkit/expr_test.go | 17 +++ pkg/definition/defkit/param.go | 21 +++ pkg/definition/defkit/param_test.go | 22 +++ pkg/definition/defkit/workflow_step.go | 89 ++++++++++-- pkg/definition/defkit/workflow_step_test.go | 127 ++++++++++++++++++ .../internal/workflowstep/check-metrics.cue | 1 - .../internal/workflowstep/notification.cue | 18 ++- .../workflowstep/restart-workflow.cue | 10 +- .../internal/workflowstep/webhook.cue | 10 +- 19 files changed, 341 insertions(+), 49 deletions(-) diff --git a/charts/vela-core/templates/defwithtemplate/check-metrics.yaml b/charts/vela-core/templates/defwithtemplate/check-metrics.yaml index c50a5128c..27341c1e2 100644 --- a/charts/vela-core/templates/defwithtemplate/check-metrics.yaml +++ b/charts/vela-core/templates/defwithtemplate/check-metrics.yaml @@ -22,7 +22,6 @@ spec: query: parameter.query metricEndpoint: parameter.metricEndpoint condition: parameter.condition - stepID: context.stepSessionID duration: parameter.duration failDuration: parameter.failDuration } diff --git a/charts/vela-core/templates/defwithtemplate/notification.yaml b/charts/vela-core/templates/defwithtemplate/notification.yaml index fae370c74..b19badd85 100644 --- a/charts/vela-core/templates/defwithtemplate/notification.yaml +++ b/charts/vela-core/templates/defwithtemplate/notification.yaml @@ -214,7 +214,7 @@ spec: ding: { if parameter.dingding != _|_ { if parameter.dingding.url.value != _|_ { - ding1: http.#Do & { + ding1: http.#HTTPDo & { $params: { method: "POST" url: parameter.dingding.url.value @@ -238,7 +238,7 @@ spec: } stringValue: util.#ConvertString & {$params: bt: base64.Decode(null, read.$returns.value.data[parameter.dingding.url.secretRef.key])} - ding2: http.#Do & { + ding2: http.#HTTPDo & { $params: { method: "POST" url: stringValue.$returns.str @@ -255,10 +255,10 @@ spec: lark: { if parameter.lark != _|_ { if parameter.lark.url.value != _|_ { - lark1: http.#Do & { + lark1: http.#HTTPDo & { $params: { method: "POST" - url: parameter.lark.message + url: parameter.lark.url.value request: { body: json.Marshal(parameter.lark.message) header: "Content-Type": "application/json" @@ -279,7 +279,7 @@ spec: } stringValue: util.#ConvertString & {$params: bt: base64.Decode(null, read.$returns.value.data[parameter.lark.url.secretRef.key])} - lark2: http.#Do & { + lark2: http.#HTTPDo & { $params: { method: "POST" url: stringValue.$returns.str @@ -297,7 +297,7 @@ spec: slack: { if parameter.slack != _|_ { if parameter.slack.url.value != _|_ { - slack1: http.#Do & { + slack1: http.#HTTPDo & { $params: { method: "POST" url: parameter.slack.url.value @@ -321,7 +321,7 @@ spec: } stringValue: util.#ConvertString & {$params: bt: base64.Decode(null, read.$returns.value.data[parameter.slack.url.secretRef.key])} - slack2: http.#Do & { + slack2: http.#HTTPDo & { $params: { method: "POST" url: stringValue.$returns.str diff --git a/charts/vela-core/templates/defwithtemplate/restart-workflow.yaml b/charts/vela-core/templates/defwithtemplate/restart-workflow.yaml index 193ca1bd3..508918ba7 100644 --- a/charts/vela-core/templates/defwithtemplate/restart-workflow.yaml +++ b/charts/vela-core/templates/defwithtemplate/restart-workflow.yaml @@ -48,14 +48,16 @@ spec: # Convert duration to seconds SECONDS=0 - if [[ "$DURATION" =~ ^([0-9]+)m$ ]]; then + if [[ "$DURATION" =~ ^([0-9]+)s$ ]]; then + SECONDS=${BASH_REMATCH[1]} + elif [[ "$DURATION" =~ ^([0-9]+)m$ ]]; then SECONDS=$((${BASH_REMATCH[1]} * 60)) elif [[ "$DURATION" =~ ^([0-9]+)h$ ]]; then SECONDS=$((${BASH_REMATCH[1]} * 3600)) elif [[ "$DURATION" =~ ^([0-9]+)d$ ]]; then SECONDS=$((${BASH_REMATCH[1]} * 86400)) else - echo "ERROR: Invalid duration format: $DURATION (expected format: 5m, 1h, or 2d)" + echo "ERROR: Invalid duration format: $DURATION (expected format: 30s, 5m, 1h, or 2d)" exit 1 fi @@ -108,8 +110,8 @@ spec: // +usage=Schedule restart at a specific RFC3339 timestamp (e.g., "2025-01-15T14:30:00Z") at?: string & =~"^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}(\\.[0-9]+)?(Z|[+-][0-9]{2}:[0-9]{2})$" // +usage=Schedule restart after a relative duration from now (e.g., "5m", "1h", "2d") - after?: string & =~"^[0-9]+(m|h|d)$" + after?: string & =~"^[0-9]+(s|m|h|d)$" // +usage=Schedule recurring restarts every specified duration (e.g., "5m", "1h", "24h") - every?: string & =~"^[0-9]+(m|h|d)$" + every?: string & =~"^[0-9]+(s|m|h|d)$" } diff --git a/charts/vela-core/templates/defwithtemplate/webhook.yaml b/charts/vela-core/templates/defwithtemplate/webhook.yaml index 8d4d503cd..859fc633e 100644 --- a/charts/vela-core/templates/defwithtemplate/webhook.yaml +++ b/charts/vela-core/templates/defwithtemplate/webhook.yaml @@ -38,9 +38,10 @@ spec: } webhook: { if parameter.url.value != _|_ { - req: http.#HTTPPost & { + req: http.#HTTPDo & { $params: { - url: parameter.url.value + method: "POST" + url: parameter.url.value request: { body: data.value header: "Content-Type": "application/json" @@ -61,9 +62,10 @@ spec: } stringValue: util.#ConvertString & {$params: bt: base64.Decode(null, read.$returns.value.data[parameter.url.secretRef.key])} - req: http.#HTTPPost & { + req: http.#HTTPDo & { $params: { - url: stringValue.$returns.str + method: "POST" + url: stringValue.$returns.str request: { body: data.value header: "Content-Type": "application/json" diff --git a/pkg/definition/definition_test.go b/pkg/definition/definition_test.go index e7da3fd84..e5106390a 100644 --- a/pkg/definition/definition_test.go +++ b/pkg/definition/definition_test.go @@ -358,5 +358,5 @@ func TestStringStatusFromCueString(t *testing.T) { statusField, ok := ast.GetFieldByPath(f, fmt.Sprintf("%s.attributes.status.details", def.GetName())) require.True(t, ok, "status field not found in CUE definition") - require.IsType(t, &ast2.StructLit{}, statusField.Value, "expected status field to be of type StructLit") + require.IsType(t, &ast2.StructLit{}, statusField.Value, "expected status field to be of type StructLit ") } diff --git a/pkg/definition/defkit/collections.go b/pkg/definition/defkit/collections.go index f02bd9724..28e450c27 100644 --- a/pkg/definition/defkit/collections.go +++ b/pkg/definition/defkit/collections.go @@ -138,6 +138,12 @@ func (c *CollectionOp) Flatten() *CollectionOp { return c } +// Dedupe removes duplicate items by a key field, keeping the first occurrence. +func (c *CollectionOp) Dedupe(keyField string) *CollectionOp { + c.ops = append(c.ops, &dedupeOp{keyField: keyField}) + return c +} + // Source returns the source value. func (c *CollectionOp) Source() Value { return c.source } diff --git a/pkg/definition/defkit/collections_test.go b/pkg/definition/defkit/collections_test.go index c44131d14..ab2ad5f92 100644 --- a/pkg/definition/defkit/collections_test.go +++ b/pkg/definition/defkit/collections_test.go @@ -111,6 +111,12 @@ var _ = Describe("Collections", func() { col := defkit.Each(ports).DefaultField("name", defkit.Format("port-%v", defkit.FieldRef("port"))) Expect(col.Operations()).To(HaveLen(1)) }) + + It("should chain Dedupe operation on CollectionOp", func() { + ports := defkit.List("ports") + col := defkit.Each(ports).Dedupe("name") + Expect(col.Operations()).To(HaveLen(1)) + }) }) Context("FieldRef", func() { diff --git a/pkg/definition/defkit/cuegen.go b/pkg/definition/defkit/cuegen.go index acb991430..7cbaa417c 100644 --- a/pkg/definition/defkit/cuegen.go +++ b/pkg/definition/defkit/cuegen.go @@ -2722,6 +2722,9 @@ func (g *CUEGenerator) writeStringParam(sb *strings.Builder, p *StringParam, ind enumParts = append(enumParts, fmt.Sprintf("%q", v)) } } + if p.IsOpenEnum() { + enumParts = append(enumParts, "string") + } sb.WriteString(fmt.Sprintf("%s%s%s: %s\n", indent, name, optional, strings.Join(enumParts, " | "))) } else { // Build constraint parts diff --git a/pkg/definition/defkit/cuegen_test.go b/pkg/definition/defkit/cuegen_test.go index 184d9c07d..9e9c4983f 100644 --- a/pkg/definition/defkit/cuegen_test.go +++ b/pkg/definition/defkit/cuegen_test.go @@ -140,6 +140,17 @@ var _ = Describe("CUEGenerator", func() { Expect(cue).To(ContainSubstring(`optionalDefault?: *"Honor" | "Ignore"`)) }) + It("should append string to enum when OpenEnum is set", func() { + comp := defkit.NewComponent("test"). + Params( + defkit.String("verbosity").Default("info").Enum("info", "debug", "warn").OpenEnum(), + ) + + cue := gen.GenerateParameterSchema(comp) + + Expect(cue).To(ContainSubstring(`*"info" | "debug" | "warn" | string`)) + }) + It("should generate // +ignore directive for ignored parameters", func() { comp := defkit.NewComponent("test"). Params( diff --git a/pkg/definition/defkit/expr.go b/pkg/definition/defkit/expr.go index cfe6aa059..f4019b55c 100644 --- a/pkg/definition/defkit/expr.go +++ b/pkg/definition/defkit/expr.go @@ -501,6 +501,7 @@ type patchKeyField struct { // Used for building array values with struct elements. type ArrayElement struct { fields map[string]Value + fieldOrder []string ops []ResourceOp // nested operations for complex structs patchKeyFields []patchKeyField // nested patchKey-annotated array fields } @@ -511,13 +512,17 @@ func (a *ArrayElement) value() {} // NewArrayElement creates a new array element builder. func NewArrayElement() *ArrayElement { return &ArrayElement{ - fields: make(map[string]Value), - ops: make([]ResourceOp, 0), + fields: make(map[string]Value), + fieldOrder: make([]string, 0), + ops: make([]ResourceOp, 0), } } // Set sets a field on the array element. func (a *ArrayElement) Set(key string, value Value) *ArrayElement { + if _, exists := a.fields[key]; !exists { + a.fieldOrder = append(a.fieldOrder, key) + } a.fields[key] = value return a } @@ -544,6 +549,9 @@ func (a *ArrayElement) PatchKeyField(field string, key string, value Value) *Arr // Fields returns all fields set on this element. func (a *ArrayElement) Fields() map[string]Value { return a.fields } +// FieldOrder returns field names in insertion order. +func (a *ArrayElement) FieldOrder() []string { return a.fieldOrder } + // Ops returns any conditional operations. func (a *ArrayElement) Ops() []ResourceOp { return a.ops } diff --git a/pkg/definition/defkit/expr_test.go b/pkg/definition/defkit/expr_test.go index afe76ee5d..9678808ec 100644 --- a/pkg/definition/defkit/expr_test.go +++ b/pkg/definition/defkit/expr_test.go @@ -286,6 +286,23 @@ var _ = Describe("Expressions", func() { fields := elem.Fields() Expect(fields).To(HaveLen(1)) }) + + It("should track field insertion order", func() { + elem := defkit.NewArrayElement(). + Set("charlie", defkit.Lit(3)). + Set("alpha", defkit.Lit(1)). + Set("bravo", defkit.Lit(2)) + Expect(elem.FieldOrder()).To(Equal([]string{"charlie", "alpha", "bravo"})) + }) + + It("should not duplicate field order on overwrite", func() { + elem := defkit.NewArrayElement(). + Set("name", defkit.Lit("old")). + Set("port", defkit.Lit(80)). + Set("name", defkit.Lit("new")) + Expect(elem.FieldOrder()).To(Equal([]string{"name", "port"})) + Expect(elem.Fields()["name"]).To(Equal(defkit.Lit("new"))) + }) }) Context("ForEachMap", func() { diff --git a/pkg/definition/defkit/param.go b/pkg/definition/defkit/param.go index 0ad2e2404..88de3c052 100644 --- a/pkg/definition/defkit/param.go +++ b/pkg/definition/defkit/param.go @@ -111,6 +111,7 @@ func (c *ParamCompareCondition) CompareValue() any { return c.value } type StringParam struct { baseParam enumValues []string // allowed enum values + openEnum bool // when true, appends | string to enum disjunction (open enum) pattern string // regex pattern constraint minLen *int // minimum length constraint maxLen *int // maximum length constraint @@ -183,6 +184,18 @@ func (p *StringParam) GetEnumValues() []string { return p.enumValues } +// OpenEnum marks the enum as open, allowing any string in addition to the +// enumerated values. This generates CUE like: "value1" | "value2" | string +func (p *StringParam) OpenEnum() *StringParam { + p.openEnum = true + return p +} + +// IsOpenEnum returns whether the enum allows arbitrary strings beyond the listed values. +func (p *StringParam) IsOpenEnum() bool { + return p.openEnum +} + // Pattern sets a regex pattern constraint for the parameter. // This generates CUE like: string & =~"pattern" func (p *StringParam) Pattern(regex string) *StringParam { @@ -437,6 +450,14 @@ func (p *BoolParam) Optional() *BoolParam { return p } +// ForceOptional makes the field optional even when it has a default value. +// Normally, fields with defaults are treated as always-present (no ? in CUE). +// This generates field?: *default | type instead of field: *default | type. +func (p *BoolParam) ForceOptional() *BoolParam { + p.forceOptional = true + return p +} + // Default sets a default value for the parameter. func (p *BoolParam) Default(value bool) *BoolParam { p.defaultValue = value diff --git a/pkg/definition/defkit/param_test.go b/pkg/definition/defkit/param_test.go index 7d8827015..bf8d7e58a 100644 --- a/pkg/definition/defkit/param_test.go +++ b/pkg/definition/defkit/param_test.go @@ -74,6 +74,17 @@ var _ = Describe("Parameters", func() { p := defkit.String("policy").Default("Honor") Expect(p.IsForceOptional()).To(BeFalse()) }) + + It("should support OpenEnum on enum", func() { + p := defkit.String("verbosity").Enum("info", "debug").OpenEnum() + Expect(p.IsOpenEnum()).To(BeTrue()) + Expect(p.GetEnumValues()).To(ConsistOf("info", "debug")) + }) + + It("should not allow string by default on enum", func() { + p := defkit.String("verbosity").Enum("info", "debug") + Expect(p.IsOpenEnum()).To(BeFalse()) + }) }) Context("IntParam", func() { @@ -122,6 +133,17 @@ var _ = Describe("Parameters", func() { Expect(p.IsOptional()).To(BeTrue()) Expect(p.GetDefault()).To(Equal(false)) }) + + It("should support ForceOptional", func() { + p := defkit.Bool("auto").Default(true).ForceOptional() + Expect(p.IsForceOptional()).To(BeTrue()) + Expect(p.HasDefault()).To(BeTrue()) + }) + + It("should not be force-optional by default", func() { + p := defkit.Bool("auto").Default(true) + Expect(p.IsForceOptional()).To(BeFalse()) + }) }) Context("FloatParam", func() { diff --git a/pkg/definition/defkit/workflow_step.go b/pkg/definition/defkit/workflow_step.go index 9226125bb..f59ab4ec9 100644 --- a/pkg/definition/defkit/workflow_step.go +++ b/pkg/definition/defkit/workflow_step.go @@ -18,6 +18,7 @@ package defkit import ( "fmt" + "sort" "strings" "sigs.k8s.io/yaml" @@ -29,12 +30,14 @@ import ( // Workflow steps define operations in an application's deployment workflow, // such as deploy, suspend, notification, approval, etc. type WorkflowStepDefinition struct { - baseDefinition // embedded common fields and methods - category string // e.g., "Application Delivery", "Notification" - scope string // e.g., "Application", "Workflow" - alias string // optional alias for definition metadata annotation - hasAlias bool // tracks whether alias was explicitly set (including empty string) - stepTemplate func(tpl *WorkflowStepTemplate) // template function for step logic (type-specific) + baseDefinition // embedded common fields and methods + category string // e.g., "Application Delivery", "Notification" + scope string // e.g., "Application", "Workflow" + labels map[string]string // arbitrary metadata labels beyond scope + alias string // optional alias for definition metadata annotation + hasAlias bool // tracks whether alias was explicitly set (including empty string) + stepTemplate func(tpl *WorkflowStepTemplate) // template function for step logic (type-specific) + rawTemplateBody string // raw CUE embedded inside template: {} before the parameter block } // WorkflowStepTemplate provides the building context for workflow step templates. @@ -51,9 +54,10 @@ type WorkflowAction interface { // BuiltinAction represents a call to a vela builtin. type BuiltinAction struct { - varName string // explicit action variable name in template (e.g., "deploy", "wait") - name string // e.g., "multicluster.#Deploy", "builtin.#Suspend" - params map[string]Value // parameters to pass + varName string // explicit action variable name in template (e.g., "deploy", "wait") + name string // e.g., "multicluster.#Deploy", "builtin.#Suspend" + params map[string]Value // parameters to pass + useFullParam bool // if true, generates $params: parameter instead of $params: { key: value } } func (b *BuiltinAction) isWorkflowAction() {} @@ -104,6 +108,16 @@ func (w *WorkflowStepDefinition) Scope(scope string) *WorkflowStepDefinition { return w } +// Labels sets arbitrary metadata labels for the workflow step definition. +// These labels appear in the definition's labels block alongside scope. +func (w *WorkflowStepDefinition) Labels(labels map[string]string) *WorkflowStepDefinition { + w.labels = labels + return w +} + +// GetLabels returns the workflow step's metadata labels. +func (w *WorkflowStepDefinition) GetLabels() map[string]string { return w.labels } + // Alias sets an optional alias for the workflow step definition. // This maps to metadata annotation `definition.oam.dev/alias` in generated YAML. func (w *WorkflowStepDefinition) Alias(alias string) *WorkflowStepDefinition { @@ -137,6 +151,23 @@ func (w *WorkflowStepDefinition) RawCUE(cue string) *WorkflowStepDefinition { return w } +// TemplateBody sets raw CUE that is embedded inside the template: { ... } block, +// between any builder-generated actions and the parameter block. +// This allows complex workflow logic (array comprehensions, context variables, etc.) +// while still using the builder API for metadata (description, category, scope, imports) +// and parameter schema definitions. +// The body should be provided at zero indentation; one tab indent is added per line when embedded. +func (w *WorkflowStepDefinition) TemplateBody(body string) *WorkflowStepDefinition { + w.rawTemplateBody = body + return w +} + +// GetRawTemplateBody returns the raw CUE template body. +func (w *WorkflowStepDefinition) GetRawTemplateBody() string { return w.rawTemplateBody } + +// HasRawTemplateBody returns true if a raw template body is set. +func (w *WorkflowStepDefinition) HasRawTemplateBody() bool { return w.rawTemplateBody != "" } + // Helper adds a helper type definition using fluent API. // The param defines the schema for the helper type. // Example: @@ -350,6 +381,14 @@ func (b *BuiltinActionBuilder) WithParams(params map[string]Value) *BuiltinActio return b } +// WithFullParameter passes the entire parameter object as $params. +// This generates: $params: parameter +// Useful for builtins (e.g., builtin.#Suspend) that accept all step parameters directly. +func (b *BuiltinActionBuilder) WithFullParameter() *BuiltinActionBuilder { + b.action.useFullParam = true + return b +} + // Build finalizes the action and adds it to the template. func (b *BuiltinActionBuilder) Build() *WorkflowStepTemplate { b.template.actions = append(b.template.actions, b.action) @@ -417,8 +456,18 @@ func (g *WorkflowStepCUEGenerator) GenerateFullDefinition(w *WorkflowStepDefinit } sb.WriteString(fmt.Sprintf("%s}\n", g.indent)) - // Write labels (scope) + // Write labels (scope + custom labels) sb.WriteString(fmt.Sprintf("%slabels: {\n", g.indent)) + if labels := w.GetLabels(); len(labels) > 0 { + keys := make([]string, 0, len(labels)) + for k := range labels { + keys = append(keys, k) + } + sort.Strings(keys) + for _, k := range keys { + sb.WriteString(fmt.Sprintf("%s\t%q: %q\n", g.indent, k, labels[k])) + } + } if w.GetScope() != "" { sb.WriteString(fmt.Sprintf("%s\t\"scope\": %q\n", g.indent, w.GetScope())) } @@ -459,6 +508,21 @@ func (g *WorkflowStepCUEGenerator) GenerateTemplate(w *WorkflowStepDefinition) s g.writeActions(&sb, wt, 1) } + // Embed raw template body if set (for complex logic not expressible via builder API). + // Each line of the body is prefixed with one tab to sit inside the template: {} block. + if w.HasRawTemplateBody() { + body := strings.TrimRight(w.rawTemplateBody, "\n") + lines := strings.Split(body, "\n") + for _, line := range lines { + if strings.TrimSpace(line) == "" { + sb.WriteString("\n") + } else { + sb.WriteString(fmt.Sprintf("%s%s\n", g.indent, line)) + } + } + sb.WriteString("\n") + } + // Generate parameter section sb.WriteString(g.generateParameterBlock(w, 1)) @@ -501,7 +565,10 @@ func (g *WorkflowStepCUEGenerator) writeBuiltinAction(sb *strings.Builder, a *Bu } sb.WriteString(fmt.Sprintf("%s%s%s: %s & {\n", indent, extraIndent, actionName, a.name)) - if len(a.params) > 0 { + if a.useFullParam { + // Pass the entire parameter object as $params (e.g., builtin.#Suspend) + sb.WriteString(fmt.Sprintf("%s%s\t$params: parameter\n", indent, extraIndent)) + } else if len(a.params) > 0 { sb.WriteString(fmt.Sprintf("%s%s\t$params: {\n", indent, extraIndent)) for paramName, paramVal := range a.params { sb.WriteString(fmt.Sprintf("%s%s\t\t%s: %s\n", indent, extraIndent, paramName, gen.valueToCUE(paramVal))) diff --git a/pkg/definition/defkit/workflow_step_test.go b/pkg/definition/defkit/workflow_step_test.go index 432c2b897..5ae2ccee5 100644 --- a/pkg/definition/defkit/workflow_step_test.go +++ b/pkg/definition/defkit/workflow_step_test.go @@ -17,6 +17,8 @@ limitations under the License. package defkit_test import ( + "strings" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -258,6 +260,131 @@ template: { Expect(cue).NotTo(ContainSubstring(`apply: kube.#Apply & {`)) Expect(cue).NotTo(ContainSubstring(`conditionalwait: builtin.#ConditionalWait & {`)) }) + + It("should generate separate if blocks for each SetIf operation", func() { + data := defkit.Object("data") + noData := defkit.Eq(defkit.ParamRef("data"), defkit.Reference("_|_")) + hasData := defkit.PathExists("parameter.data") + + step := defkit.NewWorkflowStep("webhook"). + WithImports("vela/kube", "encoding/json"). + Params(data). + Template(func(tpl *defkit.WorkflowStepTemplate) { + dataValue := defkit.NewArrayElement(). + SetIf(noData, "read", defkit.Reference("kube.#Read & {}")). + SetIf(noData, "value", defkit.Reference("json.Marshal(read.$returns.value)")). + SetIf(hasData, "value", defkit.Reference("json.Marshal(parameter.data)")) + tpl.Set("data", dataValue) + }) + + cue := step.ToCue() + Expect(strings.Count(cue, "if parameter.data == _|_ {")).To(Equal(2)) + Expect(cue).To(ContainSubstring("read: kube.#Read & {}")) + Expect(cue).To(ContainSubstring("value: json.Marshal(read.$returns.value)")) + }) + }) + + Context("Labels", func() { + It("should set and get labels", func() { + step := defkit.NewWorkflowStep("check-metrics"). + Labels(map[string]string{"catalog": "Delivery"}) + Expect(step.GetLabels()).To(HaveKeyWithValue("catalog", "Delivery")) + }) + + It("should render labels in CUE output", func() { + step := defkit.NewWorkflowStep("check-metrics"). + Description("Verify metrics"). + Category("Application Delivery"). + Labels(map[string]string{"catalog": "Delivery"}) + + cue := step.ToCue() + Expect(cue).To(ContainSubstring(`"catalog": "Delivery"`)) + }) + + It("should render multiple labels sorted alphabetically", func() { + step := defkit.NewWorkflowStep("test"). + Description("Test"). + Labels(map[string]string{"z-label": "last", "a-label": "first"}) + + cue := step.ToCue() + aIdx := strings.Index(cue, `"a-label"`) + zIdx := strings.Index(cue, `"z-label"`) + Expect(aIdx).To(BeNumerically("<", zIdx)) + }) + }) + + Context("TemplateBody", func() { + It("should set and get raw template body", func() { + step := defkit.NewWorkflowStep("test"). + TemplateBody(`check: metrics.#PromCheck & {}`) + Expect(step.HasRawTemplateBody()).To(BeTrue()) + Expect(step.GetRawTemplateBody()).To(Equal(`check: metrics.#PromCheck & {}`)) + }) + + It("should return false when no template body set", func() { + step := defkit.NewWorkflowStep("test") + Expect(step.HasRawTemplateBody()).To(BeFalse()) + Expect(step.GetRawTemplateBody()).To(BeEmpty()) + }) + + It("should embed raw template body in generated CUE", func() { + step := defkit.NewWorkflowStep("check-metrics"). + Description("Verify metrics"). + WithImports("vela/metrics"). + Params(defkit.String("query").Required()). + TemplateBody("check: metrics.#PromCheck & {\n\t$params: query: parameter.query\n}") + + cue := step.ToCue() + Expect(cue).To(ContainSubstring("check: metrics.#PromCheck & {")) + Expect(cue).To(ContainSubstring("$params: query: parameter.query")) + Expect(cue).To(ContainSubstring("parameter:")) + Expect(cue).To(ContainSubstring("query: string")) + }) + + It("should handle empty lines in template body", func() { + step := defkit.NewWorkflowStep("test"). + Description("Test"). + TemplateBody("line1: true\n\nline2: false") + + cue := step.ToCue() + Expect(cue).To(ContainSubstring("line1: true")) + Expect(cue).To(ContainSubstring("line2: false")) + }) + }) + + Context("WithFullParameter", func() { + It("should generate $params: parameter for builtin", func() { + step := defkit.NewWorkflowStep("suspend"). + Description("Suspend workflow"). + Params(defkit.String("message").Optional()). + Template(func(tpl *defkit.WorkflowStepTemplate) { + tpl.Builtin("suspend", "builtin.#Suspend"). + WithFullParameter(). + Build() + }) + + cue := step.ToCue() + Expect(cue).To(ContainSubstring("suspend: builtin.#Suspend & {")) + Expect(cue).To(ContainSubstring("$params: parameter")) + }) + + It("should use WithFullParameter over WithParams when both are set", func() { + step := defkit.NewWorkflowStep("suspend"). + Description("Suspend workflow"). + Params(defkit.String("message").Optional()). + Template(func(tpl *defkit.WorkflowStepTemplate) { + tpl.Builtin("suspend", "builtin.#Suspend"). + WithFullParameter(). + WithParams(map[string]defkit.Value{ + "message": defkit.Reference("parameter.message"), + }). + Build() + }) + + cue := step.ToCue() + Expect(cue).To(ContainSubstring("$params: parameter")) + Expect(cue).NotTo(ContainSubstring("$params: {")) + }) }) Context("Registry", func() { diff --git a/vela-templates/definitions/internal/workflowstep/check-metrics.cue b/vela-templates/definitions/internal/workflowstep/check-metrics.cue index 4ef548cbe..427f2018a 100644 --- a/vela-templates/definitions/internal/workflowstep/check-metrics.cue +++ b/vela-templates/definitions/internal/workflowstep/check-metrics.cue @@ -19,7 +19,6 @@ template: { query: parameter.query metricEndpoint: parameter.metricEndpoint condition: parameter.condition - stepID: context.stepSessionID duration: parameter.duration failDuration: parameter.failDuration } diff --git a/vela-templates/definitions/internal/workflowstep/notification.cue b/vela-templates/definitions/internal/workflowstep/notification.cue index a4ced0cef..b8cd241fd 100644 --- a/vela-templates/definitions/internal/workflowstep/notification.cue +++ b/vela-templates/definitions/internal/workflowstep/notification.cue @@ -189,9 +189,7 @@ template: { multiline?: bool min_length?: int max_length?: int - dispatch_action_config?: { - trigger_actions_on?: [...string] - } + dispatch_action_config?: trigger_actions_on?: [...string] initial_time?: string }] } @@ -214,7 +212,7 @@ template: { ding: { if parameter.dingding != _|_ { if parameter.dingding.url.value != _|_ { - ding1: http.#Do & { + ding1: http.#HTTPDo & { $params: { method: "POST" url: parameter.dingding.url.value @@ -240,7 +238,7 @@ template: { } stringValue: util.#ConvertString & {$params: bt: base64.Decode(null, read.$returns.value.data[parameter.dingding.url.secretRef.key])} - ding2: http.#Do & { + ding2: http.#HTTPDo & { $params: { method: "POST" url: stringValue.$returns.str @@ -257,10 +255,10 @@ template: { lark: { if parameter.lark != _|_ { if parameter.lark.url.value != _|_ { - lark1: http.#Do & { + lark1: http.#HTTPDo & { $params: { method: "POST" - url: parameter.lark.message + url: parameter.lark.url.value request: { body: json.Marshal(parameter.lark.message) header: "Content-Type": "application/json" @@ -283,7 +281,7 @@ template: { } stringValue: util.#ConvertString & {$params: bt: base64.Decode(null, read.$returns.value.data[parameter.lark.url.secretRef.key])} - lark2: http.#Do & { + lark2: http.#HTTPDo & { $params: { method: "POST" url: stringValue.$returns.str @@ -301,7 +299,7 @@ template: { slack: { if parameter.slack != _|_ { if parameter.slack.url.value != _|_ { - slack1: http.#Do & { + slack1: http.#HTTPDo & { $params: { method: "POST" url: parameter.slack.url.value @@ -327,7 +325,7 @@ template: { } stringValue: util.#ConvertString & {$params: bt: base64.Decode(null, read.$returns.value.data[parameter.slack.url.secretRef.key])} - slack2: http.#Do & { + slack2: http.#HTTPDo & { $params: { method: "POST" url: stringValue.$returns.str diff --git a/vela-templates/definitions/internal/workflowstep/restart-workflow.cue b/vela-templates/definitions/internal/workflowstep/restart-workflow.cue index db31ac1ce..6c8440ff2 100644 --- a/vela-templates/definitions/internal/workflowstep/restart-workflow.cue +++ b/vela-templates/definitions/internal/workflowstep/restart-workflow.cue @@ -47,14 +47,16 @@ template: { # Convert duration to seconds SECONDS=0 - if [[ "$DURATION" =~ ^([0-9]+)m$ ]]; then + if [[ "$DURATION" =~ ^([0-9]+)s$ ]]; then + SECONDS=${BASH_REMATCH[1]} + elif [[ "$DURATION" =~ ^([0-9]+)m$ ]]; then SECONDS=$((${BASH_REMATCH[1]} * 60)) elif [[ "$DURATION" =~ ^([0-9]+)h$ ]]; then SECONDS=$((${BASH_REMATCH[1]} * 3600)) elif [[ "$DURATION" =~ ^([0-9]+)d$ ]]; then SECONDS=$((${BASH_REMATCH[1]} * 86400)) else - echo "ERROR: Invalid duration format: $DURATION (expected format: 5m, 1h, or 2d)" + echo "ERROR: Invalid duration format: $DURATION (expected format: 30s, 5m, 1h, or 2d)" exit 1 fi @@ -111,8 +113,8 @@ template: { // +usage=Schedule restart at a specific RFC3339 timestamp (e.g., "2025-01-15T14:30:00Z") at?: string & =~"^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}(\\.[0-9]+)?(Z|[+-][0-9]{2}:[0-9]{2})$" // +usage=Schedule restart after a relative duration from now (e.g., "5m", "1h", "2d") - after?: string & =~"^[0-9]+(m|h|d)$" + after?: string & =~"^[0-9]+(s|m|h|d)$" // +usage=Schedule recurring restarts every specified duration (e.g., "5m", "1h", "24h") - every?: string & =~"^[0-9]+(m|h|d)$" + every?: string & =~"^[0-9]+(s|m|h|d)$" } } diff --git a/vela-templates/definitions/internal/workflowstep/webhook.cue b/vela-templates/definitions/internal/workflowstep/webhook.cue index e0564fe5d..4db3b4831 100644 --- a/vela-templates/definitions/internal/workflowstep/webhook.cue +++ b/vela-templates/definitions/internal/workflowstep/webhook.cue @@ -37,9 +37,10 @@ template: { } webhook: { if parameter.url.value != _|_ { - req: http.#HTTPPost & { + req: http.#HTTPDo & { $params: { - url: parameter.url.value + method: "POST" + url: parameter.url.value request: { body: data.value header: "Content-Type": "application/json" @@ -62,9 +63,10 @@ template: { } stringValue: util.#ConvertString & {$params: bt: base64.Decode(null, read.$returns.value.data[parameter.url.secretRef.key])} - req: http.#HTTPPost & { + req: http.#HTTPDo & { $params: { - url: stringValue.$returns.str + method: "POST" + url: stringValue.$returns.str request: { body: data.value header: "Content-Type": "application/json"