refactor: simplify jq filter initialization and remove unused libpath #743
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Evsyukov Denis <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the jq filter initialization by removing the unused libpath parameter and replacing the libjq-based implementation with itchyny/gojq. The changes simplify the code by removing legacy code paths and updating calls to the new API.
- Removed the libpath parameter from all jq filter constructions.
- Eliminated legacy implementations (libjq and jq subprocess methods) and introduced a new gojq-based implementation.
- Cleaned up related configuration and flag definitions.
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/shell-operator/bootstrap.go | Updated jq.NewFilter call syntax to remove the unused libpath parameter. |
| pkg/kube_events_manager/resource_informer.go | Removed libpath parameter from multiple jq.NewFilter calls. |
| pkg/kube_events_manager/filter_test.go | Updated test to match the new jq.NewFilter API. |
| pkg/kube/object_patch/operation.go | Removed redundant libpath parameter from filter initialization. |
| pkg/filter/jq/apply_jq.go | Introduced new implementation using itchyny/gojq. |
| pkg/filter/jq/jq_exec.go, apply_libjq_go.go, apply_jq_exec.go | Deleted obsolete implementations. |
| pkg/app/jq.go | Removed file which defined jq flags for the removed libjq implementation. |
| pkg/app/app.go | Removed call to DefineJqFlags. |
| cmd/shell-operator/main.go | Updated jq.NewFilter call in version command to align with new API. |
Files not reviewed (1)
- go.mod: Language not supported
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Evsyukov Denis <[email protected]>
…igurations Signed-off-by: Evsyukov Denis <[email protected]>
Signed-off-by: Evsyukov Denis <[email protected]>
Signed-off-by: Evsyukov Denis <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the jq filter initialization by removing the unused library path argument and replacing the legacy libjq implementation with itchyny/gojq. Key changes include:
- Removing the libpath argument from jq.NewFilter() across multiple files.
- Eliminating obsolete libjq-related code and build configurations.
- Updating tests and workflows to reflect the new jq filter implementation.
Reviewed Changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/shell-operator/bootstrap.go | Updated filter initialization by removing libpath. |
| pkg/kube_events_manager/resource_informer.go | Adjusted filter creation in event handling functions. |
| pkg/kube_events_manager/filter_test.go | Updated test filter initialization. |
| pkg/kube/object_patch/operation.go | Updated filter creation in patch operations. |
| pkg/filter/jq/apply_libjq_go.go | Removed obsolete libjq-based implementation. |
| pkg/filter/jq/apply_jq_exec.go | Removed obsolete jq exec-based implementation. |
| pkg/filter/jq/apply.go | Added a new implementation using itchyny/gojq. |
| pkg/app/jq.go | Removed jq flag definition and associated globals. |
| pkg/app/app.go | Removed flag registration for libjq filter usage. |
| cmd/shell-operator/main.go | Updated filter initialization in the version command. |
| .github/workflows/*.yaml | Removed steps for downloading prebuilt libjq libraries. |
Files not reviewed (2)
- Dockerfile: Language not supported
- go.mod: Language not supported
Signed-off-by: Evsyukov Denis <[email protected]>
Signed-off-by: Evsyukov Denis <[email protected]>
|
We need to cover jq filter with tests |
|
Added a number of tests for the jq package |
…andling Signed-off-by: Evsyukov Denis <[email protected]>
… type consistency Signed-off-by: Evsyukov Denis <[email protected]>
…in filter.go Signed-off-by: Evsyukov Denis <[email protected]>
Signed-off-by: Evsyukov Denis <[email protected]>
ldmonster
reviewed
Mar 31, 2025
ldmonster
approved these changes
Mar 31, 2025
ldmonster
reviewed
Apr 2, 2025
pkg/filter/jq/apply.go
Outdated
| return nil, err | ||
| } | ||
|
|
||
| iter := query.Run(data) |
There was a problem hiding this comment.
under hood this method can change map from input
There was a problem hiding this comment.
In order to ensure that the input data does not change, I created a separate data instance
Signed-off-by: Evsyukov Denis <[email protected]>
… in apply.go Signed-off-by: Evsyukov Denis <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
fixed: #730
What this PR does / why we need it
replace libjq library with itchyny/gojq