pre-submit cleanup

client_int_test.go

@@ -20,20 +20,16 @@ import (
 
 /*
  NOTE:  Running integration tests locally requires a configured test cluster.
-        Test failures may require manual removal of dangling resources, though
-				a complete rebuild and destruction of the integration cluster is
-				suggested.
+        Test failures may require manual removal of dangling resources.
 
- ## Creating an Integration-Test Cluster
+ ## Integration Cluster
 
  These integration tests require a properly configured cluster,
  such as that which is setup and configured in CI (see .github/workflows).
  A local KinD cluster can be started via:
    ./hack/allocate.sh && ./hack/configure.sh
- Note that kubectl, kind and yq are required.  on linux they can be installed
- with ./hack/binaries.sh
 
- ## Running Integration Tests
+ ## Integration Testing
 
  These tests can be run via the make target:
    make test-integration
@@ -42,11 +38,10 @@ import (
 
  ## Teardown and Cleanup
 
- Tests should clean up after themselves, including cluster resources and
- temp directoreis.  However some tests have not yet been updated, so failed
- tests may leave dangling files in ./testdata which must be manually removed.
- Additionally, failed tests may not clean up all their cluster resources, so
- removing the test cluster after a test run ends in failure is suggested:
+ Tests should clean up after themselves.  In the event of failures, one may
+ need to manually remove files:
+   rm -rf ./testdata/example.com
+ The test cluster is not automatically removed, as it can be reused.  To remove:
    ./hack/delete.sh
 */
 

cmd/build_test.go

@@ -2,9 +2,11 @@ package cmd
 
 import (
 	"errors"
+	"fmt"
 	"testing"
 
 	fn "knative.dev/func"
+	"knative.dev/func/builders"
 	"knative.dev/func/mock"
 )
 
@@ -14,12 +16,190 @@ func TestBuild_ConfigApplied(t *testing.T) {
 	testConfigApplied(NewBuildCmd, t)
 }
 
+func testConfigApplied(cmdFn commandConstructor, t *testing.T) {
+	t.Helper()
+	var (
+		err      error
+		home     = fmt.Sprintf("%s/testdata/TestX_ConfigApplied", cwd())
+		root     = fromTempDirectory(t)
+		f        = fn.Function{Runtime: "go", Root: root, Name: "f"}
+		pusher   = mock.NewPusher()
+		clientFn = NewTestClient(fn.WithPusher(pusher))
+	)
+	t.Setenv("XDG_CONFIG_HOME", home)
+
+	if err = fn.New().Create(f); err != nil {
+		t.Fatal(err)
+	}
+
+	// Ensure the global config setting was loaded: Registry
+	// global config in ./testdata/TestBuild_ConfigApplied sets registry
+	if err = cmdFn(clientFn).Execute(); err != nil {
+		t.Fatal(err)
+	}
+	if f, err = fn.NewFunction(root); err != nil {
+		t.Fatal(err)
+	}
+	if f.Registry != "registry.example.com/alice" {
+		t.Fatalf("expected registry 'registry.example.com/alice' got '%v'", f.Registry)
+	}
+
+	// Ensure flags are evaluated
+	cmd := cmdFn(clientFn)
+	cmd.SetArgs([]string{"--builder-image", "example.com/builder/image:v1.2.3"})
+	if err = cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+	if f, err = fn.NewFunction(root); err != nil {
+		t.Fatal(err)
+	}
+	if f.Build.BuilderImages[f.Build.Builder] != "example.com/builder/image:v1.2.3" {
+		t.Fatalf("expected builder image not set. Flags not evaluated? got %v", f.Build.BuilderImages[f.Build.Builder])
+	}
+
+	// Ensure function context loaded
+	// Update registry on the function and ensure it takes precidence (overrides)
+	// the global setting defined in home.
+	f.Registry = "registry.example.com/charlie"
+	if err := f.Write(); err != nil {
+		t.Fatal(err)
+	}
+	if err := cmdFn(clientFn).Execute(); err != nil {
+		t.Fatal(err)
+	}
+	if f, err = fn.NewFunction(root); err != nil {
+		t.Fatal(err)
+	}
+	if f.Image != "registry.example.com/charlie/f:latest" {
+		t.Fatalf("expected image 'registry.example.com/charlie/f:latest' got '%v'", f.Image)
+	}
+
+	// Ensure environment variables loaded: Push
+	// Test environment variable evaluation using FUNC_PUSH
+	t.Setenv("FUNC_PUSH", "true")
+	if err := cmdFn(clientFn).Execute(); err != nil {
+		t.Fatal(err)
+	}
+	if f, err = fn.NewFunction(root); err != nil {
+		t.Fatal(err)
+	}
+	if !pusher.PushInvoked {
+		t.Fatalf("push was not invoked when FUNC_PUSH=true")
+	}
+}
+
 // TestBuild_ConfigPrecedence ensures that the correct precidence for config
 // are applied: static < global < function context < envs < flags
 func TestBuild_ConfigPrecedence(t *testing.T) {
 	testConfigPrecedence(NewBuildCmd, t)
 }
 
+func testConfigPrecedence(cmdFn commandConstructor, t *testing.T) {
+	t.Helper()
+	var (
+		err      error
+		home     = fmt.Sprintf("%s/testdata/TestX_ConfigPrecedence", cwd())
+		builder  = mock.NewBuilder()
+		clientFn = NewTestClient(fn.WithBuilder(builder))
+	)
+
+	// Ensure static default applied via 'builder'
+	// (a degenerate case, mostly just ensuring config values are not wiped to a
+	// zero value struct, etc)
+	root := fromTempDirectory(t)
+	t.Setenv("XDG_CONFIG_HOME", home) // sets registry.example.com/global
+	f := fn.Function{Runtime: "go", Root: root, Name: "f"}
+	if err = fn.New().Create(f); err != nil {
+		t.Fatal(err)
+	}
+	if err := cmdFn(clientFn).Execute(); err != nil {
+		t.Fatal(err)
+	}
+	if f, err = fn.NewFunction(root); err != nil {
+		t.Fatal(err)
+	}
+	if f.Build.Builder != builders.Default {
+		t.Fatalf("expected static default builder '%v', got '%v'", builders.Default, f.Build.Builder)
+	}
+
+	// Ensure Global Config applied via config in ./testdata
+	root = fromTempDirectory(t)
+	t.Setenv("XDG_CONFIG_HOME", home) // sets registry.example.com/global
+	f = fn.Function{Runtime: "go", Root: root, Name: "f"}
+	if err := fn.New().Create(f); err != nil {
+		t.Fatal(err)
+	}
+	if err = cmdFn(clientFn).Execute(); err != nil {
+		t.Fatal(err)
+	}
+	if f, err = fn.NewFunction(root); err != nil {
+		t.Fatal(err)
+	}
+	if f.Registry != "registry.example.com/global" { // from ./testdata
+		t.Fatalf("expected registry 'example.com/global', got '%v'", f.Registry)
+	}
+
+	// Ensure Function context overrides global config
+	// The stanza above ensures the global config is applied.  This stanza
+	// ensures that, if set on the function, it will take precidence.
+	root = fromTempDirectory(t)
+	t.Setenv("XDG_CONFIG_HOME", home) // sets registry=example.com/global
+	f = fn.Function{Runtime: "go", Root: root, Name: "f",
+		Registry: "example.com/function"}
+	if err := fn.New().Create(f); err != nil {
+		t.Fatal(err)
+	}
+	if err = cmdFn(clientFn).Execute(); err != nil {
+		t.Fatal(err)
+	}
+	if f, err = fn.NewFunction(root); err != nil {
+		t.Fatal(err)
+	}
+	if f.Registry != "example.com/function" {
+		t.Fatalf("expected function's value for registry of 'example.com/function' to override global config setting of 'example.com/global', but got '%v'", f.Registry)
+	}
+
+	// Ensure Environment Variable overrides function context.
+	root = fromTempDirectory(t)
+	t.Setenv("XDG_CONFIG_HOME", home) // sets registry.example.com/global
+	t.Setenv("FUNC_REGISTRY", "example.com/env")
+	f = fn.Function{Runtime: "go", Root: root, Name: "f",
+		Registry: "example.com/function"}
+	if err := fn.New().Create(f); err != nil {
+		t.Fatal(err)
+	}
+	if err := cmdFn(clientFn).Execute(); err != nil {
+		t.Fatal(err)
+	}
+	if f, err = fn.NewFunction(root); err != nil {
+		t.Fatal(err)
+	}
+	if f.Registry != "example.com/env" {
+		t.Fatalf("expected FUNC_REGISTRY=example.com/env to override function's value of 'example.com/function', but got '%v'", f.Registry)
+	}
+
+	// Ensure flags override environment variables.
+	root = fromTempDirectory(t)
+	t.Setenv("XDG_CONFIG_HOME", home) // sets registry=example.com/global
+	t.Setenv("FUNC_REGISTRY", "example.com/env")
+	f = fn.Function{Runtime: "go", Root: root, Name: "f",
+		Registry: "example.com/function"}
+	if err := fn.New().Create(f); err != nil {
+		t.Fatal(err)
+	}
+	cmd := cmdFn(clientFn)
+	cmd.SetArgs([]string{"--registry=example.com/flag"})
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+	if f, err = fn.NewFunction(root); err != nil {
+		t.Fatal(err)
+	}
+	if f.Registry != "example.com/flag" {
+		t.Fatalf("expected flag 'example.com/flag' to take precidence over env var, but got '%v'", f.Registry)
+	}
+}
+
 // TestBuild_ImageFlag ensures that the image flag is used when specified.
 func TestBuild_ImageFlag(t *testing.T) {
 	var (
@@ -248,3 +428,54 @@ func TestBuild_Registry(t *testing.T) {
 func TestBuild_FunctionContext(t *testing.T) {
 	testFunctionContext(NewBuildCmd, t)
 }
+
+func testFunctionContext(cmdFn commandConstructor, t *testing.T) {
+	t.Helper()
+	root := fromTempDirectory(t)
+
+	if err := fn.New().Create(fn.Function{Runtime: "go", Root: root, Registry: TestRegistry}); err != nil {
+		t.Fatal(err)
+	}
+
+	// Build the function explicitly setting the builder to !builders.Default
+	cmd := cmdFn(NewTestClient())
+	dflt := cmd.Flags().Lookup("builder").DefValue
+
+	// The initial default value should be builders.Default (see global config)
+	if dflt != builders.Default {
+		t.Fatalf("expected flag default value '%v', got '%v'", builders.Default, dflt)
+	}
+
+	// Choose the value that is not the default
+	// We must calculate this because downstream changes the default via patches.
+	var builder string
+	if builders.Default == builders.Pack {
+		builder = builders.S2I
+	} else {
+		builder = builders.Pack
+	}
+
+	// Build with the other
+	cmd.SetArgs([]string{"--builder", builder})
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+
+	// The function should now have the builder set to the new builder
+	f, err := fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if f.Build.Builder != builder {
+		t.Fatalf("expected function to have new builder '%v', got '%v'", builder, f.Build.Builder)
+	}
+
+	// The command default should now take into account the function when
+	// determining the flag default
+	cmd = cmdFn(NewTestClient())
+	dflt = cmd.Flags().Lookup("builder").DefValue
+
+	if dflt != builder {
+		t.Fatalf("expected flag default to be function's current builder '%v', got '%v'", builder, dflt)
+	}
+}

cmd/deploy.go

@@ -270,9 +270,6 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) {
 		if f, err = client.RunPipeline(cmd.Context(), f); err != nil {
 			return
 		}
-		// TODO: remote deployments currently have no way to update the function
-		// state with values generated during the deployment process such as the
-		// ImageDigest (from pusing) or the deployed namespace (on deploy)a.
 	} else {
 		if shouldBuild(cfg.Build, f, client) { // --build or "auto" with FS changes
 			if err = client.Build(cmd.Context(), f.Root); err != nil {
@@ -679,7 +676,6 @@ func (c deployConfig) Validate(cmd *cobra.Command) (err error) {
 	}
 
 	// Can not push when specifying an --image with digest
-	// TODO: test
 	if digest != "" && c.Push {
 		return errors.New("pushing is not valid when specifying an image with digest")
 	}
@@ -782,7 +778,7 @@ func printDeployMessages(out io.Writer, cfg deployConfig) {
 	// will be ignored in favor of the local source code since --remote was not
 	// specified.
 	if !cfg.Remote && (cfg.GitURL != "" || cfg.GitBranch != "" || cfg.GitDir != "") {
-		fmt.Fprintf(out, "Warning: git settings are only applicable when running with --remote.  Local source function source code will be used.")
+		fmt.Fprintf(out, "Warning: git settings are only applicable when running with --remote.  Local source code will be used.")
 	}
 
 }