Repository: knative-sandbox/kn-plugin-func
Pull request #495 (2021-08-25 18:27:08)

feat: builders/buildpacks configured in client
 # Changes

- :gift: Recognize Builders, Builder and Buildpack in Client API function configuration when creating a new project

We have provided support for modifiying the builders and buildpacksin `func.yaml` but until now there has not been a way to set these values when creating a new function with `client.Create(cfg)`. Any values provided in `cfg` for the builders and buildpacks were ignored and the builtin defaults were used.

/kind enhancement




Patch:
diff --git a/.github/workflows/pull_requests.yaml b/.github/workflows/pull_requests.yaml
index b85a3e53..c82892b4 100644
--- a/.github/workflows/pull_requests.yaml
+++ b/.github/workflows/pull_requests.yaml
@@ -150,11 +150,3 @@ jobs:
         run: ./hack/test.sh
       - name: E2E Test
         run: make test-e2e
-      #- name: E2E Test Go
-      #  run: ./test/run_e2e_test.sh go
-      #- name: E2E Test Python
-      #  run: ./test/run_e2e_test.sh python
-      #- name: E2E Test Quarkus
-      #  run: ./test/run_e2e_test.sh quarkus
-      #- name: E2E Test Springboot
-      #  run: ./test/run_e2e_test.sh springboot
diff --git a/Makefile b/Makefile
index f6565b29..8730cfb9 100644
--- a/Makefile
+++ b/Makefile
@@ -124,7 +124,8 @@ test-integration: ## Run integration tests using an available cluster.
 	go test -tags integration ./... -v
 
 test-e2e: ## Run end-to-end tests using an available cluster.
-	./test/run_e2e_test.sh
+	./test/e2e_lifecycle_tests.sh node
+	./test/e2e_extended_tests.sh
 
 
 ######################
diff --git a/client.go b/client.go
index bca749e5..7d337153 100644
--- a/client.go
+++ b/client.go
@@ -381,15 +381,29 @@ func (c *Client) Create(cfg Function) (err error) {
 		return
 	}
 
-	// Create Function about the given root path.
-	// Loads extant config if it exists.  In-memory representation only.
+	// Root must not already be a Function
+	//
+	// Instantiate a Function struct about the given root path, but
+	// immediately exit with error (prior to actual creation) if a
+	// Function already existed at that path (Create should never
+	// clobber a pre-existing Function)
 	f, err := NewFunction(cfg.Root)
 	if err != nil {
 		return
 	}
+	if f.Initialized() {
+		err = fmt.Errorf("Function at '%v' already initialized.", cfg.Root)
+		return
+	}
 
-	// Assert the specified root is free of visible files and contentious
-	// hidden files (the ConfigFile, which indicates it is already initialized)
+	// Root must not contain any visible files
+	//
+	// We know from above that the target directory does not contain a Function,
+	// but also immediately exit if the target directoy contains any visible files
+	// at all, or any of the known hidden files that will be written.
+	// This is to ensure that if a user inadvertently chooses an incorrect directory
+	// for their new Function, the template and config file writing steps do not
+	// cause data loss.
 	if err = assertEmptyRoot(f.Root); err != nil {
 		return
 	}
@@ -426,15 +440,6 @@ func (c *Client) Create(cfg Function) (err error) {
 			f.Builders = manifest.Builders
 			f.Buildpacks = manifest.Buildpacks
 			f.HealthEndpoints = manifest.HealthEndpoints
-			if c.verbose {
-				fmt.Printf("Builder:       %s\n", f.Builder)
-				if len(f.Buildpacks) > 0 {
-					fmt.Println("Buildpacks:")
-					for _, b := range f.Buildpacks {
-						fmt.Printf("           ... %s\n", b)
-					}
-				}
-			}
 		}
 		// Remove the manifest.yaml file so the user is not confused by a
 		// configuration file that is only used for project creation/initialization
@@ -445,6 +450,37 @@ func (c *Client) Create(cfg Function) (err error) {
 		}
 	}
 
