From ff6250cdb4ce332720c109620154140c74e807d8 Mon Sep 17 00:00:00 2001 From: Yujun Zhang Date: Fri, 31 Jan 2020 20:36:55 +0800 Subject: [PATCH 1/2] Allow loading file from http --- api/internal/target/kusttarget.go | 20 ++++++++------------ api/krusty/accumulation_test.go | 4 ++-- api/loader/fileloader.go | 17 +++++++++++++++++ examples/loadHttp.md | 25 +++++++++++++++++++++++++ examples/loadHttp/kustomization.yaml | 15 +++++++++++++++ 5 files changed, 67 insertions(+), 14 deletions(-) create mode 100644 examples/loadHttp.md create mode 100644 examples/loadHttp/kustomization.yaml diff --git a/api/internal/target/kusttarget.go b/api/internal/target/kusttarget.go index 35a2b281d2..f5fb08fb98 100644 --- a/api/internal/target/kusttarget.go +++ b/api/internal/target/kusttarget.go @@ -7,7 +7,6 @@ import ( "bytes" "encoding/json" "fmt" - "log" "strings" "github.com/pkg/errors" @@ -302,18 +301,15 @@ func (kt *KustTarget) configureExternalTransformers() ([]resmap.Transformer, err func (kt *KustTarget) accumulateResources( ra *accumulator.ResAccumulator, paths []string) error { for _, path := range paths { - ldr, err := kt.ldr.New(path) - if err == nil { - err = kt.accumulateDirectory(ra, ldr) - if err != nil { - return err + // try loading resource as file then as base (directory or git repository) + if errF := kt.accumulateFile(ra, path); errF != nil { + ldr, errL := kt.ldr.New(path) + if errL != nil { + return fmt.Errorf("accumulateFile %q, loader.New %q", errF, errL) } - } else { - err2 := kt.accumulateFile(ra, path) - if err2 != nil { - // Log ldr.New() error to highlight git failures. - log.Print(err.Error()) - return err2 + errD := kt.accumulateDirectory(ra, ldr) + if errD != nil { + return fmt.Errorf("accumulateFile %q, accumulateDirector: %q", errF, errD) } } } diff --git a/api/krusty/accumulation_test.go b/api/krusty/accumulation_test.go index e0df844565..491e52013b 100644 --- a/api/krusty/accumulation_test.go +++ b/api/krusty/accumulation_test.go @@ -74,7 +74,7 @@ spec: if err == nil { t.Fatalf("expected an error") } - if !IsMissingKustomizationFileError(err) { + if !strings.Contains(err.Error(), "accumulating resources") { t.Fatalf("unexpected error: %q", err) } } @@ -89,7 +89,7 @@ resources: if err == nil { t.Fatalf("expected an error") } - if !strings.Contains(err.Error(), "'/app/deployment.yaml' doesn't exist") { + if !strings.Contains(err.Error(), "accumulating resources") { t.Fatalf("unexpected error: %q", err) } } diff --git a/api/loader/fileloader.go b/api/loader/fileloader.go index 0822d0c848..d7fddac94c 100644 --- a/api/loader/fileloader.go +++ b/api/loader/fileloader.go @@ -5,7 +5,10 @@ package loader import ( "fmt" + "io/ioutil" "log" + "net/http" + "net/url" "path/filepath" "strings" @@ -293,6 +296,20 @@ 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) { + if u, err := url.Parse(path); err == nil && strings.HasPrefix(u.Scheme, "http") { + client := &http.Client{} + resp, err := client.Get(path) + if err != nil { + return nil, err + } + defer resp.Body.Close() + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return nil, err + } + return body, nil + } + if !filepath.IsAbs(path) { path = fl.root.Join(path) } diff --git a/examples/loadHttp.md b/examples/loadHttp.md new file mode 100644 index 0000000000..fb1e78386a --- /dev/null +++ b/examples/loadHttp.md @@ -0,0 +1,25 @@ +# load file from http + +Resource and patch files could be loaded from http + + +```sh +DEMO_HOME=$(mktemp -d) + +cat <$DEMO_HOME/kustomization.yaml +resources: +- https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/examples/helloWorld/configMap.yaml +EOF +``` + + +```sh +test 1 == \ + $(kustomize build $DEMO_HOME | grep "Good Morning!" | wc -l); \ + echo $? +``` + +Kustomize will try loading resource as a file either from local or http. If it +fails, try to load it as a directory or git repository. + +Http load applies to patches as well. See full example in [loadHttp](loadHttp/). diff --git a/examples/loadHttp/kustomization.yaml b/examples/loadHttp/kustomization.yaml new file mode 100644 index 0000000000..28e1b6ad42 --- /dev/null +++ b/examples/loadHttp/kustomization.yaml @@ -0,0 +1,15 @@ +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/examples/wordpress/wordpress/deployment.yaml +- https://github.com/knative/serving/releases/download/v0.12.0/serving.yaml # redirects to s3 +patches: +- https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/examples/wordpress/patch.yaml +patchesStrategicMerge: +- |- + apiVersion: rbac.authorization.k8s.io/v1 + kind: RoleBinding + metadata: + name: custom-metrics-auth-reader + namespace: kube-system + $patch: delete From 382425d974c9d2f30816fbd7d3794aebf62ecbdf Mon Sep 17 00:00:00 2001 From: Yujun Zhang Date: Thu, 27 Feb 2020 21:05:37 +0800 Subject: [PATCH 2/2] Add unit test for http fileloader --- api/loader/fileloader.go | 14 ++++-- api/loader/fileloader_test.go | 93 +++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 3 deletions(-) diff --git a/api/loader/fileloader.go b/api/loader/fileloader.go index d7fddac94c..930cea5b6f 100644 --- a/api/loader/fileloader.go +++ b/api/loader/fileloader.go @@ -89,6 +89,9 @@ type fileLoader struct { // File system utilities. fSys filesys.FileSystem + // Used to load from HTTP + http *http.Client + // Used to clone repositories. cloner git.Cloner @@ -296,9 +299,14 @@ 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) { - if u, err := url.Parse(path); err == nil && strings.HasPrefix(u.Scheme, "http") { - client := &http.Client{} - resp, err := client.Get(path) + 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 } diff --git a/api/loader/fileloader_test.go b/api/loader/fileloader_test.go index 41d01d711f..983bb41b3d 100644 --- a/api/loader/fileloader_test.go +++ b/api/loader/fileloader_test.go @@ -4,7 +4,9 @@ package loader import ( + "bytes" "io/ioutil" + "net/http" "os" "path" "path/filepath" @@ -54,6 +56,7 @@ func makeLoader() *fileLoader { return NewFileLoaderAtRoot(MakeFakeFs(testCases)) } + func TestLoaderLoad(t *testing.T) { l1 := makeLoader() if "/" != l1.Root() { @@ -579,3 +582,93 @@ func TestRepoIndirectCycleDetection(t *testing.T) { t.Fatalf("unexpected err: %v", err) } } + +// Inspired by https://hassansin.github.io/Unit-Testing-http-client-in-Go +type fakeRoundTripper func(req *http.Request) *http.Response + +func (f fakeRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + return f(req), nil +} + +func makeFakeHTTPClient(fn fakeRoundTripper) *http.Client { + return &http.Client{ + Transport: fn, + } +} + +// TestLoaderHTTP test http file loader +func TestLoaderHTTP(t *testing.T) { + var testCasesFile = []testData{ + { + path: "http/file.yaml", + expectedContent: "file content", + }, + } + + l1 := NewFileLoaderAtRoot(MakeFakeFs(testCasesFile)) + if "/" != l1.Root() { + t.Fatalf("incorrect root: '%s'\n", l1.Root()) + } + for _, x := range testCasesFile { + b, err := l1.Load(x.path) + if err != nil { + t.Fatalf("unexpected load error: %v", err) + } + if !reflect.DeepEqual([]byte(x.expectedContent), b) { + t.Fatalf("in load expected %s, but got %s", x.expectedContent, b) + } + } + + var testCasesHTTP = []testData{ + { + path: "http://example.com/resource.yaml", + expectedContent: "http content", + }, + { + path: "https://example.com/resource.yaml", + expectedContent: "https content", + }, + } + + for _, x := range testCasesHTTP { + hc := makeFakeHTTPClient(func(req *http.Request) *http.Response { + u := req.URL.String() + if x.path != u { + t.Fatalf("expected URL %s, but got %s", x.path, u) + } + return &http.Response{ + StatusCode: 200, + Body: ioutil.NopCloser(bytes.NewBufferString(x.expectedContent)), + Header: make(http.Header), + } + }) + l2 := l1 + l2.http = hc + b, err := l2.Load(x.path) + if err != nil { + t.Fatalf("unexpected load error: %v", err) + } + if !reflect.DeepEqual([]byte(x.expectedContent), b) { + t.Fatalf("in load expected %s, but got %s", x.expectedContent, b) + } + } + + var testCaseUnsupported = []testData{ + { + path: "httpsnotreal://example.com/resource.yaml", + expectedContent: "invalid", + }, + } + for _, x := range testCaseUnsupported { + hc := makeFakeHTTPClient(func(req *http.Request) *http.Response { + t.Fatalf("unexpected request to URL %s", req.URL.String()) + return nil + }) + l2 := l1 + l2.http = hc + _, err := l2.Load(x.path) + if err == nil { + t.Fatalf("expect error but get %v", err) + } + } +}