feat: persist builder value in func.yaml (#1099)

* Persist builder value in func.yaml

* Added tests

* removed print statement for platform error

* created common function for builder persistence test

cmd/build.go

@@ -3,7 +3,6 @@ package cmd
 import (
 	"errors"
 	"fmt"
-	"os"
 
 	"github.com/AlecAivazis/survey/v2"
 	"github.com/AlecAivazis/survey/v2/terminal"
@@ -144,12 +143,33 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro
 		function.Image = config.Image
 	}
 
+	// Choose a builder based on the value of the --builder flag
+	var builder fn.Builder
+	if function.Builder == "" || cmd.Flags().Changed("builder") {
+		function.Builder = config.Builder
+	} else {
+		config.Builder = function.Builder
+	}
+
 	// All set, let's write changes in the config to the disk
 	err = function.Write()
 	if err != nil {
 		return
 	}
 
+	if config.Builder == "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 == "s2i" {
+		builder = s2i.NewBuilder(s2i.WithVerbose(config.Verbose), s2i.WithPlatform(config.Platform))
+	} else {
+		err = errors.New("unrecognized builder: valid values are: s2i, pack")
+		return
+	}
+
 	// if registry was not changed via command line flag meaning it's empty
 	// keep the same registry by setting the config.registry to empty otherwise
 	// trust viper to override the env variable with the given flag if both are specified
@@ -157,20 +177,6 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro
 		config.Registry = ""
 	}
 
