builder short name constants etc throughout (#1180)

* chore: shared builder constants etc

* pretty-print the shared unknown builder error

* update builder impls to use shared defs and validators

* error and docs text formatting

* include static default short names

* comment updates and typos

* docs paths

* use the constants for the in-package builder defaults

* use builders.All but with caviat

builders/builders.go

@@ -0,0 +1,97 @@
+/*
+	Package builders provides constants for builder implementation short names,
+	shared error types and convienience functions.
+*/
+package builders
+
+import (
+	"fmt"
+	"strconv"
+	"strings"
+
+	fn "knative.dev/kn-plugin-func"
+)
+
+const (
+	Pack    = "pack"
+	S2I     = "s2i"
+	Default = Pack
+)
+
+// Known builder names with a pretty-printed string representation
+type Known []string
+
+func All() Known {
+	return Known([]string{Pack, S2I})
+}
+
+func (k Known) String() string {
+	var b strings.Builder
+	for i, v := range k {
+		if i < len(k)-2 {
+			b.WriteString(strconv.Quote(v) + ", ")
+		} else if i < len(k)-1 {
+			b.WriteString(strconv.Quote(v) + " and ")
+		} else {
+			b.WriteString(strconv.Quote(v))
+		}
+	}
+	return b.String()
+}
+
+// ErrUnknownBuilder may be used by whomever is choosing a concrete
+// implementation of a builder to invoke based on potentially invalid input.
+type ErrUnknownBuilder struct {
+	Name  string
+	Known Known
+}
+
+func (e ErrUnknownBuilder) Error() string {
+	if len(e.Known) == 0 {
+		return fmt.Sprintf("\"%v\" is not a known builder", e.Name)
+	}
+	if len(e.Known) == 1 {
+		return fmt.Sprintf("\"%v\" is not a known builder. The available builder is %v", e.Name, e.Known)
+	}
+	return fmt.Sprintf("\"%v\" is not a known builder. Available builders are %s", e.Name, e.Known)
+}
+
+// ErrRuntimeRequired
+type ErrRuntimeRequired struct {
+	Builder string
+}
+
+func (e ErrRuntimeRequired) Error() string {
+	return fmt.Sprintf("runtime required to choose a default '%v' builder image", e.Builder)
+}
+
+// ErrNoDefaultImage
+type ErrNoDefaultImage struct {
+	Builder string
+	Runtime string
+}
+
+func (e ErrNoDefaultImage) Error() string {
+	return fmt.Sprintf("the '%v' runtime defines no default '%v' builder image", e.Runtime, e.Builder)
+}
+
+// Image is a convenience function for choosing the correct builder image
+// given a function, a builder, and defaults grouped by runtime.
+// - ErrRuntimeRequired if no runtime was provided on the given function
+// - ErrNoDefaultImage if the function has no builder image already defined
+//   for the given runtieme and there is no default in the provided map.
+func Image(f fn.Function, builder string, defaults map[string]string) (string, error) {
+	v, ok := f.BuilderImages[builder]
+	if ok {
+		return v, nil // found value
+	}
+	if f.Runtime == "" {
+		return "", ErrRuntimeRequired{Builder: builder}
+	}
+	v, ok = defaults[f.Runtime]
+	if ok {
+		return v, nil // Found default
+	}
+	return "", ErrNoDefaultImage{Builder: builder, Runtime: f.Runtime}
+
+}

builders/builders_test.go

@@ -0,0 +1,95 @@
+package builders_test
+
+import (
+	"errors"
+	"testing"
+
+	fn "knative.dev/kn-plugin-func"
+	"knative.dev/kn-plugin-func/builders"
+)
+
+// TestImage_Named ensures that a builder image is returned when
+// it exists on the function for a given builder, no defaults.
+func TestImage_Named(t *testing.T) {
+	f := fn.Function{
+		Builder: builders.Pack,
+		BuilderImages: map[string]string{
+			builders.Pack: "example.com/my/builder-image",
+		},
+	}
+
+	builderImage, err := builders.Image(f, builders.Pack, make(map[string]string))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if builderImage != "example.com/my/builder-image" {
+		t.Fatalf("expected 'example.com/my/builder-image', got '%v'", builderImage)
+	}
+}
+
+// TestImage_ErrRuntimeRequired ensures that the correct error is thrown when
+// the function has no builder image yet defined for the named builder, and
+// also no runtime to choose from the defaults.
+func TestImage_ErrRuntimeRequired(t *testing.T) {
+	_, err := builders.Image(fn.Function{}, "", make(map[string]string))
+	if err == nil {
+		t.Fatalf("did not receive expected error")
+	}
+	if !errors.Is(err, builders.ErrRuntimeRequired{}) {
+		t.Fatalf("error is not an 'ErrRuntimeRequired': '%v'", err)
+	}
+}
+
+// TestImage_ErrNoDefaultImage ensures that when
+func TestImage_ErrNoDefaultImage(t *testing.T) {
+	_, err := builders.Image(fn.Function{Runtime: "go"}, "", make(map[string]string))
+	if err == nil {
+		t.Fatalf("did not receive expected error")
+	}
+	if !errors.Is(err, builders.ErrNoDefaultImage{Runtime: "go"}) {
+		t.Fatalf("did not get 'ErrNoDefaultImage', got '%v'", err)
+	}
+}
+
+// TestImage_Defaults ensures that, when a default exists in the provided
+// map, it is chosen when both runtime is defined on the function and no
+// builder image has yet to be defined on the function.
+func TestImage_Defaults(t *testing.T) {
+	defaults := map[string]string{
+		"go": "example.com/go/default-builder-image",
+	}
+	builderImage, err := builders.Image(fn.Function{Runtime: "go"}, "", defaults)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if builderImage != "example.com/go/default-builder-image" {
+		t.Fatalf("the default was not chosen")
+	}
+}
+
+// Test_ErrUnknownBuilder ensures that the error properfly formats.
+// This error is used externally by packages which share builders but may
+// define their own custom builder, thus actually throwing this error
+// is the responsibility of whomever is instantiating builders.
+func Test_ErrUnknownBuilder(t *testing.T) {
+	var tests = []struct {
+		Known    []string
+		Expected string
+	}{
+		{[]string{},
+			`"test" is not a known builder`},
+		{[]string{"pack"},
+			`"test" is not a known builder. The available builder is "pack"`},
+		{[]string{"pack", "s2i"},
+			`"test" is not a known builder. Available builders are "pack" and "s2i"`},
+		{[]string{"pack", "s2i", "custom"},
+			`"test" is not a known builder. Available builders are "pack", "s2i" and "custom"`},
+	}
+	for _, test := range tests {
+		e := builders.ErrUnknownBuilder{Name: "test", Known: test.Known}
+		if e.Error() != test.Expected {
+			t.Fatalf("expected error \"%v\". got \"%v\"", test.Expected, e.Error())
+		}
+	}
+}

buildpacks/builder.go

@@ -15,9 +15,13 @@ import (
 	"github.com/docker/docker/client"
 
 	fn "knative.dev/kn-plugin-func"
+	"knative.dev/kn-plugin-func/builders"
 	"knative.dev/kn-plugin-func/docker"
 )
 
+// DefaultName when no WithName option is provided to NewBuilder
+const DefaultName = builders.Pack
+
 var (
 	DefaultBuilderImages = map[string]string{
 		"node":       "gcr.io/paketo-buildpacks/builder:base",
@@ -40,6 +44,7 @@ var (
 
 // Builder will build Function using Pack.
 type Builder struct {
+	name    string
 	verbose bool
 	logger  io.Writer
 	impl    Impl
@@ -52,7 +57,7 @@ type Impl interface {
 
 // NewBuilder instantiates a Buildpack-based Builder
 func NewBuilder(options ...Option) *Builder {
-	b := &Builder{}
+	b := &Builder{name: DefaultName}
 	for _, o := range options {
 		o(b)
 	}
@@ -69,6 +74,12 @@ func NewBuilder(options ...Option) *Builder {
 
 type Option func(*Builder)
 
+func WithName(n string) Option {
+	return func(b *Builder) {
+		b.name = n
+	}
+}
+
 func WithVerbose(v bool) Option {
 	return func(b *Builder) {
 		b.verbose = v
@@ -83,8 +94,8 @@ func WithImpl(i Impl) Option {
 
 // Build the Function at path.
 func (b *Builder) Build(ctx context.Context, f fn.Function) (err error) {
-	// Builder image defined on the Function if set, or from the default map.
+	// Builder image from the function if defined, default otherwise.
-	image, err := BuilderImage(f)
+	image, err := BuilderImage(f, b.name)
 	if err != nil {
 		return
 	}
@@ -165,34 +176,9 @@ func newImpl(ctx context.Context, cli client.CommonAPIClient, dockerHost string,
 	return pack.NewClient(pack.WithLogger(logging.NewSimpleLogger(logger)), pack.WithDockerClient(cli))
 }
 
-// Builder Image
+// Builder Image chooses the correct builder image or defaults.
-//
+func BuilderImage(f fn.Function, builderName string) (string, error) {
-// A value defined on the Function itself takes precidence.  If not defined,
+	return builders.Image(f, builderName, DefaultBuilderImages)
-// the default builder image for the Function's language runtime is used.
-// An inability to determine a builder image (such as an unknown language),
-// will return empty string. Errors are returned if either the runtime is not
-// populated or an inability to locate a default.
-//
-// Exported for use by Tekton in-cluster builds which do not have access to this
-// library at this time, and can therefore not instantiate and invoke this
-// package's buildpacks.Builder.Build.  Instead, they must transmit information
-// to the cluster using a Pipeline definition.
-func BuilderImage(f fn.Function) (string, error) {
-	if f.Runtime == "" {
-		return "", ErrRuntimeRequired{}
-	}
-
-	v, ok := f.BuilderImages[fn.BuilderPack]
-	if ok {
-		return v, nil
-	}
-
-	v, ok = DefaultBuilderImages[f.Runtime]
-	if ok {
-		return v, nil
-	}
-
-	return "", ErrRuntimeNotSupported{f.Runtime}
 }
 
 // podmanPreV330 returns if the daemon is podman pre v330 or errors trying.

buildpacks/builder_test.go

@@ -2,39 +2,17 @@ package buildpacks
 
 import (
 	"context"
-	"errors"
 	"testing"
 
 	pack "github.com/buildpacks/pack/pkg/client"
 	fn "knative.dev/kn-plugin-func"
+	"knative.dev/kn-plugin-func/builders"
 	. "knative.dev/kn-plugin-func/testing"
 )
 
-// Test_ErrRuntimeRequired ensures that a request to build without a runtime
+// Test_BuilderImageDefault ensures that a Function bing built which does not
-// defined for the Function yields an ErrRuntimeRequired
-func Test_ErrRuntimeRequired(t *testing.T) {
-	b := NewBuilder()
-	err := b.Build(context.Background(), fn.Function{})
-
-	if !errors.As(err, &ErrRuntimeRequired{}) {
-		t.Fatalf("expected ErrRuntimeRequired not received. Got %v", err)
-	}
-}
-
-// Test_ErrRuntimeNotSupported ensures that a request to build a function whose
-// runtime is not yet supported yields an ErrRuntimeNotSupported
-func Test_ErrRuntimeNotSupported(t *testing.T) {
-	b := NewBuilder()
-	err := b.Build(context.Background(), fn.Function{Runtime: "unsupported"})
-
-	if !errors.As(err, &ErrRuntimeNotSupported{}) {
-		t.Fatalf("expected ErrRuntimeNotSupported not received. got %v", err)
-	}
-}
-
-// Test_ImageDefault ensures that a Function bing built which does not
 // define a Builder Image will get the internally-defined default.
-func Test_ImageDefault(t *testing.T) {
+func Test_BuilderImageDefault(t *testing.T) {
 	var (
 		i = &mockImpl{}
 		b = NewBuilder(WithImpl(i))
@@ -59,18 +37,19 @@ func Test_ImageDefault(t *testing.T) {
 // image defined on the given Function if provided.
 func Test_BuilderImageConfigurable(t *testing.T) {
 	var (
-		i = &mockImpl{}             // mock underlying implementation
+		i = &mockImpl{} // mock underlying implementation
-		b = NewBuilder(WithImpl(i)) // Func Builder logic
+		b = NewBuilder( // Func Builder logic
-		f = fn.Function{            // Function with a builder image set
+			WithName(builders.Pack), WithImpl(i))
+		f = fn.Function{ // Function with a builder image set
 			Runtime: "node",
 			BuilderImages: map[string]string{
-				"pack": "example.com/user/builder-image",
+				builders.Pack: "example.com/user/builder-image",
 			},
 		}
 	)
 
 	i.BuildFn = func(ctx context.Context, opts pack.BuildOptions) error {
-		expected := f.BuilderImages["pack"]
+		expected := "example.com/user/builder-image"
 		if opts.Builder != expected {
 			t.Fatalf("expected builder image for node to be '%v', got '%v'", expected, opts.Builder)
 		}

client_test.go

@@ -21,6 +21,7 @@ import (
 
 	cloudevents "github.com/cloudevents/sdk-go/v2"
 	fn "knative.dev/kn-plugin-func"
+	"knative.dev/kn-plugin-func/builders"
 	"knative.dev/kn-plugin-func/mock"
 	. "knative.dev/kn-plugin-func/testing"
 )
@@ -943,8 +944,8 @@ func TestClient_New_BuildersPersisted(t *testing.T) {
 		Runtime: TestRuntime,
 		Root:    root,
 		BuilderImages: map[string]string{
-			"pack": "example.com/my/custom-pack-builder",
+			builders.Pack: "example.com/my/custom-pack-builder",
-			"s2i":  "example.com/my/custom-s2i-builder",
+			builders.S2I:  "example.com/my/custom-s2i-builder",
 		}}
 
 	// Create the function, which should preserve custom builders

cmd/build.go

@@ -13,6 +13,7 @@ import (
 	"knative.dev/kn-plugin-func/s2i"
 
 	fn "knative.dev/kn-plugin-func"
+	"knative.dev/kn-plugin-func/builders"
 )
 
 func NewBuildCmd(newClient ClientFactory) *cobra.Command {
@@ -49,7 +50,7 @@ and the image name is stored in the configuration file.
 		PreRunE:    bindEnv("image", "path", "builder", "registry", "confirm", "push", "builder-image", "platform"),
 	}
 
-	cmd.Flags().StringP("builder", "b", fn.DefaultBuilder, fmt.Sprintf("build strategy to use when creating the underlying image. Currently supported build strategies are %s.", fn.SupportedBuilders()))
+	cmd.Flags().StringP("builder", "b", builders.Default, fmt.Sprintf("build strategy to use when creating the underlying image. Currently supported build strategies are %s.", KnownBuilders()))
 	cmd.Flags().StringP("builder-image", "", "", "builder image, either an as a an image name or a mapping name.\nSpecified value is stored in func.yaml (as 'builder' field) for subsequent builds. ($FUNC_BUILDER_IMAGE)")
 	cmd.Flags().BoolP("confirm", "c", false, "Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)")
 	cmd.Flags().StringP("image", "i", "", "Full image name in the form [registry]/[namespace]/[name]:[tag] (optional). This option takes precedence over --registry (Env: $FUNC_IMAGE)")
@@ -75,6 +76,17 @@ and the image name is stored in the configuration file.
 	return cmd
 }
 
+// ValidateBuilder ensures that the given builder is one that the CLI
+// knows how to instantiate, returning a builkder.ErrUnknownBuilder otherwise.
+func ValidateBuilder(name string) (err error) {
+	for _, known := range KnownBuilders() {
+		if name == known {
+			return
+		}
+	}
+	return builders.ErrUnknownBuilder{Name: name, Known: KnownBuilders()}
+}
+
 func ValidNamespaceAndRegistry(path string) survey.Validator {
 	return func(val interface{}) error {
 
@@ -92,6 +104,17 @@ func ValidNamespaceAndRegistry(path string) survey.Validator {
 	}
 }
 
+// KnownBuilders are a typed string slice of builder short names which this
+// CLI understands.  Includes a customized String() representation intended
+// for use in flags and help text.
+func KnownBuilders() builders.Known {
+	// The set of builders supported by this CLI will likely always equate to
+	// the set of builders enumerated in the builders pacakage.
+	// However, future third-party integrations may support less than, or more
+	// builders, and certain environmental considerations may alter this list.
+	return builders.All()
+}
+
 func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err error) {
 	config, err := newBuildConfig().Prompt()
 	if err != nil {
@@ -158,17 +181,22 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro
 	} else {
 		config.Builder = function.Builder
 	}
-	if err = fn.ValidateBuilder(config.Builder); err != nil {
+	if err = ValidateBuilder(config.Builder); err != nil {
 		return err
 	}
-	if config.Builder == fn.BuilderPack {
+	if config.Builder == builders.Pack {
 		if config.Platform != "" {
 			err = fmt.Errorf("the --platform flag works only with s2i build")
 			return
 		}
-		builder = buildpacks.NewBuilder(buildpacks.WithVerbose(config.Verbose))
+		builder = buildpacks.NewBuilder(
-	} else if config.Builder == fn.BuilderS2i {
+			buildpacks.WithName(builders.Pack),
-		builder = s2i.NewBuilder(s2i.WithVerbose(config.Verbose), s2i.WithPlatform(config.Platform))
+			buildpacks.WithVerbose(config.Verbose))
+	} else if config.Builder == builders.S2I {
+		builder = s2i.NewBuilder(
+			s2i.WithName(builders.S2I),
+			s2i.WithVerbose(config.Verbose),
+			s2i.WithPlatform(config.Platform))
 	}
 
 	// All set, let's write changes in the config to the disk

cmd/build_test.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/spf13/cobra"
 	fn "knative.dev/kn-plugin-func"
+	"knative.dev/kn-plugin-func/builders"
 	"knative.dev/kn-plugin-func/mock"
 	. "knative.dev/kn-plugin-func/testing"
 )
@@ -229,7 +230,7 @@ func testBuilderPersistence(t *testing.T, testRegistry string, cmdBuilder func(C
 	if f, err = fn.NewFunction(root); err != nil {
 		t.Fatal(err)
 	}
-	if f.Builder != "s2i" {
+	if f.Builder != builders.S2I {
 		t.Fatal("value of builder flag not persisted when provided")
 	}
 	// Build the function without specifying a Builder
@@ -247,7 +248,7 @@ func testBuilderPersistence(t *testing.T, testRegistry string, cmdBuilder func(C
 		t.Fatal(err)
 	}
 
-	if f.Builder != "s2i" {
+	if f.Builder != builders.S2I {
 		t.Fatal("value of builder updated when not provided")
 	}
 
@@ -261,7 +262,7 @@ func testBuilderPersistence(t *testing.T, testRegistry string, cmdBuilder func(C
 	if f, err = fn.NewFunction(root); err != nil {
 		t.Fatal(err)
 	}
-	if f.Builder != "pack" {
+	if f.Builder != builders.Pack {
 		t.Fatal("value of builder flag not persisted on subsequent build")
 	}
 
@@ -283,3 +284,22 @@ func testBuilderPersistence(t *testing.T, testRegistry string, cmdBuilder func(C
 func TestBuild_BuilderPersistence(t *testing.T) {
 	testBuilderPersistence(t, "docker.io/tigerteam", NewBuildCmd)
 }
+
+// TestBuild_ValidateBuilder ensures that the validation function correctly
+// identifies valid and invalid builder short names.
+func Test_ValidateBuilder(t *testing.T) {
+	for _, name := range builders.All() {
+		if err := ValidateBuilder(name); err != nil {
+			t.Fatalf("expected builder '%v' to be valid, but got error: %v", name, err)
+		}
+	}
+
+	// This CLI creates no builders beyond those in the core reposiory.  Other
+	// users of the client library may provide their own named implementation of
+	// the fn.Builder interface. Those would have a different set of valid
+	// builders.
+
+	if err := ValidateBuilder("invalid"); err == nil {
+		t.Fatalf("did not get expected error validating an invalid builder name")
+	}
+}

cmd/completion_util.go

@@ -11,6 +11,7 @@ import (
 	"github.com/spf13/cobra"
 
 	fn "knative.dev/kn-plugin-func"
+	"knative.dev/kn-plugin-func/builders"
 	"knative.dev/kn-plugin-func/knative"
 )
 
@@ -136,5 +137,5 @@ func CompleteDeployBuildType(cmd *cobra.Command, args []string, complete string)
 }
 
 func CompleteBuildersList(cmd *cobra.Command, args []string, complete string) ([]string, cobra.ShellCompDirective) {
-	return fn.AllBuilders(), cobra.ShellCompDirectiveNoFileComp
+	return builders.All(), cobra.ShellCompDirectiveNoFileComp
 }

cmd/deploy.go

@@ -17,6 +17,7 @@ import (
 	"knative.dev/client/pkg/util"
 
 	fn "knative.dev/kn-plugin-func"
+	"knative.dev/kn-plugin-func/builders"
 	"knative.dev/kn-plugin-func/buildpacks"
 	"knative.dev/kn-plugin-func/docker"
 	"knative.dev/kn-plugin-func/docker/creds"
@@ -62,7 +63,7 @@ that is pushed to an image registry, and finally the function's Knative service
 	cmd.Flags().StringP("git-dir", "d", "", "Directory in the repo where the function is located (Env: $FUNC_GIT_DIR)")
 	cmd.Flags().StringP("build", "b", fn.DefaultBuildType, fmt.Sprintf("Build specifies the way the function should be built. Supported types are %s (Env: $FUNC_BUILD)", fn.SupportedBuildTypes(true)))
 	// Flags shared with Build specifically related to building:
-	cmd.Flags().StringP("builder", "", fn.DefaultBuilder, fmt.Sprintf("build strategy to use when creating the underlying image. Currently supported build strategies are %s.", fn.SupportedBuilders()))
+	cmd.Flags().StringP("builder", "", builders.Default, fmt.Sprintf("build strategy to use when creating the underlying image. Currently supported build strategies are %s.", KnownBuilders()))
 	cmd.Flags().StringP("builder-image", "", "", "builder image, either an as a an image name or a mapping name.\nSpecified value is stored in func.yaml (as 'builder' field) for subsequent builds. ($FUNC_BUILDER_IMAGE)")
 	cmd.Flags().StringP("image", "i", "", "Full image name in the form [registry]/[namespace]/[name]:[tag]@[digest]. This option takes precedence over --registry. Specifying digest is optional, but if it is given, 'build' and 'push' phases are disabled. (Env: $FUNC_IMAGE)")
 	cmd.Flags().StringP("registry", "r", GetDefaultRegistry(), "Registry + namespace part of the image to build, ex 'quay.io/myuser'.  The full image name is automatically determined based on the local directory name. If not provided the registry will be taken from func.yaml (Env: $FUNC_REGISTRY)")
@@ -184,16 +185,16 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err
 	} else {
 		config.Builder = function.Builder
 	}
-	if err = fn.ValidateBuilder(config.Builder); err != nil {
+	if err = ValidateBuilder(config.Builder); err != nil {
 		return err
 	}
-	if config.Builder == fn.BuilderPack {
+	if config.Builder == builders.Pack {
 		if config.Platform != "" {
 			err = fmt.Errorf("the --platform flag works only with s2i build")
 			return
 		}
 		builder = buildpacks.NewBuilder(buildpacks.WithVerbose(config.Verbose))
-	} else if config.Builder == fn.BuilderS2i {
+	} else if config.Builder == builders.S2I {
 		builder = s2i.NewBuilder(s2i.WithVerbose(config.Verbose), s2i.WithPlatform(config.Platform))
 	}
 

docs/reference/commands.txt

@@ -114,7 +114,7 @@ func build --builder=pack --builder-image cnbs/sample-builder:bionic
 
 
 Flags:
-  -b, --builder string         build strategy to use when creating the underlying image. Currently supported build strategies are "pack" or "s2i". (default "pack")
+  -b, --builder string         build strategy to use when creating the underlying image. Currently supported build strategies are "pack" and "s2i". (default "pack")
       --builder-image string   builder image, either an as a an image name or a mapping name.
                                Specified value is stored in func.yaml (as 'builder' field) for subsequent builds. ($FUNC_BUILDER_IMAGE)
   -c, --confirm                Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)
@@ -304,7 +304,7 @@ func deploy --image quay.io/myuser/myfunc -n myns
 
 Flags:
   -b, --build string           Build specifies the way the function should be built. Supported types are "disabled", "local" or "git" (Env: $FUNC_BUILD) (default "local")
-      --builder string         build strategy to use when creating the underlying image. Currently supported build strategies are "pack" or "s2i". (default "pack")
+      --builder string         build strategy to use when creating the underlying image. Currently supported build strategies are "pack" and "s2i". (default "pack")
       --builder-image string   builder image, either an as a an image name or a mapping name.
                                Specified value is stored in func.yaml (as 'builder' field) for subsequent builds. ($FUNC_BUILDER_IMAGE)
   -c, --confirm                Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)
@@ -449,7 +449,7 @@ Flags:
   -f, --format string         Format of message to send, 'http' or 'cloudevent'.  Default is to choose automatically. (Env: $FUNC_FORMAT)
   -h, --help                  help for invoke
       --id string             ID for the request data. (Env: $FUNC_ID)
-  -p, --path string           Path to the function which should have its instance invoked. (Env: $FUNC_PATH) (default ".")
+  -p, --path string           Path to the project directory (Env: $FUNC_PATH) (default ".")
       --source string         Source value for the request data. (Env: $FUNC_SOURCE) (default "/boson/fn")
   -t, --target string         Function instance to invoke.  Can be 'local', 'remote' or a URL.  Defaults to auto-discovery if not provided. (Env: $FUNC_TARGET)
       --type string           Type value for the request data. (Env: $FUNC_TYPE) (default "boson.fn")

function_builder.go

@@ -1,33 +0,0 @@
-package function
-
-import (
-	"fmt"
-)
-
-const (
-	BuilderPack    = "pack"
-	BuilderS2i     = "s2i"
-	DefaultBuilder = BuilderPack
-)
-
-func AllBuilders() []string {
-	return []string{BuilderPack, BuilderS2i}
-}
-
-// ErrInvalidBuilder indicates that the passed builder was invalid.
-type ErrInvalidBuilder error
-
-// ValidateBuilder validatest that the input Build type is valid for deploy command
-func ValidateBuilder(builder string) error {
-
-	if builder == BuilderPack || builder == BuilderS2i {
-		return nil
-	}
-
-	return ErrInvalidBuilder(fmt.Errorf("specified builder %q is not valid, allowed builders are %s", builder, SupportedBuilders()))
-}
-
-// SupportedBuilders prints string with supported build types
-func SupportedBuilders() string {
-	return fmt.Sprintf("%q or %q", BuilderPack, BuilderS2i)
-}

function_builder_unit_test.go

@@ -1,45 +0,0 @@
-//go:build !integration
-// +build !integration
-
-package function
-
-import (
-	"testing"
-)
-
-func Test_validateBuilder(t *testing.T) {
-	tests := []struct {
-		name      string
-		builder   string
-		wantError bool
-	}{
-		{
-			name:      "valid builder - pack",
-			builder:   "pack",
-			wantError: false,
-		},
-		{
-			name:      "valid builder - s2i",
-			builder:   "s2i",
-			wantError: false,
-		},
-		{
-			name:      "invalid builder",
-			builder:   "foo",
-			wantError: true,
-		},
-		{
-			name:      "builder not specified - invalid option",
-			wantError: true,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			err := ValidateBuilder(tt.builder)
-			if tt.wantError != (err != nil) {
-				t.Errorf("ValidateBuilder() = Wanted error %v but actually got %v", tt.wantError, err)
-			}
-		})
-	}
-}

pipelines/tekton/resources.go

@@ -10,6 +10,7 @@ import (
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	fn "knative.dev/kn-plugin-func"
+	"knative.dev/kn-plugin-func/builders"
 	"knative.dev/kn-plugin-func/buildpacks"
 )
 
@@ -181,7 +182,7 @@ func generatePipelineRun(f fn.Function, labels map[string]string) *pplnv1beta1.P
 // language runtime.  Errors are checked elsewhere, so at this level they
 // manifest as an inability to get a builder image = empty string.
 func getBuilderImage(f fn.Function) (name string) {
-	name, _ = buildpacks.BuilderImage(f)
+	name, _ = buildpacks.BuilderImage(f, builders.Pack)
 	return
 }
 

s2i/builder.go

@@ -26,13 +26,12 @@ import (
 	"github.com/openshift/source-to-image/pkg/scm/git"
 
 	fn "knative.dev/kn-plugin-func"
+	"knative.dev/kn-plugin-func/builders"
 	"knative.dev/kn-plugin-func/docker"
 )
 
-var (
+// DefaultName when no WithName option is provided to NewBuilder
-	// ErrRuntimeRequired indicates the required value of function runtime was not provided
+const DefaultName = builders.S2I
-	ErrRuntimeRequired = errors.New("runtime is required to build")
-)
 
 // DefaultBuilderImages for s2i builders indexed by Runtime Language
 var DefaultBuilderImages = map[string]string{
@@ -49,6 +48,7 @@ type DockerClient interface {
 
 // Builder of functions using the s2i subsystem.
 type Builder struct {
+	name     string
 	verbose  bool
 	impl     build.Builder // S2I builder implementation (aka "Strategy")
 	cli      DockerClient
@@ -57,6 +57,12 @@ type Builder struct {
 
 type Option func(*Builder)
 
+func WithName(n string) Option {
+	return func(b *Builder) {
+		b.name = n
+	}
+}
+
 // WithVerbose toggles verbose logging.
 func WithVerbose(v bool) Option {
 	return func(b *Builder) {
@@ -87,7 +93,7 @@ func WithPlatform(platform string) Option {
 
 // NewBuilder creates a new instance of a Builder with static defaults.
 func NewBuilder(options ...Option) *Builder {
-	b := &Builder{}
+	b := &Builder{name: DefaultName}
 	for _, o := range options {
 		o(b)
 	}
@@ -97,8 +103,8 @@ func NewBuilder(options ...Option) *Builder {
 func (b *Builder) Build(ctx context.Context, f fn.Function) (err error) {
 	// TODO this function currently doesn't support private s2i builder images since credentials are not set
 
-	// Builder image from the function  if defined, default otherwise.
+	// Builder image from the function if defined, default otherwise.
-	builderImage, err := builderImage(f)
+	builderImage, err := BuilderImage(f, b.name)
 	if err != nil {
 		return
 	}
@@ -361,39 +367,8 @@ func s2iScriptURL(ctx context.Context, cli DockerClient, image string) (string,
 	return "", nil
 }
 
-// builderImage for function
+// Builder Image chooses the correct builder image or defaults.
-// Uses the image defined on the function by default (for the given runtime)
+func BuilderImage(f fn.Function, builderName string) (string, error) {
-// or uses the static defaults if not defined. Returns an  ErrRuntimeRequired
+	// delegate as the logic is shared amongst builders
-// if the function failed to define a Runtime, and ErrRuntimeNotSupported if
+	return builders.Image(f, builderName, DefaultBuilderImages)
-// defined but an image exists neither in the static defaults nor in the
-// function's Builders map.
-func builderImage(f fn.Function) (string, error) {
-	if f.Runtime == "" {
-		return "", ErrRuntimeRequired
-	}
-
-	v, ok := f.BuilderImages[fn.BuilderS2i]
-	if ok {
-		return v, nil
-	}
-
-	v, ok = DefaultBuilderImages[f.Runtime]
-	if ok {
-		return v, nil
-	}
-
-	return "", ErrRuntimeNotSupported{f.Runtime}
-}
-
-func IsErrRuntimeNotSupported(err error) bool {
-	var e ErrRuntimeNotSupported
-	return errors.As(err, &e)
-}
-
-type ErrRuntimeNotSupported struct {
-	Runtime string
-}
-
-func (e ErrRuntimeNotSupported) Error() string {
-	return fmt.Sprintf("the s2i builder has no default builder image for the %q language runtime (try specifying builder image or use different build strategy)", e.Runtime)
 }

s2i/builder_test.go

@@ -26,40 +26,20 @@ import (
 
 	"github.com/openshift/source-to-image/pkg/api"
 	fn "knative.dev/kn-plugin-func"
+	"knative.dev/kn-plugin-func/builders"
 	"knative.dev/kn-plugin-func/s2i"
 	. "knative.dev/kn-plugin-func/testing"
 )
 
-// Test_ErrRuntimeRequired ensures that a request to build without a runtime
-// defined for the function yields an ErrRuntimeRequired
-func Test_ErrRuntimeRequired(t *testing.T) {
-	b := s2i.NewBuilder()
-	err := b.Build(context.Background(), fn.Function{})
-
-	if !errors.Is(err, s2i.ErrRuntimeRequired) {
-		t.Fatal("expected ErrRuntimeRequired not received")
-	}
-}
-
-// Test_ErrRuntimeNotSupported ensures that a request to build a function whose
-// runtime is not yet supported yields an ErrRuntimeNotSupported
-func Test_ErrRuntimeNotSupported(t *testing.T) {
-	b := s2i.NewBuilder()
-	err := b.Build(context.Background(), fn.Function{Runtime: "unsupported"})
-
-	if !s2i.IsErrRuntimeNotSupported(err) {
-		t.Fatal("expected ErrRuntimeNotSupported not received")
-	}
-}
-
 // Test_BuilderImageDefault ensures that a function being built which does not
 // define a Builder Image will default.
-func Test_ImageDefault(t *testing.T) {
+func Test_BuilderImageDefault(t *testing.T) {
 	var (
-		i = &mockImpl{}                                              // mock underlying s2i implementation
+		i = &mockImpl{}                  // mock underlying s2i implementation
-		c = mockDocker{}                                             // mock docker client
+		c = mockDocker{}                 // mock docker client
-		b = s2i.NewBuilder(s2i.WithImpl(i), s2i.WithDockerClient(c)) // func S2I Builder logic
+		f = fn.Function{Runtime: "node"} // function with no builder image set
-		f = fn.Function{Runtime: "node"}                             // function with no builder image set
+		b = s2i.NewBuilder(              // func S2I Builder logic
+			s2i.WithImpl(i), s2i.WithDockerClient(c))
 	)
 
 	// An implementation of the underlying S2I implementation which verifies
@@ -67,7 +47,8 @@ func Test_ImageDefault(t *testing.T) {
 	i.BuildFn = func(cfg *api.Config) (*api.Result, error) {
 		expected := s2i.DefaultBuilderImages["node"]
 		if cfg.BuilderImage != expected {
-			t.Fatalf("expected s2i config builder image '%v', got '%v'", expected, cfg.BuilderImage)
+			t.Fatalf("expected s2i config builder image '%v', got '%v'",
+				expected, cfg.BuilderImage)
 		}
 		return nil, nil
 	}
@@ -83,13 +64,14 @@ func Test_ImageDefault(t *testing.T) {
 // image defined on the given function if provided.
 func Test_BuilderImageConfigurable(t *testing.T) {
 	var (
-		i = &mockImpl{}                                              // mock underlying s2i implementation
+		i = &mockImpl{}     // mock underlying s2i implementation
-		c = mockDocker{}                                             // mock docker client
+		c = mockDocker{}    // mock docker client
-		b = s2i.NewBuilder(s2i.WithImpl(i), s2i.WithDockerClient(c)) // func S2I Builder logic
+		b = s2i.NewBuilder( // func S2I Builder logic
-		f = fn.Function{                                             // function with a builder image set
+			s2i.WithName(builders.S2I), s2i.WithImpl(i), s2i.WithDockerClient(c))
+		f = fn.Function{ // function with a builder image set
 			Runtime: "node",
 			BuilderImages: map[string]string{
-				"s2i": "example.com/user/builder-image",
+				builders.S2I: "example.com/user/builder-image",
 			},
 		}
 	)
@@ -97,7 +79,7 @@ func Test_BuilderImageConfigurable(t *testing.T) {
 	// An implementation of the underlying S2I implementation which verifies
 	// the config has arrived as expected (correct functions logic applied)
 	i.BuildFn = func(cfg *api.Config) (*api.Result, error) {
-		expected := f.BuilderImages["s2i"]
+		expected := "example.com/user/builder-image"
 		if cfg.BuilderImage != expected {
 			t.Fatalf("expected s2i config builder image for node to be '%v', got '%v'", expected, cfg.BuilderImage)
 		}
@@ -220,11 +202,11 @@ func TestS2IScriptURL(t *testing.T) {
 			f := fn.Function{
 				Runtime: "node",
 				BuilderImages: map[string]string{
-					"s2i": tt.builderImage,
+					builders.S2I: tt.builderImage,
 				},
 			}
 
-			b := s2i.NewBuilder(s2i.WithImpl(impl), s2i.WithDockerClient(cli))
+			b := s2i.NewBuilder(s2i.WithName(builders.S2I), s2i.WithImpl(impl), s2i.WithDockerClient(cli))
 			err = b.Build(context.Background(), f)
 			if err != nil {
 				t.Error(err)

test/_e2e/cmd_config_envs_test.go

@@ -15,6 +15,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
+	"knative.dev/kn-plugin-func/builders"
 	"knative.dev/kn-plugin-func/k8s"
 )
 
@@ -104,7 +105,7 @@ func TestConfigEnvs(t *testing.T) {
 	project.FunctionName = "test-config-envs"
 	project.ProjectPath = filepath.Join(os.TempDir(), project.FunctionName)
 	project.RemoteRepository = "http://github.com/boson-project/test-templates.git"
-	project.Builder = "pack"
+	project.Builder = builders.Pack
 
 	Create(t, knFunc.TestShell, project)
 	defer func() { _ = project.RemoveProjectFolder() }()

test/_e2e/cmd_config_labels_test.go

@@ -8,6 +8,8 @@ import (
 	"path/filepath"
 
 	"testing"
+
+	"knative.dev/kn-plugin-func/builders"
 )
 
 const (
@@ -57,7 +59,7 @@ func TestConfigLabel(t *testing.T) {
 	project.Template = "http"
 	project.FunctionName = "test-config-labels"
 	project.ProjectPath = filepath.Join(os.TempDir(), project.FunctionName)
-	project.Builder = "pack"
+	project.Builder = builders.Pack
 
 	Create(t, knFunc.TestShell, project)
 	defer func() { _ = project.RemoveProjectFolder() }()

test/_e2e/cmd_config_volumes_test.go

@@ -13,6 +13,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
+	"knative.dev/kn-plugin-func/builders"
 	"knative.dev/kn-plugin-func/k8s"
 )
 
@@ -101,7 +102,7 @@ func TestConfigVolumes(t *testing.T) {
 	project.FunctionName = "test-config-volumes"
 	project.ProjectPath = filepath.Join(os.TempDir(), project.FunctionName)
 	project.RemoteRepository = "http://github.com/boson-project/test-templates.git"
-	project.Builder = "pack"
+	project.Builder = builders.Pack
 
 	Create(t, knFunc.TestShell, project)
 	defer project.RemoveProjectFolder()

test/_e2e/e2e_remote_repository_test.go

@@ -7,6 +7,8 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
+
+	"knative.dev/kn-plugin-func/builders"
 )
 
 // TestRemoteRepository verifies function created using an
@@ -20,7 +22,7 @@ func TestRemoteRepository(t *testing.T) {
 	project.Template = "e2e"
 	project.FunctionName = "func-remote-repo"
 	project.ProjectPath = filepath.Join(os.TempDir(), project.FunctionName)
-	project.Builder = "pack"
+	project.Builder = builders.Pack
 
 	result := knFunc.Exec("create", project.ProjectPath,
 		"--language", project.Runtime,