Environment variables should be replaced in URLs in ADD commands.

We were previously explicitly skipping this for some reason, but Docker
seems to expand these in URLs so we should too.

integration/benchmark.json

@@ -0,0 +1 @@
+{"Command: RUN dotnet --help":133811400,"Extracting file":18158776100,"FS Unpacking":20909391300,"Fetching Extra Stages":12600,"Getting layers":3335500,"Hashing files":2472723700,"Initial FS snapshot":3706154200,"Retrieving Source Image":615160000,"Snapshotting FS":3568783100,"Total Build Time":28954788900,"Total Push Time":3842860800,"Walking filesystem":298680300,"Writing tar file":3745100,"creating directory":375701200,"creating file":669662900,"creating link":7357600,"creating symlink":137871900,"extract":16451137600,"fixing mtimes":418327800,"prepping to extract":2154366100}

integration/dockerfiles/Dockerfile_benchmark

@@ -0,0 +1,2 @@
+FROM gcr.io/dlorenc-vmtest2/dotnet2_app_builder
+RUN dotnet --help

integration/dockerfiles/Dockerfile_test_add

@@ -23,5 +23,8 @@ ARG file
 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
+ADD https://github.com/GoogleCloudPlatform/docker-credential-gcr/releases/download/1.4.3/docker-credential-gcr_linux_386-1.4.3.tar.gz context/foo /test/all/
+
+# Test environment replacement in the URL
+ENV VERSION=1.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

pkg/util/command_util.go

@@ -37,10 +37,6 @@ import (
 func ResolveEnvironmentReplacementList(values, envs []string, isFilepath bool) ([]string, error) {
 	var resolvedValues []string
 	for _, value := range values {
-		if IsSrcRemoteFileURL(value) {
-			resolvedValues = append(resolvedValues, value)
-			continue
-		}
 		resolved, err := ResolveEnvironmentReplacement(value, envs, isFilepath)
 		logrus.Debugf("Resolved %s to %s", value, resolved)
 		if err != nil {

pkg/util/command_util_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package util
 
 import (
+	"reflect"
 	"sort"
 	"testing"
 
@@ -448,3 +449,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)
+			}
+		})
+	}
+}