+	// Now that defaults are set from manifest.yaml for builders/buildpacks
+	// be sure to allow configuration to override these
+
+	// If buildpacks are provided, use them
+	if len(cfg.Buildpacks) > 0 {
+		f.Buildpacks = cfg.Buildpacks
+	}
+
+	// If builders are provided use them
+	if len(cfg.Builders) > 0 {
+		f.Builders = cfg.Builders
+		if f.Builders["default"] != "" {
+			f.Builder = f.Builders["default"]
+		}
+	}
+
+	// If a default builder is provided use it
+	if cfg.Builder != "" {
+		f.Builder = cfg.Builder
+	}
+
+	if c.verbose {
+		fmt.Printf("Builder:       %s\n", f.Builder)
+		if len(f.Buildpacks) > 0 {
+			fmt.Println("Buildpacks:")
+			for _, b := range f.Buildpacks {
+				fmt.Printf("           ... %s\n", b)
+			}
+		}
+	}
+
 	// Write out the config.
 	if err = writeConfig(f); err != nil {
 		return


Test patch:
diff --git a/client_test.go b/client_test.go
index 9bb735f3..7b557d23 100644
--- a/client_test.go
+++ b/client_test.go
@@ -9,6 +9,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"reflect"
 	"testing"
 
 	fn "knative.dev/kn-plugin-func"
@@ -712,7 +713,7 @@ func TestDeployUnbuilt(t *testing.T) {
 	}
 }
 
-// TestEmit ensures that the
+// TestEmit ensures that the client properly invokes the emitter when provided
 func TestEmit(t *testing.T) {
 	sink := "http://testy.mctestface.com"
 	emitter := mock.NewEmitter()
@@ -736,6 +737,100 @@ func TestEmit(t *testing.T) {
 	}
 }
 
+// Asserts that the client properly writes user provided Builders
+// to the Function configuration but uses internal default if
+// not provided.
+func TestWithConfiguredBuilders(t *testing.T) {
+	root := "testdata/example.com/testConfiguredBuilders" // Root from which to run the test
+	defer using(t, root)()
+
+	builders := map[string]string{
+		"custom": "docker.io/example/custom",
+	}
+	client := fn.New(fn.WithRegistry(TestRegistry))
+	if err := client.Create(fn.Function{
+		Root:     root,
+		Builders: builders,
+	}); err != nil {
+		t.Fatal(err)
+	}
+	f, err := fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Assert that our custom builder array was set
+	if !reflect.DeepEqual(f.Builders, builders) {
+		t.Fatalf("Expected %v but got %v", builders, f.Builders)
+	}
+
+	// But that the default still exists
+	if f.Builder == "" {
+		t.Fatal("Expected default builder to be set")
+	}
+}
+
+// Asserts that the client properly sets user provided Builders
+// in the Function configuration, and if one of the provided is
+// keyed as "default", this is set as the default Builder.
+func TestWithConfiguredBuildersWithDefault(t *testing.T) {
+	root := "testdata/example.com/testConfiguredBuildersWithDefault" // Root from which to run the test
+	defer using(t, root)()
+
+	builders := map[string]string{
+		"custom":  "docker.io/example/custom",
+		"default": "docker.io/example/default",
+	}
+	client := fn.New(fn.WithRegistry(TestRegistry))
+	if err := client.Create(fn.Function{
+		Root:     root,
+		Builders: builders,
+	}); err != nil {
+		t.Fatal(err)
+	}
+	f, err := fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Assert that our custom builder array was set
+	if !reflect.DeepEqual(f.Builders, builders) {
+		t.Fatalf("Expected %v but got %v", builders, f.Builders)
+	}
+
+	// Asser that the default is also set
+	if f.Builder != builders["default"] {
+		t.Fatalf("Expected %s but got %s", builders["default"], f.Builder)
+	}
+}
+
+// Asserts that the client properly sets the Buildpacks property
+// in the Function configuration when it is provided.
+func TestWithConfiguredBuildpacks(t *testing.T) {
+	root := "testdata/example.com/testConfiguredBuildpacks" // Root from which to run the test
+	defer using(t, root)()
+
+	buildpacks := []string{
+		"docker.io/example/custom-buildpack",
+	}
+	client := fn.New(fn.WithRegistry(TestRegistry))
+	if err := client.Create(fn.Function{
+		Root:       root,
+		Buildpacks: buildpacks,
+	}); err != nil {
+		t.Fatal(err)
+	}
+	f, err := fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Assert that our custom buildpacks were set
+	if !reflect.DeepEqual(f.Buildpacks, buildpacks) {
+		t.Fatalf("Expected %v but got %v", buildpacks, f.Buildpacks)
+	}
+}
+
 // Helpers ----
 
 // USING:  Make specified dir.  Return deferrable cleanup fn.
