From b32a0ac6cc7c8aff14b48182294fbebf9cc2ae75 Mon Sep 17 00:00:00 2001
From: Anna Song <annasong@google.com>
Date: Tue, 8 Nov 2022 13:51:01 -0800
Subject: [PATCH] Address TODO in PR #4652 (#4856)

* Address TODO in PR #4652

* Improve readability
---
 api/loader/fileloader.go      | 55 ++++++++++++++++++-----------------
 api/loader/fileloader_test.go |  4 +--
 2 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/api/loader/fileloader.go b/api/loader/fileloader.go
index 665feb0bd86..4bdb296522f 100644
--- a/api/loader/fileloader.go
+++ b/api/loader/fileloader.go
@@ -18,9 +18,9 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/filesys"
 )
 
-// HasRemoteFileScheme returns whether path has a url scheme that kustomize allows for
+// IsRemoteFile returns whether path has a url scheme that kustomize allows for
 // remote files. See https://github.com/kubernetes-sigs/kustomize/blob/master/examples/remoteBuild.md
-func HasRemoteFileScheme(path string) bool {
+func IsRemoteFile(path string) bool {
 	u, err := url.Parse(path)
 	return err == nil && (u.Scheme == "http" || u.Scheme == "https")
 }
@@ -299,31 +299,8 @@ func (fl *fileLoader) errIfRepoCycle(newRepoSpec *git.RepoSpec) error {
 // else an error. Relative paths are taken relative
 // to the root.
 func (fl *fileLoader) Load(path string) ([]byte, error) {
-	// TODO(annasong): replace check with HasRemoteFileScheme
-	if u, err := url.Parse(path); err == nil && (u.Scheme == "http" || u.Scheme == "https") {
-		var hc *http.Client
-		if fl.http != nil {
-			hc = fl.http
-		} else {
-			hc = &http.Client{}
-		}
-		resp, err := hc.Get(path)
-		if err != nil {
-			return nil, err
-		}
-		defer resp.Body.Close()
-		if resp.StatusCode < 200 || resp.StatusCode > 299 {
-			_, err := git.NewRepoSpecFromURL(path)
-			if err == nil {
-				return nil, errors.Errorf("URL is a git repository")
-			}
-			return nil, fmt.Errorf("%w: status code %d (%s)", ErrHTTP, resp.StatusCode, http.StatusText(resp.StatusCode))
-		}
-		body, err := io.ReadAll(resp.Body)
-		if err != nil {
-			return nil, err
-		}
-		return body, nil
+	if IsRemoteFile(path) {
+		return fl.httpClientGetContent(path)
 	}
 	if !filepath.IsAbs(path) {
 		path = fl.root.Join(path)
@@ -335,6 +312,30 @@ func (fl *fileLoader) Load(path string) ([]byte, error) {
 	return fl.fSys.ReadFile(path)
 }
 
+func (fl *fileLoader) httpClientGetContent(path string) ([]byte, error) {
+	var hc *http.Client
+	if fl.http != nil {
+		hc = fl.http
+	} else {
+		hc = &http.Client{}
+	}
+	resp, err := hc.Get(path)
+	if err != nil {
+		return nil, errors.Wrap(err)
+	}
+	defer resp.Body.Close()
+	// response unsuccessful
+	if resp.StatusCode < 200 || resp.StatusCode > 299 {
+		_, err = git.NewRepoSpecFromURL(path)
+		if err == nil {
+			return nil, errors.Errorf("URL is a git repository")
+		}
+		return nil, fmt.Errorf("%w: status code %d (%s)", ErrHTTP, resp.StatusCode, http.StatusText(resp.StatusCode))
+	}
+	content, err := io.ReadAll(resp.Body)
+	return content, errors.Wrap(err)
+}
+
 // Cleanup runs the cleaner.
 func (fl *fileLoader) Cleanup() error {
 	return fl.cleaner()
diff --git a/api/loader/fileloader_test.go b/api/loader/fileloader_test.go
index 8189cedc192..e22a5109741 100644
--- a/api/loader/fileloader_test.go
+++ b/api/loader/fileloader_test.go
@@ -21,7 +21,7 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/filesys"
 )
 
-func TestHasRemoteFileScheme(t *testing.T) {
+func TestIsRemoteFile(t *testing.T) {
 	cases := map[string]struct {
 		url   string
 		valid bool
@@ -50,7 +50,7 @@ func TestHasRemoteFileScheme(t *testing.T) {
 	for name, test := range cases {
 		test := test
 		t.Run(name, func(t *testing.T) {
-			require.Equal(t, test.valid, HasRemoteFileScheme(test.url))
+			require.Equal(t, test.valid, IsRemoteFile(test.url))
 		})
 	}
 }