From 105808ccf0e2bfb63d97dff3cf47cd4c6b7e2eef Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Fri, 25 Aug 2023 11:32:57 -0400 Subject: [PATCH] Removes backwards compatible glue for unsupported buildpack APIs (#1188) Fixes https://github.com/buildpacks/lifecycle/issues/1187 Signed-off-by: Natalie Arellano --- builder.go | 7 ---- builder_test.go | 45 ------------------------ buildpack/bom.go | 55 ++--------------------------- buildpack/build.go | 7 ---- buildpack/detect.go | 8 +++-- buildpack/detect_test.go | 33 +++++++---------- buildpack/files.go | 33 ----------------- detector.go | 5 --- detector_test.go | 76 ---------------------------------------- 9 files changed, 19 insertions(+), 250 deletions(-) diff --git a/builder.go b/builder.go index f429068d1..c4872d42b 100644 --- a/builder.go +++ b/builder.go @@ -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 { diff --git a/builder_test.go b/builder_test.go index 6fb823fc2..190e6f692 100644 --- a/builder_test.go +++ b/builder_test.go @@ -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") diff --git a/buildpack/bom.go b/buildpack/bom.go index d1b9f8a8f..e48e12327 100644 --- a/buildpack/bom.go +++ b/buildpack/bom.go @@ -1,7 +1,6 @@ package buildpack import ( - "errors" "fmt" "github.com/buildpacks/lifecycle/log" @@ -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} } @@ -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 { diff --git a/buildpack/build.go b/buildpack/build.go index 993de16e6..f5e3258c5 100644 --- a/buildpack/build.go +++ b/buildpack/build.go @@ -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 { diff --git a/buildpack/detect.go b/buildpack/detect.go index 34372c6d6..f23c5ef62 100644 --- a/buildpack/detect.go +++ b/buildpack/detect.go @@ -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 { @@ -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 @@ -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) diff --git a/buildpack/detect_test.go b/buildpack/detect_test.go index 8dd0cc27d..4a4db65f0 100644 --- a/buildpack/detect_test.go +++ b/buildpack/detect_test.go @@ -5,7 +5,6 @@ import ( "os" "path/filepath" "runtime" - "strings" "testing" "github.com/apex/log" @@ -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() { @@ -233,27 +232,23 @@ 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") @@ -261,16 +256,12 @@ func testDetect(t *testing.T, when spec.G, it spec.S) { 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`) }) }) diff --git a/buildpack/files.go b/buildpack/files.go index 9636e4ebc..bd42a40cd 100644 --- a/buildpack/files.go +++ b/buildpack/files.go @@ -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 != "" diff --git a/detector.go b/detector.go index a0c7ad5cb..f140bb6b8 100644 --- a/detector.go +++ b/detector.go @@ -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 diff --git a/detector_test.go b/detector_test.go index f5285ef99..9026c91f3 100644 --- a/detector_test.go +++ b/detector_test.go @@ -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"}},