Replace 'BuildpackMetadata' with 'BuildpackInfo'

- Also refactored 'GetLocatorType()' to use 'BuildpackInfo' Signed-off-by: Andrew Meyer <[email protected]> Signed-off-by: Micah Young <[email protected]>
kvedurmu · Feb 12, 2020 · c5992cd · c5992cd
1 parent 62f9bb6
commit c5992cd
Show file tree

Hide file tree

Showing 11 changed files with 74 additions and 106 deletions.
diff --git a/build.go b/build.go
@@ -58,8 +58,6 @@ type ContainerConfig struct {
 	Network string
 }
 
-const fromBuilderPrefix = "from=builder"
-
 func (c *Client) Build(ctx context.Context, opts BuildOptions) error {
 	imageRef, err := c.parseTagReference(opts.Image)
 	if err != nil {
@@ -99,7 +97,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error {
 		return err
 	}
 
-	fetchedBPs, order, err := c.processBuildpacks(ctx, bldr.Order(), opts.Buildpacks)
+	fetchedBPs, order, err := c.processBuildpacks(ctx, bldr.Buildpacks(), bldr.Order(), opts.Buildpacks)
 	if err != nil {
 		return err
 	}
@@ -383,11 +381,16 @@ func (c *Client) processProxyConfig(config *ProxyConfig) ProxyConfig {
 // 	----------
 // 	- group:
 //		- A
-func (c *Client) processBuildpacks(ctx context.Context, builderOrder dist.Order, declaredBPs []string) (fetchedBPs []dist.Buildpack, order dist.Order, err error) {
+func (c *Client) processBuildpacks(ctx context.Context, builderBPs []dist.BuildpackInfo, builderOrder dist.Order, declaredBPs []string) (fetchedBPs []dist.Buildpack, order dist.Order, err error) {
 	order = dist.Order{{Group: []dist.BuildpackRef{}}}
 	for _, bp := range declaredBPs {
-		switch {
-		case bp == fromBuilderPrefix:
+		locatorType, err := buildpack.GetLocatorType(bp, builderBPs)
+		if err != nil {
+			return nil, nil, err
+		}
+
+		switch locatorType {
+		case buildpack.FromBuilderLocator:
 			switch {
 			case len(order) == 0 || len(order[0].Group) == 0:
 				order = builderOrder
@@ -404,13 +407,13 @@ func (c *Client) processBuildpacks(ctx context.Context, builderOrder dist.Order,
 
 				order = newOrder
 			}
-		case isBuildpackID(bp):
+		case buildpack.IDLocator:
 			id, version := buildpack.ParseIDLocator(bp)
 			order = appendBuildpackToOrder(order, dist.BuildpackInfo{
 				ID:      id,
 				Version: version,
 			})
-		default:
+		case buildpack.URILocator:
 			err := ensureBPSupport(bp)
 			if err != nil {
 				return fetchedBPs, order, errors.Wrapf(err, "checking support")
@@ -428,6 +431,8 @@ func (c *Client) processBuildpacks(ctx context.Context, builderOrder dist.Order,
 
 			fetchedBPs = append(fetchedBPs, fetchedBP)
 			order = appendBuildpackToOrder(order, fetchedBP.Descriptor().Info)
+		default:
+			return nil, nil, fmt.Errorf("invalid buildpack string %s", style.Symbol(bp))
 		}
 	}
 
@@ -447,19 +452,6 @@ func appendBuildpackToOrder(order dist.Order, bpInfo dist.BuildpackInfo) (newOrd
 	return newOrder
 }
 
-func isBuildpackID(bp string) bool {
-	if strings.HasPrefix(bp, fromBuilderPrefix+":") {
-		return true
-	}
-
-	if !paths.IsURI(bp) {
-		if _, err := os.Stat(bp); err != nil {
-			return true
-		}
-	}
-	return false
-}
-
 func ensureBPSupport(bpPath string) (err error) {
 	p := bpPath
 	if paths.IsURI(bpPath) {

diff --git a/build_test.go b/build_test.go
@@ -74,13 +74,9 @@ func testBuild(t *testing.T, when spec.G, it spec.S) {
 			"1234",
 			"5678",
 			builder.Metadata{
-				Buildpacks: []builder.BuildpackMetadata{
-					{
-						BuildpackInfo: dist.BuildpackInfo{ID: "buildpack.1.id", Version: "buildpack.1.version"},
-					},
-					{
-						BuildpackInfo: dist.BuildpackInfo{ID: "buildpack.2.id", Version: "buildpack.2.version"},
-					},
+				Buildpacks: []dist.BuildpackInfo{
+					{ID: "buildpack.1.id", Version: "buildpack.1.version"},
+					{ID: "buildpack.2.id", Version: "buildpack.2.version"},
 				},
 				Stack: builder.StackMetadata{
 					RunImage: builder.RunImageMetadata{
@@ -892,7 +888,7 @@ func testBuild(t *testing.T, when spec.G, it spec.S) {
 					ClearCache: true,
 					Buildpacks: []string{"missing.bp@version"},
 				}),
-					"no versions of buildpack 'missing.bp' were found on the builder",
+					"invalid buildpack string 'missing.bp@version'",
 				)
 			})
 
@@ -947,24 +943,18 @@ func testBuild(t *testing.T, when spec.G, it spec.S) {
 								{BuildpackInfo: dist.BuildpackInfo{ID: "some-other-buildpack-id", Version: "some-other-buildpack-version"}},
 							}},
 						})
-						h.AssertEq(t, bldr.Buildpacks(), []builder.BuildpackMetadata{
+						h.AssertEq(t, bldr.Buildpacks(), []dist.BuildpackInfo{
 							{
-								BuildpackInfo: dist.BuildpackInfo{
-									ID:      "buildpack.1.id",
-									Version: "buildpack.1.version",
-								},
+								ID:      "buildpack.1.id",
+								Version: "buildpack.1.version",
 							},
 							{
-								BuildpackInfo: dist.BuildpackInfo{
-									ID:      "buildpack.2.id",
-									Version: "buildpack.2.version",
-								},
+								ID:      "buildpack.2.id",
+								Version: "buildpack.2.version",
 							},
 							{
-								BuildpackInfo: dist.BuildpackInfo{
-									ID:      "some-other-buildpack-id",
-									Version: "some-other-buildpack-version",
-								},
+								ID:      "some-other-buildpack-id",
+								Version: "some-other-buildpack-version",
 							},
 						})
 					})
@@ -1004,11 +994,11 @@ func testBuild(t *testing.T, when spec.G, it spec.S) {
 								{BuildpackInfo: tgzBuildpackInfo},
 							}},
 						})
-						h.AssertEq(t, bldr.Buildpacks(), []builder.BuildpackMetadata{
-							{BuildpackInfo: buildpack1Info},
-							{BuildpackInfo: buildpack2Info},
-							{BuildpackInfo: dirBuildpackInfo},
-							{BuildpackInfo: tgzBuildpackInfo},
+						h.AssertEq(t, bldr.Buildpacks(), []dist.BuildpackInfo{
+							buildpack1Info,
+							buildpack2Info,
+							dirBuildpackInfo,
+							tgzBuildpackInfo,
 						})
 					})
 				})
@@ -1050,10 +1040,10 @@ func testBuild(t *testing.T, when spec.G, it spec.S) {
 								{BuildpackInfo: dist.BuildpackInfo{ID: "some-other-buildpack-id", Version: "some-other-buildpack-version"}},
 							}},
 						})
-						h.AssertEq(t, bldr.Buildpacks(), []builder.BuildpackMetadata{
-							{BuildpackInfo: dist.BuildpackInfo{ID: "buildpack.1.id", Version: "buildpack.1.version"}},
-							{BuildpackInfo: dist.BuildpackInfo{ID: "buildpack.2.id", Version: "buildpack.2.version"}},
-							{BuildpackInfo: dist.BuildpackInfo{ID: "some-other-buildpack-id", Version: "some-other-buildpack-version"}},
+						h.AssertEq(t, bldr.Buildpacks(), []dist.BuildpackInfo{
+							{ID: "buildpack.1.id", Version: "buildpack.1.version"},
+							{ID: "buildpack.2.id", Version: "buildpack.2.version"},
+							{ID: "some-other-buildpack-id", Version: "some-other-buildpack-version"},
 						})
 					})
 				})

