diff --git a/builders/builders.go b/builders/builders.go new file mode 100644 index 00000000..845e8c82 --- /dev/null +++ b/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} + +} diff --git a/builders/builders_test.go b/builders/builders_test.go new file mode 100644 index 00000000..dc0a3edb --- /dev/null +++ b/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()) + } + } +} diff --git a/buildpacks/builder.go b/buildpacks/builder.go index f22f8b2f..1d2e944b 100644 --- a/buildpacks/builder.go +++ b/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. - image, err := BuilderImage(f) + // Builder image from the function if defined, default otherwise. + 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 -// -// 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. 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} +// Builder Image chooses the correct builder image or defaults. +func BuilderImage(f fn.Function, builderName string) (string, error) { + return builders.Image(f, builderName, DefaultBuilderImages) } // podmanPreV330 returns if the daemon is podman pre v330 or errors trying. diff --git a/buildpacks/builder_test.go b/buildpacks/builder_test.go index ea5eefbe..cedf19bb 100644 --- a/buildpacks/builder_test.go +++ b/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 -// 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 +// Test_BuilderImageDefault 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 - b = NewBuilder(WithImpl(i)) // Func Builder logic - f = fn.Function{ // Function with a builder image set + i = &mockImpl{} // mock underlying implementation + b = NewBuilder( // Func Builder logic + 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) } diff --git a/client_test.go b/client_test.go index ac1bc3a7..26566cea 100644 --- a/client_test.go +++ b/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", - "s2i": "example.com/my/custom-s2i-builder", + builders.Pack: "example.com/my/custom-pack-builder", + builders.S2I: "example.com/my/custom-s2i-builder", }} // Create the function, which should preserve custom builders diff --git a/cmd/build.go b/cmd/build.go index 929b0929..eee6334b 100644 --- a/cmd/build.go +++ b/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)) - } else if config.Builder == fn.BuilderS2i { - builder = s2i.NewBuilder(s2i.WithVerbose(config.Verbose), s2i.WithPlatform(config.Platform)) + builder = buildpacks.NewBuilder( + buildpacks.WithName(builders.Pack), + 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 diff --git a/cmd/build_test.go b/cmd/build_test.go index 34025467..da1cf08b 100644 --- a/cmd/build_test.go +++ b/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") + } +} diff --git a/cmd/completion_util.go b/cmd/completion_util.go index 4a0124c5..84c74be6 100644 --- a/cmd/completion_util.go +++ b/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 } diff --git a/cmd/deploy.go b/cmd/deploy.go index 17d18717..fee34e15 100644 --- a/cmd/deploy.go +++ b/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)) } diff --git a/docs/reference/commands.txt b/docs/reference/commands.txt index 00dd0cfc..d384313e 100644 --- a/docs/reference/commands.txt +++ b/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") diff --git a/function_builder.go b/function_builder.go deleted file mode 100644 index 38c6dc98..00000000 --- a/function_builder.go +++ /dev/null @@ -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) -} diff --git a/function_builder_unit_test.go b/function_builder_unit_test.go deleted file mode 100644 index 3a681bed..00000000 --- a/function_builder_unit_test.go +++ /dev/null @@ -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) - } - }) - } -} diff --git a/pipelines/tekton/resources.go b/pipelines/tekton/resources.go index e9cadb37..790d9344 100644 --- a/pipelines/tekton/resources.go +++ b/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 } diff --git a/s2i/builder.go b/s2i/builder.go index fd6f5c0f..b5ad461b 100644 --- a/s2i/builder.go +++ b/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 ( - // ErrRuntimeRequired indicates the required value of function runtime was not provided - ErrRuntimeRequired = errors.New("runtime is required to build") -) +// DefaultName when no WithName option is provided to NewBuilder +const DefaultName = builders.S2I // 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. - builderImage, err := builderImage(f) + // Builder image from the function if defined, default otherwise. + 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 -// 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 := 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) +// Builder Image chooses the correct builder image or defaults. +func BuilderImage(f fn.Function, builderName string) (string, error) { + // delegate as the logic is shared amongst builders + return builders.Image(f, builderName, DefaultBuilderImages) } diff --git a/s2i/builder_test.go b/s2i/builder_test.go index deac11e2..381ea428 100644 --- a/s2i/builder_test.go +++ b/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 - 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 + i = &mockImpl{} // mock underlying s2i implementation + c = mockDocker{} // mock docker client + 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 - c = mockDocker{} // mock docker client - b = s2i.NewBuilder(s2i.WithImpl(i), s2i.WithDockerClient(c)) // func S2I Builder logic - f = fn.Function{ // function with a builder image set + i = &mockImpl{} // mock underlying s2i implementation + c = mockDocker{} // mock docker client + b = s2i.NewBuilder( // func S2I Builder logic + 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) diff --git a/test/_e2e/cmd_config_envs_test.go b/test/_e2e/cmd_config_envs_test.go index 48c6bf17..045a5932 100644 --- a/test/_e2e/cmd_config_envs_test.go +++ b/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() }() diff --git a/test/_e2e/cmd_config_labels_test.go b/test/_e2e/cmd_config_labels_test.go index 7cb72199..266f769b 100644 --- a/test/_e2e/cmd_config_labels_test.go +++ b/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() }() diff --git a/test/_e2e/cmd_config_volumes_test.go b/test/_e2e/cmd_config_volumes_test.go index a66d9e48..715e45df 100644 --- a/test/_e2e/cmd_config_volumes_test.go +++ b/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() diff --git a/test/_e2e/e2e_remote_repository_test.go b/test/_e2e/e2e_remote_repository_test.go index f6457a41..6379e1fa 100644 --- a/test/_e2e/e2e_remote_repository_test.go +++ b/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,