From 36f4784377b518cb236c5ec2500ffe840836d022 Mon Sep 17 00:00:00 2001 From: Daniel Mikusa Date: Thu, 15 Apr 2021 10:45:41 -0400 Subject: [PATCH 1/5] Modifications to enable libcnb to be compatible with both versions 0.5 and 0.6 of the API Signed-off-by: Daniel Mikusa --- build.go | 16 +++++++++--- build_test.go | 56 +++++++++++++++++++++++++++++++++++------ detect.go | 5 ++-- detect_test.go | 4 +-- internal/layer_api_5.go | 16 ++++++++++++ layer.go | 13 ++++++++++ layer_test.go | 47 +++++++++++++++++++++++++++++++++- 7 files changed, 141 insertions(+), 16 deletions(-) create mode 100644 internal/layer_api_5.go diff --git a/build.go b/build.go index 0b0c00e..4520465 100644 --- a/build.go +++ b/build.go @@ -165,8 +165,9 @@ func Build(builder Builder, options ...Option) { } logger.Debugf("Buildpack: %+v", ctx.Buildpack) - if strings.TrimSpace(ctx.Buildpack.API) != "0.6" { - config.exitHandler.Error(errors.New("this version of libcnb is only compatible with buildpack API 0.6")) + API := strings.TrimSpace(ctx.Buildpack.API) + if API != "0.5" && API != "0.6" { + config.exitHandler.Error(errors.New("this version of libcnb is only compatible with buildpack API 0.5 and 0.6")) return } @@ -273,7 +274,16 @@ func Build(builder Builder, options ...Option) { file = filepath.Join(ctx.Layers.Path, fmt.Sprintf("%s.toml", layer.Name)) logger.Debugf("Writing layer metadata: %s <= %+v", file, layer) - if err = config.tomlWriter.Write(file, layer); err != nil { + var toWrite interface{} = layer + if API == "0.5" { + toWrite = internal.LayerAPI5{ + Build: layer.LayerTypes.Build, + Cache: layer.LayerTypes.Cache, + Launch: layer.LayerTypes.Launch, + Metadata: layer.Metadata, + } + } + if err = config.tomlWriter.Write(file, toWrite); err != nil { config.exitHandler.Error(fmt.Errorf("unable to write layer metadata %s\n%w", file, err)) return } diff --git a/build_test.go b/build_test.go index e07f9b3..2a0eb6b 100644 --- a/build_test.go +++ b/build_test.go @@ -21,6 +21,7 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" "testing" . "github.com/onsi/gomega" @@ -28,6 +29,7 @@ import ( "github.com/stretchr/testify/mock" "github.com/buildpacks/libcnb" + "github.com/buildpacks/libcnb/internal" "github.com/buildpacks/libcnb/mocks" ) @@ -39,6 +41,7 @@ func testBuild(t *testing.T, context spec.G, it spec.S) { builder *mocks.Builder buildpackPath string buildpackPlanPath string + bpTOMLContents string commandPath string environmentWriter *mocks.EnvironmentWriter exitHandler *mocks.ExitHandler @@ -64,9 +67,8 @@ func testBuild(t *testing.T, context spec.G, it spec.S) { Expect(err).NotTo(HaveOccurred()) Expect(os.Setenv("CNB_BUILDPACK_DIR", buildpackPath)).To(Succeed()) - Expect(ioutil.WriteFile(filepath.Join(buildpackPath, "buildpack.toml"), - []byte(` -api = "0.6" + bpTOMLContents = ` +api = "##API_VERSION##" [buildpack] id = "test-id" @@ -96,7 +98,9 @@ mixins = ["test-name"] [metadata] test-key = "test-value" -`), +` + Expect(ioutil.WriteFile(filepath.Join(buildpackPath, "buildpack.toml"), + []byte(strings.Replace(bpTOMLContents, "##API_VERSION##", "0.6", 1)), 0644), ).To(Succeed()) @@ -171,11 +175,11 @@ test-key = "test-value" Expect(os.RemoveAll(platformPath)).To(Succeed()) }) - context("buildpack API is not 0.6", func() { + context("buildpack API is not 0.5 or 0.6", func() { it.Before(func() { Expect(ioutil.WriteFile(filepath.Join(buildpackPath, "buildpack.toml"), []byte(` -api = "0.5" +api = "0.4" [buildpack] id = "test-id" @@ -193,7 +197,7 @@ version = "1.1.1" ) Expect(exitHandler.Calls[0].Arguments.Get(0)).To(MatchError( - "this version of libcnb is only compatible with buildpack API 0.6", + "this version of libcnb is only compatible with buildpack API 0.5 and 0.6", )) }) }) @@ -389,7 +393,43 @@ version = "1.1.1" Expect(environmentWriter.Calls[3].Arguments[1]).To(Equal(map[string]string{"test-profile": "test-value"})) }) - it("writes layer metadata", func() { + it("writes 0.5 layer metadata", func() { + Expect(ioutil.WriteFile(filepath.Join(buildpackPath, "buildpack.toml"), + []byte(strings.Replace(bpTOMLContents, "##API_VERSION##", "0.5", 1)), + 0644), + ).To(Succeed()) + + layer := libcnb.Layer{ + Name: "test-name", + Path: filepath.Join(layersPath, "test-name"), + LayerTypes: libcnb.LayerTypes{ + Build: true, + Cache: true, + Launch: true, + }, + Metadata: map[string]interface{}{"test-key": "test-value"}, + } + layerContributor.On("Contribute", mock.Anything).Return(layer, nil) + layerContributor.On("Name").Return("test-name") + result := libcnb.BuildResult{Layers: []libcnb.LayerContributor{layerContributor}} + builder.On("Build", mock.Anything).Return(result, nil) + + libcnb.Build(builder, + libcnb.WithArguments([]string{commandPath, layersPath, platformPath, buildpackPlanPath}), + libcnb.WithTOMLWriter(tomlWriter), + ) + + Expect(tomlWriter.Calls[0].Arguments[0]).To(Equal(filepath.Join(layersPath, "test-name.toml"))) + + layer5, ok := tomlWriter.Calls[0].Arguments[1].(internal.LayerAPI5) + Expect(ok).To(BeTrue()) + Expect(layer5.Build).To(BeTrue()) + Expect(layer5.Cache).To(BeTrue()) + Expect(layer5.Launch).To(BeTrue()) + Expect(layer5.Metadata).To(Equal(map[string]interface{}{"test-key": "test-value"})) + }) + + it("writes 0.6 layer metadata", func() { layer := libcnb.Layer{ Name: "test-name", Path: filepath.Join(layersPath, "test-name"), diff --git a/detect.go b/detect.go index e386183..71a9da7 100644 --- a/detect.go +++ b/detect.go @@ -115,8 +115,9 @@ func Detect(detector Detector, options ...Option) { } logger.Debugf("Buildpack: %+v", ctx.Buildpack) - if strings.TrimSpace(ctx.Buildpack.API) != "0.6" { - config.exitHandler.Error(errors.New("this version of libcnb is only compatible with buildpack API 0.6")) + API := strings.TrimSpace(ctx.Buildpack.API) + if API != "0.5" && API != "0.6" { + config.exitHandler.Error(errors.New("this version of libcnb is only compatible with buildpack API 0.5 and 0.6")) return } diff --git a/detect_test.go b/detect_test.go index 76ab266..1bb5ad3 100644 --- a/detect_test.go +++ b/detect_test.go @@ -135,7 +135,7 @@ test-key = "test-value" Expect(os.RemoveAll(platformPath)).To(Succeed()) }) - context("buildpack API is not 0.6", func() { + context("buildpack API is not 0.5 or 0.6", func() { it.Before(func() { Expect(ioutil.WriteFile(filepath.Join(buildpackPath, "buildpack.toml"), []byte(` @@ -157,7 +157,7 @@ version = "1.1.1" ) Expect(exitHandler.Calls[0].Arguments.Get(0)).To(MatchError( - "this version of libcnb is only compatible with buildpack API 0.6", + "this version of libcnb is only compatible with buildpack API 0.5 and 0.6", )) }) }) diff --git a/internal/layer_api_5.go b/internal/layer_api_5.go new file mode 100644 index 0000000..e4d2e3d --- /dev/null +++ b/internal/layer_api_5.go @@ -0,0 +1,16 @@ +package internal + +// LayerAPI5 is used for backwards compatibility with serialization/deserialization of the layer toml +type LayerAPI5 struct { + // Build indicates that a layer should be used for builds. + Build bool `toml:"build"` + + // Cache indicates that a layer should be cached. + Cache bool `toml:"cache"` + + // Launch indicates that a layer should be used for launch. + Launch bool `toml:"launch"` + + // Metadata is the metadata associated with the layer. + Metadata map[string]interface{} `toml:"metadata"` +} diff --git a/layer.go b/layer.go index ec4835c..a4df307 100644 --- a/layer.go +++ b/layer.go @@ -22,6 +22,7 @@ import ( "path/filepath" "github.com/BurntSushi/toml" + "github.com/buildpacks/libcnb/internal" ) // Exec represents the exec.d layer location @@ -146,5 +147,17 @@ func (l *Layers) Layer(name string) (Layer, error) { return Layer{}, fmt.Errorf("unable to decode layer metadata %s\n%w", f, err) } + if !layer.Build && !layer.Cache && !layer.Launch { + // if all three are false, that could mean we have a API <= 0.5 TOML file + // try parsing the <= 0.5 API format where these were top level attributes + buf := internal.LayerAPI5{} + if _, err := toml.DecodeFile(f, &buf); err != nil && !os.IsNotExist(err) { + return Layer{}, fmt.Errorf("unable to decode layer metadata %s\n%w", f, err) + } + layer.Build = buf.Build + layer.Cache = buf.Cache + layer.Launch = buf.Launch + } + return layer, nil } diff --git a/layer_test.go b/layer_test.go index bafc858..f768a14 100644 --- a/layer_test.go +++ b/layer_test.go @@ -111,7 +111,28 @@ func testLayer(t *testing.T, context spec.G, it spec.S) { Expect(l.Profile).To(Equal(libcnb.Profile{})) }) - it("reads existing metadata", func() { + it("reads existing 0.5 metadata", func() { + Expect(ioutil.WriteFile( + filepath.Join(path, "test-name.toml"), + []byte(` +launch = true +build = false + +[metadata] +test-key = "test-value" + `), + 0644), + ).To(Succeed()) + + l, err := layers.Layer("test-name") + Expect(err).NotTo(HaveOccurred()) + + Expect(l.Metadata).To(Equal(map[string]interface{}{"test-key": "test-value"})) + Expect(l.Launch).To(BeTrue()) + Expect(l.Build).To(BeFalse()) + }) + + it("reads existing 0.6 metadata", func() { Expect(ioutil.WriteFile( filepath.Join(path, "test-name.toml"), []byte(` @@ -132,5 +153,29 @@ test-key = "test-value" Expect(l.Launch).To(BeTrue()) Expect(l.Build).To(BeFalse()) }) + + it("reads existing 0.6 metadata with launch, build and cache all false", func() { + Expect(ioutil.WriteFile( + filepath.Join(path, "test-name.toml"), + []byte(` +[types] +launch = false +build = false +cache = false + +[metadata] +test-key = "test-value" + `), + 0644), + ).To(Succeed()) + + l, err := layers.Layer("test-name") + Expect(err).NotTo(HaveOccurred()) + + Expect(l.Metadata).To(Equal(map[string]interface{}{"test-key": "test-value"})) + Expect(l.Launch).To(BeFalse()) + Expect(l.Build).To(BeFalse()) + Expect(l.Cache).To(BeFalse()) + }) }) } From 982e2d490973a8451801c02a99ca63a9d12af32d Mon Sep 17 00:00:00 2001 From: Daniel Mikusa Date: Mon, 19 Apr 2021 09:27:36 -0400 Subject: [PATCH 2/5] Added comment to explain coding decision of inlining bools Signed-off-by: Daniel Mikusa --- internal/layer_api_5.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/layer_api_5.go b/internal/layer_api_5.go index e4d2e3d..203bd5a 100644 --- a/internal/layer_api_5.go +++ b/internal/layer_api_5.go @@ -2,6 +2,8 @@ package internal // LayerAPI5 is used for backwards compatibility with serialization/deserialization of the layer toml type LayerAPI5 struct { + // bool's are inlined to instead of using libcnb.LayerTypes to break a circular package reference, internal -> libcnb -> internal -> ... + // Build indicates that a layer should be used for builds. Build bool `toml:"build"` From dccb6e6bcda70cb87ca1c61eca6f21c323798c2f Mon Sep 17 00:00:00 2001 From: Daniel Mikusa Date: Mon, 19 Apr 2021 09:37:24 -0400 Subject: [PATCH 3/5] Added WARNING if you try to set process.Default=true when using API 0.5 Signed-off-by: Daniel Mikusa --- build.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/build.go b/build.go index 4520465..1c36d64 100644 --- a/build.go +++ b/build.go @@ -325,6 +325,15 @@ func Build(builder Builder, options ...Option) { if !launch.isEmpty() { file = filepath.Join(ctx.Layers.Path, "launch.toml") logger.Debugf("Writing application metadata: %s <= %+v", file, launch) + + if API == "0.5" { + for _, process := range launch.Processes { + if process.Default { + logger.Info("WARNING: Launch layer is setting default=true, but that is not supported until API version 0.6. This setting will be ignored.") + } + } + } + if err = config.tomlWriter.Write(file, launch); err != nil { config.exitHandler.Error(fmt.Errorf("unable to write application metadata %s\n%w", file, err)) return From 9785aecd0d8b6292124d1bac8ef7072ebaa7b836 Mon Sep 17 00:00:00 2001 From: Daniel Mikusa Date: Tue, 20 Apr 2021 15:01:47 -0400 Subject: [PATCH 4/5] Update build.go Co-authored-by: Emily Casey Signed-off-by: Daniel Mikusa --- build.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.go b/build.go index 1c36d64..b85708b 100644 --- a/build.go +++ b/build.go @@ -167,7 +167,7 @@ func Build(builder Builder, options ...Option) { API := strings.TrimSpace(ctx.Buildpack.API) if API != "0.5" && API != "0.6" { - config.exitHandler.Error(errors.New("this version of libcnb is only compatible with buildpack API 0.5 and 0.6")) + config.exitHandler.Error(errors.New("this version of libcnb is only compatible with buildpack APIs 0.5 and 0.6")) return } From f26c239b906850bc3432db94e478a584a5935c56 Mon Sep 17 00:00:00 2001 From: Daniel Mikusa Date: Tue, 20 Apr 2021 15:13:31 -0400 Subject: [PATCH 5/5] Use text/template instead of strings.Replace Signed-off-by: Daniel Mikusa --- build_test.go | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/build_test.go b/build_test.go index 2a0eb6b..e698c83 100644 --- a/build_test.go +++ b/build_test.go @@ -17,12 +17,13 @@ package libcnb_test import ( + "bytes" "fmt" "io/ioutil" "os" "path/filepath" - "strings" "testing" + "text/template" . "github.com/onsi/gomega" "github.com/sclevine/spec" @@ -49,6 +50,7 @@ func testBuild(t *testing.T, context spec.G, it spec.S) { layersPath string platformPath string tomlWriter *mocks.TOMLWriter + buildpackTOML *template.Template workingDir string ) @@ -68,7 +70,7 @@ func testBuild(t *testing.T, context spec.G, it spec.S) { Expect(os.Setenv("CNB_BUILDPACK_DIR", buildpackPath)).To(Succeed()) bpTOMLContents = ` -api = "##API_VERSION##" +api = "{{.APIVersion}}" [buildpack] id = "test-id" @@ -99,10 +101,14 @@ mixins = ["test-name"] [metadata] test-key = "test-value" ` - Expect(ioutil.WriteFile(filepath.Join(buildpackPath, "buildpack.toml"), - []byte(strings.Replace(bpTOMLContents, "##API_VERSION##", "0.6", 1)), - 0644), - ).To(Succeed()) + buildpackTOML, err = template.New("buildpack.toml").Parse(bpTOMLContents) + Expect(err).ToNot(HaveOccurred()) + + var b bytes.Buffer + err = buildpackTOML.Execute(&b, map[string]string{"APIVersion": "0.6"}) + Expect(err).ToNot(HaveOccurred()) + + Expect(ioutil.WriteFile(filepath.Join(buildpackPath, "buildpack.toml"), b.Bytes(), 0644)).To(Succeed()) f, err := ioutil.TempFile("", "build-buildpackplan-path") Expect(err).NotTo(HaveOccurred()) @@ -197,7 +203,7 @@ version = "1.1.1" ) Expect(exitHandler.Calls[0].Arguments.Get(0)).To(MatchError( - "this version of libcnb is only compatible with buildpack API 0.5 and 0.6", + "this version of libcnb is only compatible with buildpack APIs 0.5 and 0.6", )) }) }) @@ -394,10 +400,11 @@ version = "1.1.1" }) it("writes 0.5 layer metadata", func() { - Expect(ioutil.WriteFile(filepath.Join(buildpackPath, "buildpack.toml"), - []byte(strings.Replace(bpTOMLContents, "##API_VERSION##", "0.5", 1)), - 0644), - ).To(Succeed()) + var b bytes.Buffer + err := buildpackTOML.Execute(&b, map[string]string{"APIVersion": "0.5"}) + Expect(err).ToNot(HaveOccurred()) + + Expect(ioutil.WriteFile(filepath.Join(buildpackPath, "buildpack.toml"), b.Bytes(), 0644)).To(Succeed()) layer := libcnb.Layer{ Name: "test-name",