From 49b464fd4d6d130d2dacee67b97c21caaf980c0d Mon Sep 17 00:00:00 2001 From: Sylvain Rabot Date: Wed, 8 Dec 2021 14:27:11 +0100 Subject: [PATCH 1/4] Handle HTTP error codes in file loader GitHub release files like https://github.com/fluxcd/helm-controller/releases/download/v0.14.0/helm-controller.crds.yaml seems to be hosted on Azure and it seems that there are egress limits that can be reached, e.g.: ```xml ServerBusyEgress is over the account limit. RequestId:f4a46b38-001e-0046-2437-ec16e2000000 Time:2021-12-08T13:28:03.8542138Z ``` This patch allows to have a clear Error message instead of just `missing Resource metadata`: ``` Error: accumulating resources: accumulation err='accumulating resources from 'https://github.com/fluxcd/source-controller/releases/download/v0.19.0/source-controller.crds.yaml': URL returned error 503 (Service Unavailable)': evalsymlink failure on '/private/var/folders/hq/ttl6jyh539q55fz6282w0jyc0000gn/T/kustomize-3508224975/releases/download/v0.19.0/source-controller.crds.yaml' : lstat /private/var/folders/hq/ttl6jyh539q55fz6282w0jyc0000gn/T/kustomize-3508224975/releases: no such file or directory ``` Signed-off-by: Sylvain Rabot --- api/loader/fileloader.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/api/loader/fileloader.go b/api/loader/fileloader.go index 92bfc3f69..c2e7bb9e2 100644 --- a/api/loader/fileloader.go +++ b/api/loader/fileloader.go @@ -313,6 +313,9 @@ func (fl *fileLoader) Load(path string) ([]byte, error) { return nil, err } defer resp.Body.Close() + if resp.StatusCode < 200 || resp.StatusCode > 299 { + return nil, fmt.Errorf("URL returned error %d (%s)", resp.StatusCode, http.StatusText(resp.StatusCode)) + } body, err := ioutil.ReadAll(resp.Body) if err != nil { return nil, err From e65e571ed1342b3baffe067230e0c81858208408 Mon Sep 17 00:00:00 2001 From: Sylvain Rabot Date: Wed, 8 Dec 2021 18:24:29 +0100 Subject: [PATCH 2/4] Do not try to load HTTP resources from FS when error occurs It is useless and it clogs the error message. Signed-off-by: Sylvain Rabot --- api/internal/target/kusttarget.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/api/internal/target/kusttarget.go b/api/internal/target/kusttarget.go index 85f25165f..ba3f700fb 100644 --- a/api/internal/target/kusttarget.go +++ b/api/internal/target/kusttarget.go @@ -366,6 +366,9 @@ func (kt *KustTarget) accumulateResources( for _, path := range paths { // try loading resource as file then as base (directory or git repository) if errF := kt.accumulateFile(ra, path, origin); errF != nil { + if strings.HasPrefix(path, "https://") || strings.HasPrefix(path, "http://") { + return nil, errF + } ldr, err := kt.ldr.New(path) if err != nil { return nil, errors.Wrapf( From 738573b0794630bd143914c1b480f1f03c6c6463 Mon Sep 17 00:00:00 2001 From: Sylvain Rabot Date: Fri, 10 Dec 2021 09:27:37 +0100 Subject: [PATCH 3/4] Error on HTTP resources are not nescessarly protocol related Signed-off-by: Sylvain Rabot --- api/internal/target/kusttarget.go | 4 +++- api/loader/errors.go | 5 +++++ api/loader/fileloader.go | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 api/loader/errors.go diff --git a/api/internal/target/kusttarget.go b/api/internal/target/kusttarget.go index ba3f700fb..140c97a9f 100644 --- a/api/internal/target/kusttarget.go +++ b/api/internal/target/kusttarget.go @@ -19,6 +19,7 @@ import ( "sigs.k8s.io/kustomize/api/internal/plugins/loader" "sigs.k8s.io/kustomize/api/internal/utils" "sigs.k8s.io/kustomize/api/konfig" + load "sigs.k8s.io/kustomize/api/loader" "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" "sigs.k8s.io/kustomize/api/types" @@ -366,7 +367,8 @@ func (kt *KustTarget) accumulateResources( for _, path := range paths { // try loading resource as file then as base (directory or git repository) if errF := kt.accumulateFile(ra, path, origin); errF != nil { - if strings.HasPrefix(path, "https://") || strings.HasPrefix(path, "http://") { + // not much we can do if the error is an HTTP error so we bail out + if errors.Is(errF, load.ErrorHTTP) { return nil, errF } ldr, err := kt.ldr.New(path) diff --git a/api/loader/errors.go b/api/loader/errors.go new file mode 100644 index 000000000..5281a1cd7 --- /dev/null +++ b/api/loader/errors.go @@ -0,0 +1,5 @@ +package loader + +import "fmt" + +var ErrorHTTP = fmt.Errorf("HTTP Error") diff --git a/api/loader/fileloader.go b/api/loader/fileloader.go index c2e7bb9e2..4426097cf 100644 --- a/api/loader/fileloader.go +++ b/api/loader/fileloader.go @@ -314,7 +314,7 @@ func (fl *fileLoader) Load(path string) ([]byte, error) { } defer resp.Body.Close() if resp.StatusCode < 200 || resp.StatusCode > 299 { - return nil, fmt.Errorf("URL returned error %d (%s)", resp.StatusCode, http.StatusText(resp.StatusCode)) + return nil, fmt.Errorf("%w: status code %d (%s)", ErrorHTTP, resp.StatusCode, http.StatusText(resp.StatusCode)) } body, err := ioutil.ReadAll(resp.Body) if err != nil { From 16495c6ed713f8e4c8e2c6a265b535b2955806ec Mon Sep 17 00:00:00 2001 From: Sylvain Rabot Date: Fri, 10 Dec 2021 09:28:54 +0100 Subject: [PATCH 4/4] Test HTTP Error thrown by the file loader Signed-off-by: Sylvain Rabot --- api/krusty/remoteload_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/api/krusty/remoteload_test.go b/api/krusty/remoteload_test.go index 04f87dd0b..081c0c301 100644 --- a/api/krusty/remoteload_test.go +++ b/api/krusty/remoteload_test.go @@ -4,12 +4,15 @@ package krusty_test import ( + "fmt" + "net/http" "path/filepath" "testing" "github.com/stretchr/testify/assert" "sigs.k8s.io/kustomize/api/internal/utils" "sigs.k8s.io/kustomize/api/krusty" + "sigs.k8s.io/kustomize/api/loader" "sigs.k8s.io/kustomize/kyaml/filesys" ) @@ -74,6 +77,31 @@ spec: assert.NoError(t, fSys.RemoveAll(tmpDir.String())) } +func TestRemoteResourceWithHTTPError(t *testing.T) { + fSys := filesys.MakeFsOnDisk() + b := krusty.MakeKustomizer(krusty.MakeDefaultOptions()) + tmpDir, err := filesys.NewTmpConfirmedDir() + assert.NoError(t, err) + + url404 := "https://github.com/thisisa404.yaml" + kusto := filepath.Join(tmpDir.String(), "kustomization.yaml") + assert.NoError(t, fSys.WriteFile(kusto, []byte(fmt.Sprintf(` +resources: +- %s +`, url404)))) + + _, err = b.Run(fSys, tmpDir.String()) + if utils.IsErrTimeout(err) { + // Don't fail on timeouts. + t.SkipNow() + } + + httpErr := fmt.Errorf("%w: status code %d (%s)", loader.ErrorHTTP, 404, http.StatusText(404)) + accuFromErr := fmt.Errorf("accumulating resources from '%s': %w", url404, httpErr) + expectedErr := fmt.Errorf("accumulating resources: %w", accuFromErr) + assert.EqualErrorf(t, err, expectedErr.Error(), url404) +} + func TestRemoteResourceAnnoOrigin(t *testing.T) { fSys := filesys.MakeFsOnDisk() b := krusty.MakeKustomizer(krusty.MakeDefaultOptions())