Repository navigation
Feat: $( ) expressions in workflow step properties, resolved when each step runs - #271
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 34 files
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
View guided diff | Re-trigger cubic
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #271 +/- ##
===========================================
+ Coverage 62.49% 79.15% +16.66%
===========================================
Files 62 81 +19
Lines 4415 6150 +1735
===========================================
+ Hits 2759 4868 +2109
+ Misses 1324 1282 -42
+ Partials 332 0 -332
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
297b520 to
925f429
Compare
An Application's workflow step expressions were substituted once, before the workflow ran, so a step could read nothing the run produced. The workflow engine now resolves them when each step starts (kubevela/workflow#271), and KubeVela gives it its own roots for an Application that opts in: - context and source, answered as the step's render would, with what each step's sources resolved to recorded against it; - a source holds for the whole run: each is resolved the first time a step reads it, kept in the workflow context against the run, and typed again against its schema for every later step. A new run, such as a new revision, reads afresh; - steps and each are roots a workflow step may read, so the parser and admission plan them, and admission runs the workflow's checks: a read of a step or output that does not exist, of a later step in a workflow run in order, a cycle, or each outside a forEach body is refused. A step still may not read component. status.workflow.dependencies carries what each step waits on, from its dependsOn or its expressions. status.sources keeps reporting what the current run's steps read after they have run. The up-front pass (resolveWorkflowStepSources) goes, forEach.items included: the workflow resolves items when the loop starts. Signed-off-by: Brian Kane <briankane1@gmail.com>
…workflow fork kubevela/workflow#271 is not merged yet, and a fork-only commit cannot be pinned under the upstream module path. Do not merge this commit; swap it for the upstream version once kubevela#271 lands. Signed-off-by: Brian Kane <briankane1@gmail.com>
925f429 to
7888b50
Compare
An Application's workflow step expressions were substituted once, before the workflow ran, so a step could read nothing the run produced. The workflow engine now resolves them when each step starts (kubevela/workflow#271), and KubeVela gives it its own roots for an Application that opts in: - context and source, answered as the step's render would, with what each step's sources resolved to recorded against it; - a source holds for the whole run: each is resolved the first time a step reads it, kept in the workflow context against the run, and typed again against its schema for every later step. A new run, such as a new revision, reads afresh; - steps and each are roots a workflow step may read, so the parser and admission plan them, and admission runs the workflow's checks: a read of a step or output that does not exist, of a later step in a workflow run in order, a cycle, or each outside a forEach body is refused. A step still may not read component. status.workflow.dependencies carries what each step waits on, from its dependsOn or its expressions. status.sources keeps reporting what the current run's steps read after they have run. The up-front pass (resolveWorkflowStepSources) goes, forEach.items included: the workflow resolves items when the loop starts. Signed-off-by: Brian Kane <briankane1@gmail.com>
…workflow fork kubevela/workflow#271 is not merged yet, and a fork-only commit cannot be pinned under the upstream module path. Do not merge this commit; swap it for the upstream version once kubevela#271 lands. Signed-off-by: Brian Kane <briankane1@gmail.com>
An Application's workflow step expressions were substituted once, before the workflow ran, so a step could read nothing the run produced. The workflow engine now resolves them when each step starts (kubevela/workflow#271), and KubeVela gives it its own roots for an Application that opts in: - context and source, answered as the step's render would, with what each step's sources resolved to recorded against it; - a source holds for the whole run: each is resolved the first time a step reads it, kept in the workflow context against the run, and typed again against its schema for every later step. A new run, such as a new revision, reads afresh; - steps and each are roots a workflow step may read, so the parser and admission plan them, and admission runs the workflow's checks: a read of a step or output that does not exist, of a later step in a workflow run in order, a cycle, or each outside a forEach body is refused. A step still may not read component. status.workflow.dependencies carries what each step waits on, from its dependsOn or its expressions. status.sources keeps reporting what the current run's steps read after they have run. The up-front pass (resolveWorkflowStepSources) goes, forEach.items included: the workflow resolves items when the loop starts. Signed-off-by: Brian Kane <briankane1@gmail.com>
…workflow fork kubevela/workflow#271 is not merged yet, and a fork-only commit cannot be pinned under the upstream module path. Do not merge this commit; swap it for the upstream version once kubevela#271 lands. Signed-off-by: Brian Kane <briankane1@gmail.com>
7888b50 to
52c473a
Compare
There was a problem hiding this comment.
All reported issues were addressed across 17 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
An Application's workflow step expressions were substituted once, before the workflow ran, so a step could read nothing the run produced. The workflow engine now resolves them when each step starts (kubevela/workflow#271), and KubeVela gives it its own roots for an Application that opts in: - context and source, answered as the step's render would, with what each step's sources resolved to recorded against it; - a source holds for the whole run: each is resolved the first time a step reads it, kept in the workflow context against the run, and typed again against its schema for every later step. A new run, such as a new revision, reads afresh; - steps and each are roots a workflow step may read, so the parser and admission plan them, and admission runs the workflow's checks: a read of a step or output that does not exist, of a later step in a workflow run in order, a cycle, or each outside a forEach body is refused. A step still may not read component. status.workflow.dependencies carries what each step waits on, from its dependsOn or its expressions. status.sources keeps reporting what the current run's steps read after they have run. The up-front pass (resolveWorkflowStepSources) goes, forEach.items included: the workflow resolves items when the loop starts. Signed-off-by: Brian Kane <briankane1@gmail.com>
…workflow fork kubevela/workflow#271 is not merged yet, and a fork-only commit cannot be pinned under the upstream module path. Do not merge this commit; swap it for the upstream version once kubevela#271 lands. Signed-off-by: Brian Kane <briankane1@gmail.com>
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
ac4981d to
58045d9
Compare
An Application's workflow step expressions were substituted once, before the workflow ran, so a step could read nothing the run produced. The workflow engine now resolves them when each step starts (kubevela/workflow#271), and KubeVela gives it its own roots for an Application that opts in: - context and source, answered as the step's render would, with what each step's sources resolved to recorded against it; - a source holds for the whole run: each is resolved the first time a step reads it, kept in the workflow context against the run, and typed again against its schema for every later step. A new run, such as a new revision, reads afresh; - steps and each are roots a workflow step may read, so the parser and admission plan them, and admission runs the workflow's checks: a read of a step or output that does not exist, of a later step in a workflow run in order, a cycle, or each outside a forEach body is refused. A step still may not read component. status.workflow.dependencies carries what each step waits on, from its dependsOn or its expressions. status.sources keeps reporting what the current run's steps read after they have run. The up-front pass (resolveWorkflowStepSources) goes, forEach.items included: the workflow resolves items when the loop starts. Signed-off-by: Brian Kane <briankane1@gmail.com>
…workflow fork kubevela/workflow#271 is not merged yet, and a fork-only commit cannot be pinned under the upstream module path. Do not merge this commit; swap it for the upstream version once kubevela#271 lands. Signed-off-by: Brian Kane <briankane1@gmail.com>
…h step runs A step's properties may hold $( ) expressions, evaluated on the kubevela/pkg CEL engine when the step starts rather than before the run, so a step can read what earlier steps produced: - steps.<name>.outputs.<output> reads an output that step declares. A read waits while that step has still to finish, and fails once it failed. A skipped step's outputs read as absent, as inputs read them: a guarded read takes its default, an unguarded one fails. Inside a forEach iteration a body step's outputs are that iteration's; outside the loop they are the lists it collected. - each.item and each.index read the current forEach iteration, by the names inputs[].from uses. - context reads the step's context, as a CUE template does. - forEach.items may be an expression, resolved once when the loop starts. A read of steps.X is an implied dependsOn X, by the rules KubeVela uses for component reads: not to the reader itself, a later step in a workflow run in order, or one closing a cycle, and not to an earlier step in order, which the order already puts first. Unlike a written dependsOn, it does not skip its reader when the step read is skipped. status.dependencies records what each step waits on and whether its dependsOn or its expressions said so. A host adds roots of its own through StepGeneratorOptions.Expressions, with a resolver for each. A resolver is told the run and step it answers for (RunFrom), so it can hold a value for the whole run in the workflow context. The step's own properties are resolved before a StepConvertor runs, so text a convertor adds is left for whatever it belongs to. A WorkflowRun opts in with the EnableCelExpressions feature gate (workflow.enableCelExpressions in the chart) and the annotation workflowrun.oam.dev/cel-expressions: "true". Admission then checks every expression without evaluating it, and refuses a read that does not compile, names a step or output that does not exist or an output two steps declare, uses each outside a forEach body, or could never be answered (the reader itself, a later step in order, a cycle). Signed-off-by: Brian Kane <briankane1@gmail.com>
58045d9 to
53ec13e
Compare
An Application's workflow step expressions were substituted once, before the workflow ran, so a step could read nothing the run produced. The workflow engine now resolves them when each step starts (kubevela/workflow#271), and KubeVela gives it its own roots for an Application that opts in: - context and source, answered as the step's render would, with what each step's sources resolved to recorded against it; - a source holds for the whole run: each is resolved the first time a step reads it, kept in the workflow context against the run, and typed again against its schema for every later step, and reporting why it was served (a stale value says so). A new run, or a new revision, reads afresh; - steps and each are roots a workflow step may read, so the parser and admission plan them, and admission runs the workflow's checks: a read of a step or output that does not exist, of a later step in a workflow run in order, a cycle, or each outside a forEach body is refused. A step still may not read component. status.workflow.dependencies carries what each step waits on, from its dependsOn or its expressions; an edge a component read implies is an expression's, as it is in status.dependencies. A component read no longer adds a dependsOn to an earlier step in a workflow run in order: the order already puts that step first, and the edge only stopped an earlier failure skipping the reader. A group's own mode now orders its sub-steps, and an edge lifted to the reader's group that would wait on one of its own sub-steps is left out. status.sources keeps reporting what the current run's steps read after they have run. The up-front pass (resolveWorkflowStepSources) goes, forEach.items included: the workflow resolves items when the loop starts. Signed-off-by: Brian Kane <briankane1@gmail.com>
…workflow fork kubevela/workflow#271 is not merged yet, and a fork-only commit cannot be pinned under the upstream module path. Do not merge this commit; swap it for the upstream version once kubevela#271 lands. Signed-off-by: Brian Kane <briankane1@gmail.com>
Only a list, or one $( ) expression that resolves to one, can be looped over; text around an expression, or two of them, resolve to a string that the loop then refuses as a run fails. Admission now refuses it, and the message for an expression without the opt-in names the annotations to set. Signed-off-by: Brian Kane <briankane1@gmail.com>
A convertor that edited the step it was given in place also edited the step as written, which the step's expressions are resolved from, and the caller's. Signed-off-by: Brian Kane <briankane1@gmail.com>
…ead cannot wait on itself A document holding $\u0028 decodes to $( but was not seen as holding an expression, so it was neither resolved nor checked. And in a workflow run in order, a read of an earlier step that itself waits for the reader through dependsOn could never be answered; admission now refuses it as a cycle. Signed-off-by: Brian Kane <briankane1@gmail.com>
770eab2 to
1eea8fc
Compare
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
The engine does not count whitespace around a single expression as text, so admission accepts such items and the loop runs over the list they read. Both are now tested. Signed-off-by: Brian Kane <briankane1@gmail.com>
eb7b07c to
d660855
Compare
…workflow fork kubevela/workflow#271 is not merged yet, and a fork-only commit cannot be pinned under the upstream module path. Do not merge this commit; swap it for the upstream version once kubevela#271 lands. Signed-off-by: Brian Kane <briankane1@gmail.com>
Build against kubevela/workflow f746a3a (kubevela/workflow#271), which resolves step expressions as each step runs. Signed-off-by: Brian Kane <briankane1@gmail.com>
* Feat(workflow): resolve step expressions when each step runs An Application's workflow step expressions were substituted once, before the workflow ran, so a step could read nothing the run produced. The workflow engine now resolves them when each step starts (kubevela/workflow#271), and KubeVela gives it its own roots for an Application that opts in: - context and source, answered as the step's render would, with what each step's sources resolved to recorded against it; - a source holds for the whole run: each is resolved the first time a step reads it, kept in the workflow context against the run, and typed again against its schema for every later step, and reporting why it was served (a stale value says so). A new run, or a new revision, reads afresh; - steps and each are roots a workflow step may read, so the parser and admission plan them, and admission runs the workflow's checks: a read of a step or output that does not exist, of a later step in a workflow run in order, a cycle, or each outside a forEach body is refused. A step still may not read component. status.workflow.dependencies carries what each step waits on, from its dependsOn or its expressions; an edge a component read implies is an expression's, as it is in status.dependencies. A component read no longer adds a dependsOn to an earlier step in a workflow run in order: the order already puts that step first, and the edge only stopped an earlier failure skipping the reader. A group's own mode now orders its sub-steps, and an edge lifted to the reader's group that would wait on one of its own sub-steps is left out. status.sources keeps reporting what the current run's steps read after they have run. The up-front pass (resolveWorkflowStepSources) goes, forEach.items included: the workflow resolves items when the loop starts. Signed-off-by: Brian Kane <briankane1@gmail.com> * Fix(sources): pin the sources a step resolved when a later read fails A step whose read of one source failed left the sources it had already resolved unpinned, so its retry, or a later step, could read a newer value than the one the run started with. What resolved is now written to the run's pins before the failure is returned. Signed-off-by: Brian Kane <briankane1@gmail.com> * Fix(application): keep every source read a workflow step records A step resolves its forEach.items and its properties separately, so a step reading one source in both was recorded twice, and the second recording replaced the first in status.sources, dropping what the items read. A reader's values now accumulate across its recordings, one per property and source attribute. Signed-off-by: Brian Kane <briankane1@gmail.com> * Fix(application): keep a kept step's resolutions beside a live reader A workflow step kept from the last report lost its resolutions, the storage key and expiry its value came from, as soon as any other reader resolved the same binding in the reconcile. The kept resolutions now join the live ones, by storage key with the live entry winning, whenever a kept reader is reported. Signed-off-by: Brian Kane <briankane1@gmail.com> * Docs(workflow): say what WithStepReads does to each dependency The GoDoc now says it marks the entries that are reads as an expression's, adds the reads the engine did not record, and sorts by step. Signed-off-by: Brian Kane <briankane1@gmail.com> * Test(workflow): prove a restart follows the edges reads add The reader was marked skipped, and a skipped step is restarted with or without an edge, so the test passed without the read edges. Every other step now succeeded, and the same restart without the edges keeps the readers. Signed-off-by: Brian Kane <briankane1@gmail.com> * Test(webhook): require errors before reading the step reads aggregate An empty list aggregates to nil, so a refusal that stopped happening panicked rather than failing the assertion. Signed-off-by: Brian Kane <briankane1@gmail.com> * Test(e2e): step expressions' cleanup and absence checks name their cause The cleanup also waits for the source cache's ConfigMaps and Secrets in vela-system to go, and a ConfigMap read tells absence from an API error, so a check that it was never applied cannot pass on a failed read. Signed-off-by: Brian Kane <briankane1@gmail.com> * Chore: bump kubevela/workflow to the step expressions change Build against kubevela/workflow f746a3a (kubevela/workflow#271), which resolves step expressions as each step runs. Signed-off-by: Brian Kane <briankane1@gmail.com> --------- Signed-off-by: Brian Kane <briankane1@gmail.com>
Description of your changes
Lets a step's properties hold
$( )expressions, evaluated on the kubevela/pkg CEL engine when the step starts rather than before the run, so a step can read what earlier steps produced.steps.<name>.outputs.<output>reads an output that step declares;each.itemandeach.indexread the current forEach iteration, by the namesinputs[].fromuses;contextreads the step's context, as a CUE template does. Inside an iteration a body step's outputs are that iteration's; outside the loop they are the lists it collected.waiting for step list); a read of a step that failed fails. A skipped step's outputs read as absent, as an input reads one as null: a guarded read (has(steps.a.outputs.o) ? ... : default) takes its default, an unguarded one fails, and the implieddependsOndoes not skip the reader as a written one would.forEach.itemsmay be an expression, resolved once when the loop starts.steps.Xis an implieddependsOn: X, by the rules KubeVela uses for component reads: not to the reader itself, a later step in a workflow run in order, or one that closes a cycle.status.dependencieslists what each step waits on and whetherdependsOnor an expression said so, in the shape the Application uses for components:{step, dependsOn, source}.StepGeneratorOptions.Expressionsadds a host's roots with a resolver for each (KubeVela addssource). A resolver is told the run and step it answers for (expressions.RunFrom), so it can hold a value for the whole run in the workflow context. A step's own properties are resolved before aStepConvertorruns, so text a convertor adds, such as a component's properties, is left for whatever it belongs to.EnableCelExpressionsfeature gate (chartworkflow.enableCelExpressions, off by default) and the annotationworkflowrun.oam.dev/cel-expressions: "true". Without both, properties are left as written.stepsoutputs that exist and that only one step declares;eachsits in a forEach body; and no read could wait forever (the reader itself, a later step in order, a cycle).Part of kubevela/kubevela#7414.
I have:
make reviewableto ensure this PR is ready for review. Ran gofmt, golangci-lint and staticcheck, and regenerated the CRD and chart README; not the full target.backport release-x.ylabels to auto-backport this PR if necessary.How has this code been tested
dependsOn, admission, and the run scope a host resolver gets.$$(escape, and resolution before a convertor.each, and a DAG ordered only by its reads.e2e/expressions_test.go) on k3d; the e2e install turns the gate on.TestSendEmail(and itslegacycopy) and the "request step resolving headers from a Kubernetes Secret" e2e spec fail locally for reasons unrelated to this change: the first needs network access the sandbox denies, the second calls a mock server by cluster DNS, which a controller run on the host cannot resolve.Special notes for your reviewer
status.dependencies. KubeVela carries it into the Application'sstatus.workflowin the companion PR.sourcevalues are typed against their schema in a way the workflow cannot store faithfully, so it keeps them itself throughRunFrom.Summary by cubic
Lets workflow step properties and
forEach.itemshold$( )CEL expressions, resolved when the step starts instead of before the run, so a step can read what earlier steps produced.Behavior
steps.<name>.outputs.<output>reads a declared output;each.item,each.index, andcontextare also available, and$(written as$\u0028is still treated as an expression.steps.XimpliesdependsOn: X; reads wait for running steps and fail if the step failed. A skipped step's outputs read as absent, so a guarded read takes its default, and the implied dependsOn holds the reader pending without skipping it as a written one would. A forEach body's reads see that iteration's values.StepGeneratorOptions.Expressions; a step's properties resolve before aStepConvertorruns, which receives a copy so in-place edits can't alter the step as written.status.dependencieslists what each step waits on and why.Migration
EnableCelExpressionsfeature gate (workflow.enableCelExpressions, off by default) and theworkflowrun.oam.dev/cel-expressions: "true"annotation; without both, properties are left as written.eachoutside a forEach body, or could wait forever, including an in-order read of an earlier step that waits on the reader. AforEach.itemsmust be a list or one$( )expression that resolves to one; text around an expression is refused, while whitespace around it is still treated as one. A referenced workflow is checked with its own steps, mode, and roots only, since nothing else resolves its WorkflowRun.Written for commit d660855. Summary will update on new commits.