diff --git a/cmd/config_labels_test.go b/cmd/config_labels_test.go
index 9b85460f..4ceee9af 100644
--- a/cmd/config_labels_test.go
+++ b/cmd/config_labels_test.go
@@ -129,3 +129,63 @@ func TestNewConfigLabelsCmd(t *testing.T) {
 	run("remove", arrowDown, enter)
 	assertLabel(fn.Labels{p("e","f"), p("c", "d")})
 }
+
+
+func TestListLabels(t *testing.T) {
+
+	p := func (k,v string) fn.Label {
+		return fn.Label{Key: &k, Value: &v}
+	}
+
+	var loaderSaver mockFunctionLoaderSaver
+	labels := &loaderSaver.f.Labels
+
+	*labels = append(*labels, p("a", "b"), p("c", "d"))
+
+	cmd := NewConfigLabelsCmd(&loaderSaver)
+
+	ctx := context.Background()
+	c, _, err := vt10x.NewVT10XConsole()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer c.Close()
+
+	cmd.SetArgs(make([]string, 0))
+
+	errChan := make(chan error, 1)
+	func () {
+		var err error
+		defer func() {
+			errChan <- err
+		}()
+		defer withMockedStdio(t, c)()
+		err = cmd.ExecuteContext(ctx)
+	}()
+
+	expected := []string{
+		`Configured labels:`,
+		`-  Label with key "a" and value "b"`,
+		`-  Label with key "c" and value "d"`,
+	}
+
+	// prevents the ExpectString() function from waiting indefinitely
+	// in case when expected string is not printed to stdout nor the stdout is closed
+	go func() {
+		time.Sleep(time.Second * 5)
+		c.Close()
+	}()
+
+	for _, s := range expected {
+		out, err := c.ExpectString(s)
+		if err != nil {
+			t.Errorf("unexpected output: %q, err: %v\n", out, err)
+		}
+	}
+
+	err = <- errChan
+	if err != nil {
+		t.Fatal(err)
+	}
+
+}
diff --git a/test/_e2e/e2e_functions_test.go b/test/_e2e/e2e_functions_test.go
index a19e2f82..6ca93b2f 100644
--- a/test/_e2e/e2e_functions_test.go
+++ b/test/_e2e/e2e_functions_test.go
@@ -1,4 +1,4 @@
-// +build e2e e2efunc
+// +build e2elc
 
 package e2e
 
diff --git a/test/_e2e/main_test.go b/test/_e2e/main_test.go
index 561070e1..dac201de 100644
--- a/test/_e2e/main_test.go
+++ b/test/_e2e/main_test.go
@@ -1,4 +1,4 @@
-// +build e2e
+// +build e2e e2elc
 
 package e2e
 
