fix: always show accumulation errors

[colinodell]

For accumulation errors when...

1. The file load fails due to malformed YAML
2. The base load succeeds without issues
3. The directory fails to accumulate

... we will now report the accumulation errors up the stack. Previously, only the malformed YAML error was returned, masking the actual accumulation errors that were the real problem.

Sending just the YAML error makes sense in situations where the base load completely fails and we're pretty confident the base isn't actually a base. But, if we were able to successfully load that base, then the YAML error alone is pretty irrelevant because we know the resource was not a file after all.

Fixes #5692

[k8s-ci-robot]

Hi @colinodell. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[stormqueen1990]

Hi there, @colinodell! 👋🏻

Thank you for your contribution!

Could you please cover this scenario with a test?
I'm mostly looking for some coverage that could prevent regressions in the future.

[k8s-ci-robot]

This PR has multiple commits, and the default merge method is: merge.
You can request commits to be squashed using the label: tide/merge-method-squash

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[colinodell]

Hi @stormqueen1990,

Writing this test was very difficult, especially since kusttarget_test.go (which was supposed to test the YAML error handling) didn't already cover the bottom half of accumulateResources() 😕 But after a few hours I was able to implement something I'm happy with, partially inspired by how accumulation_test.go uses a test server to mock remote responses. Let me know what you think or if further changes are needed here.

Cheers!

[stormqueen1990]

Hi there, @colinodell! 👋🏻

Thank you very much for your contribution. Appreciate the effort to put together a test for this too! 🙏🏻

/lgtm
/ok-to-test

[colinodell]

/retest

[colinodell]

/retest

[stormqueen1990]

/lgtm
/label tide/merge-method-squash

[koba1t]

Thanks @colinodell!
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: colinodell, koba1t

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [koba1t]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment