extensions get the same targets validation as buildpacks (#1108)

* extensions get the same targets validation as buildpacks Signed-off-by: Joe Kimmel <[email protected]> * extensions default to */* for os/arch detection Signed-off-by: Joe Kimmel <[email protected]> --------- Signed-off-by: Joe Kimmel <[email protected]>
buildpacks · Jun 6, 2023 · f9b8e93 · f9b8e93
1 parent 28084b7
commit f9b8e93
Show file tree

Hide file tree

Showing 8 changed files with 105 additions and 34 deletions.
diff --git a/buildpack/bp_descriptor.go b/buildpack/bp_descriptor.go
@@ -124,6 +124,10 @@ func (d *BpDescriptor) String() string {
 	return d.Buildpack.Name + " " + d.Buildpack.Version
 }
 
+func (d *BpDescriptor) TargetsList() []TargetMetadata {
+	return d.Targets
+}
+
 func (bg Group) Append(group ...Group) Group {
 	for _, g := range group {
 		bg.Group = append(bg.Group, g.Group...)

diff --git a/buildpack/descriptor.go b/buildpack/descriptor.go
@@ -9,8 +9,11 @@ const (
 type Descriptor interface {
 	API() string
 	Homepage() string
+	TargetsList() []TargetMetadata
 }
 
+// BaseInfo is information shared by both buildpacks and extensions.
+// For buildpacks it winds up under the toml `buildpack` key along with SBOM info, but extensions have no SBOMs.
 type BaseInfo struct {
 	ClearEnv bool   `toml:"clear-env,omitempty"`
 	Homepage string `toml:"homepage,omitempty"`

diff --git a/buildpack/ext_descriptor.go b/buildpack/ext_descriptor.go
@@ -3,15 +3,17 @@
 package buildpack
 
 import (
+	"os"
 	"path/filepath"
 
 	"github.com/BurntSushi/toml"
 )
 
 type ExtDescriptor struct {
-	WithAPI     string  `toml:"api"`
-	Extension   ExtInfo `toml:"extension"`
-	WithRootDir string  `toml:"-"`
+	WithAPI     string           `toml:"api"`
+	Extension   ExtInfo          `toml:"extension"`
+	WithRootDir string           `toml:"-"`
+	Targets     []TargetMetadata `toml:"targets"`
 }
 
 type ExtInfo struct {
@@ -29,7 +31,38 @@ func ReadExtDescriptor(path string) (*ExtDescriptor, error) {
 	if descriptor.WithRootDir, err = filepath.Abs(filepath.Dir(path)); err != nil {
 		return &ExtDescriptor{}, err
 	}
-	return descriptor, nil
+	err = descriptor.inferTargets()
+	return descriptor, err
+}
+
+func (d *ExtDescriptor) inferTargets() error {
+	if len(d.Targets) == 0 {
+		binDir := filepath.Join(d.WithRootDir, "bin")
+		if stat, _ := os.Stat(binDir); stat != nil {
+			binFiles, err := os.ReadDir(binDir)
+			if err != nil {
+				return err
+			}
+			var windowsDetected, linuxDetected bool
+			for i := 0; i < len(binFiles); i++ { // detect and generate files are optional
+				bf := binFiles[len(binFiles)-i-1] // we're iterating backwards b/c os.ReadDir sorts "foo.exe" after "foo" but we want to preferentially detect windows first.
+				fname := bf.Name()
+				if !windowsDetected && (fname == "detect.exe" || fname == "detect.bat" || fname == "generate.exe" || fname == "generate.bat") {
+					d.Targets = append(d.Targets, TargetMetadata{OS: "windows", Arch: "*"})
+					windowsDetected = true
+				}
+				if !linuxDetected && (fname == "detect" || fname == "generate") {
+					d.Targets = append(d.Targets, TargetMetadata{OS: "linux", Arch: "*"})
+					linuxDetected = true
+				}
+			}
+		}
+	}
+	// fallback: if nothing worked just mark it */*
+	if len(d.Targets) == 0 {
+		d.Targets = append(d.Targets, TargetMetadata{OS: "*", Arch: "*"})
+	}
+	return nil
 }
 
 func (d *ExtDescriptor) API() string {
@@ -51,3 +84,7 @@ func (d *ExtDescriptor) RootDir() string {
 func (d *ExtDescriptor) String() string {
 	return d.Extension.Name + " " + d.Extension.Version
 }
+
+func (d *ExtDescriptor) TargetsList() []TargetMetadata {
+	return d.Targets
+}
diff --git a/buildpack/ext_descriptor_test.go b/buildpack/ext_descriptor_test.go
@@ -27,6 +27,25 @@ func testExtDescriptor(t *testing.T, when spec.G, it spec.S) {
 			h.AssertEq(t, descriptor.Extension.Name, "Extension A")
 			h.AssertEq(t, descriptor.Extension.Version, "v1")
 			h.AssertEq(t, descriptor.Extension.Homepage, "Extension A Homepage")
+			h.AssertEq(t, len(descriptor.Targets), 1)
+			h.AssertEq(t, descriptor.Targets[0].OS, "linux")
+			h.AssertEq(t, descriptor.Targets[0].Arch, "*")
+		})
+		it("infers */* if there's no files to infer from", func() {
+			path := filepath.Join("testdata", "extension", "by-id", "B", "v1", "extension.toml")
+			descriptor, err := buildpack.ReadExtDescriptor(path)
+			h.AssertNil(t, err)
+			h.AssertEq(t, len(descriptor.Targets), 1)
+			h.AssertEq(t, descriptor.Targets[0].OS, "*")
+			h.AssertEq(t, descriptor.Targets[0].Arch, "*")
+		})
+		it("slices, it dices, it even does windows", func() {
+			path := filepath.Join("testdata", "extension", "by-id", "D", "v1", "extension.toml")
+			descriptor, err := buildpack.ReadExtDescriptor(path)
+			h.AssertNil(t, err)
+			h.AssertEq(t, len(descriptor.Targets), 1)
+			h.AssertEq(t, descriptor.Targets[0].OS, "windows")
+			h.AssertEq(t, descriptor.Targets[0].Arch, "*")
 		})
 	})
 }
