feat: adds http patch support to cel expressions#54
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #54 +/- ##
==========================================
+ Coverage 70.35% 70.59% +0.24%
==========================================
Files 80 80
Lines 2540 2636 +96
==========================================
+ Hits 1787 1861 +74
- Misses 602 612 +10
- Partials 151 163 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Gagan H R <hrgagan4@gmail.com>
Signed-off-by: Gagan H R <hrgagan4@gmail.com>
083edda to
2329184
Compare
There was a problem hiding this comment.
Pull request overview
Adds HTTP PUT and PATCH support to the Kyverno SDK CEL HTTP library so CEL expressions can perform full update operations (beyond existing GET/POST) with optional headers and request bodies.
Changes:
- Extend the CEL environment with
Put/putandPatch/patchmember functions (overloads with/without headers). - Implement PUT/PATCH request handling in the CEL binding layer and the underlying HTTP context implementation.
- Add unit tests covering successful PUT/PATCH calls and argument validation errors.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
extensions/cel/libs/http/types.go |
Extends the exported HTTP context interface to include Put and Patch. |
extensions/cel/libs/http/lib.go |
Registers CEL function overloads for Put/put and Patch/patch. |
extensions/cel/libs/http/impl.go |
Adds CEL function bindings that validate/convert args and invoke ContextInterface.Put/Patch. |
extensions/cel/libs/http/http.go |
Implements Put and Patch request creation/dispatch in contextImpl. |
extensions/cel/libs/http/impl_test.go |
Adds tests validating PUT/PATCH behavior and error handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type ContextInterface interface { | ||
| Get(url string, headers map[string]string) (any, error) | ||
| Post(url string, data any, headers map[string]string) (any, error) | ||
| Put(url string, data any, headers map[string]string) (any, error) | ||
| Patch(url string, data any, headers map[string]string) (any, error) | ||
| Client(caBundle string) (ContextInterface, error) |
There was a problem hiding this comment.
Adding new methods to the exported ContextInterface is a breaking Go API change for any downstream users that may implement this interface. If backwards-compatibility is required, consider introducing a new interface (e.g. ContextInterfaceV2), embedding the old interface, or exposing PUT/PATCH via concrete types while keeping the original interface unchanged.
| body, err := buildRequestData(data) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to encode request data: %w", err) | ||
| } | ||
| req, err := http.NewRequestWithContext(context.TODO(), "PUT", url, body) |
There was a problem hiding this comment.
buildRequestData returns an error message that hard-codes "HTTP POST data", but this code path is now used by PUT (and PATCH below). Consider making the error message method-agnostic (or passing the HTTP method into the helper) so failures during PUT/PATCH encoding don't misleadingly mention POST.
| assert.NoError(t, err) | ||
| assert.NotNil(t, env) | ||
| ast, issues := env.Compile(`http.Put("http://localhost:8080", { "key": dyn("value"), "foo": dyn(2) })`) | ||
| fmt.Println(issues.String()) |
There was a problem hiding this comment.
This fmt.Println(issues.String()) writes to stdout during go test runs and makes CI output noisy. Consider removing it, or switching to t.Log/Logf only when issues != nil and issues.Err() != nil (or equivalent) to log failures only.
| fmt.Println(issues.String()) | |
| if issues != nil { | |
| t.Log(issues.String()) | |
| } |
| ast, issues := env.Compile(`http.Put("http://localhost:8080", {"key": "value"}, {"Authorization": "Bearer token"})`) | ||
| fmt.Println(issues.String()) | ||
| assert.Nil(t, issues) |
There was a problem hiding this comment.
This fmt.Println(issues.String()) writes to stdout during go test runs and makes CI output noisy. Consider removing it, or switching to t.Log/Logf only when issues != nil and issues.Err() != nil (or equivalent) to log failures only.
| ast, issues := env.Compile(`http.Patch("http://localhost:8080", {"key": "value"})`) | ||
| fmt.Println(issues.String()) | ||
| assert.Nil(t, issues) | ||
| assert.NotNil(t, ast) |
There was a problem hiding this comment.
This fmt.Println(issues.String()) writes to stdout during go test runs and makes CI output noisy. Consider removing it, or switching to t.Log/Logf only when issues != nil and issues.Err() != nil (or equivalent) to log failures only.
| ast, issues := env.Compile(`http.Patch("http://localhost:8080", {"key": "value"}, {"Authorization": "Bearer token", "Content-Type": "application/json"})`) | ||
| fmt.Println(issues.String()) | ||
| assert.Nil(t, issues) | ||
| assert.NotNil(t, ast) |
There was a problem hiding this comment.
This fmt.Println(issues.String()) writes to stdout during go test runs and makes CI output noisy. Consider removing it, or switching to t.Log/Logf only when issues != nil and issues.Err() != nil (or equivalent) to log failures only.
Explanation
This PR adds HTTP PATCH method support to the Kyverno SDK CEL HTTP library. Currently, the library only supports GET and POST operations. The PATCH method is a fundamental part of RESTful API interactions. This enhancement allows users to make PATCH requests with custom headers and request bodies from within CEL expressions, enabling full resource update capabilities in policy expressions.
Related issue
Fixes kyverno/kyverno#15471
Dependent on the merge of PR #53 since this PR is built on top of that. Will rebase this branch with main once #53 is merged
Proposed Changes
The PATCH implementation mirrors the existing PATCH method pattern to maintain consistency across the codebase. These changes will add support to the patch method (say, for example http.Patch() being used in the CEL expressions)
Checklist
Further Comments