diff --git a/integration/dockerfiles/Dockerfile_test_add b/integration/dockerfiles/Dockerfile_test_add index 65f530ad1d..3df0c58647 100644 --- a/integration/dockerfiles/Dockerfile_test_add +++ b/integration/dockerfiles/Dockerfile_test_add @@ -24,4 +24,7 @@ COPY $file /arg # Finally, test adding a remote URL, concurrently with a normal file ADD https://github.com/GoogleCloudPlatform/docker-credential-gcr/releases/download/v1.4.3/docker-credential-gcr_linux_386-1.4.3.tar.gz context/foo /test/all/ -ADD https://github.com/GoogleCloudPlatform/docker-credential-gcr/releases/download/v1.4.3-static/docker-credential-gcr_linux_amd64-1.4.3.tar.gz /destination + +# Test environment replacement in the URL +ENV VERSION=v1.4.3 +ADD https://github.com/GoogleCloudPlatform/docker-credential-gcr/releases/download/${VERSION}-static/docker-credential-gcr_linux_amd64-1.4.3.tar.gz /destination diff --git a/pkg/commands/add.go b/pkg/commands/add.go index 7bcbce810d..b66b56db21 100644 --- a/pkg/commands/add.go +++ b/pkg/commands/add.go @@ -61,7 +61,10 @@ func (a *AddCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui for _, src := range srcs { fullPath := filepath.Join(a.buildcontext, src) if util.IsSrcRemoteFileURL(src) { - urlDest := util.URLDestinationFilepath(src, dest, config.WorkingDir) + urlDest, err := util.URLDestinationFilepath(src, dest, config.WorkingDir, replacementEnvs) + if err != nil { + return err + } logrus.Infof("Adding remote URL %s to %s", src, urlDest) if err := util.DownloadFileToDest(src, urlDest); err != nil { return err diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index c217fc74d3..e64fadf55b 100644 --- a/pkg/util/command_util.go +++ b/pkg/util/command_util.go @@ -37,11 +37,13 @@ import ( func ResolveEnvironmentReplacementList(values, envs []string, isFilepath bool) ([]string, error) { var resolvedValues []string for _, value := range values { + var resolved string + var err error if IsSrcRemoteFileURL(value) { - resolvedValues = append(resolvedValues, value) - continue + resolved, err = ResolveEnvironmentReplacement(value, envs, false) + } else { + resolved, err = ResolveEnvironmentReplacement(value, envs, isFilepath) } - resolved, err := ResolveEnvironmentReplacement(value, envs, isFilepath) logrus.Debugf("Resolved %s to %s", value, resolved) if err != nil { return nil, err @@ -165,20 +167,24 @@ func DestinationFilepath(src, dest, cwd string) (string, error) { } // URLDestinationFilepath gives the destination a file from a remote URL should be saved to -func URLDestinationFilepath(rawurl, dest, cwd string) string { +func URLDestinationFilepath(rawurl, dest, cwd string, envs []string) (string, error) { if !IsDestDir(dest) { if !filepath.IsAbs(dest) { - return filepath.Join(cwd, dest) + return filepath.Join(cwd, dest), nil } - return dest + return dest, nil } urlBase := filepath.Base(rawurl) + urlBase, err := ResolveEnvironmentReplacement(urlBase, envs, true) + if err != nil { + return "", err + } destPath := filepath.Join(dest, urlBase) if !filepath.IsAbs(dest) { destPath = filepath.Join(cwd, destPath) } - return destPath + return destPath, nil } func IsSrcsValid(srcsAndDest instructions.SourcesAndDest, resolvedSources []string, root string) error { diff --git a/pkg/util/command_util_test.go b/pkg/util/command_util_test.go index c4c6fead44..f7a4bf2111 100644 --- a/pkg/util/command_util_test.go +++ b/pkg/util/command_util_test.go @@ -17,6 +17,7 @@ limitations under the License. package util import ( + "reflect" "sort" "testing" @@ -27,14 +28,12 @@ var testURL = "https://github.com/GoogleContainerTools/runtimes-common/blob/mast var testEnvReplacement = []struct { path string - command string envs []string isFilepath bool expectedPath string }{ { - path: "/simple/path", - command: "WORKDIR /simple/path", + path: "/simple/path", envs: []string{ "simple=/path/", }, @@ -42,8 +41,7 @@ var testEnvReplacement = []struct { expectedPath: "/simple/path", }, { - path: "/simple/path/", - command: "WORKDIR /simple/path/", + path: "/simple/path/", envs: []string{ "simple=/path/", }, @@ -51,8 +49,7 @@ var testEnvReplacement = []struct { expectedPath: "/simple/path/", }, { - path: "${a}/b", - command: "WORKDIR ${a}/b", + path: "${a}/b", envs: []string{ "a=/path/", "b=/path2/", @@ -61,8 +58,7 @@ var testEnvReplacement = []struct { expectedPath: "/path/b", }, { - path: "/$a/b", - command: "COPY ${a}/b /c/", + path: "/$a/b", envs: []string{ "a=/path/", "b=/path2/", @@ -71,8 +67,7 @@ var testEnvReplacement = []struct { expectedPath: "/path/b", }, { - path: "/$a/b/", - command: "COPY /${a}/b /c/", + path: "/$a/b/", envs: []string{ "a=/path/", "b=/path2/", @@ -81,8 +76,7 @@ var testEnvReplacement = []struct { expectedPath: "/path/b/", }, { - path: "\\$foo", - command: "COPY \\$foo /quux", + path: "\\$foo", envs: []string{ "foo=/path/", }, @@ -90,8 +84,14 @@ var testEnvReplacement = []struct { expectedPath: "$foo", }, { - path: "8080/$protocol", - command: "EXPOSE 8080/$protocol", + path: "8080/$protocol", + envs: []string{ + "protocol=udp", + }, + expectedPath: "8080/udp", + }, + { + path: "8080/$protocol", envs: []string{ "protocol=udp", }, @@ -183,6 +183,7 @@ var urlDestFilepathTests = []struct { cwd string dest string expectedDest string + envs []string }{ { url: "https://something/something", @@ -202,12 +203,19 @@ var urlDestFilepathTests = []struct { dest: "/dest/", expectedDest: "/dest/something", }, + { + url: "https://something/$foo.tar.gz", + cwd: "/test", + dest: "/foo/", + expectedDest: "/foo/bar.tar.gz", + envs: []string{"foo=bar"}, + }, } func Test_UrlDestFilepath(t *testing.T) { for _, test := range urlDestFilepathTests { - actualDest := URLDestinationFilepath(test.url, test.dest, test.cwd) - testutil.CheckErrorAndDeepEqual(t, false, nil, test.expectedDest, actualDest) + actualDest, err := URLDestinationFilepath(test.url, test.dest, test.cwd, test.envs) + testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedDest, actualDest) } } @@ -448,3 +456,57 @@ func Test_RemoteUrls(t *testing.T) { } } + +func TestResolveEnvironmentReplacementList(t *testing.T) { + type args struct { + values []string + envs []string + isFilepath bool + } + tests := []struct { + name string + args args + want []string + wantErr bool + }{ + { + name: "url", + args: args{ + values: []string{ + "https://google.com/$foo", "$bar", + }, + envs: []string{ + "foo=baz", + "bar=bat", + }, + }, + want: []string{"https://google.com/baz", "bat"}, + }, + { + name: "mixed", + args: args{ + values: []string{ + "$foo", "$bar$baz", "baz", + }, + envs: []string{ + "foo=FOO", + "bar=BAR", + "baz=BAZ", + }, + }, + want: []string{"FOO", "BARBAZ", "baz"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ResolveEnvironmentReplacementList(tt.args.values, tt.args.envs, tt.args.isFilepath) + if (err != nil) != tt.wantErr { + t.Errorf("ResolveEnvironmentReplacementList() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("ResolveEnvironmentReplacementList() = %v, want %v", got, tt.want) + } + }) + } +}