feat: configurable s2i builder images (#1024)

adds support for the builderImages func.yaml attribute

cmd/build.go

@@ -160,7 +160,7 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro
 	if config.Builder == "pack" {
 		builder = buildpacks.NewBuilder(config.Verbose)
 	} else if config.Builder == "s2i" {
-		builder = s2i.NewBuilder(config.Verbose)
+		builder = s2i.NewBuilder(s2i.WithVerbose(config.Verbose))
 	} else {
 		err = errors.New("unrecognized builder: valid values are: s2i, pack")
 		return

function.go

@@ -71,6 +71,13 @@ type Function struct {
 	// 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.
+	// builderImages:
+	//   pack: example.com/user/my-pack-node-builder
+	//   s2i: example.com/user/my-s2i-node-builder
+	BuilderImages map[string]string `yaml:"builderImages,omitempty"`
+
 	// Optional list of buildpacks to use when building the function
 	Buildpacks []string `yaml:"buildpacks"`
 

s2i/builder.go

@@ -39,30 +39,41 @@ var DefaultBuilderImages = map[string]string{
 // Builder of Functions using the s2i subsystem.
 type Builder struct {
 	verbose bool
+	impl    build.Builder // S2I builder implementation (aka "Strategy")
+}
+
+type Option func(*Builder)
+
+// WithVerbose toggles verbose logging.
+func WithVerbose(v bool) Option {
+	return func(b *Builder) {
+		b.verbose = v
+	}
+}
+
+// WithImpl sets an optional S2I Builder implementation override to use in
+// place of what will be generated by the S2I build "strategy" system based
+// on the config.  Used for mocking the implementation during tests.
+func WithImpl(s build.Builder) Option {
+	return func(b *Builder) {
+		b.impl = s
+	}
 }
 
 // NewBuilder creates a new instance of a Builder with static defaults.
-func NewBuilder(verbose bool) *Builder {
-	return &Builder{verbose: verbose}
+func NewBuilder(options ...Option) *Builder {
+	b := &Builder{}
+	for _, o := range options {
+		o(b)
+	}
+	return b
 }
 
 func (b *Builder) Build(ctx context.Context, f fn.Function) (err error) {
-	// Ensure the Function has a builder specified
-	if f.Builder == "" {
-		f.Builder, err = defaultBuilderImage(f)
-		if err != nil {
-			return
-		}
-	}
-
-	client, _, err := docker.NewClient(dockerClient.DefaultDockerHost)
+	// Builder image from the Function  if defined, default otherwise.
+	builderImage, err := builderImage(f)
 	if err != nil {
-		return err
-	}
-	defer client.Close()
-
-	if isPodman(ctx, client) {
-		client = podmanDockerClient{client}
+		return
 	}
 
 	// Build Config
@@ -70,7 +81,7 @@ func (b *Builder) Build(ctx context.Context, f fn.Function) (err error) {
 	cfg.Quiet = !b.verbose
 	cfg.Tag = f.Image
 	cfg.Source = &git.URL{URL: url.URL{Path: f.Root}, Type: git.URLTypeLocal}
-	cfg.BuilderImage = f.Builder
+	cfg.BuilderImage = builderImage
 	cfg.BuilderPullPolicy = api.DefaultBuilderPullPolicy
 	cfg.PreviousImagePullPolicy = api.DefaultPreviousImagePullPolicy
 	cfg.RuntimeImagePullPolicy = api.DefaultRuntimeImagePullPolicy
@@ -102,12 +113,15 @@ func (b *Builder) Build(ctx context.Context, f fn.Function) (err error) {
 		return errors.New("Unable to build via the s2i builder.")
 	}
 
-	builder, _, err := strategies.Strategy(client, cfg, build.Overrides{})
-	if err != nil {
-		return
+	// Create the S2I builder instance if not overridden
+	if b.impl == nil {
+		if b.impl, err = newImpl(ctx, cfg); err != nil {
+			return
+		}
 	}
 
-	result, err := builder.Build(cfg)
+	// Perform the build
+	result, err := b.impl.Build(cfg)
 	if err != nil {
 		return
 	}
@@ -120,15 +134,41 @@ func (b *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) {
+// builderImage for Function
+// Uses the image defined on the Function by default (for the given runtime)
+// or uses the static defaults if not defined. Returns an  ErrRuntimeRequired
+// if the Function failed to define a Runtime, and ErrRuntimeNotSupported if
+// 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 := DefaultBuilderImages[f.Runtime]
-	if !ok {
-		return "", ErrRuntimeNotSupported
+
+	v, ok := f.BuilderImages["s2i"]
+	if ok {
+		return v, nil
 	}
-	return v, nil
+
+	v, ok = DefaultBuilderImages[f.Runtime]
+	if ok {
+		return v, nil
+	}
+
+	return "", ErrRuntimeNotSupported
+}
+
+// new S2I implementation using a docker client wrapped as necessary in the
+// case of podman.
+func newImpl(ctx context.Context, cfg *api.Config) (impl build.Builder, err error) {
+	client, _, err := docker.NewClient(dockerClient.DefaultDockerHost)
+	if err != nil {
+		return
+	}
+	defer client.Close()
+	if isPodman(ctx, client) {
+		client = podmanDockerClient{client}
+	}
+	impl, _, err = strategies.Strategy(client, cfg, build.Overrides{})
+	return
 }

s2i/builder_test.go

@@ -5,14 +5,16 @@ import (
 	"errors"
 	"testing"
 
+	"github.com/openshift/source-to-image/pkg/api"
 	fn "knative.dev/kn-plugin-func"
 	"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(true)
+	b := s2i.NewBuilder()
 	err := b.Build(context.Background(), fn.Function{})
 
 	if !errors.Is(err, s2i.ErrRuntimeRequired) {
@@ -23,10 +25,126 @@ func Test_ErrRuntimeRequired(t *testing.T) {
 // 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(true)
+	b := s2i.NewBuilder()
 	err := b.Build(context.Background(), fn.Function{Runtime: "unsupported"})
 
 	if !errors.Is(err, s2i.ErrRuntimeNotSupported) {
 		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) {
+	var (
+		i = &mockImpl{}                     // mock underlying s2i implementation
+		b = s2i.NewBuilder(s2i.WithImpl(i)) // Func S2I Builder logic
+		f = fn.Function{Runtime: "node"}    // Function with no builder image set
+	)
+
+	// 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 := s2i.DefaultBuilderImages["node"]
+		if cfg.BuilderImage != expected {
+			t.Fatalf("expected s2i config builder image '%v', got '%v'", expected, cfg.BuilderImage)
+		}
+		return nil, nil
+	}
+
+	// Invoke Build, which runs Function Builder logic before invoking the
+	// mock impl above.
+	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 s2i implementation
+		b = s2i.NewBuilder(s2i.WithImpl(i)) // Func S2I Builder logic
+		f = fn.Function{                    // Function with a builder image set
+			Runtime: "node",
+			BuilderImages: map[string]string{
+				"s2i": "example.com/user/builder-image",
+			},
+		}
+	)
+
+	// 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"]
+		if cfg.BuilderImage != expected {
+			t.Fatalf("expected s2i config builder image for node to be '%v', got '%v'", expected, cfg.BuilderImage)
+		}
+		return nil, nil
+	}
+
+	// Invoke Build, which runs Function Builder logic before invoking the
+	// mock impl above.
+	if err := b.Build(context.Background(), f); err != nil {
+		t.Fatal(err)
+	}
+}
+
+// Test_Verbose ensures that the verbosity flag is propagated to the
+// S2I builder implementation.
+func Test_BuilderVerbose(t *testing.T) {
+	assert := func(verbose bool) {
+		i := &mockImpl{
+			BuildFn: func(cfg *api.Config) (r *api.Result, err error) {
+				if cfg.Quiet == verbose {
+					t.Fatalf("expected s2i quiet mode to be !%v when verbose %v", verbose, verbose)
+				}
+				return &api.Result{Messages: []string{"message"}}, nil
+			}}
+		if err := s2i.NewBuilder(s2i.WithVerbose(verbose), s2i.WithImpl(i)).Build(context.Background(), fn.Function{Runtime: "node"}); err != nil {
+			t.Fatal(err)
+		}
+	}
+	assert(true)  // when verbose is on, quiet should remain off
+	assert(false) // when verbose is off, quiet should be toggled on
+}
+
+// Test_BuildEnvs ensures that build environment variables on the Function
+// are interpolated and passed to the S2I build implementation in the final
+// build config.
+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 = s2i.NewBuilder(s2i.WithImpl(i))
+	)
+	i.BuildFn = func(cfg *api.Config) (r *api.Result, err error) {
+		for _, v := range cfg.Environment {
+			if v.Name == envName && v.Value == "interpolated" {
+				return // success!
+			} else if v.Name == envName && v.Value == envValue {
+				t.Fatal("build env was not interpolated")
+			}
+		}
+		t.Fatal("build envs not added to builder impl config")
+		return
+	}
+	if err := b.Build(context.Background(), f); err != nil {
+		t.Fatal(err)
+	}
+}
+
+// mockImpl is a mock implementation of an S2I builder.
+type mockImpl struct {
+	BuildFn func(*api.Config) (*api.Result, error)
+}
+
+func (i *mockImpl) Build(cfg *api.Config) (*api.Result, error) {
+	return i.BuildFn(cfg)
+}

schema/func_yaml-schema.json

@@ -86,6 +86,14 @@
 					},
 					"type": "object"
 				},
+				"builderImages": {
+					"patternProperties": {
+						".*": {
+							"type": "string"
+						}
+					},
+					"type": "object"
+				},
 				"buildpacks": {
 					"items": {
 						"type": "string"