Repository: knative-sandbox/kn-plugin-func
Pull request #1045 (2022-06-08 19:04:29)

[release-0.24] feat: builder images map migration
This is an automated cherry-pick of #1033

knative-prow[bot]: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative-sandbox/kn-plugin-func/pull/1045#" title="Author self-approved">knative-prow-robot</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **salaboy** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @salaboy` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative-sandbox%2Fkn-plugin-func).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/knative-sandbox/kn-plugin-func/blob/release-0.24/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["salaboy"]} -->

Patch:
diff --git a/buildpacks/builder.go b/buildpacks/builder.go
index d18d7245..b91f576f 100644
--- a/buildpacks/builder.go
+++ b/buildpacks/builder.go
@@ -19,150 +19,217 @@ import (
 	"github.com/buildpacks/pack/pkg/logging"
 )
 
-// DefaultBuilderImages for Pack builders indexed by Runtime Language
-var DefaultBuilderImages = map[string]string{
-	"node":       "gcr.io/paketo-buildpacks/builder:base",
-	"typescript": "gcr.io/paketo-buildpacks/builder:base",
-	"go":         "gcr.io/paketo-buildpacks/builder:base",
-	"python":     "gcr.io/paketo-buildpacks/builder:base",
-	"quarkus":    "gcr.io/paketo-buildpacks/builder:base",
-	"rust":       "gcr.io/paketo-buildpacks/builder:base",
-	"springboot": "gcr.io/paketo-buildpacks/builder:base",
-}
+var (
+	DefaultBuilderImages = map[string]string{
+		"node":       "gcr.io/paketo-buildpacks/builder:base",
+		"typescript": "gcr.io/paketo-buildpacks/builder:base",
+		"go":         "gcr.io/paketo-buildpacks/builder:base",
+		"python":     "gcr.io/paketo-buildpacks/builder:base",
+		"quarkus":    "gcr.io/paketo-buildpacks/builder:base",
+		"rust":       "gcr.io/paketo-buildpacks/builder:base",
+		"springboot": "gcr.io/paketo-buildpacks/builder:base",
+	}
+
+	trustedBuilderImagePrefixes = []string{
+		"quay.io/boson",
+		"gcr.io/paketo-buildpacks",
+		"docker.io/paketobuildpacks",
+	}
 
-//Builder holds the configuration that will be passed to
-//Buildpack builder
+	v330 = semver.MustParse("v3.3.0") // for checking podman version
+)
+
+// Builder will build Function using Pack.
 type Builder struct {
 	verbose bool
+	logger  io.Writer
+	impl    Impl
 }
 
-//NewBuilder builds the new Builder configuration
-func NewBuilder(verbose bool) *Builder {
-	return &Builder{verbose: verbose}
+// Impl allows for the underlying implementation to be mocked for tests.
+type Impl interface {
+	Build(context.Context, pack.BuildOptions) error
 }
 
-var v330 = semver.MustParse("v3.3.0")
-
-// Build the Function at path.
-func (builder *Builder) Build(ctx context.Context, f fn.Function) (err error) {
-
-	// Use the builder found in the Function configuration file if it exists,
-	// or a default for the language if not provided
-	packBuilder := BuilderImage(f)
-	if packBuilder == "" {
-		return fmt.Errorf("builder image not found for function of language '%v'", f.Runtime)
-	}
-
-	// Build options for the pack client.
-	var network string
-	if runtime.GOOS == "linux" {
-		network = "host"
+// NewBuilder instantiates a Buildpack-based Builder
+func NewBuilder(options ...Option) *Builder {
+	b := &Builder{}
+	for _, o := range options {
+		o(b)
 	}
 
-	// log output is either STDOUt or kept in a buffer to be printed on error.
-	var logWriter io.Writer
-	if builder.verbose {
-		// pass stdout as non-closeable writer
-		// otherwise pack client would close it which is bad
-		logWriter = stdoutWrapper{os.Stdout}
+	// Stream logs to stdout or buffer only for display on error.
+	if b.verbose {
+		b.logger = stdoutWrapper{os.Stdout}
 	} else {
-		logWriter = &bytes.Buffer{}
+		b.logger = &bytes.Buffer{}
 	}
 
-	cli, dockerHost, err := docker.NewClient(client.DefaultDockerHost)
-	if err != nil {
-		return err
-	}
-	defer cli.Close()
+	return b
+}
 
-	version, err := cli.ServerVersion(ctx)
-	if err != nil {
-		return err
+type Option func(*Builder)
+
+func WithVerbose(v bool) Option {
+	return func(b *Builder) {
+		b.verbose = v
 	}
+}
 
-	var daemonIsPodmanBeforeV330 bool
-	for _, component := range version.Components {
-		if component.Name == "Podman Engine" {
-			v := semver.MustParse(version.Version)
-			if v.Compare(v330) < 0 {
-				daemonIsPodmanBeforeV330 = true
-			}
-			break
-		}
+func WithImpl(i Impl) Option {
+	return func(b *Builder) {
+		b.impl = i
 	}
+}
 
-	buildEnvs, err := fn.Interpolate(f.BuildEnvs)
+// 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.
+	image, err := BuilderImage(f)
 	if err != nil {
-		return err
+		return
 	}
 
-	var isTrustedBuilderFunc = func(b string) bool {
-		return !daemonIsPodmanBeforeV330 &&
-			(strings.HasPrefix(packBuilder, "quay.io/boson") ||
-				strings.HasPrefix(packBuilder, "gcr.io/paketo-buildpacks") ||
-				strings.HasPrefix(packBuilder, "docker.io/paketobuildpacks"))
-	}
-	packOpts := pack.BuildOptions{
+	// Pack build options
+	opts := pack.BuildOptions{
 		AppPath:        f.Root,
 		Image:          f.Image,
 		LifecycleImage: "quay.io/boson/lifecycle:0.13.2",
-		Builder:        packBuilder,
-		Env:            buildEnvs,
+		Builder:        image,
 		Buildpacks:     f.Buildpacks,
-		TrustBuilder:   isTrustedBuilderFunc,
-		DockerHost:     dockerHost,
 		ContainerConfig: struct {
 			Network string
 			Volumes []string
-		}{Network: network, Volumes: nil},
+		}{Network: "", Volumes: nil},
 	}
-
-	// Client with a logger which is enabled if in Verbose mode and a dockerClient that supports SSH docker daemon connection.
-	packClient, err := pack.NewClient(pack.WithLogger(logging.NewSimpleLogger(logWriter)), pack.WithDockerClient(cli))
-	if err != nil {
-		return
+	if opts.Env, err = fn.Interpolate(f.BuildEnvs); err != nil {
+		return err
+	}
+	if runtime.GOOS == "linux" {
+		opts.ContainerConfig.Network = "host"
 	}
 
-	// Build based using the given builder.
-	if err = packClient.Build(ctx, packOpts); err != nil {
-		if ctx.Err() != nil {
-			// received SIGINT
+	// Instantate the pack build client implementation
+	// (and update build opts as necessary)
+	if b.impl == nil {
+		if b.impl, err = newImpl(ctx, &opts, b.logger); err != nil {
 			return
-		} else if !builder.verbose {
-			// If the builder was not showing logs, embed the full logs in the error.
-			err = fmt.Errorf("failed to build the function (output: %q): %w", logWriter.(*bytes.Buffer).String(), err)
 		}
 	}
 
+	// Perform the build
+	if err = b.impl.Build(ctx, opts); err != nil {
+		if ctx.Err() != nil {
+			return // SIGINT
+		} else if !b.verbose {
+			err = fmt.Errorf("failed to build the function (output: %q): %w", b.logger.(*bytes.Buffer).String(), err)
+		}
+	}
 	return
 }
 
-// hack this makes stdout non-closeable
-type stdoutWrapper struct {
-	impl io.Writer
-}
+// newImpl returns an instance of the builder implementatoin.  Note that this
+// also mutates the provided options' DockerHost and TrustBuilder.
+func newImpl(ctx context.Context, opts *pack.BuildOptions, logger io.Writer) (impl Impl, err error) {
+	cli, dockerHost, err := docker.NewClient(client.DefaultDockerHost)
+	if err != nil {
+		return
+	}
+	defer cli.Close()
 
-func (s stdoutWrapper) Write(p []byte) (n int, err error) {
-	return s.impl.Write(p)
+	opts.DockerHost = dockerHost
+
+	daemonIsPodmanPreV330, err := podmanPreV330(ctx, cli)
+	if err != nil {
+		return
+	}
+
+	opts.TrustBuilder = func(_ string) bool {
+		if daemonIsPodmanPreV330 {
+			return false
+		}
+		for _, v := range trustedBuilderImagePrefixes {
+			if strings.HasPrefix(opts.Builder, v) {
+				return true
+			}
+		}
+		return false
+	}
+
+	// Client with a logger which is enabled if in Verbose mode and a dockerClient that supports SSH docker daemon connection.
+	return pack.NewClient(pack.WithLogger(logging.NewSimpleLogger(logger)), pack.WithDockerClient(cli))
 }
 
-// Builder Image for a Function being built using Buildpack.
+// Builder Image
 //
 // A value defined on the Function itself takes precidence.  If not defined,
 // 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.
+// 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) (builder string) {
-	// NOTE this will be updated when func.yaml is expanded to support
-	// differing builder images for different build strategies (buildpac vs s2i)
-	if f.Builder != "" {
-		return f.Builder
+func BuilderImage(f fn.Function) (string, error) {
+	if f.Runtime == "" {
+		return "", ErrRuntimeRequired{}
+	}
+
+	v, ok := f.BuilderImages["pack"]
+	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.
+func podmanPreV330(ctx context.Context, cli client.CommonAPIClient) (b bool, err error) {
+	version, err := cli.ServerVersion(ctx)
+	if err != nil {
+		return
+	}
+
+	for _, component := range version.Components {
+		if component.Name == "Podman Engine" {
+			v := semver.MustParse(version.Version)
+			if v.Compare(v330) < 0 {
+				return true, nil
+			}
+			break
+		}
 	}
-	builder = DefaultBuilderImages[f.Runtime]
 	return
 }
+
+// stdoutWrapper is a hack that makes stdout non-closeable
+type stdoutWrapper struct {
+	impl io.Writer
+}
+
+func (s stdoutWrapper) Write(p []byte) (n int, err error) {
+	return s.impl.Write(p)
+}
+
+// Errors
+
+type ErrRuntimeRequired struct{}
+
+func (e ErrRuntimeRequired) Error() string {
+	return "Pack requires the Function define a language runtime"
+}
+
+type ErrRuntimeNotSupported struct {
+	Runtime string
+}
+
+func (e ErrRuntimeNotSupported) Error() string {
+	return fmt.Sprintf("Pack builder has no default builder image for the '%v' language runtime.  Please provide one.", e.Runtime)
+}
diff --git a/client.go b/client.go
index 4a6188d2..4dac5512 100644
--- a/client.go
+++ b/client.go
@@ -557,18 +557,6 @@ func (c *Client) Create(cfg Function) (err error) {
 		return
 	}
 
-	// TODO: Create a status structure and return it for clients to use
-	// for output, such as from the CLI.
-	if c.verbose {
-		fmt.Printf("Builder:       %s\n", f.Builder)
-		if len(f.Buildpacks) > 0 {
-			fmt.Println("Buildpacks:")
-			for _, b := range f.Buildpacks {
-				fmt.Printf("           ... %s\n", b)
-			}
-		}
-		fmt.Println("Function project created")
-	}
 	return
 }
 
diff --git a/cmd/build.go b/cmd/build.go
index 20f0b42f..2662b2fc 100644
--- a/cmd/build.go
+++ b/cmd/build.go
@@ -100,7 +100,7 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro
 		return
 	}
 
-	function, err := functionWithOverrides(config.Path, functionOverrides{BuilderImage: config.BuilderImage, Image: config.Image})
+	function, err := functionWithOverrides(config.Path, functionOverrides{Image: config.Image})
 	if err != nil {
 		return
 	}
@@ -158,7 +158,7 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro
 	// Choose a builder based on the value of the --builder flag
 	var builder fn.Builder
 	if config.Builder == "pack" {
-		builder = buildpacks.NewBuilder(config.Verbose)
+		builder = buildpacks.NewBuilder(buildpacks.WithVerbose(config.Verbose))
 	} else if config.Builder == "s2i" {
 		builder = s2i.NewBuilder(s2i.WithVerbose(config.Verbose))
 	} else {
@@ -166,6 +166,11 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro
 		return
 	}
 
+	// Use the user-provided builder image, if supplied
+	if config.BuilderImage != "" {
+		function.BuilderImages[config.Builder] = config.BuilderImage
+	}
+
 	// Create a client using the registry defined in config plus any additional
 	// options provided (such as mocks for testing)
 	client, done := newClient(ClientConfig{Verbose: config.Verbose},
diff --git a/cmd/client.go b/cmd/client.go
index 1533d5b2..43e8e00f 100644
--- a/cmd/client.go
+++ b/cmd/client.go
@@ -68,7 +68,7 @@ func NewClient(cfg ClientConfig, options ...fn.Option) (*fn.Client, func()) {
 			fn.WithVerbose(cfg.Verbose),
 			fn.WithProgressListener(p),
 			fn.WithTransport(t),
-			fn.WithBuilder(buildpacks.NewBuilder(cfg.Verbose)),
+			fn.WithBuilder(buildpacks.NewBuilder(buildpacks.WithVerbose(cfg.Verbose))),
 			fn.WithRemover(knative.NewRemover(cfg.Namespace, cfg.Verbose)),
 			fn.WithDescriber(knative.NewDescriber(cfg.Namespace, cfg.Verbose)),
 			fn.WithLister(knative.NewLister(cfg.Namespace, cfg.Verbose)),
diff --git a/cmd/completion_util.go b/cmd/completion_util.go
index d70d4967..26d70a53 100644
--- a/cmd/completion_util.go
+++ b/cmd/completion_util.go
@@ -114,8 +114,8 @@ func CompleteBuilderImageList(cmd *cobra.Command, args []string, complete string
 		return
 	}
 
-	builderImages = make([]string, 0, len(f.Builders))
-	for name := range f.Builders {
+	builderImages = make([]string, 0, len(f.BuilderImages))
+	for name := range f.BuilderImages {
 		if len(complete) == 0 {
 			builderImages = append(builderImages, name)
 			continue
diff --git a/cmd/root.go b/cmd/root.go
index 1838c8ab..4a2ee8dd 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -168,9 +168,8 @@ func bindEnv(flags ...string) bindFunc {
 }
 
 type functionOverrides struct {
-	Image        string
-	Namespace    string
-	BuilderImage string
+	Image     string
+	Namespace string
 }
 
 // functionWithOverrides sets the namespace and image strings for the
@@ -187,7 +186,6 @@ func functionWithOverrides(root string, overrides functionOverrides) (f fn.Funct
 		src  string
 		dest *string
 	}{
-		{overrides.BuilderImage, &f.Builder},
 		{overrides.Image, &f.Image},
 		{overrides.Namespace, &f.Namespace},
 	}
diff --git a/function.go b/function.go
index 7a2850a5..a7c4276b 100644
--- a/function.go
+++ b/function.go
@@ -64,15 +64,9 @@ type Function struct {
 	// in case build type "git" is being used
 	Git Git `yaml:"git"`
 
-	// Builder represents the CNCF Buildpack builder image for a function
-	Builder string `yaml:"builder"`
-
-	// Map containing known builders.
-	// e.g. { "jvm": "docker.io/example/quarkus-jvm-builder" }
-	Builders map[string]string `yaml:"builders"`
-
 	// BuilderImages define optional explicit builder images to use by
-	// builder implementations in leau of the in-code defaults.
+	// builder implementations in leau of the in-code defaults.  They key
+	// is the builder's short name.  For example:
 	// builderImages:
 	//   pack: example.com/user/my-pack-node-builder
 	//   s2i: example.com/user/my-s2i-node-builder
@@ -121,8 +115,8 @@ type HealthEndpoints struct {
 
 // BuildConfig defines builders and buildpacks
 type BuildConfig struct {
-	Buildpacks []string          `yaml:"buildpacks,omitempty"`
-	Builders   map[string]string `yaml:"builders,omitempty"`
+	Buildpacks    []string          `yaml:"buildpacks,omitempty"`
+	BuilderImages map[string]string `yaml:"builderImages,omitempty"`
 }
 
 // Invocation defines hints on how to accomplish a Function invocation.
@@ -169,7 +163,7 @@ func NewFunction(path string) (f Function, err error) {
 	if err != nil {
 		return
 	}
-	if err = yaml.UnmarshalStrict(bb, &f); err != nil {
+	if err = yaml.Unmarshal(bb, &f); err != nil {
 		err = formatUnmarshalError(err) // human-friendly unmarshalling errors
 		return
 	}
@@ -486,7 +480,7 @@ func hasInitializedFunction(path string) (bool, error) {
 		return false, err
 	}
 	f := Function{}
-	if err = yaml.UnmarshalStrict(bb, &f); err != nil {
+	if err = yaml.Unmarshal(bb, &f); err != nil {
 		return false, err
 	}
 	if f, err = f.Migrate(); err != nil {
diff --git a/function_migrations.go b/function_migrations.go
index 4863a588..412569e9 100644
--- a/function_migrations.go
+++ b/function_migrations.go
@@ -1,9 +1,12 @@
 package function
 
 import (
+	"io/ioutil"
+	"path/filepath"
 	"time"
 
 	"github.com/coreos/go-semver/semver"
+	"gopkg.in/yaml.v2"
 )
 
 // Migrate applies any necessary migrations, returning a new migrated
@@ -74,13 +77,15 @@ func (f Function) Migrated() bool {
 // version for the migration if necessary)
 var migrations = []migration{
 	{"0.19.0", migrateToCreationStamp},
+	{"0.23.0", migrateToBuilderImages},
 	// New Migrations Here.
 }
 
 // Individual Migration implementations
 // ------------------------------------
 
-// migrateToCreationStamp is the initial migration which brings a Function from
+// migrateToCreationStamp
+// The initial migration which brings a Function from
 // some unknown point in the past to the point at which it is versioned,
 // migrated and includes a creation timestamp.  Without this migration,
 // instantiation of old functions will fail with a "Function at path X not
@@ -112,3 +117,73 @@ func migrateToCreationStamp(f Function, m migration) (Function, error) {
 	f.Version = m.version // Record this migration was evaluated.
 	return f, nil
 }
+
+// migrateToBuilderImages
+// Prior to this migration, 'builder' and 'builders' attributes of a Function
+// were specific to buildpack builds.  In addition, the separation of the two
+// fields was to facilitate the use of "named" inbuilt builders, which ended
+// up not being necessary.  With the addition of the S2I builder implementation,
+// it is necessary to differentiate builders for use when building via Pack vs
+// builder for use when building with S2I.  Furthermore, now that the builder
+// itself is a user-supplied parameter, the short-hand of calling builder images
+// simply "builder" is not possible, since that term more correctly refers to
+// the builder being used (S2I, pack, or some future implementation), while this
+// field very specifically refers to the image the chosen builder should use
+// (in leau of the inbuilt default).
+//
+// For an example of the situation:  the 'builder' member used to instruct the
+// system to use that builder _image_ in all cases.  This of course restricts
+// the system from being able to build with anything other than the builder
+// implementation to which that builder image applies (pack or s2i).  Further,
+// always including this value in the serialized func.yaml causes this value to
+// be pegged/immutable (without manual intervention), which hampers our ability
+// to change out the underlying builder image with future versions.
+//
+// The 'builder' and 'builders' members have therefore been removed in favor
+// of 'builderImages', which is keyed by the short name of the builder
+// implementation (currently 'pack' and 's2i').  Its existence is optional,
+// with the default value being provided in the associated builder's impl.
+// Should the value exist, this indicates the user has overridden the value,
+// or is using a fully custom language pack.
+//
+// This migration allows pre-builder-image Functions to load despite their
+// inclusion of the now removed 'builder' member.  If the user had provided
+// a customized builder image, that value is preserved as the builder image
+// for the 'pack' builder in the new version (s2i did not exist prior).
+// See associated unit tests.
+func migrateToBuilderImages(f1 Function, m migration) (Function, error) {
+	// Load the Function using pertinent parts of the previous version's schema:
+	f0Filename := filepath.Join(f1.Root, FunctionFile)
+	bb, err := ioutil.ReadFile(f0Filename)
+	if err != nil {
+		return f1, err
+	}
+	f0 := migrateToBuilderImages_previousFunction{}
+	if err = yaml.Unmarshal(bb, &f0); err != nil {
+		return f1, err
+	}
+
+	// At time of this migration, the default pack builder image for all language
+	// runtimes is:
+	defaultPackBuilderImage := "gcr.io/paketo-buildpacks/builder:base"
+
+	// If the old Function had defined something custom
+	if f0.Builder != "" && f0.Builder != defaultPackBuilderImage {
+		// carry it forward as the new pack builder image
+		if f1.BuilderImages == nil {
+			f1.BuilderImages = make(map[string]string)
+		}
+		f1.BuilderImages["pack"] = f0.Builder
+	}
+
+	// Flag f1 as having had the migration applied
+	f1.Version = m.version
+	return f1, nil
+
+}
+
+// The pertinent asspects of the Function schema prior to the builder images
+// migration.
+type migrateToBuilderImages_previousFunction struct {
+	Builder string `yaml:"builder"`
+}
diff --git a/pipelines/tekton/resources.go b/pipelines/tekton/resources.go
index 47f76c7a..ae6fc430 100644
--- a/pipelines/tekton/resources.go
+++ b/pipelines/tekton/resources.go
@@ -125,7 +125,7 @@ func generatePipelineRun(f fn.Function, labels map[string]string) *pplnv1beta1.P
 				},
 				{
 					Name:  "builderImage",
-					Value: *pplnv1beta1.NewArrayOrString(buildpacks.BuilderImage(f)),
+					Value: *pplnv1beta1.NewArrayOrString(getBuilderImage(f)),
 				},
 			},
 
@@ -155,6 +155,15 @@ func generatePipelineRun(f fn.Function, labels map[string]string) *pplnv1beta1.P
 	}
 }
 
+// guilderImage returns the builder image to use when building the Function
+// with the Pack strategy if it can be calculated (the Function has a defined
+// 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)
+	return
+}
+
 func getPipelineName(f fn.Function) string {
 	return fmt.Sprintf("%s-%s-pipeline", f.Name, f.BuildType)
 }
diff --git a/schema/func_yaml-schema.json b/schema/func_yaml-schema.json
index ab238586..7ff3a474 100644
--- a/schema/func_yaml-schema.json
+++ b/schema/func_yaml-schema.json
@@ -29,8 +29,6 @@
 				"imageDigest",
 				"build",
 				"git",
-				"builder",
-				"builders",
 				"buildpacks",
 				"volumes",
 				"buildEnvs",
@@ -75,17 +73,6 @@
 					"$schema": "http://json-schema.org/draft-04/schema#",
 					"$ref": "#/definitions/Git"
 				},
-				"builder": {
-					"type": "string"
-				},
-				"builders": {
-					"patternProperties": {
-						".*": {
-							"type": "string"
-						}
-					},
-					"type": "object"
-				},
 				"builderImages": {
 					"patternProperties": {
 						".*": {
diff --git a/template.go b/template.go
index 8f712174..d7620a21 100644
--- a/template.go
+++ b/template.go
@@ -37,14 +37,8 @@ func (t template) Write(ctx context.Context, f *Function) error {
 	// so it's values are treated as defaults.
 	// TODO: this begs the question: should the Template's manifest.yaml actually
 	// be a partially-populated func.yaml?
-	if f.Builder == "" { // as a special first case, this default comes from itself
-		f.Builder = f.Builders["default"]
-		if f.Builder == "" { // still nothing?  then use the template
-			f.Builder = t.config.Builders["default"]
-		}
-	}
-	if len(f.Builders) == 0 {
-		f.Builders = t.config.Builders
+	if len(f.BuilderImages) == 0 {
+		f.BuilderImages = t.config.BuilderImages
 	}
 	if len(f.Buildpacks) == 0 {
 		f.Buildpacks = t.config.Buildpacks


Test patch:
diff --git a/.github/workflows/test-integration.yaml b/.github/workflows/test-integration.yaml
index e1e386b9..becea8a7 100644
--- a/.github/workflows/test-integration.yaml
+++ b/.github/workflows/test-integration.yaml
@@ -21,7 +21,5 @@ jobs:
         run: ./hack/allocate.sh
       - name: Local Registry
         run: ./hack/registry.sh
-      - name: Verify Configuration
-        run: ./hack/test.sh
       - name: Integration Test
         run: make test-integration
diff --git a/buildpacks/builder_test.go b/buildpacks/builder_test.go
new file mode 100644
index 00000000..ea5eefbe
--- /dev/null
+++ b/buildpacks/builder_test.go
@@ -0,0 +1,121 @@
+package buildpacks
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	pack "github.com/buildpacks/pack/pkg/client"
+	fn "knative.dev/kn-plugin-func"
+	. "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 := 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) {
+	var (
+		i = &mockImpl{}
+		b = NewBuilder(WithImpl(i))
+		f = fn.Function{Runtime: "node"}
+	)
+
+	i.BuildFn = func(ctx context.Context, opts pack.BuildOptions) error {
+		expected := DefaultBuilderImages["node"]
+		if opts.Builder != expected {
+			t.Fatalf("expected pack builder image '%v', got '%v'", expected, opts.Builder)
+		}
+		return nil
+	}
+
+	if err := b.Build(context.Background(), f); err != nil {
+		t.Fatal(err)
+	}
+
+}
+
+// Test_BuilderImageConfigurable ensures that the builder will use the builder
+// image defined on the given Function if provided.
+func Test_BuilderImageConfigurable(t *testing.T) {
+	var (
+		i = &mockImpl{}             // mock underlying implementation
+		b = NewBuilder(WithImpl(i)) // Func Builder logic
+		f = fn.Function{            // Function with a builder image set
+			Runtime: "node",
+			BuilderImages: map[string]string{
+				"pack": "example.com/user/builder-image",
+			},
+		}
+	)
+
+	i.BuildFn = func(ctx context.Context, opts pack.BuildOptions) error {
+		expected := f.BuilderImages["pack"]
+		if opts.Builder != expected {
+			t.Fatalf("expected builder image for node to be '%v', got '%v'", expected, opts.Builder)
+		}
+		return nil
+	}
+
+	if err := b.Build(context.Background(), f); err != nil {
+		t.Fatal(err)
+	}
+}
+
+// Test_BuildEnvs ensures that build environment variables are interpolated and
+// provided in Build Options
+func Test_BuildEnvs(t *testing.T) {
+	defer WithEnvVar(t, "INTERPOLATE_ME", "interpolated")()
+	var (
+		envName  = "NAME"
+		envValue = "{{ env:INTERPOLATE_ME }}"
+		f        = fn.Function{
+			Runtime:   "node",
+			BuildEnvs: []fn.Env{{Name: &envName, Value: &envValue}},
+		}
+		i = &mockImpl{}
+		b = NewBuilder(WithImpl(i))
+	)
+	i.BuildFn = func(ctx context.Context, opts pack.BuildOptions) error {
+		for k, v := range opts.Env {
+			if k == envName && v == "interpolated" {
+				return nil // success!
+			} else if k == envName && v == envValue {
+				t.Fatal("build env was not interpolated")
+			}
+		}
+		t.Fatal("build envs not added to builder options")
+		return nil
+	}
+	if err := b.Build(context.Background(), f); err != nil {
+		t.Fatal(err)
+	}
+}
+
+type mockImpl struct {
+	BuildFn func(context.Context, pack.BuildOptions) error
+}
+
+func (i mockImpl) Build(ctx context.Context, opts pack.BuildOptions) error {
+	return i.BuildFn(ctx, opts)
+}
diff --git a/client_int_test.go b/client_int_test.go
index fdefc04a..94c628fb 100644
--- a/client_int_test.go
+++ b/client_int_test.go
@@ -203,7 +203,7 @@ func TestRemoteRepositories(t *testing.T) {
 // newClient creates an instance of the func client whose concrete impls
 // match those created by the kn func plugin CLI.
 func newClient(verbose bool) *fn.Client {
-	builder := buildpacks.NewBuilder(verbose)
+	builder := buildpacks.NewBuilder(buildpacks.WithVerbose(verbose))
 	pusher := docker.NewPusher(docker.WithVerbose(verbose))
 	deployer := knative.NewDeployer(DefaultNamespace, verbose)
 	remover := knative.NewRemover(DefaultNamespace, verbose)
diff --git a/client_test.go b/client_test.go
index 6511aedc..3356773e 100644
--- a/client_test.go
+++ b/client_test.go
@@ -899,8 +899,8 @@ func TestClient_Deploy_UnbuiltErrors(t *testing.T) {
 	}
 }
 
-// TestClient_New_BuildersPersisted Asserts that the client preserves user-
-// provided Builders
+// TestClient_New_BuilderImagesPersisted Asserts that the client preserves user-
+// provided Builder Images
 func TestClient_New_BuildersPersisted(t *testing.T) {
 	root := "testdata/example.com/testConfiguredBuilders" // Root from which to run the test
 	defer Using(t, root)()
@@ -910,8 +910,9 @@ func TestClient_New_BuildersPersisted(t *testing.T) {
 	f0 := fn.Function{
 		Runtime: TestRuntime,
 		Root:    root,
-		Builders: map[string]string{
-			"custom": "docker.io/example/custom",
+		BuilderImages: map[string]string{
+			"pack": "example.com/my/custom-pack-builder",
+			"s2i":  "example.com/my/custom-s2i-builder",
 		}}
 
 	// Create the Function, which should preserve custom builders
@@ -926,8 +927,8 @@ func TestClient_New_BuildersPersisted(t *testing.T) {
 	}
 
 	// Assert that our custom builders were retained
-	if !reflect.DeepEqual(f0.Builders, f1.Builders) {
-		t.Fatalf("Expected %v but got %v", f0.Builders, f1.Builders)
+	if !reflect.DeepEqual(f0.BuilderImages, f1.BuilderImages) {
+		t.Fatalf("Expected %v but got %v", f0.BuilderImages, f1.BuilderImages)
 	}
 
 	// A Default Builder(image) is not asserted here, because that is
@@ -937,41 +938,6 @@ func TestClient_New_BuildersPersisted(t *testing.T) {
 	// A builder image may also be manually specified of course.
 }
 
-// TestClient_New_BuilderDefault ensures that if a custom builder is
-// provided of name "default", this is chosen as the default builder instead
-// of the inbuilt static default.
-func TestClient_New_BuilderDefault(t *testing.T) {
-	root := "testdata/example.com/testConfiguredBuildersWithDefault" // Root from which to run the test
-	defer Using(t, root)()
-
-	builders := map[string]string{
-		"custom":  "docker.io/example/custom",
-		"default": "docker.io/example/default",
-	}
-	client := fn.New(fn.WithRegistry(TestRegistry))
-	if err := client.New(context.Background(), fn.Function{
-		Runtime:  TestRuntime,
-		Root:     root,
-		Builders: builders,
-	}); err != nil {
-		t.Fatal(err)
-	}
-	f, err := fn.NewFunction(root)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	// Assert that our custom builder array was set
-	if !reflect.DeepEqual(f.Builders, builders) {
-		t.Fatalf("Expected %v but got %v", builders, f.Builders)
-	}
-
-	// Asser that the default is also set
-	if f.Builder != builders["default"] {
-		t.Fatalf("Expected %s but got %s", builders["default"], f.Builder)
-	}
-}
-
 // TestClient_New_BuildpacksPersisted ensures that provided buildpacks are
 // persisted on new Functions.
 func TestClient_New_BuildpacksPersisted(t *testing.T) {
diff --git a/function_migrations_unit_test.go b/function_migrations_unit_test.go
index 8b372dd8..8842cc44 100644
--- a/function_migrations_unit_test.go
+++ b/function_migrations_unit_test.go
@@ -82,6 +82,48 @@ func TestMigrateToCreationStamp(t *testing.T) {
 	}
 }
 
+// TestMigrateToBuilderImages ensures that the migration which migrates
+// from "builder" and "builders" to "builderImages" is applied.  This results
+// in the attributes being removed and no errors on load of the function with
+// old schema.
+func TestMigrateToBuilderImagesDefault(t *testing.T) {
+	// Load a Function created prior to the adoption of the builder images map
+	// (was created with 'builder' and 'builders' which does not support different
+	// builder implementations.
+	root := "testdata/migrations/v0.23.0"
+
+	// Without the migration, instantiating the older Function would error
+	// because its strict unmarshalling would fail parsing the unexpected
+	// 'builder' and 'builders' members.
+	_, err := NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
+// TestMigrateToBuilderImagesCustom ensures that the migration to builderImages
+// correctly carries forward a customized value for 'builder'.
+func TestMigrateToBuilderImagesCustom(t *testing.T) {
+	// An early version of a Function which includes a customized value for
+	// the 'builder'.  This should be correctly carried forward to
+	// the namespaced 'builderImages' map as image for the "pack" builder.
+	root := "testdata/migrations/v0.23.0-customized"
+	expected := "example.com/user/custom-builder" // set in testdata func.yaml
+
+	f, err := NewFunction(root)
+	if err != nil {
+		t.Fatal(f)
+	}
+	i, ok := f.BuilderImages["pack"]
+	if !ok {
+		t.Fatal("migrated Function does not include the pack builder images")
+	}
+	if i != expected {
+		t.Fatalf("migrated Function expected builder image '%v', got '%v'", expected, i)
+	}
+
+}
+
 func latestMigrationVersion() string {
 	return migrations[len(migrations)-1].version
 }
diff --git a/testdata/migrations/v0.23.0-customized/func.yaml b/testdata/migrations/v0.23.0-customized/func.yaml
new file mode 100644
index 00000000..ea10251d
--- /dev/null
+++ b/testdata/migrations/v0.23.0-customized/func.yaml
@@ -0,0 +1,26 @@
+version: 0.0.0
+name: testfunc
+namespace: ""
+runtime: go
+registry: ""
+image: ""
+imageDigest: ""
+build: local
+git: {}
+builder: "example.com/user/custom-builder"
+builders: {}
+buildpacks:
+- paketo-buildpacks/go-dist
+- ghcr.io/boson-project/go-function-buildpack:tip
+volumes: []
+buildEnvs: []
+envs: []
+annotations: {}
+options: {}
+labels: []
+healthEndpoints:
+  liveness: /health/liveness
+  readiness: /health/readiness
+created: 2022-05-25T22:44:47.36886+09:00
+invocation:
+  format: http
diff --git a/testdata/migrations/v0.23.0/func.yaml b/testdata/migrations/v0.23.0/func.yaml
new file mode 100644
index 00000000..4089f602
--- /dev/null
+++ b/testdata/migrations/v0.23.0/func.yaml
@@ -0,0 +1,26 @@
+version: 0.0.0
+name: testfunc
+namespace: ""
+runtime: go
+registry: ""
+image: ""
+imageDigest: ""
+build: local
+git: {}
+builder: ""
+builders: {}
+buildpacks:
+- paketo-buildpacks/go-dist
+- ghcr.io/boson-project/go-function-buildpack:tip
+volumes: []
+buildEnvs: []
+envs: []
+annotations: {}
+options: {}
+labels: []
+healthEndpoints:
+  liveness: /health/liveness
+  readiness: /health/readiness
+created: 2022-05-25T22:44:47.36886+09:00
+invocation:
+  format: http