Skip to content

Commit

Permalink
Removes backwards compatible glue for unsupported buildpack APIs (#1188)
Browse files Browse the repository at this point in the history 
Fixes #1187

Signed-off-by: Natalie Arellano <[email protected]>
  • Loading branch information
@natalieparellano
natalieparellano authored Aug 25, 2023
1 parent a12fbff commit 105808c
Show file tree
Hide file tree
Showing 9 changed files with 19 additions and 250 deletions.
7 changes: 0 additions & 7 deletions builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,6 @@ func (b *Builder) Build() (*files.BuildMetadata, error) {
b.Logger.Debugf("Finished running build for buildpack %s", bp)
}

if b.PlatformAPI.LessThan("0.4") {
b.Logger.Debug("Updating BOM entries")
for i := range launchBOM {
launchBOM[i].ConvertMetadataToVersion()
}
}

if b.PlatformAPI.AtLeast("0.8") {
b.Logger.Debug("Copying SBOM files")
if err := b.copySBOMFiles(inputs.LayersDir, bomFiles); err != nil {
Expand Down
45 changes: 0 additions & 45 deletions builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -952,51 +952,6 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) {
})
})

when("platform api < 0.4", func() {
it.Before(func() {
builder.PlatformAPI = api.MustParse("0.3")
})

when("build metadata", func() {
when("bom", func() {
it("converts metadata.version to top level version", func() {
bpA := &buildpack.BpDescriptor{Buildpack: buildpack.BpInfo{BaseInfo: buildpack.BaseInfo{ID: "A", Version: "v1"}}}
dirStore.EXPECT().LookupBp("A", "v1").Return(bpA, nil)
executor.EXPECT().Build(*bpA, gomock.Any(), gomock.Any()).Return(buildpack.BuildOutputs{
LaunchBOM: []buildpack.BOMEntry{
{
Require: buildpack.Require{
Name: "dep1",
Metadata: map[string]interface{}{"version": string("v1")},
},
Buildpack: buildpack.GroupElement{ID: "A", Version: "v1"},
},
},
}, nil)
bpB := &buildpack.BpDescriptor{Buildpack: buildpack.BpInfo{BaseInfo: buildpack.BaseInfo{ID: "B", Version: "v1"}}}
dirStore.EXPECT().LookupBp("B", "v2").Return(bpB, nil)
executor.EXPECT().Build(*bpB, gomock.Any(), gomock.Any())

metadata, err := builder.Build()
h.AssertNil(t, err)

if s := cmp.Diff(metadata.BOM, []buildpack.BOMEntry{
{
Require: buildpack.Require{
Name: "dep1",
Version: "v1",
Metadata: map[string]interface{}{"version": string("v1")},
},
Buildpack: buildpack.GroupElement{ID: "A", Version: "v1"},
},
}); s != "" {
t.Fatalf("Unexpected:\n%s\n", s)
}
})
})
})
})

