Skip to content

Commit

Permalink
enforcing api error if pipeline render fails instead of ignoring erro…
Browse files Browse the repository at this point in the history
…r and allowing it to pass
  • Loading branch information
Catalin-Stratulat-Ericsson committed Nov 26, 2024
1 parent 52d2a94 commit 33b52b3
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 6 deletions.
4 changes: 3 additions & 1 deletion pkg/engine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,9 @@ func (cad *cadEngine) UpdatePackageResources(ctx context.Context, repositoryObj
}

renderStatus, err := cad.taskHandler.DoPRResourceMutations(ctx, pr2Update, draft, oldRes, newRes)

if err != nil {
return nil, renderStatus, err
}
// No lifecycle change when updating package resources; updates are done.
repoPkgRev, err := draft.Close(ctx, "")
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions pkg/registry/porch/packagerevisionresources.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ func (r *packageRevisionResources) Update(ctx context.Context, name string, objI

rev, renderStatus, err := r.cad.UpdatePackageResources(ctx, &repositoryObj, oldRepoPkgRev, oldApiPkgRevResources, newObj)
if err != nil {
err := fmt.Errorf("%w%s%s%s", err, "\n\nKpt function pipeline error occurred during render.", "\nPackage has NOT been pushed to remote.", "\nAmend local package using error message above & retry.")
return nil, false, apierrors.NewInternalError(err)
}

Expand Down
17 changes: 12 additions & 5 deletions pkg/task/generictaskhandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,19 +225,26 @@ func (th *genericTaskHandler) DoPRResourceMutations(ctx context.Context, pr2Upda

var renderStatus *api.RenderStatus
if len(appliedResources.Contents) > 0 {
// a porch package which fails render validation will no longer be accepted.
// render the package
// Render failure will not fail the overall API operation.
// Render failure WILL fail the overall API operation.
// The render error and result is captured as part of renderStatus above
// and is returned in packageresourceresources API's status field. We continue with
// saving the non-rendered resources to avoid losing user's changes.
// and supress this err.
_, renderStatus, _ = applyResourceMutations(ctx,
// and is returned in PackageRevisionResources API's status field.
// We do not push the package further to remote
// the users changes are captured on their local
// and can be amended using the error returned to properly validate/mutate the package before retrying
// we no longer suppress this err.
_, renderStatus, err = applyResourceMutations(ctx,
draft,
appliedResources,
[]mutation{&renderPackageMutation{
runnerOptions: runnerOptions,
runtime: th.runtime,
}})
if err != nil {
klog.Errorf("Kpt function pipeline error occurred in render. Package has NOT been pushed to remote. Modify and Fix local package and retry.")
return renderStatus, err
}
} else {
renderStatus = nil
}
Expand Down

0 comments on commit 33b52b3

Please sign in to comment.