Updated PR Branch with master

2026-10-20 09:41:17 +02:00 · 2019-04-09 02:18:28 +05:30
parent 8b459b57aa
commit fa2a2c803b
75 changed files with 2221 additions and 647 deletions
@@ -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
@@ -76,6 +78,22 @@ func ResolveEnvironmentReplacement(value string, envs []string, isFilepath bool)
 	return fp, nil
 }

+func ResolveEnvAndWildcards(sd instructions.SourcesAndDest, buildcontext string, envs []string) ([]string, string, error) {
+	// First, resolve any environment replacement
+	resolvedEnvs, err := ResolveEnvironmentReplacementList(sd, envs, true)
+	if err != nil {
+		return nil, "", err
+	}
+	dest := resolvedEnvs[len(resolvedEnvs)-1]
+	// Resolve wildcards and get a list of resolved sources
+	srcs, err := ResolveSources(resolvedEnvs[0:len(resolvedEnvs)-1], buildcontext)
+	if err != nil {
+		return nil, "", err
+	}
+	err = IsSrcsValid(sd, srcs, buildcontext)
+	return srcs, dest, err
+}
+
 // ContainsWildcards returns true if any entry in paths contains wildcards
 func ContainsWildcards(paths []string) bool {
 	for _, path := range paths {
@@ -88,23 +106,22 @@ func ContainsWildcards(paths []string) bool {

 // ResolveSources resolves the given sources if the sources contains wildcards
 // It returns a list of resolved sources
-func ResolveSources(srcsAndDest instructions.SourcesAndDest, root string) ([]string, error) {
-	srcs := srcsAndDest[:len(srcsAndDest)-1]
+func ResolveSources(srcs []string, root string) ([]string, error) {
 	// If sources contain wildcards, we first need to resolve them to actual paths
-	if ContainsWildcards(srcs) {
-		logrus.Debugf("Resolving srcs %v...", srcs)
-		files, err := RelativeFiles("", root)
-		if err != nil {
-			return nil, err
-		}
-		srcs, err = matchSources(srcs, files)
-		if err != nil {
-			return nil, err
-		}
-		logrus.Debugf("Resolved sources to %v", srcs)
+	if !ContainsWildcards(srcs) {
+		return srcs, nil
 	}
-	// Check to make sure the sources are valid
-	return srcs, IsSrcsValid(srcsAndDest, srcs, root)
+	logrus.Infof("Resolving srcs %v...", srcs)
+	files, err := RelativeFiles("", root)
+	if err != nil {
+		return nil, err
+	}
+	resolved, err := matchSources(srcs, files)
+	if err != nil {
+		return nil, err
+	}
+	logrus.Debugf("Resolved sources to %v", resolved)
+	return resolved, nil
 }

 // matchSources returns a list of sources that match wildcards
@@ -165,20 +182,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 {
@@ -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)
 	}
 }

@@ -400,7 +408,6 @@ var testResolveSources = []struct {
 			"context/foo",
 			"context/b*",
 			testURL,
-			"dest/",
 		},
 		expectedList: []string{
 			"context/foo",
@@ -448,3 +455,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)
+			}
+		})
+	}
+}
@@ -216,27 +216,16 @@ func extractFile(dest string, hdr *tar.Header, tr io.Reader) error {
 		if err != nil {
 			return err
 		}
-		// manually set permissions on file, since the default umask (022) will interfere
-		if err = os.Chmod(path, mode); err != nil {
-			return err
-		}
 		if _, err = io.Copy(currFile, tr); err != nil {
 			return err
 		}
-		if err = currFile.Chown(uid, gid); err != nil {
+		if err = setFilePermissions(path, mode, uid, gid); err != nil {
 			return err
 		}
 		currFile.Close()
 	case tar.TypeDir:
 		logrus.Debugf("creating dir %s", path)
-		if err := os.MkdirAll(path, mode); err != nil {
-			return err
-		}
-		// In some cases, MkdirAll doesn't change the permissions, so run Chmod
-		if err := os.Chmod(path, mode); err != nil {
-			return err
-		}
-		if err := os.Chown(path, uid, gid); err != nil {
+		if err := mkdirAllWithPermissions(path, mode, uid, gid); err != nil {
 			return err
 		}

@@ -429,10 +418,7 @@ func CreateFile(path string, reader io.Reader, perm os.FileMode, uid uint32, gid
 	if _, err := io.Copy(dest, reader); err != nil {
 		return err
 	}
-	if err := dest.Chmod(perm); err != nil {
-		return err
-	}
-	return dest.Chown(int(uid), int(gid))
+	return setFilePermissions(path, perm, int(uid), int(gid))
 }

 // AddVolumePath adds the given path to the volume whitelist.
@@ -492,13 +478,11 @@ func CopyDir(src, dest, buildcontext string) ([]string, error) {
 		if fi.IsDir() {
 			logrus.Debugf("Creating directory %s", destPath)

+			mode := fi.Mode()
 			uid := int(fi.Sys().(*syscall.Stat_t).Uid)
 			gid := int(fi.Sys().(*syscall.Stat_t).Gid)

-			if err := os.MkdirAll(destPath, fi.Mode()); err != nil {
-				return nil, err
-			}
-			if err := os.Chown(destPath, uid, gid); err != nil {
+			if err := mkdirAllWithPermissions(destPath, mode, uid, gid); err != nil {
 				return nil, err
 			}
 		} else if fi.Mode()&os.ModeSymlink != 0 {
@@ -614,3 +598,24 @@ func HasFilepathPrefix(path, prefix string, prefixMatchOnly bool) bool {
 func Volumes() []string {
 	return volumes
 }
+
+func mkdirAllWithPermissions(path string, mode os.FileMode, uid, gid int) error {
+	if err := os.MkdirAll(path, mode); err != nil {
+		return err
+	}
+	if err := os.Chown(path, uid, gid); err != nil {
+		return err
+	}
+	// In some cases, MkdirAll doesn't change the permissions, so run Chmod
+	// Must chmod after chown because chown resets the file mode.
+	return os.Chmod(path, mode)
+}
+
+func setFilePermissions(path string, mode os.FileMode, uid, gid int) error {
+	if err := os.Chown(path, uid, gid); err != nil {
+		return err
+	}
+	// manually set permissions on file, since the default umask (022) will interfere
+	// Must chmod after chown because chown resets the file mode.
+	return os.Chmod(path, mode)
+}
@@ -503,6 +503,30 @@ func TestExtractFile(t *testing.T) {
 				filesAreHardlinks("/bin/uncompress", "/bin/gzip"),
 			},
 		},
+		{
+			name:     "file with setuid bit",
+			contents: []byte("helloworld"),
+			hdrs:     []*tar.Header{fileHeader("./bar", "helloworld", 04644)},
+			checkers: []checker{
+				fileExists("/bar"),
+				fileMatches("/bar", []byte("helloworld")),
+				permissionsMatch("/bar", 0644|os.ModeSetuid),
+			},
+		},
+		{
+			name:     "dir with sticky bit",
+			contents: []byte("helloworld"),
+			hdrs: []*tar.Header{
+				dirHeader("./foo", 01755),
+				fileHeader("./foo/bar", "helloworld", 0644),
+			},
+			checkers: []checker{
+				fileExists("/foo/bar"),
+				fileMatches("/foo/bar", []byte("helloworld")),
+				permissionsMatch("/foo/bar", 0644),
+				permissionsMatch("/foo", 0755|os.ModeDir|os.ModeSticky),
+			},
+		},
 	}

 	for _, tc := range tcs {