diff --git a/create_builder_test.go b/create_builder_test.go
@@ -360,9 +360,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) {
 					ID:      "bp.one",
 					Version: "1.2.3",
 				}
-				h.AssertEq(t, bldr.Buildpacks(), []builder.BuildpackMetadata{{
-					BuildpackInfo: bpInfo,
-				}})
+				h.AssertEq(t, bldr.Buildpacks(), []dist.BuildpackInfo{bpInfo})
 				h.AssertEq(t, bldr.Order(), dist.Order{{
 					Group: []dist.BuildpackRef{{
 						BuildpackInfo: bpInfo,
@@ -417,9 +415,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) {
 					ID:      "bp.one",
 					Version: "1.2.3",
 				}
-				h.AssertEq(t, bldr.Buildpacks(), []builder.BuildpackMetadata{{
-					BuildpackInfo: bpInfo,
-				}})
+				h.AssertEq(t, bldr.Buildpacks(), []dist.BuildpackInfo{bpInfo})
 				h.AssertEq(t, bldr.Order(), dist.Order{{
 					Group: []dist.BuildpackRef{{
 						BuildpackInfo: bpInfo,

diff --git a/inspect_builder.go b/inspect_builder.go
@@ -18,17 +18,12 @@ type BuilderInfo struct {
 	Mixins          []string
 	RunImage        string
 	RunImageMirrors []string
-	Buildpacks      []builder.BuildpackMetadata
+	Buildpacks      []dist.BuildpackInfo
 	Order           dist.Order
 	Lifecycle       builder.LifecycleDescriptor
 	CreatedBy       builder.CreatorMetadata
 }
 
-type BuildpackInfo struct {
-	ID      string
-	Version string
-}
-
 func (c *Client) InspectBuilder(name string, daemon bool) (*BuilderInfo, error) {
 	img, err := c.imageFetcher.Fetch(context.Background(), name, daemon, false)
 	if err != nil {

diff --git a/inspect_builder_test.go b/inspect_builder_test.go
@@ -12,7 +12,6 @@ import (
 	"github.com/sclevine/spec"
 	"github.com/sclevine/spec/report"
 
-	"github.com/buildpacks/pack/internal/builder"
 	"github.com/buildpacks/pack/internal/dist"
 	"github.com/buildpacks/pack/internal/image"
 	"github.com/buildpacks/pack/internal/logging"
@@ -140,11 +139,9 @@ func testInspectBuilder(t *testing.T, when spec.G, it spec.S) {
 					it("sets the buildpacks", func() {
 						builderInfo, err := subject.InspectBuilder("some/builder", useDaemon)
 						h.AssertNil(t, err)
-						h.AssertEq(t, builderInfo.Buildpacks[0], builder.BuildpackMetadata{
-							BuildpackInfo: dist.BuildpackInfo{
-								ID:      "test.bp.one",
-								Version: "1.0.0",
-							},
+						h.AssertEq(t, builderInfo.Buildpacks[0], dist.BuildpackInfo{
+							ID:      "test.bp.one",
+							Version: "1.0.0",
 						})
 					})
 

diff --git a/internal/builder/builder.go b/internal/builder/builder.go
@@ -160,7 +160,7 @@ func (b *Builder) LifecycleDescriptor() LifecycleDescriptor {
 	return b.lifecycleDescriptor
 }
 
-func (b *Builder) Buildpacks() []BuildpackMetadata {
+func (b *Builder) Buildpacks() []dist.BuildpackInfo {
 	return b.metadata.Buildpacks
 }
 
@@ -189,9 +189,7 @@ func (b *Builder) Mixins() []string {
 
 func (b *Builder) AddBuildpack(bp dist.Buildpack) {
 	b.additionalBuildpacks = append(b.additionalBuildpacks, bp)
-	b.metadata.Buildpacks = append(b.metadata.Buildpacks, BuildpackMetadata{
-		BuildpackInfo: bp.Descriptor().Info,
-	})
+	b.metadata.Buildpacks = append(b.metadata.Buildpacks, bp.Descriptor().Info)
 }
 
 func (b *Builder) SetLifecycle(lifecycle Lifecycle) error {
@@ -349,7 +347,7 @@ func (b *Builder) Save(logger logging.Logger) error {
 	return b.image.Save()
 }
 
-func processOrder(buildpacks []BuildpackMetadata, order dist.Order) (dist.Order, error) {
+func processOrder(buildpacks []dist.BuildpackInfo, order dist.Order) (dist.Order, error) {
 	resolvedOrder := dist.Order{}
 
 	for gi, g := range order {
@@ -359,7 +357,7 @@ func processOrder(buildpacks []BuildpackMetadata, order dist.Order) (dist.Order,
 			var matchingBps []dist.BuildpackInfo
 			for _, bp := range buildpacks {
 				if bpRef.ID == bp.ID {
-					matchingBps = append(matchingBps, bp.BuildpackInfo)
+					matchingBps = append(matchingBps, bp)
 				}
 			}
 
@@ -395,7 +393,7 @@ func hasBuildpackWithVersion(bps []dist.BuildpackInfo, version string) bool {
 	return false
 }
 
-func validateBuildpacks(stackID string, mixins []string, lifecycleDescriptor LifecycleDescriptor, allBuildpacks []BuildpackMetadata, bpsToValidate []dist.Buildpack) error {
+func validateBuildpacks(stackID string, mixins []string, lifecycleDescriptor LifecycleDescriptor, allBuildpacks []dist.BuildpackInfo, bpsToValidate []dist.Buildpack) error {
 	bpLookup := map[string]interface{}{}
 
 	for _, bp := range allBuildpacks {

diff --git a/internal/builder/metadata.go b/internal/builder/metadata.go
@@ -7,22 +7,18 @@ const (
 )
 
 type Metadata struct {
-	Description string              `json:"description"`
-	Buildpacks  []BuildpackMetadata `json:"buildpacks"`
-	Stack       StackMetadata       `json:"stack"`
-	Lifecycle   LifecycleMetadata   `json:"lifecycle"`
-	CreatedBy   CreatorMetadata     `json:"createdBy"`
+	Description string               `json:"description"`
+	Buildpacks  []dist.BuildpackInfo `json:"buildpacks"`
+	Stack       StackMetadata        `json:"stack"`
+	Lifecycle   LifecycleMetadata    `json:"lifecycle"`
+	CreatedBy   CreatorMetadata      `json:"createdBy"`
 }
 
 type CreatorMetadata struct {
 	Name    string `json:"name"`
 	Version string `json:"version"`
 }
 
-type BuildpackMetadata struct {
-	dist.BuildpackInfo
-}
-
 type LifecycleMetadata struct {
 	LifecycleInfo
 	API LifecycleAPI `json:"api"`

diff --git a/internal/buildpack/locatortype.go b/internal/buildpack/locatortype.go
@@ -6,6 +6,7 @@ import (
 
 	"github.com/google/go-containerregistry/pkg/name"
 
+	"github.com/buildpacks/pack/internal/dist"
 	"github.com/buildpacks/pack/internal/paths"
 	"github.com/buildpacks/pack/internal/style"
 )
@@ -35,13 +36,13 @@ func (l LocatorType) String() string {
 // GetLocatorType determines which type of locator is designated by the given input.
 // If a type cannot be determined, `INVALID_LOCATOR` will be returned. If an error
 // is encountered, it will be returned.
-func GetLocatorType(locator string, idsFromBuilder []string) (LocatorType, error) {
+func GetLocatorType(locator string, buildpacksFromBuilder []dist.BuildpackInfo) (LocatorType, error) {
 	if locator == fromBuilderPrefix {
 		return FromBuilderLocator, nil
 	}
 
 	if strings.HasPrefix(locator, fromBuilderPrefix+":") {
-		if !builderMatchFound(locator, idsFromBuilder) {
+		if !builderMatchFound(locator, buildpacksFromBuilder) {
 			return InvalidLocator, fmt.Errorf("%s is not a valid identifier", style.Symbol(locator))
 		}
 		return IDLocator, nil
@@ -59,7 +60,7 @@ func GetLocatorType(locator string, idsFromBuilder []string) (LocatorType, error
 		return URILocator, nil
 	}
 
-	if builderMatchFound(locator, idsFromBuilder) {
+	if builderMatchFound(locator, buildpacksFromBuilder) {
 		return IDLocator, nil
 	}
 
@@ -70,11 +71,10 @@ func GetLocatorType(locator string, idsFromBuilder []string) (LocatorType, error
 	return InvalidLocator, nil
 }
 
-func builderMatchFound(locator string, candidates []string) bool {
+func builderMatchFound(locator string, candidates []dist.BuildpackInfo) bool {
 	id, version := ParseIDLocator(locator)
 	for _, c := range candidates {
-		candID, candVer := ParseIDLocator(c)
-		if id == candID && (version == "" || version == candVer) {
+		if id == c.ID && (version == "" || version == c.Version) {
 			return true
 		}
 	}