-	// Choose a builder based on the value of the --builder flag
-	var builder fn.Builder
-	if config.Builder == "pack" {
-		if config.Platform != "" {
-			fmt.Fprintln(os.Stderr, "the --platform flag works only with s2i build")
-		}
-		builder = buildpacks.NewBuilder(buildpacks.WithVerbose(config.Verbose))
-	} else if config.Builder == "s2i" {
-		builder = s2i.NewBuilder(s2i.WithVerbose(config.Verbose), s2i.WithPlatform(config.Platform))
-	} else {
-		err = errors.New("unrecognized builder: valid values are: s2i, pack")
-		return
-	}
-
 	// Use the user-provided builder image, if supplied
 	if config.BuilderImage != "" {
 		function.BuilderImages[config.Builder] = config.BuilderImage

cmd/build_test.go

@@ -1,11 +1,13 @@
 package cmd
 
 import (
+	"context"
 	"fmt"
 	"io/ioutil"
 	"os"
 	"testing"
 
+	"github.com/spf13/cobra"
 	fn "knative.dev/kn-plugin-func"
 	"knative.dev/kn-plugin-func/mock"
 	. "knative.dev/kn-plugin-func/testing"
@@ -150,3 +152,97 @@ created: 2009-11-10 23:00:00`,
 		})
 	}
 }
+
+func testBuilderPersistence(t *testing.T, testRegistry string, cmdBuilder func(ClientFactory) *cobra.Command) {
+	root, rm := Mktemp(t)
+	defer rm()
+
+	client := fn.New(fn.WithRegistry(testRegistry))
+
+	f := fn.Function{Runtime: "go", Root: root, Name: "myfunc", Registry: testRegistry}
+
+	if err := client.New(context.Background(), f); err != nil {
+		t.Fatal(err)
+	}
+
+	cmd := cmdBuilder(NewClientFactory(func() *fn.Client {
+		return client
+	}))
+
+	cmd.SetArgs([]string{"--registry", testRegistry})
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+
+	var err error
+	// Assert the Function has persisted a value of builder (has a default)
+	if f, err = fn.NewFunction(root); err != nil {
+		t.Fatal(err)
+	}
+	if f.Builder == "" {
+		t.Fatal("value of builder not persisted using a flag default")
+	}
+
+	// Build the Function, specifying a Builder
+	cmd.SetArgs([]string{"--builder=s2i"})
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+	// Assert the Function has persisted the value of builder
+	if f, err = fn.NewFunction(root); err != nil {
+		t.Fatal(err)
+	}
+	if f.Builder != "s2i" {
+		t.Fatal("value of builder flag not persisted when provided")
+	}
+	// Build the function without specifying a Builder
+	cmd = cmdBuilder(NewClientFactory(func() *fn.Client {
+		return client
+	}))
+	cmd.SetArgs([]string{"--registry", testRegistry})
+
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+
+	// Assert the function has retained its original value
+	if f, err = fn.NewFunction(root); err != nil {
+		t.Fatal(err)
+	}
+
+	if f.Builder != "s2i" {
+		t.Fatal("value of builder updated when not provided")
+	}
+
+	// Build the Function again using a different builder
+	cmd.SetArgs([]string{"--builder=pack"})
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+
+	// Assert the Function has persisted the new value
+	if f, err = fn.NewFunction(root); err != nil {
+		t.Fatal(err)
+	}
+	if f.Builder != "pack" {
+		t.Fatal("value of builder flag not persisted on subsequent build")
+	}
+
+	// Build the Function, specifying a platform with "pack" Builder
+	cmd.SetArgs([]string{"--platform", "linux"})
+	if err := cmd.Execute(); err == nil {
+		t.Fatal("Expected error")
+	}
+
+	// Set an invalid builder
+	cmd.SetArgs([]string{"--builder", "invalid"})
+	if err := cmd.Execute(); err == nil {
+		t.Fatal("Expected error")
+	}
+}
+
+// TestBuild_BuilderPersistence ensures that the builder chosen is read from
+// the Function by default, and is able to be overridden by flags/env vars.
+func TestBuild_BuilderPersistence(t *testing.T) {
+	testBuilderPersistence(t, "docker.io/tigerteam", NewBuildCmd)
+}

cmd/deploy.go

@@ -170,6 +170,25 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err
 		function.Image = config.Image
 	}
 
+	// Choose a builder based on the value of the --builder flag
+	var builder fn.Builder
+	if function.Builder == "" || cmd.Flags().Changed("builder") {
+		function.Builder = config.Builder
+	} else {
+		config.Builder = function.Builder
+	}
+	if config.Builder == "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 == "s2i" {
+		builder = s2i.NewBuilder(s2i.WithVerbose(config.Verbose), s2i.WithPlatform(config.Platform))
+	} else {
+		err = errors.New("unrecognized builder: valid values are: s2i, pack")
+		return
+	}
 	// All set, let's write changes in the config to the disk
 	err = function.Write()
 	if err != nil {
@@ -188,20 +207,6 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err
 		config.Registry = ""
 	}
 
-	// Choose a builder based on the value of the --builder flag
-	var builder fn.Builder
-	if config.Builder == "pack" {
-		if config.Platform != "" {
-			fmt.Fprintln(os.Stderr, "the --platform flag works only with s2i build")
-		}
-		builder = buildpacks.NewBuilder(buildpacks.WithVerbose(config.Verbose))
-	} else if config.Builder == "s2i" {
-		builder = s2i.NewBuilder(s2i.WithVerbose(config.Verbose), s2i.WithPlatform(config.Platform))
-	} else {
-		err = errors.New("unrecognized builder: valid values are: s2i, pack")
-		return
-	}
-
 	// Use the user-provided builder image, if supplied
 	if config.BuilderImage != "" {
 		function.BuilderImages[config.Builder] = config.BuilderImage

cmd/deploy_test.go

@@ -270,3 +270,9 @@ runtime: go`,
 		})
 	}
 }
+
+// TestDeploy_BuilderPersistence ensures that the builder chosen is read from
+// the Function by default, and is able to be overridden by flags/env vars.
+func TestDeploy_BuilderPersistence(t *testing.T) {
+	testBuilderPersistence(t, "docker.io/tigerteam", NewDeployCmd)
+}

function.go

@@ -75,6 +75,10 @@ type Function struct {
 	// Optional list of buildpacks to use when building the function
 	Buildpacks []string `yaml:"buildpacks"`
 
+	// Builder is the name of the subsystem that will complete the underlying
+	// build (Pack, s2i, remote pipeline, etc)
+	Builder string `yaml:"builder"`
+
 	// List of volumes to be mounted to the function
 	Volumes []Volume `yaml:"volumes"`
 

schema/func_yaml-schema.json

@@ -30,6 +30,7 @@
 				"build",
 				"git",
 				"buildpacks",
+				"builder",
 				"volumes",
 				"buildEnvs",
 				"envs",
@@ -87,6 +88,9 @@
 					},
 					"type": "array"
 				},
+				"builder": {
+					"type": "string"
+				},
 				"volumes": {
 					"items": {
 						"$schema": "http://json-schema.org/draft-04/schema#",