diff --git a/test/_e2e/trigger_events_test.go b/test/_e2e/trigger_events_test.go
index 86bd28d9..00e36deb 100644
--- a/test/_e2e/trigger_events_test.go
+++ b/test/_e2e/trigger_events_test.go
@@ -25,11 +25,10 @@ func (s SimpleTestEvent) pushTo(url string, t *testing.T) (body string, statusCo
 	req.Header.Add("Content-Type", s.ContentType)
 	resp, err := client.Do(req)
 
-	t.Logf("event POST %v -> %v", url, resp.Status)
-
 	if err != nil {
 		return "", 0, err
 	}
+	t.Logf("event POST %v -> %v", url, resp.Status)
 	defer resp.Body.Close()
 	b, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
diff --git a/test/e2e_extended_tests.sh b/test/e2e_extended_tests.sh
new file mode 100755
index 00000000..c4d83027
--- /dev/null
+++ b/test/e2e_extended_tests.sh
@@ -0,0 +1,51 @@
+#!/usr/bin/env bash
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+#
+# Runs E2E additional set of tests against kn func cli.
+# By default it will run e2e tests against 'func' binary, but you can change it to use 'kn func' instead
+#
+# The following environment variable can be set in order to customize e2e execution:
+#
+# E2E_USE_KN_FUNC    When set to "true" indicates e2e to issue func command using kn cli.
+#
+# E2E_REGISTRY_URL   Indicates a specific registry (i.e: "quay.io/user") should be used. Make sure
+#                    to authenticate to the registry (i.e: docker login ...) prior to execute the script
+#
+# E2E_FUNC_BIN_PATH  Path to func binary. Derived by this script
+#
+
+set -o errexit
+set -o nounset
+set -o pipefail
+
+use_kn_func=${E2E_USE_KN_FUNC:-}
+
+curdir=$(pwd)
+cd $(dirname $0)
+cd ../
+
+# Make sure 'func' binary is built in case KN FUNC was not required for testing
+if [[ ! -f func && "$use_kn_func" != "true" ]]; then
+  echo "func binary not found. Please run 'make build' prior to run e2e."
+  exit 1
+fi
+
+export E2E_FUNC_BIN_PATH=$(pwd)/func
+
+go clean -testcache
+go test -v -test.v -tags="e2e" ./test/_e2e/
+ret=$?
+
+cd $curdir
+exit $ret
diff --git a/test/run_e2e_test.sh b/test/e2e_lifecycle_tests.sh
similarity index 76%
rename from test/run_e2e_test.sh
rename to test/e2e_lifecycle_tests.sh
index bf04a524..b6e8ae85 100755
--- a/test/run_e2e_test.sh
+++ b/test/e2e_lifecycle_tests.sh
@@ -12,9 +12,14 @@
 # limitations under the License.
 
 #
-# Runs E2E tests against for func cli.
+# Runs basic lifecycle E2E tests against kn func cli for a given language/runtime.
 # By default it will run e2e tests against 'func' binary, but you can change it to use 'kn func' instead
 #
+# Use:
+#   ./e2e_lifecycle_tests.sh <language>  (defaults to "node")
+# Example:
+#   ./e2e_lifecycle_tests.sh python
+#
 # The following environment variable can be set in order to customize e2e execution:
 #
 # E2E_USE_KN_FUNC    When set to "true" indicates e2e to issue func command using kn cli.
@@ -24,26 +29,19 @@
 #
 # E2E_FUNC_BIN_PATH  Path to func binary. Derived by this script
 #
-# In additional to these env variables, for convinience you can run only the tests specific per runtime
-# without running other e2e tests, by passing the runtime as an argument for this script. Example:
-#
-#   ./run_e2e_test.sh python
-#
 
 set -o errexit
 set -o nounset
 set -o pipefail
 
 runtime=${1:-}
-tag=e2e
 use_kn_func=${E2E_USE_KN_FUNC:-}
 
-curdir=`pwd`
-cd `dirname $0`
-projectdir=../
-cd $projectdir
+curdir=$(pwd)
+cd $(dirname $0)
+cd ../
 
-# Make sure 'func' binary is built in case KN FUNC was not required for this test
+# Make sure 'func' binary is built in case KN FUNC was not required for testing
 if [[ ! -f func && "$use_kn_func" != "true" ]]; then
   echo "func binary not found. Please run 'make build' prior to run e2e."
   exit 1
@@ -51,14 +49,12 @@ fi
 
 if [[ "$runtime" != "" ]]; then
   export E2E_RUNTIME=$runtime
-  tag=e2efunc
 fi
 
-export E2E_FUNC_BIN_PATH=`pwd`/func
-echo Binary $E2E_FUNC_BIN_PATH
+export E2E_FUNC_BIN_PATH=$(pwd)/func
 
 go clean -testcache
-go test -v -test.v -tags="${tag}" ./test/_e2e/
+go test -v -test.v -tags="e2elc" ./test/_e2e/
 ret=$?
 
 cd $curdir