diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 000000000..558937839 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,11 @@ +# Description + +Please include a thorough summary of the changes. + +This fixes ... \ This new feature can be used to ... + +# Context + +Summary of issue + +Description and justification of any interesting decisions made diff --git a/changelog/v0.35.0/fix-AppendErrHandler-not-appending-errors-to-original-object.yaml b/changelog/v0.35.0/fix-AppendErrHandler-not-appending-errors-to-original-object.yaml new file mode 100644 index 000000000..1f258c06f --- /dev/null +++ b/changelog/v0.35.0/fix-AppendErrHandler-not-appending-errors-to-original-object.yaml @@ -0,0 +1,7 @@ +changelog: + - type: FIX + issueLink: https://github.com/solo-io/skv2/issues/517 + description: Fix issue where the AppendErrHandler was not appending errors to the original object. + - type: BREAKING_CHANGE + issueLink: https://github.com/solo-io/skv2/issues/517 + description: Usages of AppendErrHandler will now need to pass it by reference (&). diff --git a/contrib/pkg/output/errhandlers/err_handlers.go b/contrib/pkg/output/errhandlers/err_handlers.go index 21f423539..6c1e1aa52 100644 --- a/contrib/pkg/output/errhandlers/err_handlers.go +++ b/contrib/pkg/output/errhandlers/err_handlers.go @@ -9,23 +9,33 @@ import ( // this file contains ErrorHandlers for handling errors created when writing output snapshots +func ResourceWriteError(resource ezkube.Object, err error) error { + return eris.Wrapf(err, "writing resource %v failed", sets.Key(resource)) +} +func ResourceDeleteError(resource ezkube.Object, err error) error { + return eris.Wrapf(err, "deleting resource %v failed", sets.Key(resource)) +} +func ListError(err error) error { + return eris.Wrapf(err, "listing resources failed") +} + type AppendingErrHandler struct { errs error } -func (a AppendingErrHandler) HandleWriteError(resource ezkube.Object, err error) { - a.errs = multierror.Append(a.errs, eris.Wrapf(err, "writing resource %v failed", sets.Key(resource))) +func (a *AppendingErrHandler) HandleWriteError(resource ezkube.Object, err error) { + a.errs = multierror.Append(a.errs, ResourceWriteError(resource, err)) } -func (a AppendingErrHandler) HandleDeleteError(resource ezkube.Object, err error) { - a.errs = multierror.Append(a.errs, eris.Wrapf(err, "deleting resource %v failed", sets.Key(resource))) +func (a *AppendingErrHandler) HandleDeleteError(resource ezkube.Object, err error) { + a.errs = multierror.Append(a.errs, ResourceDeleteError(resource, err)) } -func (a AppendingErrHandler) HandleListError(err error) { - a.errs = multierror.Append(a.errs, eris.Wrapf(err, "listing failed")) +func (a *AppendingErrHandler) HandleListError(err error) { + a.errs = multierror.Append(a.errs, ListError(err)) } -// returns the errors collected by the handler -func (a AppendingErrHandler) Errors() error { +// Errors returns the errors collected by the handler +func (a *AppendingErrHandler) Errors() error { return a.errs } diff --git a/contrib/pkg/output/errhandlers/err_handlers_suite_test.go b/contrib/pkg/output/errhandlers/err_handlers_suite_test.go new file mode 100644 index 000000000..07dbc9ca9 --- /dev/null +++ b/contrib/pkg/output/errhandlers/err_handlers_suite_test.go @@ -0,0 +1,13 @@ +package errhandlers_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestCodegen(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Error Handlers Suite") +} diff --git a/contrib/pkg/output/errhandlers/err_handlers_test.go b/contrib/pkg/output/errhandlers/err_handlers_test.go new file mode 100644 index 000000000..0c3f4a5b4 --- /dev/null +++ b/contrib/pkg/output/errhandlers/err_handlers_test.go @@ -0,0 +1,94 @@ +package errhandlers + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/pkg/errors" + v1 "github.com/solo-io/skv2/codegen/test/api/things.test.io/v1" + "github.com/solo-io/skv2/contrib/pkg/output" + "github.com/solo-io/skv2/pkg/ezkube" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var _ = Describe("Error Handlers", func() { + var ( + errorHandler output.ErrorHandler + fakeResource = &v1.Paint{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-name", + Namespace: "fake-ns", + }, + } + ) + + Context("AppendingErrHandler", func() { + BeforeEach(func() { + errorHandler = &AppendingErrHandler{} + }) + + DescribeTable("should append errors to the error handler object", func(mutation errorMutation, expects errorExpectation) { + mutation(errorHandler, fakeResource) + expects(errorHandler.(*AppendingErrHandler).Errors(), fakeResource) + }, + Entry("when HandleListError is called", addListError, expectListError), + Entry("when HandleWriteError is called", addWriteError, expectWriteError), + Entry("when HandleDeleteError is called", addDeleteError, expectDeleteError), + Entry("when multiple error handler functions are invoked", func(handler output.ErrorHandler, resource ezkube.Object) { + addListError(handler, resource) + addWriteError(handler, resource) + addDeleteError(handler, resource) + }, func(err error, resource ezkube.Object) { + Expect(err).To(And( + MatchError(ListError(err)), + MatchError(ResourceWriteError(resource, err)), + MatchError(ResourceDeleteError(resource, err)), + )) + }), + ) + + It("should not have an error when no errors occur", func() { + errs := errorHandler.(*AppendingErrHandler).Errors() + Expect(errs).NotTo(HaveOccurred()) + }) + }) +}) + +type errorMutation func(handler output.ErrorHandler, resource ezkube.Object) + +func addListError(handler output.ErrorHandler, _ ezkube.Object) { + handler.HandleListError(errors.New("listing resources failed")) +} +func addWriteError(handler output.ErrorHandler, resource ezkube.Object) { + handler.HandleWriteError(resource, errors.New("writing resource failed")) +} +func addDeleteError(handler output.ErrorHandler, resource ezkube.Object) { + handler.HandleDeleteError(resource, errors.New("deleting resource failed")) +} + +// errorExpectation is a function that expects a specific error to occur +type errorExpectation func(err error, resource ezkube.Object) + +func expectListError(err error, resource ezkube.Object) { + ExpectWithOffset(1, err).To(HaveOccurred()) + ExpectWithOffset(1, err).To(And( + MatchError(ListError(err)), + Not(MatchError(ResourceWriteError(resource, err))), + Not(MatchError(ResourceDeleteError(resource, err))), + )) +} +func expectWriteError(err error, resource ezkube.Object) { + ExpectWithOffset(1, err).To(HaveOccurred()) + ExpectWithOffset(1, err).To(And( + MatchError(ResourceWriteError(resource, err)), + Not(MatchError(ListError(err))), + Not(MatchError(ResourceDeleteError(resource, err))), + )) +} +func expectDeleteError(err error, resource ezkube.Object) { + ExpectWithOffset(1, err).To(HaveOccurred()) + ExpectWithOffset(1, err).To(And( + MatchError(ResourceDeleteError(resource, err)), + Not(MatchError(ListError(err))), + Not(MatchError(ResourceWriteError(resource, err))), + )) +}