diff --git a/buildpack/testdata/extension/by-id/D/v1/bin/detect.bat b/buildpack/testdata/extension/by-id/D/v1/bin/detect.bat
@@ -0,0 +1 @@
+ECHO "I am batchman"
diff --git a/buildpack/testdata/extension/by-id/D/v1/bin/generate.bat b/buildpack/testdata/extension/by-id/D/v1/bin/generate.bat
@@ -0,0 +1,3 @@
+ECHO "like most .bats, I am using ECHOlocation"
+ECHO %cd%
+
diff --git a/buildpack/testdata/extension/by-id/D/v1/extension.toml b/buildpack/testdata/extension/by-id/D/v1/extension.toml
@@ -0,0 +1,7 @@
+api = "0.9"
+
+[extension]
+id = "A"
+name = "Extension A"
+version = "v1"
+homepage = "Extension A Homepage"
diff --git a/detector.go b/detector.go
@@ -221,12 +221,11 @@ func (d *Detector) detectGroup(group buildpack.Group, done []buildpack.GroupElem
 
 		// Lookup element in store.  <-- "the store" is the directory where all the buildpacks are.
 		var (
-			descriptor   buildpack.Descriptor
-			bpDescriptor *buildpack.BpDescriptor
-			err          error
+			descriptor buildpack.Descriptor
+			err        error
 		)
 		if groupEl.Kind() == buildpack.KindBuildpack {
-			bpDescriptor, err = d.DirStore.LookupBp(groupEl.ID, groupEl.Version)
+			bpDescriptor, err := d.DirStore.LookupBp(groupEl.ID, groupEl.Version)
 			if err != nil {
 				return nil, nil, err
 			}
@@ -237,39 +236,37 @@ func (d *Detector) detectGroup(group buildpack.Group, done []buildpack.GroupElem
 				// FIXME: cyclical references lead to infinite recursion
 				return d.detectOrder(order, done, group.Group[i+1:], groupEl.Optional, wg)
 			}
-
-			if d.PlatformAPI.AtLeast("0.12") {
-				targetMatch := false
-				if isWildcard(d.AnalyzeMD.RunImageTarget()) || hasWildcard(bpDescriptor.Targets) {
-					targetMatch = true
-				} else {
-					for i := range bpDescriptor.Targets {
-						d.Logger.Debugf("Checking for match against descriptor:", bpDescriptor.Targets[i])
-						if platform.TargetSatisfiedForBuild(*d.AnalyzeMD.RunImage.TargetMetadata, bpDescriptor.Targets[i]) {
-							targetMatch = true
-							break
-						}
-					}
-				}
-				if !targetMatch && !groupEl.Optional {
-					markDone(groupEl, bpDescriptor)
-					d.Runs.Store(
-						keyFor(groupEl),
-						buildpack.DetectOutputs{
-							Code: -1,
-							Err:  fmt.Errorf("unable to satisfy Target OS/Arch constraints; run image: %v, buildpack: %v", d.AnalyzeMD.RunImage.TargetMetadata, bpDescriptor.Targets),
-						})
-					continue
-				}
-			}
-
 			descriptor = bpDescriptor // standardize the type so below we don't have to care whether it was an extension
 		} else {
 			descriptor, err = d.DirStore.LookupExt(groupEl.ID, groupEl.Version)
 			if err != nil {
 				return nil, nil, err
 			}
 		}
+		if d.PlatformAPI.AtLeast("0.12") {
+			targetMatch := false
+			if isWildcard(d.AnalyzeMD.RunImageTarget()) || hasWildcard(descriptor.TargetsList()) {
+				targetMatch = true
+			} else {
+				for i := range descriptor.TargetsList() {
+					d.Logger.Debugf("Checking for match against descriptor:", descriptor.TargetsList()[i])
+					if platform.TargetSatisfiedForBuild(*d.AnalyzeMD.RunImage.TargetMetadata, descriptor.TargetsList()[i]) {
+						targetMatch = true
+						break
+					}
+				}
+			}
+			if !targetMatch && !groupEl.Optional {
+				markDone(groupEl, descriptor)
+				d.Runs.Store(
+					keyFor(groupEl),
+					buildpack.DetectOutputs{
+						Code: -1,
+						Err:  fmt.Errorf("unable to satisfy Target OS/Arch constraints; run image: %v, buildpack: %v", d.AnalyzeMD.RunImage.TargetMetadata, descriptor.TargetsList()),
+					})
+				continue
+			}
+		}
 
 		markDone(groupEl, descriptor)
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		ECHO "like most .bats, I am using ECHOlocation"
		ECHO %cd%