From 6b5b88c2bc53d74da600cabb85f9c8b796005893 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Fri, 4 Oct 2019 14:27:24 -0700 Subject: [PATCH 1/3] fix creating abs path for urls --- cmd/executor/cmd/root.go | 22 +++++--- cmd/executor/cmd/root_test.go | 99 +++++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 8 deletions(-) create mode 100644 cmd/executor/cmd/root_test.go diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index 7eb34bf904..344b798449 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -168,7 +168,7 @@ func cacheFlagsValid() error { // resolveDockerfilePath resolves the Dockerfile path to an absolute path func resolveDockerfilePath() error { - if match, _ := regexp.MatchString("^https?://", opts.DockerfilePath); match { + if isUrl(opts.DockerfilePath) { return nil } if util.FilepathExists(opts.DockerfilePath) { @@ -241,13 +241,8 @@ func resolveRelativePaths() error { } for _, p := range optsPaths { - // Skip empty path - if *p == "" { - continue - } - // Skip path that is already absolute - if filepath.IsAbs(*p) { - logrus.Debugf("Path %s is absolute, skipping", *p) + if path := *p; skipPath(path) { + logrus.Debugf("Skip resolving path %s", path) continue } @@ -266,3 +261,14 @@ func exit(err error) { fmt.Println(err) os.Exit(1) } + +func isUrl(path string) bool { + if match, _ := regexp.MatchString("^https?://", path); match { + return true + } + return false +} + +func skipPath(path string) bool { + return path == "" || isUrl(path) || filepath.IsAbs(path) +} diff --git a/cmd/executor/cmd/root_test.go b/cmd/executor/cmd/root_test.go new file mode 100644 index 0000000000..ee629cffe5 --- /dev/null +++ b/cmd/executor/cmd/root_test.go @@ -0,0 +1,99 @@ +/* +Copyright 2018 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cmd + +import ( + "testing" + + "github.com/GoogleContainerTools/kaniko/testutil" +) + +func TestSkipPath(t *testing.T) { + tests := []struct { + description string + path string + expected bool + }{ + { + description: "path is a http url", + path: "http://test", + expected: true, + }, + { + description: "path is a https url", + path: "https://test", + expected: true, + }, + { + description: "path is a empty", + path: "", + expected: true, + }, + { + description: "path is already abs", + path: "/tmp/test", + expected: true, + }, + { + description: "path is relative", + path: ".././test", + }, + } + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + testutil.CheckDeepEqual(t, tt.expected, skipPath(tt.path)) + }) + } +} + +func TestIsUrl(t *testing.T) { + tests := []struct { + description string + path string + expected bool + }{ + { + description: "path is a http url", + path: "http://test", + expected: true, + }, + { + description: "path is a https url", + path: "https://test", + expected: true, + }, + { + description: "path is a empty", + path: "", + }, + { + description: "path is already abs", + path: "/tmp/test", + }, + { + description: "path is relative", + path: ".././test", + }, + } + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + testutil.CheckDeepEqual(t, tt.expected, isUrl(tt.path)) + }) + } +} From 230c4c9b07fc1916bea7c8423f41cbed75c948ac Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Fri, 4 Oct 2019 14:39:53 -0700 Subject: [PATCH 2/3] fix linter --- cmd/executor/cmd/root.go | 6 +++--- cmd/executor/cmd/root_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index 344b798449..53c3eeadcb 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -168,7 +168,7 @@ func cacheFlagsValid() error { // resolveDockerfilePath resolves the Dockerfile path to an absolute path func resolveDockerfilePath() error { - if isUrl(opts.DockerfilePath) { + if isURL(opts.DockerfilePath) { return nil } if util.FilepathExists(opts.DockerfilePath) { @@ -262,7 +262,7 @@ func exit(err error) { os.Exit(1) } -func isUrl(path string) bool { +func isURL(path string) bool { if match, _ := regexp.MatchString("^https?://", path); match { return true } @@ -270,5 +270,5 @@ func isUrl(path string) bool { } func skipPath(path string) bool { - return path == "" || isUrl(path) || filepath.IsAbs(path) + return path == "" || isURL(path) || filepath.IsAbs(path) } diff --git a/cmd/executor/cmd/root_test.go b/cmd/executor/cmd/root_test.go index ee629cffe5..e760c1f06f 100644 --- a/cmd/executor/cmd/root_test.go +++ b/cmd/executor/cmd/root_test.go @@ -93,7 +93,7 @@ func TestIsUrl(t *testing.T) { for _, tt := range tests { t.Run(tt.description, func(t *testing.T) { - testutil.CheckDeepEqual(t, tt.expected, isUrl(tt.path)) + testutil.CheckDeepEqual(t, tt.expected, isURL(tt.path)) }) } } From bb9ace058fa78fbdad9b31237f1be486c8e995cb Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Fri, 4 Oct 2019 14:43:21 -0700 Subject: [PATCH 3/3] address Don's comment --- cmd/executor/cmd/root.go | 4 ++-- cmd/executor/cmd/root_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index 53c3eeadcb..566accfa1c 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -241,7 +241,7 @@ func resolveRelativePaths() error { } for _, p := range optsPaths { - if path := *p; skipPath(path) { + if path := *p; shdSkip(path) { logrus.Debugf("Skip resolving path %s", path) continue } @@ -269,6 +269,6 @@ func isURL(path string) bool { return false } -func skipPath(path string) bool { +func shdSkip(path string) bool { return path == "" || isURL(path) || filepath.IsAbs(path) } diff --git a/cmd/executor/cmd/root_test.go b/cmd/executor/cmd/root_test.go index e760c1f06f..cec6dff693 100644 --- a/cmd/executor/cmd/root_test.go +++ b/cmd/executor/cmd/root_test.go @@ -56,7 +56,7 @@ func TestSkipPath(t *testing.T) { for _, tt := range tests { t.Run(tt.description, func(t *testing.T) { - testutil.CheckDeepEqual(t, tt.expected, skipPath(tt.path)) + testutil.CheckDeepEqual(t, tt.expected, shdSkip(tt.path)) }) } }