From 260d07cca2e68eabbb833d83fb0d1b68efbdb4fb Mon Sep 17 00:00:00 2001 From: Daniel Mikusa Date: Mon, 25 Apr 2022 15:05:45 -0400 Subject: [PATCH] Rewrite LayerContributor - Check if the layer has been restored properly. If the layer is cache or build and the layer TOML exists, then the layer directory should also exist and have contents. If it does not exist or is empty then the layer was not restored and we need to reload the laery. This adds checks for this condition and will print a different message to indicate that the layer is being reloaded, instead of contributed fresh. - Add sherpa utilites for checking if a file, directory or symlink exist. These were needed for the refactoring above and are also common across other buildpacks. A set of tests is also provided. - Fixes test "adds expected Syft SBOM file" which was previously passing because a previous test run creates the SBOM file this test requires. Refactoring of the test class exposed this error. The test has been corrected. Signed-off-by: Daniel Mikusa --- layer.go | 84 ++++++++++++++++++++----- layer_test.go | 107 ++++++++++++++++++++++++++----- sherpa/exists.go | 63 +++++++++++++++++++ sherpa/exists_test.go | 143 ++++++++++++++++++++++++++++++++++++++++++ sherpa/init_test.go | 1 + 5 files changed, 367 insertions(+), 31 deletions(-) create mode 100644 sherpa/exists.go create mode 100644 sherpa/exists_test.go diff --git a/layer.go b/layer.go index df0a005..01a833c 100644 --- a/layer.go +++ b/layer.go @@ -18,6 +18,7 @@ package libpak import ( "fmt" + "io/fs" "os" "path/filepath" "reflect" @@ -64,34 +65,31 @@ type LayerFunc func() (libcnb.Layer, error) // Contribute is the function to call when implementing your libcnb.LayerContributor. func (l *LayerContributor) Contribute(layer libcnb.Layer, f LayerFunc) (libcnb.Layer, error) { - raw, err := toml.Marshal(l.ExpectedMetadata) + layerRestored, err := l.checkIfLayerRestored(layer) if err != nil { - return libcnb.Layer{}, fmt.Errorf("unable to encode metadata\n%w", err) + return libcnb.Layer{}, fmt.Errorf("unable to check metadata\n%w", err) } - expected := map[string]interface{}{} - if err := toml.Unmarshal(raw, &expected); err != nil { - return libcnb.Layer{}, fmt.Errorf("unable to decode metadata\n%w", err) + expected, cached, err := l.checkIfMetadataMatches(layer) + if err != nil { + return libcnb.Layer{}, fmt.Errorf("unable to check metadata\n%w", err) } - l.Logger.Debugf("Expected metadata: %+v", expected) - l.Logger.Debugf("Actual metadata: %+v", layer.Metadata) - - // TODO: compare entire layer not just metadata (in case build, launch, or cache have changed) - if reflect.DeepEqual(expected, layer.Metadata) { + if cached && layerRestored { l.Logger.Headerf("%s: %s cached layer", color.BlueString(l.Name), color.GreenString("Reusing")) layer.LayerTypes = l.ExpectedTypes return layer, nil } - l.Logger.Headerf("%s: %s to layer", color.BlueString(l.Name), color.YellowString("Contributing")) - - if err := os.RemoveAll(layer.Path); err != nil { - return libcnb.Layer{}, fmt.Errorf("unable to remove existing layer directory %s\n%w", layer.Path, err) + if !layerRestored { + l.Logger.Headerf("%s: %s cached layer", color.BlueString(l.Name), color.RedString("Reloading")) + } else { + l.Logger.Headerf("%s: %s to layer", color.BlueString(l.Name), color.YellowString("Contributing")) } - if err := os.MkdirAll(layer.Path, 0755); err != nil { - return libcnb.Layer{}, fmt.Errorf("unable to create layer directory %s\n%w", layer.Path, err) + err = l.reset(layer) + if err != nil { + return libcnb.Layer{}, fmt.Errorf("unable to reset\n%w", err) } layer, err = f() @@ -105,6 +103,60 @@ func (l *LayerContributor) Contribute(layer libcnb.Layer, f LayerFunc) (libcnb.L return layer, nil } +func (l *LayerContributor) checkIfMetadataMatches(layer libcnb.Layer) (map[string]interface{}, bool, error) { + raw, err := toml.Marshal(l.ExpectedMetadata) + if err != nil { + return map[string]interface{}{}, false, fmt.Errorf("unable to encode metadata\n%w", err) + } + + expected := map[string]interface{}{} + if err := toml.Unmarshal(raw, &expected); err != nil { + return map[string]interface{}{}, false, fmt.Errorf("unable to decode metadata\n%w", err) + } + + l.Logger.Debugf("Expected metadata: %+v", expected) + l.Logger.Debugf("Actual metadata: %+v", layer.Metadata) + + return expected, reflect.DeepEqual(expected, layer.Metadata), nil +} + +func (l *LayerContributor) checkIfLayerRestored(layer libcnb.Layer) (bool, error) { + layerTOML := fmt.Sprintf("%s.toml", layer.Path) + tomlExists, err := sherpa.FileExists(layerTOML) + if err != nil { + return false, fmt.Errorf("unable to check if layer TOML tomlExists %s\n%w", layerTOML, err) + } + + layerDirExists, err := sherpa.DirExists(layer.Path) + if err != nil { + return false, fmt.Errorf("unable to check if layer directory exists %s\n%w", layer.Path, err) + } + + var dirContents []fs.DirEntry + if layerDirExists { + dirContents, err = os.ReadDir(layer.Path) + if err != nil { + return false, fmt.Errorf("unable to read directory %s\n%w", layer.Path, err) + } + } + + l.Logger.Debugf("Check If Layer Restored -> tomlExists: %s, layerDirExists: %s, dirContents: %s, cache: %s, build: %s", + tomlExists, layerDirExists, dirContents, l.ExpectedTypes.Cache, l.ExpectedTypes.Build) + return !(tomlExists && (!layerDirExists || len(dirContents) == 0) && (l.ExpectedTypes.Cache || l.ExpectedTypes.Build)), nil +} + +func (l *LayerContributor) reset(layer libcnb.Layer) error { + if err := os.RemoveAll(layer.Path); err != nil { + return fmt.Errorf("unable to remove existing layer directory %s\n%w", layer.Path, err) + } + + if err := os.MkdirAll(layer.Path, 0755); err != nil { + return fmt.Errorf("unable to create layer directory %s\n%w", layer.Path, err) + } + + return nil +} + // DependencyLayerContributor is a helper for implementing a libcnb.LayerContributor for a BuildpackDependency in order // to get consistent logging and avoidance. type DependencyLayerContributor struct { diff --git a/layer_test.go b/layer_test.go index 34745dc..814a4d5 100644 --- a/layer_test.go +++ b/layer_test.go @@ -37,14 +37,16 @@ func testLayer(t *testing.T, context spec.G, it spec.S) { var ( Expect = NewWithT(t).Expect - layer libcnb.Layer + layersDir string + layer libcnb.Layer ) it.Before(func() { var err error - layer.Path, err = ioutil.TempDir("", "layer") + layersDir, err = ioutil.TempDir("", "layer") Expect(err).NotTo(HaveOccurred()) + layer.Path = filepath.Join(layersDir, "test-layer") layer.Exec.Path = layer.Path layer.Metadata = map[string]interface{}{} @@ -52,7 +54,7 @@ func testLayer(t *testing.T, context spec.G, it spec.S) { }) it.After(func() { - Expect(os.RemoveAll(layer.Path)).To(Succeed()) + Expect(os.RemoveAll(layersDir)).To(Succeed()) }) context("LayerContributor", func() { @@ -96,6 +98,90 @@ func testLayer(t *testing.T, context spec.G, it spec.S) { Expect(called).To(BeTrue()) }) + context("reloads layers not restored", func() { + var called bool + + it.Before(func() { + layer.Metadata = map[string]interface{}{ + "alpha": "test-alpha", + "bravo": map[string]interface{}{ + "bravo-1": "test-bravo-1", + "bravo-2": "test-bravo-2", + }, + } + }) + + it("calls function with matching metadata but no layer directory on cache layer", func() { + Expect(ioutil.WriteFile(fmt.Sprintf("%s.toml", layer.Path), []byte{}, 0644)).To(Succeed()) + Expect(os.RemoveAll(layer.Path)).To(Succeed()) + lc.ExpectedTypes.Cache = true + + _, err := lc.Contribute(layer, func() (libcnb.Layer, error) { + called = true + return layer, nil + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(called).To(BeTrue()) + }) + + it("calls function with matching metadata but no layer directory on build layer", func() { + Expect(ioutil.WriteFile(fmt.Sprintf("%s.toml", layer.Path), []byte{}, 0644)).To(Succeed()) + Expect(os.RemoveAll(layer.Path)).To(Succeed()) + lc.ExpectedTypes.Build = true + + _, err := lc.Contribute(layer, func() (libcnb.Layer, error) { + called = true + return layer, nil + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(called).To(BeTrue()) + }) + + it("calls function with matching metadata but an empty layer directory on build layer", func() { + Expect(ioutil.WriteFile(fmt.Sprintf("%s.toml", layer.Path), []byte{}, 0644)).To(Succeed()) + Expect(os.MkdirAll(layer.Path, 0755)).To(Succeed()) + lc.ExpectedTypes.Build = true + + _, err := lc.Contribute(layer, func() (libcnb.Layer, error) { + called = true + return layer, nil + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(called).To(BeTrue()) + }) + + it("does not call function with matching metadata when layer directory exists and has a file in it", func() { + Expect(ioutil.WriteFile(fmt.Sprintf("%s.toml", layer.Path), []byte{}, 0644)).To(Succeed()) + Expect(os.MkdirAll(layer.Path, 0755)).To(Succeed()) + Expect(ioutil.WriteFile(filepath.Join(layer.Path, "foo"), []byte{}, 0644)).To(Succeed()) + lc.ExpectedTypes.Build = true + + _, err := lc.Contribute(layer, func() (libcnb.Layer, error) { + called = true + return layer, nil + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(called).To(BeFalse()) + }) + + it("does not call function with matching metadata when layer TOML missing", func() { + Expect(os.MkdirAll(layer.Path, 0755)).To(Succeed()) + layer.Build = true + + _, err := lc.Contribute(layer, func() (libcnb.Layer, error) { + called = true + return layer, nil + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(called).To(BeFalse()) + }) + }) + it("does not call function with matching metadata", func() { layer.Metadata = map[string]interface{}{ "alpha": "test-alpha", @@ -675,22 +761,13 @@ func testLayer(t *testing.T, context spec.G, it spec.S) { }) it("adds expected Syft SBOM file", func() { - layer.Metadata = map[string]interface{}{ - "id": buildpack.Info.ID, - "name": buildpack.Info.Name, - "version": buildpack.Info.Version, - "homepage": buildpack.Info.Homepage, - "clear-env": buildpack.Info.ClearEnvironment, - "description": "", - "sbom-formats": []interface{}{}, - "keywords": []interface{}{}, - } + layer.Metadata = map[string]interface{}{} _, err := hlc.Contribute(layer) Expect(err).NotTo(HaveOccurred()) - Expect(filepath.Join(layer.Exec.FilePath("test-name-1"))).NotTo(BeAnExistingFile()) - Expect(filepath.Join(layer.Exec.FilePath("test-name-2"))).NotTo(BeAnExistingFile()) + Expect(filepath.Join(layer.Exec.FilePath("test-name-1"))).To(BeAnExistingFile()) + Expect(filepath.Join(layer.Exec.FilePath("test-name-2"))).To(BeAnExistingFile()) outputFile := layer.SBOMPath(libcnb.SyftJSON) Expect(outputFile).To(BeARegularFile()) diff --git a/sherpa/exists.go b/sherpa/exists.go new file mode 100644 index 0000000..9580a7c --- /dev/null +++ b/sherpa/exists.go @@ -0,0 +1,63 @@ +/* + * Copyright 2018-2022 the original author or authors. + * + * 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 + * + * https://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. + */ + +package sherpa + +import "os" + +// Exists returns true if the path exists. +func Exists(path string) (bool, error) { + if _, err := os.Stat(path); err == nil { + return true, nil + } else if os.IsNotExist(err) { + return false, nil + } else { + return false, err + } +} + +// FileExists returns true if the path exists and is a regular file. +func FileExists(path string) (bool, error) { + if stat, err := os.Stat(path); err == nil { + return stat.Mode().IsRegular(), nil + } else if os.IsNotExist(err) { + return false, nil + } else { + return false, err + } +} + +// DirExists returns true if the path exists and is a directory. +func DirExists(path string) (bool, error) { + if stat, err := os.Stat(path); err == nil { + return stat.IsDir(), nil + } else if os.IsNotExist(err) { + return false, nil + } else { + return false, err + } +} + +// SymlinkExists returns true if the path exists and is a symlink. +func SymlinkExists(path string) (bool, error) { + if stat, err := os.Lstat(path); err == nil { + return stat.Mode()&os.ModeSymlink != 0, nil + } else if os.IsNotExist(err) { + return false, nil + } else { + return false, err + } +} diff --git a/sherpa/exists_test.go b/sherpa/exists_test.go new file mode 100644 index 0000000..b357c66 --- /dev/null +++ b/sherpa/exists_test.go @@ -0,0 +1,143 @@ +/* + * Copyright 2018-2022 the original author or authors. + * + * 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 + * + * https://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. + */ + +package sherpa_test + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/paketo-buildpacks/libpak/sherpa" + "github.com/sclevine/spec" + + . "github.com/onsi/gomega" +) + +func testExists(t *testing.T, when spec.G, it spec.S) { + var ( + Expect = NewWithT(t).Expect + testPath string + ) + + it.Before(func() { + var err error + testPath, err = ioutil.TempDir("", "exists") + Expect(err).NotTo(HaveOccurred()) + }) + + it.After(func() { + Expect(os.RemoveAll(testPath)).To(Succeed()) + }) + + when("checking something exists", func() { + it("should return true if path is a file", func() { + path := filepath.Join(testPath, "test-file") + Expect(os.WriteFile(path, []byte{}, 0644)).To(Succeed()) + exists, err := sherpa.Exists(path) + Expect(err).ToNot(HaveOccurred()) + Expect(exists).To(BeTrue()) + }) + + it("should return true if path is a directory", func() { + path := filepath.Join(testPath, "test-dir") + Expect(os.Mkdir(path, 0755)).To(Succeed()) + exists, err := sherpa.Exists(path) + Expect(err).ToNot(HaveOccurred()) + Expect(exists).To(BeTrue()) + }) + + it("should return false if path does not exist", func() { + exists, err := sherpa.Exists(filepath.Join(testPath, "does-not-exist")) + Expect(err).ToNot(HaveOccurred()) + Expect(exists).To(BeFalse()) + }) + }) + + when("checking a directory exists", func() { + it("should return true if path is a directory", func() { + path := filepath.Join(testPath, "test-dir") + Expect(os.Mkdir(path, 0755)).To(Succeed()) + exists, err := sherpa.DirExists(path) + Expect(err).ToNot(HaveOccurred()) + Expect(exists).To(BeTrue()) + }) + + it("should return false if path is a file", func() { + path := filepath.Join(testPath, "test-file") + Expect(os.WriteFile(path, []byte{}, 0644)).To(Succeed()) + exists, err := sherpa.DirExists(path) + Expect(err).ToNot(HaveOccurred()) + Expect(exists).To(BeFalse()) + }) + + it("should return false if path does not exist", func() { + exists, err := sherpa.FileExists(filepath.Join(testPath, "does-not-exist")) + Expect(err).ToNot(HaveOccurred()) + Expect(exists).To(BeFalse()) + }) + }) + + when("checking a file exists", func() { + it("should return true if path is a file", func() { + path := filepath.Join(testPath, "test-file") + Expect(os.WriteFile(path, []byte{}, 0644)).To(Succeed()) + exists, err := sherpa.FileExists(path) + Expect(err).ToNot(HaveOccurred()) + Expect(exists).To(BeTrue()) + }) + + it("should return false if path is a directory", func() { + path := filepath.Join(testPath, "test-dir") + Expect(os.Mkdir(path, 0755)).To(Succeed()) + exists, err := sherpa.FileExists(path) + Expect(err).ToNot(HaveOccurred()) + Expect(exists).To(BeFalse()) + BeARegularFile() + }) + + it("should return false if path does not exist", func() { + exists, err := sherpa.FileExists(filepath.Join(testPath, "does-not-exist")) + Expect(err).ToNot(HaveOccurred()) + Expect(exists).To(BeFalse()) + }) + }) + + when("checking a symlink exists", func() { + it("should return true if path is a symlink", func() { + path := filepath.Join(testPath, "tmp-link") + Expect(os.Symlink(".", path)).ToNot(HaveOccurred()) + exists, err := sherpa.SymlinkExists(path) + Expect(err).ToNot(HaveOccurred()) + Expect(exists).To(BeTrue()) + }) + + it("should return false if path is a directory", func() { + path := filepath.Join(testPath, "test-dir") + Expect(os.Mkdir(path, 0755)).To(Succeed()) + exists, err := sherpa.SymlinkExists(path) + Expect(err).ToNot(HaveOccurred()) + Expect(exists).To(BeFalse()) + }) + + it("should return false if path does not exist", func() { + exists, err := sherpa.SymlinkExists(filepath.Join(testPath, "does-not-exist")) + Expect(err).ToNot(HaveOccurred()) + Expect(exists).To(BeFalse()) + }) + }) +} diff --git a/sherpa/init_test.go b/sherpa/init_test.go index f524e5d..387879e 100644 --- a/sherpa/init_test.go +++ b/sherpa/init_test.go @@ -27,6 +27,7 @@ func TestUnit(t *testing.T) { suite := spec.New("libpak/sherpa", spec.Report(report.Terminal{})) suite("CopyFile", testCopyFile) suite("EnvVar", testEnvVar) + suite("Exists", testExists) suite("FileListing", testFileListing) suite("NodeJS", testNodeJS) suite("Sherpa", testSherpa)