Skip to content

Trim or redact apply-setters results#1266

Open
mozesl-nokia wants to merge 12 commits into
kptdev:mainfrom
nokia:trim-apply-setters-results
Open

Trim or redact apply-setters results#1266
mozesl-nokia wants to merge 12 commits into
kptdev:mainfrom
nokia:trim-apply-setters-results

Conversation

@mozesl-nokia

@mozesl-nokia mozesl-nokia commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

The apply-setters function currently writes whatever it has set to the log. This can expose sensitive data and generally bloat logs for large values.

In this PR, we redact well-known patterns based on both the value and the field path, as well as truncate long values.

Also did quite a bit of restructuring and refactoring to use the new kpt api module.

The large line diff is because of the restructuring of the files. The new logic starts from commit fdc5e86 .

AI was used in the creation of this PR:

  • Composer 2.5 was used to generate the redaction logic

Copilot AI review requested due to automatic review settings June 25, 2026 11:19
@dosubot dosubot Bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Jun 25, 2026
@netlify

netlify Bot commented Jun 25, 2026

Copy link
Copy Markdown

Deploy Preview for krm-function-catalog canceled.

Name Link
🔨 Latest commit 0dea530
🔍 Latest deploy log https://app.netlify.com/projects/krm-function-catalog/deploys/6a4f527a86bc9700082e5a31

@mozesl-nokia mozesl-nokia added enhancement New feature or request go Pull requests that update Go code and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Jun 25, 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.

Pull request overview

This PR updates the apply-setters Go function to reduce the risk of leaking sensitive data and to keep function results/log output smaller by redacting likely-secret values (based on field path, setter names, and value patterns) and truncating long values. It also refactors result construction to use the new github.com/kptdev/kpt/api/fnresult/v1 ResultItem model and restructures visitor/walk logic into clearer files.

Changes:

  • Redact likely-secret values and truncate long values before emitting ResultItem messages/fields.
  • Refactor apply-setters results to produce richer framework.Result output (message/severity/file/resourceRef/field current+proposed values).
  • Restructure visitor/walker code and update dependencies to include github.com/kptdev/kpt/api.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
functions/go/apply-setters/main.go Maps internal ResultItems to framework.Result output fields (message/severity/file/resourceRef/field values).
functions/go/apply-setters/go.mod Adds dependency on github.com/kptdev/kpt/api and updates indirect requirements.
functions/go/apply-setters/go.sum Updates module checksums consistent with dependency changes.
functions/go/apply-setters/applysetters/apply_setters.go Switches results to fnresult.ResultItem and captures ResourceIdentifier metadata for results.
functions/go/apply-setters/applysetters/walk.go Keeps traversal logic; now relies on visitor definitions moved into visit.go.
functions/go/apply-setters/applysetters/visit.go Adds visitor implementation plus buildResultItem to sanitize/redact/truncate output.
functions/go/apply-setters/applysetters/trim.go Implements redaction heuristics and truncation for safe result logging.
functions/go/apply-setters/applysetters/trim_test.go Adds focused unit tests for redaction/truncation behavior.
functions/go/apply-setters/applysetters/apply_setters_test.go Extends tests to assert redaction behavior in emitted results.

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

Comment thread functions/go/apply-setters/applysetters/trim.go
Comment thread functions/go/apply-setters/applysetters/visit.go
Comment thread functions/go/apply-setters/applysetters/visit.go
@dosubot dosubot Bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Jun 25, 2026
@mozesl-nokia

Copy link
Copy Markdown
Contributor Author

We could consider using some third party lib for matching patterns, it looks like the coverage with these patterns is a bit lackluster...

Comment thread .github/workflows/ci.yaml Outdated
Comment thread functions/go/apply-setters/applysetters/visit.go
@mozesl-nokia mozesl-nokia force-pushed the trim-apply-setters-results branch from f49960e to 1f5fc49 Compare July 1, 2026 08:44
@dosubot dosubot Bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Jul 1, 2026
@mozesl-nokia mozesl-nokia requested a review from Copilot July 1, 2026 11:39

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 14 out of 16 changed files in this pull request and generated 3 comments.

Comment thread functions/go/apply-setters/applysetters/trim.go Outdated
Comment thread functions/go/apply-setters/applysetters/apply_setters_test.go Outdated
Comment thread functions/go/apply-setters/applysetters/apply_setters_test.go Outdated
@mozesl-nokia mozesl-nokia force-pushed the trim-apply-setters-results branch from 91b515c to 434cd92 Compare July 2, 2026 08:31
aravindtga
aravindtga previously approved these changes Jul 7, 2026
Comment thread functions/go/apply-setters/go.mod Outdated
@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Jul 7, 2026
liamfallon
liamfallon previously approved these changes Jul 9, 2026
Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com>
Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com>
Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com>
Assisted-by: Cursor:composer-2.5
Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com>
Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com>
Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com>
Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com>
Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com>
Assisted-by: Cursor:composer-2.5
Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com>
Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com>
Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com>
Signed-off-by: Mózes László Máté <laszlo.mozes@nokia.com>
@mozesl-nokia mozesl-nokia dismissed stale reviews from liamfallon and aravindtga via 0dea530 July 9, 2026 07:49
@mozesl-nokia mozesl-nokia force-pushed the trim-apply-setters-results branch from 87b0e8a to 0dea530 Compare July 9, 2026 07:49
@mozesl-nokia mozesl-nokia requested review from a team, aravindtga and liamfallon July 9, 2026 07:49
@mozesl-nokia

Copy link
Copy Markdown
Contributor Author

Will wait for #1275 before rebase

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

Labels

enhancement New feature or request go Pull requests that update Go code lgtm This PR has been approved by a maintainer size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants