feat: s2i builder with preliminary node support (#923)

* fix: stuck build ticker

* feat: s2i builder prototype

* default builders

* use s2i fork with updated docker

* in-code builder defaults

* s2i builder verbosity constructor arg

* typed errors

* typed error tests

* remove unneeded env code

* s2i build e2e test

* e2e tests

* update licenses

* cleanup

* codegen debug

* update licenses

* Revert "Update actions (#921)"

This reverts commit 8312b5c.

* update licenses

* e2e test updates

* use GetDefaultDockerConfig for s2i config

* docker.NewClient docs

Author: lkingland

buildpacks/builder.go

@@ -3,7 +3,6 @@ package buildpacks
 import (
 	"bytes"
 	"context"
-	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -21,6 +20,12 @@ 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",
+	"go":   "gcr.io/paketo-buildpacks/builder:base",
+}
+
 //Builder holds the configuration that will be passed to
 //Buildpack builder
 type Builder struct {
@@ -46,7 +51,10 @@ func (builder *Builder) Build(ctx context.Context, f fn.Function) (err error) {
 			packBuilder = pb
 		}
 	} else {
-		return errors.New("no buildpack configured for function")
+		packBuilder, err = defaultBuilderImage(f)
+		if err != nil {
+			return
+		}
 	}
 
 	// Build options for the pack client.
@@ -139,6 +147,16 @@ func (builder *Builder) Build(ctx context.Context, f fn.Function) (err error) {
 	return
 }
 
+// defaultBuilderImage for the given function based on its runtime, or an
+// error if no default is defined for the given runtime.
+func defaultBuilderImage(f fn.Function) (string, error) {
+	v, ok := DefaultBuilderImages[f.Runtime]
+	if !ok {
+		return "", fmt.Errorf("Pack builder has no default builder image specified for the '%v' language runtime.  Please provide one.", f.Runtime)
+	}
+	return v, nil
+}
+
 // hack this makes stdout non-closeable
 type stdoutWrapper struct {
 	impl io.Writer

client.go

@@ -644,7 +644,6 @@ func (c *Client) printBuildActivity(ctx context.Context) {
 	}
 	i := 0
 	ticker := time.NewTicker(10 * time.Second)
-	defer ticker.Stop()
 	go func() {
 		for {
 			select {
@@ -654,6 +653,7 @@ func (c *Client) printBuildActivity(ctx context.Context) {
 				i = i % len(m)
 			case <-ctx.Done():
 				c.progressListener.Stopping()
+				ticker.Stop()
 				return
 			}
 		}

client_test.go

@@ -900,8 +900,7 @@ func TestClient_Deploy_UnbuiltErrors(t *testing.T) {
 }
 
 // TestClient_New_BuildersPersisted Asserts that the client preserves user-
-// provided Builders on the Function configuration with the internal default
-// if not provided.
+// provided Builders
 func TestClient_New_BuildersPersisted(t *testing.T) {
 	root := "testdata/example.com/testConfiguredBuilders" // Root from which to run the test
 	defer Using(t, root)()
@@ -931,10 +930,11 @@ func TestClient_New_BuildersPersisted(t *testing.T) {
 		t.Fatalf("Expected %v but got %v", f0.Builders, f1.Builders)
 	}
 
-	// But that the default exists
-	if f1.Builder == "" {
-		t.Fatal("Expected default builder to be set")
-	}
+	// A Default Builder(image) is not asserted here, because that is
+	// the responsibility of the Builder(type) being used to build the Function.
+	// The builder (Buildpack,s2i, etc) will have a default builder image for
+	// the given Function or will error that the Function is not supported.
+	// A builder image may also be manually specified of course.
 }
 
 // TestClient_New_BuilderDefault ensures that if a custom builder is

cmd/build.go

@@ -9,6 +9,9 @@ import (
 	"github.com/ory/viper"
 	"github.com/spf13/cobra"
 
+	"knative.dev/kn-plugin-func/buildpacks"
+	"knative.dev/kn-plugin-func/s2i"
+
 	fn "knative.dev/kn-plugin-func"
 )
 
@@ -40,17 +43,18 @@ and the image name is stored in the configuration file.
 {{.Name}} build --builder cnbs/sample-builder:bionic
 `,
 		SuggestFor: []string{"biuld", "buidl", "built"},
-		PreRunE:    bindEnv("image", "path", "builder", "registry", "confirm", "push"),
+		PreRunE:    bindEnv("image", "path", "builder", "registry", "confirm", "push", "builder-image"),
 	}
 
-	cmd.Flags().StringP("builder", "b", "", "Buildpack builder, either an as a an image name or a mapping name.\nSpecified value is stored in func.yaml for subsequent builds.")
+	cmd.Flags().StringP("builder", "b", "pack", "builder to use when creating the underlying image. Currently supported builders are 'pack' and 's2i'.")
+	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 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)")
 	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)")
 	cmd.Flags().BoolP("push", "u", false, "Attempt to push the function image after being successfully built")
 	setPathFlag(cmd)
 
-	if err := cmd.RegisterFlagCompletionFunc("builder", CompleteBuilderList); err != nil {
+	if err := cmd.RegisterFlagCompletionFunc("builder", CompleteBuilderImageList); err != nil {
 		fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err)
 	}
 
@@ -89,7 +93,7 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro
 		return
 	}
 
-	function, err := functionWithOverrides(config.Path, functionOverrides{Builder: config.Builder, Image: config.Image})
+	function, err := functionWithOverrides(config.Path, functionOverrides{BuilderImage: config.BuilderImage, Image: config.Image})
 	if err != nil {
 		return
 	}
@@ -144,15 +148,22 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro
 		config.Registry = ""
 	}
 
-	// TODO(lkingland): The below deferred options gathering is what will
-	// re-enable the addition of alternative implementations of the Builder,
-	// unblocking PR https://github.com/knative-sandbox/kn-plugin-func/pull/842
-	// the implementation of which will be inserted here.
+	// Choose a builder based on the value of the --builder flag
+	var builder fn.Builder
+	if config.Builder == "pack" {
+		builder = buildpacks.NewBuilder(config.Verbose)
+	} else if config.Builder == "s2i" {
+		builder = s2i.NewBuilder(config.Verbose)
+	} else {
+		err = errors.New("unrecognized builder")
+		return
+	}
 
 	// 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},
-		fn.WithRegistry(config.Registry))
+		fn.WithRegistry(config.Registry),
+		fn.WithBuilder(builder))
 	defer done()
 
 	err = client.Build(cmd.Context(), config.Path)
@@ -184,18 +195,27 @@ type buildConfig struct {
 	// Confirm: confirm values arrived upon from environment plus flags plus defaults,
 	// with interactive prompting (only applicable when attached to a TTY).
 	Confirm bool
+
+	// Builder is the name of the subsystem that will complete the underlying
+	// build (Pack, s2i, remote pipeline, etc).  Currently ad-hoc rather than
+	// an enumerated field.  See the Client constructory for logic.
 	Builder string
+
+	// BuilderImage is the image (name or mapping) to use for building.  Usually
+	// set automatically.
+	BuilderImage string
 }
 
 func newBuildConfig() buildConfig {
 	return buildConfig{
-		Image:    viper.GetString("image"),
-		Path:     viper.GetString("path"),
-		Registry: viper.GetString("registry"),
-		Verbose:  viper.GetBool("verbose"), // defined on root
-		Confirm:  viper.GetBool("confirm"),
-		Builder:  viper.GetString("builder"),
-		Push:     viper.GetBool("push"),
+		Image:        viper.GetString("image"),
+		Path:         viper.GetString("path"),
+		Registry:     viper.GetString("registry"),
+		Verbose:      viper.GetBool("verbose"), // defined on root
+		Confirm:      viper.GetBool("confirm"),
+		Builder:      viper.GetString("builder"),
+		BuilderImage: viper.GetString("builder-image"),
+		Push:         viper.GetBool("push"),
 	}
 }
 

cmd/completion_util.go

@@ -95,7 +95,7 @@ func CompleteRegistryList(cmd *cobra.Command, args []string, toComplete string)
 	return
 }
 
-func CompleteBuilderList(cmd *cobra.Command, args []string, complete string) (strings []string, directive cobra.ShellCompDirective) {
+func CompleteBuilderImageList(cmd *cobra.Command, args []string, complete string) (strings []string, directive cobra.ShellCompDirective) {
 	directive = cobra.ShellCompDirectiveError
 
 	var (

cmd/root.go

@@ -168,9 +168,9 @@ func bindEnv(flags ...string) bindFunc {
 }
 
 type functionOverrides struct {
-	Image     string
-	Namespace string
-	Builder   string
+	Image        string
+	Namespace    string
+	BuilderImage string
 }
 
 // functionWithOverrides sets the namespace and image strings for the
@@ -187,7 +187,7 @@ func functionWithOverrides(root string, overrides functionOverrides) (f fn.Funct
 		src  string
 		dest *string
 	}{
-		{overrides.Builder, &f.Builder},
+		{overrides.BuilderImage, &f.Builder},
 		{overrides.Image, &f.Image},
 		{overrides.Namespace, &f.Namespace},
 	}

docker/docker_client.go

@@ -19,6 +19,14 @@ import (
 	"github.com/docker/docker/client"
 )
 
+// NewClient creates a new docker client.
+// reads the DOCKER_HOST envvar but it may or may not return it as dockerHost.
+//  - For local connection (unix socket and windows named pipe) it returns the
+//    DOCKER_HOST directly.
+//  - For ssh connections it reads the DOCKER_HOST from the ssh remote.
+//  - For TCP connections it returns "" so it defaults in the remote (note that
+//    one should not be use client.DefaultDockerHost in this situation). This is
+//    needed beaus of TCP+tls connections.
 func NewClient(defaultHost string) (dockerClient client.CommonAPIClient, dockerHost string, err error) {
 	var _url *url.URL
 

go.mod

@@ -9,7 +9,7 @@ require (
 	github.com/alecthomas/jsonschema v0.0.0-20210526225647-edb03dcab7bc
 	github.com/buildpacks/pack v0.24.0
 	github.com/cloudevents/sdk-go/v2 v2.5.0
-	github.com/containers/image/v5 v5.10.6
+	github.com/containers/image/v5 v5.19.1
 	github.com/coreos/go-semver v0.3.0
 	github.com/docker/cli v20.10.12+incompatible
 	github.com/docker/docker v20.10.12+incompatible
@@ -23,6 +23,7 @@ require (
 	github.com/hinshun/vt10x v0.0.0-20180809195222-d55458df857c
 	github.com/mitchellh/go-homedir v1.1.0
 	github.com/opencontainers/image-spec v1.0.3-0.20220114050600-8b9d41f48198
+	github.com/openshift/source-to-image v1.3.1
 	github.com/ory/viper v1.7.5
 	github.com/pkg/errors v0.9.1
 	github.com/spf13/cobra v1.3.0
@@ -45,6 +46,9 @@ require (
 )
 
 replace (
+	// update docker to be compatible with version used by pack and removes invalid pseudo-version
+	github.com/openshift/source-to-image => github.com/boson-project/source-to-image v1.3.2
+
 	// Pin k8s.io dependencies to align with Knative and Tekton needs
 	k8s.io/api => k8s.io/api v0.22.5
 	k8s.io/apimachinery => k8s.io/apimachinery v0.22.5