feat: Add CEL-based conditional function execution (#4388) - #4469
feat: Add CEL-based conditional function execution (#4388)#4469SurbhiAgarwal1 wants to merge 6 commits into
Conversation
✅ Deploy Preview for kptdocs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
2037fa3 to
b9a1ab1
Compare
|
Comment from #4391: Closing this PR in favor of a clean rebase. The branch had accumulated 61 commits including upstream commits from other contributors, making it messy to review. All the feedback from this review has been addressed in the new PR which has a single clean commit: Thank you all for the thorough review! |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 32 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
thirdparty/kyaml/runfn/runfn.go:1
NewFunctionRunner’s error is discarded andSetConditionis called without validatingopts.CELEnvironment. This can both (a) mask construction failures and (b) silently ignore the condition at runtime whenCELEnvironmentis nil (sinceFilter()only evaluates conditions whencelEnv != nil). Handle theerrfromNewFunctionRunner, and ifr.Conditionis set, return an error whenopts.CELEnvironmentis nil (or makeSetConditionreturn an error and enforce it here).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 30 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
documentation/content/en/book/04-using-functions/_index.md:1
- Two doc mismatches with the implementation: (1) the CEL resource map normalization in
resourceToMapguaranteesapiVersion,kind, andmetadatakeys, but notspec/status—either update the docs or ensure those keys are present; (2) the skipped example says “Successfully executed 1 function(s)” but the new behavior and e2e expectations indicate skipped functions should not count as executed (so this should be0).
---
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
The tests are failing because we have merged the conditions and renederstatus to kpt recently. I thought that by deleting the The following `diff.patch files work for me on the tests locally on my machine: e2e/testdata/fn-render/condition/condition-met/.expected/diff.patch e2e/testdata/fn-render/condition/condition-not-met/.expected/diff.patch Can you try restoring the two Sorry for mucking with your PR 😢 |
abd89dd to
df1189f
Compare
|
Thank you @liamfallon! I've restored both diff.patch files with exactly the contents you provided in commit df1189f. The tests should pass now. Sorry for the confusion! |
df1189f to
ad63ed9
Compare
298cc5d to
6d4aed1
Compare
6d4aed1 to
26362cc
Compare
Hi @SurbhiAgarwal1 Unfortunately, I have little experience with Windows development and I would suggest that we do not fully support Windows development. Again, thanks for your efforts with this but we do need to get this completed asap. Can you address the above so we can proceed swiftly. |
8ced243 to
83f04f2
Compare
|
You're completely right about the WSL symlink behavior. I've updated the tests to drop the file-inspection approach and replaced it with a strict (I also ran into the local Docker/Podman failures myself because I had originally cloned the repo on my Windows mount instead of natively inside WSL, but cloning it directly inside WSL fixed it completely!) |
|
It look like you've addressed most of the review comments, with just a few remaining. There's also a merge conflict and some CI failures that need attention. We're happy to help you get those resolved and move the reviews along - feel free to reach out if you have any questions. cc/ @liamfallon @efiacor |
|
Hi @SurbhiAgarwal1 , have you resolved all the comments above? |
c497258 to
2cff771
Compare
|
|
This feature has been getting more traction from the community recently and we'd like to prioritise getting it merged. Could you pick this up and address the outstanding items?
If you're blocked on anything or need any help, feel free to reach out - we're happy to assist. cc/ @liamfallon @efiacor |
Add CEL-based conditional function execution to kpt pipelines. A new optional 'when' field is added to the Function type in the Kptfile pipeline. When specified, the CEL expression is evaluated against the current list of KRM resources. If the expression returns false, the function is skipped. If omitted or returns true, the function executes normally. Changes: - Added 'when' (CelCondition) field to Function type in api/kptfile/v1/types.go - Added CELEnvironment in pkg/lib/runneroptions/celenv.go using google/cel-go - Integrated condition check in FunctionRunner.Filter() in pkg/fn/runtime/runner.go - Functions skipped due to condition show [SKIPPED] in CLI output - Added 'when' and 'skipped' fields to PipelineStepResult for render status tracking - CEL limits (CelCheckFrequency, CelCostLimit) are configurable on RunnerOptions - Added InitCELEnvironment() method to RunnerOptions for proper error handling - Updated all callers of InitDefaults() to also call InitCELEnvironment() - Added E2E testdata for condition-met and condition-not-met cases - Added unit tests for CEL evaluation covering builtin, exec, and container runtimes - Updated documentation: kptfile schema reference and book/04-using-functions - Windows test skips use runtime.GOOS == 'windows' check Signed-off-by: SurbhiAgarwal1 <surbhi.agarwal@example.com>
2cff771 to
e0f128a
Compare
Signed-off-by: SurbhiAgarwal1 <surbhi.agarwal@example.com>
Signed-off-by: SurbhiAgarwal1 <surbhi.agarwal@example.com>
Signed-off-by: SurbhiAgarwal1 <surbhi.agarwal@example.com>
Signed-off-by: SurbhiAgarwal1 <surbhi.agarwal@example.com>
…e expectations Signed-off-by: SurbhiAgarwal1 <surbhi.agarwal@example.com>
|
Hi @aravindtga @efiacor @liamfallon, I have updated the PR with all fixes and test fixture updates:
All commits are signed off and pushed to the branch. Ready for your review! |
efiacor
left a comment
There was a problem hiding this comment.
We will have to wave this through and pickup the additional pieces afterwards
@aravindtga


Description
This PR adds CEL-based conditional function execution to kpt, implementing #4388.
A new optional
conditionfield is added to theFunctiontype in the Kptfile pipeline. When specified, the CEL expression is evaluated against the current list of KRM resources. If the expression returnsfalse, the function is skipped. If omitted or returnstrue, the function executes normally.Motivation
In many real-world scenarios, you want a pipeline function to run only when certain resources exist or meet specific criteria. Without this feature, users had to maintain separate Kptfiles or manually manage which functions run. The
conditionfield makes pipelines more dynamic and reduces the need for workarounds.Changes
conditionfield toFunctiontype inpkg/api/kptfile/v1/types.goCELEnvironmentinpkg/lib/runneroptions/celenv.gousinggoogle/cel-goFunctionRunner.Filter()ininternal/fnruntime/runner.go[SKIPPED]in CLI outputInitCELEnvironment()method toRunnerOptionsfor proper error handlingInitDefaults()to also callInitCELEnvironment()condition-metandcondition-not-metcasesExample Usage