From 1acb95e921c179afc9207c49dc98ba295e49c8f5 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 16 Sep 2024 09:34:53 +0100 Subject: [PATCH 1/5] hack: lie more about ownership --- pkg/util/util.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/util/util.go b/pkg/util/util.go index 7501984fbe..67eeb81907 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -24,7 +24,6 @@ import ( "io" "math" "os" - "path/filepath" "strconv" "strings" "sync" @@ -123,8 +122,10 @@ func CacheHasher() func(string) (string, error) { // likely that the file will be owned by the UID/GID that is running // envbuilder, which in this case is not guaranteed to be root. // Let's just pretend that it is, cross our fingers, and hope for the best. - lyingAboutOwnership := !fi.IsDir() && - strings.HasSuffix(filepath.Clean(filepath.Dir(p)), ".envbuilder.tmp") + isRoot := os.Geteuid() == 0 + //lyingAboutOwnership := !fi.IsDir() && + //strings.HasSuffix(filepath.Clean(filepath.Dir(p)), ".envbuilder.tmp") + lyingAboutOwnership := !fi.IsDir() && !isRoot if lyingAboutOwnership { h.Write([]byte(strconv.FormatUint(uint64(0), 36))) h.Write([]byte(",")) From a6f639371c8870751e08848e12e7a15ceafdfa5c Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 18 Sep 2024 15:52:04 +0100 Subject: [PATCH 2/5] use sync.OnceValue --- pkg/util/util.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/pkg/util/util.go b/pkg/util/util.go index 67eeb81907..22875cdce2 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -37,6 +37,13 @@ import ( "golang.org/x/sys/unix" ) +// In order to avoid failing cache probes when a cache probe operation +// is run as a non-root user, we need to pretend that files are owned +// by root. +var isRoot = sync.OnceValue(func() bool { + return os.Getuid() == 0 +}) + // Hasher returns a hash function, used in snapshotting to determine if a file has changed func Hasher() func(string) (string, error) { pool := sync.Pool{ @@ -122,10 +129,7 @@ func CacheHasher() func(string) (string, error) { // likely that the file will be owned by the UID/GID that is running // envbuilder, which in this case is not guaranteed to be root. // Let's just pretend that it is, cross our fingers, and hope for the best. - isRoot := os.Geteuid() == 0 - //lyingAboutOwnership := !fi.IsDir() && - //strings.HasSuffix(filepath.Clean(filepath.Dir(p)), ".envbuilder.tmp") - lyingAboutOwnership := !fi.IsDir() && !isRoot + lyingAboutOwnership := !fi.IsDir() && !isRoot() if lyingAboutOwnership { h.Write([]byte(strconv.FormatUint(uint64(0), 36))) h.Write([]byte(",")) From e0a7370421e7118f5816b8d36fe5d6ae73c9ddcc Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 18 Sep 2024 15:58:46 +0100 Subject: [PATCH 3/5] improve comment + logging --- pkg/util/util.go | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/pkg/util/util.go b/pkg/util/util.go index 22875cdce2..3788e061bb 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -25,7 +25,6 @@ import ( "math" "os" "strconv" - "strings" "sync" "syscall" "time" @@ -41,7 +40,9 @@ import ( // is run as a non-root user, we need to pretend that files are owned // by root. var isRoot = sync.OnceValue(func() bool { - return os.Getuid() == 0 + b := os.Getuid() == 0 + logrus.Debugf("kaniko running as root: %t", b) + return b }) // Hasher returns a hash function, used in snapshotting to determine if a file has changed @@ -120,15 +121,13 @@ func CacheHasher() func(string) (string, error) { h.Write([]byte(fi.Mode().String())) - // Cian: this is a disgusting hack, but it removes the need for the - // envbuilder binary to be owned by root when doing a cache probe. - // We want to ignore UID and GID changes for the envbuilder binary - // specifically. When building and pushing an image using the envbuilder - // image, the embedded envbuilder binary will most likely be owned by - // root:root. However, when performing a cache probe operation, it is more - // likely that the file will be owned by the UID/GID that is running - // envbuilder, which in this case is not guaranteed to be root. - // Let's just pretend that it is, cross our fingers, and hope for the best. + // Cian: this is a disgusting hack. When building and pushing an image + // using the envbuilder image, the files in .envbuilder will most likely + // be owned by root:root. However, when performing a cache probe operation, + // it is almost certain that the UID/GID that is running the cache probe + // operation is not root. This means that the cache probe operation will + // fail unless we lie about the UID/GID of the files used to build the + // image. lyingAboutOwnership := !fi.IsDir() && !isRoot() if lyingAboutOwnership { h.Write([]byte(strconv.FormatUint(uint64(0), 36))) From b7f13d0d1d8e2895e54bb1e9837e889cb3b12b26 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 23 Sep 2024 09:32:18 +0100 Subject: [PATCH 4/5] skip failing test if not run as root --- pkg/executor/cache_probe_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/executor/cache_probe_test.go b/pkg/executor/cache_probe_test.go index 86b54848b2..52677a053e 100644 --- a/pkg/executor/cache_probe_test.go +++ b/pkg/executor/cache_probe_test.go @@ -20,6 +20,7 @@ import ( "fmt" "net/http/httptest" "net/url" + "os" "path/filepath" "strings" "testing" @@ -165,6 +166,9 @@ COPY foo/baz.txt copied/ }) t.Run("MultiStage", func(t *testing.T) { + if os.Getuid() != 0 { + t.Skip("this test fails because DoBuild is not running as the root user") + } // Share cache between both builds. regCache := setupCacheRegistry(t) @@ -218,9 +222,13 @@ COPY foo/baz.txt copied/ opts, fn = prepare() defer fn() image2, err := DoCacheProbe(opts) - testutil.CheckNoError(t, err) + if err != nil { + t.Fatalf("cache probe failed: %+v", err) + } digest2, err := image2.Digest() - testutil.CheckNoError(t, err) + if err != nil { + t.Fatalf("digest failed: %+v", err) + } if digest1.String() != digest2.String() { t.Errorf("expected %s, got %s", digest1.String(), digest2.String()) From f813228f8ac7cccb0fbdf6a9cb9b1ec954456aa2 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 23 Sep 2024 09:51:07 +0100 Subject: [PATCH 5/5] move definition inside CacheHasher --- pkg/util/util.go | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/pkg/util/util.go b/pkg/util/util.go index 3788e061bb..97703337ac 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -36,15 +36,6 @@ import ( "golang.org/x/sys/unix" ) -// In order to avoid failing cache probes when a cache probe operation -// is run as a non-root user, we need to pretend that files are owned -// by root. -var isRoot = sync.OnceValue(func() bool { - b := os.Getuid() == 0 - logrus.Debugf("kaniko running as root: %t", b) - return b -}) - // Hasher returns a hash function, used in snapshotting to determine if a file has changed func Hasher() func(string) (string, error) { pool := sync.Pool{ @@ -104,6 +95,13 @@ type CacheHasherFileInfoSum interface { // CacheHasher takes into account everything the regular hasher does except for mtime func CacheHasher() func(string) (string, error) { + // In order to avoid failing cache probes when a cache probe operation + // is run as a non-root user, we need to pretend that files are owned + // by root. + isRoot := os.Getuid() == 0 + if isRoot { + logrus.Debugf("kaniko running as root: %t", isRoot) + } hasher := func(p string) (string, error) { h := md5.New() fi, err := filesystem.FS.Lstat(p) @@ -128,7 +126,7 @@ func CacheHasher() func(string) (string, error) { // operation is not root. This means that the cache probe operation will // fail unless we lie about the UID/GID of the files used to build the // image. - lyingAboutOwnership := !fi.IsDir() && !isRoot() + lyingAboutOwnership := !fi.IsDir() && !isRoot if lyingAboutOwnership { h.Write([]byte(strconv.FormatUint(uint64(0), 36))) h.Write([]byte(","))