From c5f2d462a1bfd801c5deaaf02ac86fe4cf339155 Mon Sep 17 00:00:00 2001 From: Luke Kingland Date: Thu, 26 Jan 2023 15:44:24 +0900 Subject: [PATCH] pre-submit cleanup --- client_int_test.go | 19 ++-- cmd/build_test.go | 231 +++++++++++++++++++++++++++++++++++++++++++++ cmd/deploy.go | 6 +- cmd/deploy_test.go | 229 -------------------------------------------- cmd/root.go | 2 +- 5 files changed, 240 insertions(+), 247 deletions(-) diff --git a/client_int_test.go b/client_int_test.go index 448ada62a..e39044fff 100644 --- a/client_int_test.go +++ b/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 */ diff --git a/cmd/build_test.go b/cmd/build_test.go index 8fbb84ace..0f9884513 100644 --- a/cmd/build_test.go +++ b/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) + } +} diff --git a/cmd/deploy.go b/cmd/deploy.go index 299de9ea3..d2c7c4b62 100644 --- a/cmd/deploy.go +++ b/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.") } } diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index e5f6cd91f..02b2e120b 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -29,190 +29,12 @@ func TestDeploy_ConfigApplied(t *testing.T) { testConfigApplied(NewDeployCmd, 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") - } -} - // TestDeploy_ConfigPrecedence ensures that the correct precidence for config // are applied: static < global < function context < envs < flags func TestDeploy_ConfigPrecedence(t *testing.T) { testConfigPrecedence(NewDeployCmd, 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) - } -} - // TestDeploy_FunctionContext ensures that the function contextually relevant // to the current command is loaded and used for flag defaults by spot-checking // the builder setting. @@ -220,57 +42,6 @@ func TestDeploy_FunctionContext(t *testing.T) { testFunctionContext(NewDeployCmd, 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) - } -} - // TestDeploy_Default ensures that running deploy on a valid default Function // (only required options populated; all else default) completes successfully. func TestDeploy_Default(t *testing.T) { diff --git a/cmd/root.go b/cmd/root.go index 26dc4d610..4afb398f7 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -147,7 +147,7 @@ func effectivePath() (path string) { p = fs.StringP("path", "p", "", "") ) fs.SetOutput(io.Discard) - fs.ParseErrorsWhitelist.UnknownFlags = true + fs.ParseErrorsWhitelist.UnknownFlags = true //nolint:woke err := fs.Parse(os.Args[1:]) if err != nil { fmt.Fprintf(os.Stderr, "error preparsing flags: %v\n", err)