Merge pull request #973 from cvgw/u/cgwippern/fix-issue-704

Fix issue #704
2026-10-21 09:11:07 +02:00 · 2020-01-23 13:00:34 -08:00
parent 6e80e627ac 96093359c1
commit a2aae6274d
18 changed files with 1715 additions and 19 deletions
@@ -167,7 +167,13 @@ func (cr *CachingCopyCommand) ExecuteCommand(config *v1.Config, buildArgs *docke
 	if cr.img == nil {
 		return errors.New(fmt.Sprintf("cached command image is nil %v", cr.String()))
 	}
-	cr.extractedFiles, err = util.GetFSFromImage(RootDir, cr.img, cr.extractFn)
+
+	layers, err := cr.img.Layers()
+	if err != nil {
+		return errors.Wrapf(err, "retrieve image layers")
+	}
+
+	cr.extractedFiles, err = util.GetFSFromLayers(RootDir, layers, util.ExtractFunc(cr.extractFn), util.IncludeWhiteout())

 	logrus.Infof("extractedFiles: %s", cr.extractedFiles)
 	if err != nil {
@@ -182,7 +188,10 @@ func (cr *CachingCopyCommand) FilesUsedFromContext(config *v1.Config, buildArgs
 }

 func (cr *CachingCopyCommand) FilesToSnapshot() []string {
-	return cr.extractedFiles
+	f := cr.extractedFiles
+	logrus.Debugf("files extracted by caching copy command %s", f)
+
+	return f
 }

 func (cr *CachingCopyCommand) String() string {
@@ -197,7 +197,18 @@ func (cr *CachingRunCommand) ExecuteCommand(config *v1.Config, buildArgs *docker
 	if cr.img == nil {
 		return errors.New(fmt.Sprintf("command image is nil %v", cr.String()))
 	}
-	cr.extractedFiles, err = util.GetFSFromImage(constants.RootDir, cr.img, cr.extractFn)
+
+	layers, err := cr.img.Layers()
+	if err != nil {
+		return errors.Wrap(err, "retrieving image layers")
+	}
+
+	cr.extractedFiles, err = util.GetFSFromLayers(
+		constants.RootDir,
+		layers,
+		util.ExtractFunc(cr.extractFn),
+		util.IncludeWhiteout(),
+	)
 	if err != nil {
 		return errors.Wrap(err, "extracting fs from image")
 	}
@@ -206,7 +217,10 @@ func (cr *CachingRunCommand) ExecuteCommand(config *v1.Config, buildArgs *docker
 }

 func (cr *CachingRunCommand) FilesToSnapshot() []string {
-	return cr.extractedFiles
+	f := cr.extractedFiles
+	logrus.Debugf("files extracted from caching run command %s", f)
+
+	return f
 }

 func (cr *CachingRunCommand) String() string {
@@ -0,0 +1,143 @@
+/*
+Copyright 2020 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.
+*/
+
+// Code generated by MockGen. DO NOT EDIT.
+// Source: github.com/google/go-containerregistry/pkg/v1 (interfaces: Layer)
+
+// Package mockv1 is a generated GoMock package.
+package mockv1
+
+import (
+	io "io"
+	reflect "reflect"
+
+	gomock "github.com/golang/mock/gomock"
+	v1 "github.com/google/go-containerregistry/pkg/v1"
+	types "github.com/google/go-containerregistry/pkg/v1/types"
+)
+
+// MockLayer is a mock of Layer interface
+type MockLayer struct {
+	ctrl     *gomock.Controller
+	recorder *MockLayerMockRecorder
+}
+
+// MockLayerMockRecorder is the mock recorder for MockLayer
+type MockLayerMockRecorder struct {
+	mock *MockLayer
+}
+
+// NewMockLayer creates a new mock instance
+func NewMockLayer(ctrl *gomock.Controller) *MockLayer {
+	mock := &MockLayer{ctrl: ctrl}
+	mock.recorder = &MockLayerMockRecorder{mock}
+	return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use
+func (m *MockLayer) EXPECT() *MockLayerMockRecorder {
+	return m.recorder
+}
+
+// Compressed mocks base method
+func (m *MockLayer) Compressed() (io.ReadCloser, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Compressed")
+	ret0, _ := ret[0].(io.ReadCloser)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// Compressed indicates an expected call of Compressed
+func (mr *MockLayerMockRecorder) Compressed() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Compressed", reflect.TypeOf((*MockLayer)(nil).Compressed))
+}
+
+// DiffID mocks base method
+func (m *MockLayer) DiffID() (v1.Hash, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "DiffID")
+	ret0, _ := ret[0].(v1.Hash)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// DiffID indicates an expected call of DiffID
+func (mr *MockLayerMockRecorder) DiffID() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DiffID", reflect.TypeOf((*MockLayer)(nil).DiffID))
+}
+
+// Digest mocks base method
+func (m *MockLayer) Digest() (v1.Hash, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Digest")
+	ret0, _ := ret[0].(v1.Hash)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// Digest indicates an expected call of Digest
+func (mr *MockLayerMockRecorder) Digest() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Digest", reflect.TypeOf((*MockLayer)(nil).Digest))
+}
+
+// MediaType mocks base method
+func (m *MockLayer) MediaType() (types.MediaType, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "MediaType")
+	ret0, _ := ret[0].(types.MediaType)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// MediaType indicates an expected call of MediaType
+func (mr *MockLayerMockRecorder) MediaType() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MediaType", reflect.TypeOf((*MockLayer)(nil).MediaType))
+}
+
+// Size mocks base method
+func (m *MockLayer) Size() (int64, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Size")
+	ret0, _ := ret[0].(int64)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// Size indicates an expected call of Size
+func (mr *MockLayerMockRecorder) Size() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Size", reflect.TypeOf((*MockLayer)(nil).Size))
+}
+
+// Uncompressed mocks base method
+func (m *MockLayer) Uncompressed() (io.ReadCloser, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Uncompressed")
+	ret0, _ := ret[0].(io.ReadCloser)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// Uncompressed indicates an expected call of Uncompressed
+func (mr *MockLayerMockRecorder) Uncompressed() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Uncompressed", reflect.TypeOf((*MockLayer)(nil).Uncompressed))
+}
@@ -71,25 +71,58 @@ var excluded []string

 type ExtractFunction func(string, *tar.Header, io.Reader) error

+type FSConfig struct {
+	includeWhiteout bool
+	extractFunc     ExtractFunction
+}
+
+type FSOpt func(*FSConfig)
+
+func IncludeWhiteout() FSOpt {
+	return func(opts *FSConfig) {
+		opts.includeWhiteout = true
+	}
+}
+
+func ExtractFunc(extractFunc ExtractFunction) FSOpt {
+	return func(opts *FSConfig) {
+		opts.extractFunc = extractFunc
+	}
+}
+
 // GetFSFromImage extracts the layers of img to root
 // It returns a list of all files extracted
 func GetFSFromImage(root string, img v1.Image, extract ExtractFunction) ([]string, error) {
-	if extract == nil {
-		return nil, errors.New("must supply an extract function")
-	}
 	if img == nil {
 		return nil, errors.New("image cannot be nil")
 	}
-	if err := DetectFilesystemWhitelist(constants.WhitelistPath); err != nil {
-		return nil, err
-	}
-	logrus.Debugf("Mounted directories: %v", whitelist)
+
 	layers, err := img.Layers()
 	if err != nil {
 		return nil, err
 	}
-	extractedFiles := []string{}

+	return GetFSFromLayers(root, layers, ExtractFunc(extract))
+}
+
+func GetFSFromLayers(root string, layers []v1.Layer, opts ...FSOpt) ([]string, error) {
+	cfg := new(FSConfig)
+
+	for _, opt := range opts {
+		opt(cfg)
+	}
+
+	if cfg.extractFunc == nil {
+		return nil, errors.New("must supply an extract function")
+	}
+
+	if err := DetectFilesystemWhitelist(constants.WhitelistPath); err != nil {
+		return nil, err
+	}
+
+	logrus.Debugf("Mounted directories: %v", whitelist)
+
+	extractedFiles := []string{}
 	for i, l := range layers {
 		if mediaType, err := l.MediaType(); err == nil {
 			logrus.Tracef("Extracting layer %d of media type %s", i, mediaType)
@@ -102,29 +135,41 @@ func GetFSFromImage(root string, img v1.Image, extract ExtractFunction) ([]strin
 			return nil, err
 		}
 		defer r.Close()
+
 		tr := tar.NewReader(r)
 		for {
 			hdr, err := tr.Next()
 			if err == io.EOF {
 				break
 			}
+
 			if err != nil {
 				return nil, errors.Wrap(err, fmt.Sprintf("error reading tar %d", i))
 			}
+
 			path := filepath.Join(root, filepath.Clean(hdr.Name))
 			base := filepath.Base(path)
 			dir := filepath.Dir(path)
+
 			if strings.HasPrefix(base, ".wh.") {
 				logrus.Debugf("Whiting out %s", path)
+
 				name := strings.TrimPrefix(base, ".wh.")
 				if err := os.RemoveAll(filepath.Join(dir, name)); err != nil {
 					return nil, errors.Wrapf(err, "removing whiteout %s", hdr.Name)
 				}
-				continue
+
+				if !cfg.includeWhiteout {
+					logrus.Debug("not including whiteout files")
+					continue
+				}
+
 			}
-			if err := extract(root, hdr, tr); err != nil {
+
+			if err := cfg.extractFunc(root, hdr, tr); err != nil {
 				return nil, err
 			}
+
 			extractedFiles = append(extractedFiles, filepath.Join(root, filepath.Clean(hdr.Name)))
 		}
 	}
@@ -20,6 +20,7 @@ import (
 	"archive/tar"
 	"bytes"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -29,7 +30,11 @@ import (
 	"testing"
 	"time"

+	"github.com/GoogleContainerTools/kaniko/pkg/mocks/go-containerregistry/mockv1"
 	"github.com/GoogleContainerTools/kaniko/testutil"
+	"github.com/golang/mock/gomock"
+	v1 "github.com/google/go-containerregistry/pkg/v1"
+	"github.com/google/go-containerregistry/pkg/v1/types"
 )

 func Test_DetectFilesystemWhitelist(t *testing.T) {
@@ -966,3 +971,277 @@ func Test_CopyFile_skips_self(t *testing.T) {
 		t.Fatalf("expected file contents to be %q, but got %q", expected, actual)
 	}
 }
+
+func fakeExtract(dest string, hdr *tar.Header, tr io.Reader) error {
+	return nil
+}
+
+func Test_GetFSFromLayers_with_whiteouts_include_whiteout_enabled(t *testing.T) {
+	ctrl := gomock.NewController(t)
+
+	root, err := ioutil.TempDir("", "layers-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.Remove(root)
+
+	opts := []FSOpt{
+		// I'd rather use the real func (util.ExtractFile)
+		// but you have to be root to chown
+		ExtractFunc(fakeExtract),
+		IncludeWhiteout(),
+	}
+
+	expectErr := false
+
+	f := func(expectedFiles []string, tw *tar.Writer) {
+		for _, f := range expectedFiles {
+			f := strings.TrimPrefix(strings.TrimPrefix(f, root), "/")
+
+			hdr := &tar.Header{
+				Name: f,
+				Mode: 0644,
+				Size: int64(len("Hello World\n")),
+			}
+
+			if err := tw.WriteHeader(hdr); err != nil {
+				t.Fatal(err)
+			}
+
+			if _, err := tw.Write([]byte("Hello World\n")); err != nil {
+				t.Fatal(err)
+			}
+		}
+
+		if err := tw.Close(); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	expectedFiles := []string{
+		filepath.Join(root, "foobar"),
+	}
+
+	buf := new(bytes.Buffer)
+	tw := tar.NewWriter(buf)
+
+	f(expectedFiles, tw)
+
+	mockLayer := mockv1.NewMockLayer(ctrl)
+	mockLayer.EXPECT().MediaType().Return(types.OCILayer, nil)
+
+	rc := ioutil.NopCloser(buf)
+	mockLayer.EXPECT().Uncompressed().Return(rc, nil)
+
+	secondLayerFiles := []string{
+		filepath.Join(root, ".wh.foobar"),
+	}
+
+	buf = new(bytes.Buffer)
+	tw = tar.NewWriter(buf)
+
+	f(secondLayerFiles, tw)
+
+	mockLayer2 := mockv1.NewMockLayer(ctrl)
+	mockLayer2.EXPECT().MediaType().Return(types.OCILayer, nil)
+
+	rc = ioutil.NopCloser(buf)
+	mockLayer2.EXPECT().Uncompressed().Return(rc, nil)
+
+	layers := []v1.Layer{
+		mockLayer,
+		mockLayer2,
+	}
+
+	expectedFiles = append(expectedFiles, secondLayerFiles...)
+
+	actualFiles, err := GetFSFromLayers(root, layers, opts...)
+
+	assertGetFSFromLayers(
+		t,
+		actualFiles,
+		expectedFiles,
+		err,
+		expectErr,
+	)
+}
+
+func Test_GetFSFromLayers_with_whiteouts_include_whiteout_disabled(t *testing.T) {
+	ctrl := gomock.NewController(t)
+
+	root, err := ioutil.TempDir("", "layers-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.Remove(root)
+
+	opts := []FSOpt{
+		// I'd rather use the real func (util.ExtractFile)
+		// but you have to be root to chown
+		ExtractFunc(fakeExtract),
+	}
+
+	expectErr := false
+
+	f := func(expectedFiles []string, tw *tar.Writer) {
+		for _, f := range expectedFiles {
+			f := strings.TrimPrefix(strings.TrimPrefix(f, root), "/")
+
+			hdr := &tar.Header{
+				Name: f,
+				Mode: 0644,
+				Size: int64(len("Hello world\n")),
+			}
+
+			if err := tw.WriteHeader(hdr); err != nil {
+				t.Fatal(err)
+			}
+
+			if _, err := tw.Write([]byte("Hello world\n")); err != nil {
+				t.Fatal(err)
+			}
+		}
+
+		if err := tw.Close(); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	expectedFiles := []string{
+		filepath.Join(root, "foobar"),
+	}
+
+	buf := new(bytes.Buffer)
+	tw := tar.NewWriter(buf)
+
+	f(expectedFiles, tw)
+
+	mockLayer := mockv1.NewMockLayer(ctrl)
+	mockLayer.EXPECT().MediaType().Return(types.OCILayer, nil)
+
+	rc := ioutil.NopCloser(buf)
+	mockLayer.EXPECT().Uncompressed().Return(rc, nil)
+
+	secondLayerFiles := []string{
+		filepath.Join(root, ".wh.foobar"),
+	}
+
+	buf = new(bytes.Buffer)
+	tw = tar.NewWriter(buf)
+
+	f(secondLayerFiles, tw)
+
+	mockLayer2 := mockv1.NewMockLayer(ctrl)
+	mockLayer2.EXPECT().MediaType().Return(types.OCILayer, nil)
+
+	rc = ioutil.NopCloser(buf)
+	mockLayer2.EXPECT().Uncompressed().Return(rc, nil)
+
+	layers := []v1.Layer{
+		mockLayer,
+		mockLayer2,
+	}
+
+	actualFiles, err := GetFSFromLayers(root, layers, opts...)
+
+	assertGetFSFromLayers(
+		t,
+		actualFiles,
+		expectedFiles,
+		err,
+		expectErr,
+	)
+}
+
+func Test_GetFSFromLayers(t *testing.T) {
+	ctrl := gomock.NewController(t)
+
+	root, err := ioutil.TempDir("", "layers-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.Remove(root)
+
+	opts := []FSOpt{
+		// I'd rather use the real func (util.ExtractFile)
+		// but you have to be root to chown
+		ExtractFunc(fakeExtract),
+	}
+
+	expectErr := false
+	expectedFiles := []string{
+		filepath.Join(root, "foobar"),
+	}
+
+	buf := new(bytes.Buffer)
+	tw := tar.NewWriter(buf)
+
+	for _, f := range expectedFiles {
+		f := strings.TrimPrefix(strings.TrimPrefix(f, root), "/")
+
+		hdr := &tar.Header{
+			Name: f,
+			Mode: 0644,
+			Size: int64(len("Hello world\n")),
+		}
+
+		if err := tw.WriteHeader(hdr); err != nil {
+			t.Fatal(err)
+		}
+
+		if _, err := tw.Write([]byte("Hello world\n")); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	if err := tw.Close(); err != nil {
+		t.Fatal(err)
+	}
+
+	mockLayer := mockv1.NewMockLayer(ctrl)
+	mockLayer.EXPECT().MediaType().Return(types.OCILayer, nil)
+
+	rc := ioutil.NopCloser(buf)
+	mockLayer.EXPECT().Uncompressed().Return(rc, nil)
+
+	layers := []v1.Layer{
+		mockLayer,
+	}
+
+	actualFiles, err := GetFSFromLayers(root, layers, opts...)
+
+	assertGetFSFromLayers(
+		t,
+		actualFiles,
+		expectedFiles,
+		err,
+		expectErr,
+	)
+}
+
+func assertGetFSFromLayers(
+	t *testing.T,
+	actualFiles []string,
+	expectedFiles []string,
+	err error,
+	expectErr bool,
+) {
+	if !expectErr && err != nil {
+		t.Error(err)
+		t.FailNow()
+	} else if expectErr && err == nil {
+		t.Error("expected err to not be nil")
+		t.FailNow()
+	}
+
+	if len(actualFiles) != len(expectedFiles) {
+		t.Errorf("expected %s to equal %s", actualFiles, expectedFiles)
+		t.FailNow()
+	}
+
+	for i := range expectedFiles {
+		if actualFiles[i] != expectedFiles[i] {
+			t.Errorf("expected %s to equal %s", actualFiles[i], expectedFiles[i])
+		}
+	}
+}