From 88c89f422a94fd573f3f393f9abda25f217ed943 Mon Sep 17 00:00:00 2001 From: Colin O'Dell Date: Fri, 24 May 2024 23:13:10 -0400 Subject: [PATCH] fix: always show accumulation errors (#5693) * fix: always show accumulation errors if the resource was successfully loaded as a base * chore: regression test * chore: fix lint violations --- api/internal/target/kusttarget.go | 3 -- api/internal/target/kusttarget_test.go | 70 ++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/api/internal/target/kusttarget.go b/api/internal/target/kusttarget.go index 44c82bc59..cdb419559 100644 --- a/api/internal/target/kusttarget.go +++ b/api/internal/target/kusttarget.go @@ -449,9 +449,6 @@ func (kt *KustTarget) accumulateResources( ra, err = kt.accumulateDirectory(ra, ldr, false) } if err != nil { - if kusterr.IsMalformedYAMLError(errF) { // Some error occurred while tyring to decode YAML file - return nil, errF - } return nil, errors.WrapPrefixf( err, "accumulation err='%s'", errF.Error()) } diff --git a/api/internal/target/kusttarget_test.go b/api/internal/target/kusttarget_test.go index 515a1efa1..7993df5ac 100644 --- a/api/internal/target/kusttarget_test.go +++ b/api/internal/target/kusttarget_test.go @@ -6,6 +6,8 @@ package target_test import ( "encoding/base64" "fmt" + "net/http" + "net/http/httptest" "reflect" "testing" "time" @@ -560,3 +562,71 @@ func (l loaderNewThrowsError) Load(location string) ([]byte, error) { func (l loaderNewThrowsError) Cleanup() error { return l.baseLoader.Cleanup() //nolint:wrapcheck // baseLoader's error is sufficient } + +func TestErrorMessageForMalformedYAMLAndInvalidBase(t *testing.T) { + // These testcases verify behavior for the scenario described in + // https://github.com/kubernetes-sigs/kustomize/issues/5692 . + + // Use a test server to fake the remote file response + handler := http.NewServeMux() + handler.HandleFunc("/", func(out http.ResponseWriter, req *http.Request) { + // Per issue #5692, the server should return a 200 status code with a response body that fails to parse as YAML + out.WriteHeader(http.StatusOK) + _, _ = out.Write([]byte(` + +`)) + }) + svr := httptest.NewServer(handler) + defer svr.Close() + + th := kusttest_test.MakeHarness(t) + th.WriteF("/should-fail/kustomization.yml", "resources:\n- "+svr.URL) + th.WriteF("/should-fail/remote-repo/kustomization.yml", "this: is not a kustomization file!") + + ldrWrapper := func(baseLoader ifc.Loader) ifc.Loader { + return &loaderWithRenamedRoots{ + baseLoader: baseLoader, + fakeRootMap: map[string]string{ + // Use the "remote-repo" subdir instead of the remote git repo + svr.URL: "remote-repo", + }, + } + } + + _, err := makeAndLoadKustTargetWithLoaderOverride(t, th.GetFSys(), "/should-fail", ldrWrapper).AccumulateTarget() + require.Error(t, err) + errString := err.Error() + assert.Contains(t, errString, "accumulating resources from '"+svr.URL+"'") + assert.Contains(t, errString, "MalformedYAMLError: yaml: line 3: mapping values are not allowed in this context") + assert.Contains(t, errString, `invalid Kustomization: json: unknown field "this"`) +} + +// loaderWithRenamedRoots is a loader that can map New() roots to some other name +type loaderWithRenamedRoots struct { + baseLoader ifc.Loader + fakeRootMap map[string]string +} + +func (l loaderWithRenamedRoots) Repo() string { + return l.baseLoader.Repo() +} + +func (l loaderWithRenamedRoots) Root() string { + return l.baseLoader.Root() +} + +func (l loaderWithRenamedRoots) New(newRoot string) (ifc.Loader, error) { + if otherRoot, ok := l.fakeRootMap[newRoot]; ok { + return l.baseLoader.New(otherRoot) //nolint:wrapcheck // baseLoader's error is sufficient + } + + return l.baseLoader.New(newRoot) //nolint:wrapcheck // baseLoader's error is sufficient +} + +func (l loaderWithRenamedRoots) Load(path string) ([]byte, error) { + return l.baseLoader.Load(path) //nolint:wrapcheck // baseLoader's error is sufficient +} + +func (l loaderWithRenamedRoots) Cleanup() error { + return l.baseLoader.Cleanup() //nolint:wrapcheck // baseLoader's error is sufficient +}