Skip to content

feat: Add CEL-based conditional function execution (#4388)#4469

Open
SurbhiAgarwal1 wants to merge 11 commits into
kptdev:mainfrom
SurbhiAgarwal1:feat/cel-clean-v2
Open

feat: Add CEL-based conditional function execution (#4388)#4469
SurbhiAgarwal1 wants to merge 11 commits into
kptdev:mainfrom
SurbhiAgarwal1:feat/cel-clean-v2

Conversation

@SurbhiAgarwal1

@SurbhiAgarwal1 SurbhiAgarwal1 commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Description

This PR adds CEL-based conditional function execution to kpt, implementing #4388.

A new optional condition 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.

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 condition field makes pipelines more dynamic and reduces the need for workarounds.

Changes

  • Added condition field to Function type in pkg/api/kptfile/v1/types.go
  • Added CELEnvironment in pkg/lib/runneroptions/celenv.go using google/cel-go
  • Integrated condition check in FunctionRunner.Filter() in internal/fnruntime/runner.go
  • Functions skipped due to condition show [SKIPPED] in CLI output
  • 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 all 3 runtimes (builtin, exec, container)
  • Updated documentation: kptfile schema reference and book/04-using-functions

Example Usage

pipeline:
  mutators:
    - image: ghcr.io/kptdev/krm-functions-catalog/set-namespace:v0.4
      condition: "resources.exists(r, r.kind == 'ConfigMap' && r.metadata.name == 'env-config')"
      configMap:
        namespace: production

Copilot AI review requested due to automatic review settings April 7, 2026 18:29
@netlify

netlify Bot commented Apr 7, 2026

Copy link
Copy Markdown

Deploy Preview for kptdocs ready!

Name Link
🔨 Latest commit 83f04f2
🔍 Latest deploy log https://app.netlify.com/projects/kptdocs/deploys/6a42518728c1f00008a4bc66
😎 Deploy Preview https://deploy-preview-4469--kptdocs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@dosubot dosubot Bot added size:XL This PR changes 500-999 lines, ignoring generated files. documentation Improvements or additions to documentation enhancement New feature or request go Pull requests that update Go code labels Apr 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@liamfallon

Copy link
Copy Markdown
Contributor

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:

Removed unnecessary type alias file (celeval.go)
Grouped constants in const() block
Replaced interface{} with any
Removed panic from InitDefaults, added InitCELEnvironment() error method
Added e2e tests for condition-met and condition-not-met
Added immutability test
Added documentation (kptfile schema + book/04-using-functions)
Removed all unwanted files (krm-functions-catalog, output.txt, etc.)
Fixed diff.patch for renderStatus field

Thank you all for the thorough review!

Copilot AI review requested due to automatic review settings April 8, 2026 16:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 and SetCondition is called without validating opts.CELEnvironment. This can both (a) mask construction failures and (b) silently ignore the condition at runtime when CELEnvironment is nil (since Filter() only evaluates conditions when celEnv != nil). Handle the err from NewFunctionRunner, and if r.Condition is set, return an error when opts.CELEnvironment is nil (or make SetCondition return an error and enforce it here).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/util/render/executor.go Outdated
Comment thread pkg/lib/runneroptions/runneroptions.go Outdated
Comment thread commands/fn/render/cmdrender.go Outdated
Comment thread documentation/content/en/book/04-using-functions/_index.md Outdated
Comment thread e2e/testdata/live-apply/json-output/config.yaml
Comment thread e2e/testdata/live-apply/apply-depends-on/config.yaml
Copilot AI review requested due to automatic review settings April 8, 2026 18:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 resourceToMap guarantees apiVersion, kind, and metadata keys, but not spec/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 be 0).
---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread thirdparty/kyaml/runfn/runfn.go Outdated
Comment thread pkg/fn/runtime/runner.go
Comment thread pkg/fn/runtime/runner.go
Comment thread documentation/content/en/book/04-using-functions/_index.md Outdated
@liamfallon

liamfallon commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

@SurbhiAgarwal1

The tests are failing because we have merged the conditions and renederstatus to kpt recently. I thought that by deleting the diff.patch files, the comparison would just check the output and the tests would pass. However we also need to add the difference in the Kprfile after render to the diff.patch files. Now that I deleted the diff.patch files in the PR, I can't see any option to restore them on the Github GUI.

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

diff --git a/Kptfile b/Kptfile
index eb90ac3..d305dc0 100644
--- a/Kptfile
+++ b/Kptfile
@@ -5,4 +5,14 @@ metadata:
 pipeline:
   mutators:
     - image: ghcr.io/kptdev/krm-functions-catalog/no-op
-      condition: "resources.exists(r, r.kind == 'ConfigMap' && r.metadata.name == 'app-config')"
+      condition: resources.exists(r, r.kind == 'ConfigMap' && r.metadata.name == 'app-config')
+status:
+  conditions:
+    - type: Rendered
+      status: "True"
+      reason: RenderSuccess
+  renderStatus:
+    mutationSteps:
+      - image: ghcr.io/kptdev/krm-functions-catalog/no-op
+        exitCode: 0

e2e/testdata/fn-render/condition/condition-not-met/.expected/diff.patch

diff --git a/Kptfile b/Kptfile
index eb90ac3..b055f32 100644
--- a/Kptfile
+++ b/Kptfile
@@ -5,4 +5,14 @@ metadata:
 pipeline:
   mutators:
     - image: ghcr.io/kptdev/krm-functions-catalog/no-op
-      condition: "resources.exists(r, r.kind == 'ConfigMap' && r.metadata.name == 'app-config')"
+      condition: resources.exists(r, r.kind == 'ConfigMap' && r.metadata.name == 'app-config')
+status:
+  conditions:
+    - type: Rendered
+      status: "True"
+      reason: RenderSuccess
+  renderStatus:
+    mutationSteps:
+      - image: ghcr.io/kptdev/krm-functions-catalog/no-op
+        exitCode: 0
+        skipped: true

Can you try restoring the two diff.patch files with the contents above and see if that fixes the tests?

Sorry for mucking with your PR 😢

@SurbhiAgarwal1

Copy link
Copy Markdown
Contributor Author

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!

Copilot AI review requested due to automatic review settings April 8, 2026 19:04
@SurbhiAgarwal1 SurbhiAgarwal1 force-pushed the feat/cel-clean-v2 branch 2 times, most recently from 298cc5d to 6d4aed1 Compare April 8, 2026 23:04
@efiacor

efiacor commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Hi @CsatariGergely,

Yes, I'm still working on this. Unfortunately, I've been facing some issues with my computer recently, which has delayed my progress. I'll get the environment set up again, rebase the branch, and update the PR as soon as possible.

Thank you for your patience.

Hi @SurbhiAgarwal1
How can we assist in moving this PR along?

Comment thread run_e2e.sh Outdated
Comment thread internal/kptops/doc.go Outdated
Comment thread api/kptfile/v1/types.go Outdated
Comment thread test-local/Kptfile Outdated
Comment thread go.mod Outdated
Comment thread api/kptfile/v1/types.go Outdated
Comment thread commands/fn/render/cmdrender_test.go Outdated
assert.NoError(t, err)
err = os.Symlink(filepath.Join("path", "to", "pkg", "dir"), "foo")
assert.NoError(t, err)
if err != nil {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have issues with running these tests due to Windows permissions, this needs to be addressed somehow.
These should work in WSL though.

Comment thread commands/pkg/diff/cmddiff_test.go Outdated
Comment thread documentation/content/en/book/04-using-functions/_index.md Outdated
Comment thread pkg/fn/runtime/celeval_test.go Outdated
@@ -0,0 +1,160 @@
// Copyright 2026 The kpt and Nephio Authors

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All new headers should be:

Copyright 2026 The kpt Authors

Comment thread thirdparty/cmdconfig/commands/cmdeval/cmdeval_test.go Outdated
Comment thread thirdparty/kyaml/runfn/runfn.go Outdated
Comment thread pkg/fn/runtime/runner.go Outdated
Comment thread .gitattributes Outdated
Comment thread go.mod Outdated

@efiacor efiacor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice progress made. Please address all comments/suggestions to proceed. Thanks

Comment thread api/kptfile/v1/types.go Outdated
Comment thread documentation/content/en/reference/schema/kptfile/kptfile.yaml Outdated
Comment thread e2e/testdata/fn-render/condition/condition-met/Kptfile Outdated
Comment thread e2e/testdata/fn-render/condition/condition-not-met/Kptfile Outdated
Comment thread pkg/fn/runtime/condition_test.go Outdated
Comment thread pkg/fn/runtime/runner.go Outdated
Comment thread pkg/fn/runtime/runner.go Outdated
Comment thread pkg/fn/runtime/runner.go Outdated
Comment thread pkg/lib/runneroptions/celenv.go Outdated
Comment thread e2e/testdata/fn-render/condition/condition-met/.expected/config.yaml Outdated
Comment thread e2e/testdata/fn-render/condition/condition-not-met/.expected/config.yaml Outdated
+ renderStatus:
+ mutationSteps:
+ - image: ghcr.io/kptdev/krm-functions-catalog/no-op:latest
+ exitCode: 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when should be introduced into the pipeline result as suggested here before.

Comment thread commands/pkg/get/cmdget_test.go Outdated
Comment thread commands/pkg/update/cmdupdate_test.go
Comment thread pkg/lib/runneroptions/celenv.go Outdated
)

const (
celCheckFrequency = 100

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes @efiacor It would be preferable to expose these as configurable options rather than hardcoded constants. Since the when field accepts arbitrary user-written CEL expressions, complex expressions could hit the hardcoded limits today. Having configurable limits would provide the flexibility needed without requiring code changes.

Comment thread pkg/lib/util/get/get_test.go Outdated
})
defer clean()

// Skip if the test repo doesn't have real symlinks (e.g. Windows/WSL with

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WSL natively supports symlinks, so these tests should work fine there. The problem only arises when the repo is cloned on Windows (or with core.symlinks=false) and then accessed from WSL - in that case git stores symlink targets as plain files rather than actual symlinks. The solution for WSL is simply to clone the repo within WSL itself.

For Windows, a runtime.GOOS == "windows" check is the appropriate way to skip these tests. I'd prefer this change to be scoped specifically to Windows rather than using a broader file-inspection approach that could inadvertently skip tests on other platforms.

Comment thread thirdparty/kyaml/runfn/runfn.go
@SurbhiAgarwal1 SurbhiAgarwal1 force-pushed the feat/cel-clean-v2 branch 3 times, most recently from 0debe62 to 5b6eb9b Compare June 23, 2026 21:00
@SurbhiAgarwal1

SurbhiAgarwal1 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Hi @efiacor and @aravindtga ,

I finally tracked down the root cause of the CEL condition test failures! It turns out cmdrender.go and cmdeval.go were silently ignoring errors from InitCELEnvironment(). If the CEL environment failed to initialize, the functions would just silently bypass condition evaluations instead of surfacing the error.

I have fixed those files to explicitly handle the initialization error and fail loudly. I've pushed the fixes to the branch. The test suite requires some Unix commands (like cp) so it flakes on my local Windows setup, but these error propagation fixes should resolve the condition evaluation skips we were seeing in CI. Let me know if everything looks green on your end!

