From a965403652e010523ce7b2e22e3023c70f171cf6 Mon Sep 17 00:00:00 2001 From: Josh Ghiloni Date: Thu, 4 Nov 2021 10:02:40 -0400 Subject: [PATCH 1/3] Add struct tag to Buildpack.Path field Most Go applications use github.com/burntsushi/toml as their toml parsing library. When struct tags are omitted, the library will do case-insensitive deserialization, so a property called "path" will successfully unmarshal into the Path struct field. However, if you marshal a Buildpack instance to toml, the "Path = " field appears regardless of whether it is set or not, and technically violates the buildpack toml spec. This PR adds a struct tag to not only marshal the field to the "path" property in the resultant toml, but also adds the "omitempty" modifier to ignore the field altogether when not set. Signed-off-by: Josh Ghiloni --- buildpack.go | 2 +- buildpack_test.go | 65 +++++++++++++++++++++++++++++++++++++++++++++++ init_test.go | 1 + 3 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 buildpack_test.go diff --git a/buildpack.go b/buildpack.go index acee58e..6ae4520 100644 --- a/buildpack.go +++ b/buildpack.go @@ -91,7 +91,7 @@ type Buildpack struct { Info BuildpackInfo `toml:"buildpack"` // Path is the path to the buildpack. - Path string + Path string `toml:"path,omitempty"` // Stacks is the collection of stacks supported by the buildpack. Stacks []BuildpackStack `toml:"stacks"` diff --git a/buildpack_test.go b/buildpack_test.go new file mode 100644 index 0000000..aa2d6b0 --- /dev/null +++ b/buildpack_test.go @@ -0,0 +1,65 @@ +/* + * Copyright 2018-2020 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 libcnb_test + +import ( + "bytes" + "testing" + + "github.com/BurntSushi/toml" + "github.com/buildpacks/libcnb" + "github.com/sclevine/spec" + + . "github.com/onsi/gomega" +) + +func testBuildpackTOML(t *testing.T, context spec.G, it spec.S) { + var ( + Expect = NewWithT(t).Expect + ) + + it("does not serialize an empty Path field", func() { + bp := libcnb.Buildpack{ + API: "0.6", + Info: libcnb.BuildpackInfo{ + ID: "test-buildpack/sample", + Name: "sample", + }, + } + + output := &bytes.Buffer{} + + Expect(toml.NewEncoder(output).Encode(bp)).To(Succeed()) + Expect(output.String()).NotTo(ContainSubstring("Path = ")) + }) + + it("serializes a non-empty Path field", func() { + bp := libcnb.Buildpack{ + API: "0.6", + Info: libcnb.BuildpackInfo{ + ID: "test-buildpack/sample", + Name: "sample", + }, + Path: "../buildpack", + } + + output := &bytes.Buffer{} + + Expect(toml.NewEncoder(output).Encode(bp)).To(Succeed()) + Expect(output.String()).To(ContainSubstring(`path = "../buildpack"`)) + }) +} diff --git a/init_test.go b/init_test.go index 313577b..f0c9f07 100644 --- a/init_test.go +++ b/init_test.go @@ -33,5 +33,6 @@ func TestUnit(t *testing.T) { suite("Main", testMain) suite("Platform", testPlatform) suite("ExecD", testExecD) + suite("BuildpackTOML", testBuildpackTOML) suite.Run(t) } From 33e2828ac73bbbf29d7b774d1c12474e27641d5d Mon Sep 17 00:00:00 2001 From: Josh Ghiloni Date: Thu, 4 Nov 2021 11:20:15 -0400 Subject: [PATCH 2/3] Omit Path field from serialization altogether Signed-off-by: Josh Ghiloni --- buildpack.go | 2 +- buildpack_test.go | 19 ++----------------- 2 files changed, 3 insertions(+), 18 deletions(-) diff --git a/buildpack.go b/buildpack.go index 6ae4520..aa16af6 100644 --- a/buildpack.go +++ b/buildpack.go @@ -91,7 +91,7 @@ type Buildpack struct { Info BuildpackInfo `toml:"buildpack"` // Path is the path to the buildpack. - Path string `toml:"path,omitempty"` + Path string `toml:"-"` // Stacks is the collection of stacks supported by the buildpack. Stacks []BuildpackStack `toml:"stacks"` diff --git a/buildpack_test.go b/buildpack_test.go index aa2d6b0..03e823b 100644 --- a/buildpack_test.go +++ b/buildpack_test.go @@ -32,22 +32,7 @@ func testBuildpackTOML(t *testing.T, context spec.G, it spec.S) { Expect = NewWithT(t).Expect ) - it("does not serialize an empty Path field", func() { - bp := libcnb.Buildpack{ - API: "0.6", - Info: libcnb.BuildpackInfo{ - ID: "test-buildpack/sample", - Name: "sample", - }, - } - - output := &bytes.Buffer{} - - Expect(toml.NewEncoder(output).Encode(bp)).To(Succeed()) - Expect(output.String()).NotTo(ContainSubstring("Path = ")) - }) - - it("serializes a non-empty Path field", func() { + it("does not serialize the Path field", func() { bp := libcnb.Buildpack{ API: "0.6", Info: libcnb.BuildpackInfo{ @@ -60,6 +45,6 @@ func testBuildpackTOML(t *testing.T, context spec.G, it spec.S) { output := &bytes.Buffer{} Expect(toml.NewEncoder(output).Encode(bp)).To(Succeed()) - Expect(output.String()).To(ContainSubstring(`path = "../buildpack"`)) + Expect(output.String()).NotTo(ContainSubstring("ath = ")) // match on path and Path }) } From b2b6b4b4c1654938eeefdfb6cdcdd954b45df426 Mon Sep 17 00:00:00 2001 From: Josh Ghiloni Date: Thu, 4 Nov 2021 13:33:50 -0400 Subject: [PATCH 3/3] Make substring match more explicit in buildpack toml test Signed-off-by: Josh Ghiloni --- buildpack_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildpack_test.go b/buildpack_test.go index 03e823b..b357d6e 100644 --- a/buildpack_test.go +++ b/buildpack_test.go @@ -45,6 +45,6 @@ func testBuildpackTOML(t *testing.T, context spec.G, it spec.S) { output := &bytes.Buffer{} Expect(toml.NewEncoder(output).Encode(bp)).To(Succeed()) - Expect(output.String()).NotTo(ContainSubstring("ath = ")) // match on path and Path + Expect(output.String()).NotTo(Or(ContainSubstring("Path = "), ContainSubstring("path = "))) }) }