when("platform api < 0.6", func() {
it.Before(func() {
builder.PlatformAPI = api.MustParse("0.5")
Expand Down
55 changes: 2 additions & 53 deletions buildpack/bom.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package buildpack

import (
"errors"
"fmt"

"github.com/buildpacks/lifecycle/log"
Expand All @@ -11,7 +10,8 @@ type BOMValidator interface {
ValidateBOM(GroupElement, []BOMEntry) ([]BOMEntry, error)
}

func NewBOMValidator(bpAPI string, layersDir string, logger log.Logger) BOMValidator {
// NewBOMValidator returns a validator for the legacy unstructured BOM.
func NewBOMValidator(_ string, layersDir string, logger log.Logger) BOMValidator {
return &defaultBOMValidator{logger: logger, layersDir: layersDir}
}

Expand Down Expand Up @@ -55,57 +55,6 @@ func (v *defaultBOMValidator) processBOM(buildpack GroupElement, bom []BOMEntry)
return WithBuildpack(buildpack, bom)
}

type v05To06BOMValidator struct{}

func (v *v05To06BOMValidator) ValidateBOM(bp GroupElement, bom []BOMEntry) ([]BOMEntry, error) {
if err := v.validateBOM(bom); err != nil {
return []BOMEntry{}, err
}
return v.processBOM(bp, bom), nil
}

func (v *v05To06BOMValidator) validateBOM(bom []BOMEntry) error {
for _, entry := range bom {
if entry.Version != "" {
return fmt.Errorf("bom entry '%s' has a top level version which is not allowed. The buildpack should instead set metadata.version", entry.Name)
}
}
return nil
}

func (v *v05To06BOMValidator) processBOM(buildpack GroupElement, bom []BOMEntry) []BOMEntry {
return WithBuildpack(buildpack, bom)
}

type legacyBOMValidator struct{}

func (v *legacyBOMValidator) ValidateBOM(bp GroupElement, bom []BOMEntry) ([]BOMEntry, error) {
if err := v.validateBOM(bom); err != nil {
return []BOMEntry{}, err
}
return v.processBOM(bp, bom), nil
}

func (v *legacyBOMValidator) validateBOM(bom []BOMEntry) error {
for _, entry := range bom {
if version, ok := entry.Metadata["version"]; ok {
metadataVersion := fmt.Sprintf("%v", version)
if entry.Version != "" && entry.Version != metadataVersion {
return errors.New("top level version does not match metadata version")
}
}
}
return nil
}

func (v *legacyBOMValidator) processBOM(buildpack GroupElement, bom []BOMEntry) []BOMEntry {
bom = WithBuildpack(buildpack, bom)
for i := range bom {
bom[i].convertVersionToMetadata()
}
return bom
}

func WithBuildpack(bp GroupElement, bom []BOMEntry) []BOMEntry {
var out []BOMEntry
for _, entry := range bom {
Expand Down
7 changes: 0 additions & 7 deletions buildpack/build.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,6 @@ type BuildExecutor interface {
type DefaultBuildExecutor struct{}

func (e *DefaultBuildExecutor) Build(d BpDescriptor, inputs BuildInputs, logger log.Logger) (BuildOutputs, error) {
if api.MustParse(d.WithAPI).Equal(api.MustParse("0.2")) {
logger.Debug("Updating plan entries")
for i := range inputs.Plan.Entries {
inputs.Plan.Entries[i].convertMetadataToVersion()
}
}

logger.Debug("Creating plan directory")
planDir, err := os.MkdirTemp("", launch.EscapeID(d.Buildpack.ID)+"-")
if err != nil {
Expand Down
8 changes: 5 additions & 3 deletions buildpack/detect.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (e *DefaultDetectExecutor) Detect(d Descriptor, inputs DetectInputs, logger
}
}

func detectBp(d BpDescriptor, inputs DetectInputs, logger log.Logger) DetectOutputs {
func detectBp(d BpDescriptor, inputs DetectInputs, _ log.Logger) DetectOutputs {
planDir, planPath, err := processBuildpackPaths()
defer os.RemoveAll(planDir)
if err != nil {
Expand All @@ -78,7 +78,8 @@ func detectBp(d BpDescriptor, inputs DetectInputs, logger log.Logger) DetectOutp
result.Code = -1
}
if result.hasTopLevelVersions() || result.Or.hasTopLevelVersions() {
logger.Warnf(`buildpack %s has a "version" key. This key is deprecated in build plan requirements in buildpack API 0.3. "metadata.version" should be used instead`, d.Buildpack.ID)
result.Err = fmt.Errorf(`buildpack %s has a "version" key which is not supported. "metadata.version" should be used instead`, d.Buildpack.ID)
result.Code = -1
}

return result
Expand Down Expand Up @@ -115,7 +116,8 @@ func detectExt(d ExtDescriptor, inputs DetectInputs, logger log.Logger) DetectOu
result.Code = -1
}
if result.hasTopLevelVersions() || result.Or.hasTopLevelVersions() {
logger.Warnf(`extension %s has a "version" key. This key is deprecated in build plan requirements in buildpack API 0.3. "metadata.version" should be used instead`, d.Extension.ID)
result.Err = fmt.Errorf(`extension %s has a "version" key which is not supported. "metadata.version" should be used instead`, d.Extension.ID)
result.Code = -1
}
if result.hasRequires() || result.Or.hasRequires() {
result.Err = fmt.Errorf(`extension %s outputs "requires" which is not allowed`, d.Extension.ID)
Expand Down
33 changes: 12 additions & 21 deletions buildpack/detect_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"os"
"path/filepath"
"runtime"
"strings"
"testing"

"github.com/apex/log"
Expand Down Expand Up @@ -216,7 +215,7 @@ func testDetect(t *testing.T, when spec.G, it spec.S) {
if err == nil {
t.Fatalf("Expected error")
}
h.AssertEq(t, err.Error(), `buildpack A has a "version" key and a "metadata.version" which cannot be specified together. "metadata.version" should be used instead`)
h.AssertEq(t, err.Error(), `buildpack A has a "version" key which is not supported. "metadata.version" should be used instead`)
})

it("errors if there is an alternate plan with both a top level version and a metadata version", func() {
Expand All @@ -233,44 +232,36 @@ func testDetect(t *testing.T, when spec.G, it spec.S) {
if err == nil {
t.Fatalf("Expected error")
}
h.AssertEq(t, err.Error(), `buildpack A has a "version" key and a "metadata.version" which cannot be specified together. "metadata.version" should be used instead`)
h.AssertEq(t, err.Error(), `buildpack A has a "version" key which is not supported. "metadata.version" should be used instead`)
})

it("warns if the plan has a top level version", func() {
it("errors if the plan has a top level version", func() {
toappfile("\n[[requires]]\n name = \"dep2\"\n version = \"some-version\"", "detect-plan-A-v1.toml")

detectRun := executor.Detect(descriptor, inputs, logger)

h.AssertEq(t, detectRun.Code, 0)
h.AssertEq(t, detectRun.Code, -1)
err := detectRun.Err
if err != nil {
t.Fatalf("Unexpected error:\n%s\n", err)
}
if s := h.AllLogs(logHandler); !strings.Contains(s,
`buildpack A has a "version" key. This key is deprecated in build plan requirements in buildpack API 0.3. "metadata.version" should be used instead`,
) {
t.Fatalf("Expected log to contain warning:\n%s\n", s)
if err == nil {
t.Fatalf("Expected error")
}
h.AssertEq(t, err.Error(), `buildpack A has a "version" key which is not supported. "metadata.version" should be used instead`)
})

it("warns if there is an alternate plan with a top level version", func() {
it("errors if there is an alternate plan with a top level version", func() {
toappfile("\n[[provides]]\n name = \"dep2-missing\"", "detect-plan-A-v1.toml")
toappfile("\n[[or]]", "detect-plan-A-v1.toml")
toappfile("\n[[or.provides]]\n name = \"dep1-present\"", "detect-plan-A-v1.toml")
toappfile("\n[[or.requires]]\n name = \"dep1-present\"\n version = \"some-version\"", "detect-plan-A-v1.toml")

detectRun := executor.Detect(descriptor, inputs, logger)

h.AssertEq(t, detectRun.Code, 0)
h.AssertEq(t, detectRun.Code, -1)
err := detectRun.Err
if err != nil {
t.Fatalf("Unexpected error:\n%s\n", err)
}
if s := h.AllLogs(logHandler); !strings.Contains(s,
`buildpack A has a "version" key. This key is deprecated in build plan requirements in buildpack API 0.3. "metadata.version" should be used instead`,
) {
t.Fatalf("Expected log to contain warning:\n%s\n", s)
if err == nil {
t.Fatalf("Expected error")
}
h.AssertEq(t, err.Error(), `buildpack A has a "version" key which is not supported. "metadata.version" should be used instead`)
})
})

Expand Down
33 changes: 0 additions & 33 deletions buildpack/files.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,45 +108,12 @@ type BOMEntry struct {
Buildpack GroupElement `toml:"buildpack" json:"buildpack"`
}

func (bom *BOMEntry) ConvertMetadataToVersion() {
if version, ok := bom.Metadata["version"]; ok {
metadataVersion := fmt.Sprintf("%v", version)
bom.Version = metadataVersion
}
}

func (bom *BOMEntry) convertVersionToMetadata() {
if bom.Version != "" {
if bom.Metadata == nil {
bom.Metadata = make(map[string]interface{})
}
bom.Metadata["version"] = bom.Version
bom.Version = ""
}
}

type Require struct {
Name string `toml:"name" json:"name"`
Version string `toml:"version,omitempty" json:"version,omitempty"`
Metadata map[string]interface{} `toml:"metadata" json:"metadata"`
}

func (r *Require) convertMetadataToVersion() {
if version, ok := r.Metadata["version"]; ok {
r.Version = fmt.Sprintf("%v", version)
}
}

func (r *Require) ConvertVersionToMetadata() {
if r.Version != "" {
if r.Metadata == nil {
r.Metadata = make(map[string]interface{})
}
r.Metadata["version"] = r.Version
r.Version = ""
}
}

func (r *Require) hasDoublySpecifiedVersions() bool {
if _, ok := r.Metadata["version"]; ok {
return r.Version != ""
Expand Down
5 changes: 0 additions & 5 deletions detector.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,6 @@ func (d *Detector) DetectOrder(order buildpack.Order) (buildpack.Group, files.Pl
} else if err == ErrFailedDetection {
err = buildpack.NewError(err, buildpack.ErrTypeFailedDetection)
}
for i := range planEntries {
for j := range planEntries[i].Requires {
planEntries[i].Requires[j].ConvertVersionToMetadata()
}
}
return buildpack.Group{Group: filter(detected, buildpack.KindBuildpack), GroupExtensions: filter(detected, buildpack.KindExtension)},
files.Plan{Entries: planEntries},
err
Expand Down
76 changes: 0 additions & 76 deletions detector_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -568,82 +568,6 @@ func testDetector(t *testing.T, when spec.G, it spec.S) {
}
})

it("converts top level versions to metadata versions", func() {
bpA1 := &buildpack.BpDescriptor{
Buildpack: buildpack.BpInfo{BaseInfo: buildpack.BaseInfo{ID: "A", Version: "v1"}},
}
dirStore.EXPECT().LookupBp("A", "v1").Return(bpA1, nil).AnyTimes()

bpB1 := &buildpack.BpDescriptor{
Buildpack: buildpack.BpInfo{BaseInfo: buildpack.BaseInfo{ID: "B", Version: "v1"}},
}
dirStore.EXPECT().LookupBp("B", "v1").Return(bpB1, nil).AnyTimes()

executor.EXPECT().Detect(bpA1, gomock.Any(), gomock.Any())
executor.EXPECT().Detect(bpB1, gomock.Any(), gomock.Any())

group := []buildpack.GroupElement{
{ID: "A", Version: "v1"},
{ID: "B", Version: "v1"},
}
resolver.EXPECT().Resolve(group, detector.Runs).Return(group, []files.BuildPlanEntry{
{
Providers: []buildpack.GroupElement{
{ID: "A", Version: "v1"},
},
Requires: []buildpack.Require{
{
Name: "dep1",
Version: "some-version",
},
},
},
{
Providers: []buildpack.GroupElement{
{ID: "B", Version: "v1"},
},
Requires: []buildpack.Require{
{
Name: "dep2",
Version: "some-already-exists-version",
Metadata: map[string]interface{}{"version": "some-already-exists-version"},
},
},
},
}, nil)

detector.Order = buildpack.Order{{Group: group}}
found, plan, err := detector.Detect()
if err != nil {
t.Fatalf("Unexpected error:\n%s\n", err)
}

if s := cmp.Diff(found, buildpack.Group{Group: group}); s != "" {
t.Fatalf("Unexpected group:\n%s\n", s)
}

if !hasEntries(plan.Entries, []files.BuildPlanEntry{
{
Providers: []buildpack.GroupElement{
{ID: "A", Version: "v1"},
},
Requires: []buildpack.Require{
{Name: "dep1", Metadata: map[string]interface{}{"version": "some-version"}},
},
},
{
Providers: []buildpack.GroupElement{
{ID: "B", Version: "v1"},
},
Requires: []buildpack.Require{
{Name: "dep2", Metadata: map[string]interface{}{"version": "some-already-exists-version"}},
},
},
}) {
t.Fatalf("Unexpected entries:\n%+v\n", plan.Entries)
}
})

it("updates detect runs for each buildpack", func() {
bpA1 := &buildpack.BpDescriptor{
Buildpack: buildpack.BpInfo{BaseInfo: buildpack.BaseInfo{ID: "A", Version: "v1"}},
Expand Down

0 comments on commit 105808c

Please sign in to comment.