-
Notifications
You must be signed in to change notification settings - Fork 226
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
kpt fn render
ensures validators don't mutate resources
#1896
Conversation
Ignore this PR for now, @Shell32-Natsu identified an issue with this, so I am going to fix that. |
87e281c
to
cbe0210
Compare
internal/cmdrender/executor.go
Outdated
|
||
// runMutators runs a set of mutators functions on given input resources. | ||
func (pn *pkgNode) runMutators(ctx context.Context, hctx *hydrationContext, input []*yaml.RNode) ([]*yaml.RNode, error) { | ||
const op errors.Op = "pipeline.runMutators" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this op
, since we don't want to expose internal implementation details to users, I suspect does it really have a use case? Is it only used in debug? Is there a debug mode for kpt? And why don't we directly add a real stack trace here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is no longer useful, we took out the stacktrace printing. Taking it out.
return nil, errors.E(op, pn.pkg.UniquePath, err) | ||
} | ||
// print a new line after a pipeline running | ||
pr.Printf("\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the original code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha, I remember that I added this. :0
* kpt fn render ensures validators don't mutate resources
With this PR,
kpt fn render
ensures that validators in a pipeline don't mutate the resources.This is achieved by splitting the execution of
mutators
fromvalidators
. Now we run validators on a copy of mutated resources.Note: It will be nice if
kpt fn render
can flag a validator function that mutates the resources. I think that is doable, but need be investigated how best to do that.fixes #1860