diff --git a/cmd/build.go b/cmd/build.go index 046b4875..20f0b42f 100644 --- a/cmd/build.go +++ b/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 diff --git a/function.go b/function.go index 6b821a30..7a2850a5 100644 --- a/function.go +++ b/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"` diff --git a/s2i/builder.go b/s2i/builder.go index 05436cd1..9ab541f5 100644 --- a/s2i/builder.go +++ b/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 } diff --git a/s2i/builder_test.go b/s2i/builder_test.go index 284bfe3c..81061d25 100644 --- a/s2i/builder_test.go +++ b/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) +} diff --git a/schema/func_yaml-schema.json b/schema/func_yaml-schema.json index c2cc2fa3..ab238586 100644 --- a/schema/func_yaml-schema.json +++ b/schema/func_yaml-schema.json @@ -86,6 +86,14 @@ }, "type": "object" }, + "builderImages": { + "patternProperties": { + ".*": { + "type": "string" + } + }, + "type": "object" + }, "buildpacks": { "items": { "type": "string"