@efiacor

efiacor commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Hi @efiacor and @aravindtga ,

I finally tracked down the root cause of the CEL condition test failures! It turns out cmdrender.go and cmdeval.go were silently ignoring errors from InitCELEnvironment(). If the CEL environment failed to initialize, the functions would just silently bypass condition evaluations instead of surfacing the error.

I have fixed those files to explicitly handle the initialization error and fail loudly. I've pushed the fixes to the branch. The test suite requires some Unix commands (like cp) so it flakes on my local Windows setup, but these error propagation fixes should resolve the condition evaluation skips we were seeing in CI. Let me know if everything looks green on your end!

Hi @SurbhiAgarwal1
We do need to try to "resolve" any code review comments left by reviewers. Perhaps you have missed some due to how the github UI "folds" multiple comments. Please expand, reply and|or mark as resolved.
We cannot re review until all previous comments are addressed AND the CI runs are fully passing.

Unfortunately, I have little experience with Windows development and I would suggest that we do not fully support Windows development.
Some of our contributors are successfully using WSL. Perhaps this could be an option for you.

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.

SurbhiAgarwal1 and others added 11 commits June 29, 2026 16:33
Signed-off-by: SurbhiAgarwal1 <agarwalsurbhi1807@gmail.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>
Signed-off-by: SurbhiAgarwal1 <surbhi.agarwal@example.com>
Signed-off-by: SurbhiAgarwal1 <surbhi.agarwal@example.com>
Signed-off-by: SurbhiAgarwal1 <surbhi.agarwal@example.com>
…eck, configure CEL limits, fix windows skips

Signed-off-by: SurbhiAgarwal1 <surbhi.agarwal@example.com>
Signed-off-by: SurbhiAgarwal1 <surbhi.agarwal@example.com>
Signed-off-by: SurbhiAgarwal1 <SurbhiAgarwal1@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>
@SurbhiAgarwal1

Copy link
Copy Markdown
Contributor Author

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 runtime.GOOS == "windows" check instead. This ensures the tests are only skipped on native Windows, while preserving the test assertions for Linux, macOS, and properly cloned WSL environments.

(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!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request go Pull requests that update Go code lgtm size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants