From 399e0a9ce209ba829485a9d9539091eeaea73a0d Mon Sep 17 00:00:00 2001 From: Luke Kingland <58986931+lkingland@users.noreply.github.com> Date: Mon, 6 Feb 2023 17:43:35 +0900 Subject: [PATCH] feat: deploy command global config with function context (#1434) * feat: deploy command global config function context * fix static check errors * fix test * fix remote persists test * env opaque box test * use keyed fields (govet) * mock deployer expansion * ClearEnvs now in shared testing pkg * effectivePath now uses plfags * list directly uses config for default namespace * Improved Namespace calculation and Caching - Built() now a client package function - Namespace evaluation delegeate to implementations This enables the func.yaml to avert dirtiness checks on rebuilds (caching) - Build and Deploy update build stamp on completion Enables dirtiness checks to be unaffects, leading to better caching. - CLI Namespace checks no longer require k8s env evaluation for default - Fixed missing condition check in TestDeploy_Namespace - Fixes some minor linter and spelling errors - Global config does not directly set namespace - it is corrected to be deferred evaluation used by implementations. - Skips writing func.yaml on f.Write if there were no modifications, which leads to better caching (no timestamp update means Built == true) - HasImage removed in favor of a corrected fn.Built * linting, misspellings and docs rebuild * warn user if git settings exist on a non-remote build * lint error fix * test flag unsetting persists * allow unknown flags in preparsing of flags * pre-submit cleanup * update cmd to use client.Init --- client.go | 79 ++- client_int_test.go | 1 - client_test.go | 18 +- cmd/build.go | 25 +- cmd/build_test.go | 46 +- cmd/delete_test.go | 4 +- cmd/deploy.go | 560 +++++++++--------- cmd/deploy_test.go | 259 ++++++-- cmd/describe_test.go | 17 +- cmd/list.go | 16 +- cmd/list_test.go | 2 +- cmd/root.go | 29 +- cmd/root_test.go | 13 +- cmd/run.go | 2 +- .../func/config.yaml | 0 .../func/config.yaml | 0 config/config.go | 10 +- docs/reference/func_build.md | 4 +- docs/reference/func_delete.md | 2 +- docs/reference/func_deploy.md | 23 +- docs/reference/func_describe.md | 2 +- function.go | 20 +- mock/deployer.go | 15 +- testing/testing.go | 13 + 24 files changed, 723 insertions(+), 437 deletions(-) rename cmd/testdata/{TestBuild_ConfigApplied => TestX_ConfigApplied}/func/config.yaml (100%) rename cmd/testdata/{TestBuild_ConfigPrecidence => TestX_ConfigPrecedence}/func/config.yaml (100%) diff --git a/client.go b/client.go index 8dbc6ddf5..03bdad104 100644 --- a/client.go +++ b/client.go @@ -722,7 +722,7 @@ func (c *Client) Deploy(ctx context.Context, path string) (err error) { // Functions must be built (have an associated image) before being deployed. // Note that externally built images may be specified in the func.yaml - if !f.HasImage() { + if !Built(f.Root) { return ErrNotBuilt } @@ -735,6 +735,19 @@ func (c *Client) Deploy(ctx context.Context, path string) (err error) { // Deploy a new or Update the previously-deployed function c.progressListener.Increment("⬆️ Deploying function to the cluster") result, err := c.deployer.Deploy(ctx, f) + if err != nil { + fmt.Printf("deploy error: %v\n", err) + return + } + + // Update the function with the namespace into which the function was + // deployed + f.Deploy.Namespace = result.Namespace + + // saves namespace to function's metadata (func.yaml) + if err = f.Write(); err != nil { + return + } if result.Status == Deployed { c.progressListener.Increment(fmt.Sprintf("✅ Function deployed in namespace %q and exposed at URL: \n %v", result.Namespace, result.URL)) @@ -742,7 +755,9 @@ func (c *Client) Deploy(ctx context.Context, path string) (err error) { c.progressListener.Increment(fmt.Sprintf("✅ Function updated in namespace %q and exposed at URL: \n %v", result.Namespace, result.URL)) } - return err + // Metadata generated from deploying (namespace) should not trigger a rebuild + // through a staleness check, so update the build stamp we checked earlier. + return updateBuildStamp(f) } // RunPipeline runs a Pipeline to build and deploy the function. @@ -956,29 +971,40 @@ func (c *Client) Push(ctx context.Context, path string) (err error) { return } - if !f.HasImage() { + if !Built(f.Root) { return ErrNotBuilt } - imageDigest, err := c.pusher.Push(ctx, f) - if err != nil { + if f.ImageDigest, err = c.pusher.Push(ctx, f); err != nil { return } - // Record the Image Digest pushed. - f.ImageDigest = imageDigest - return f.Write() + if err = f.Write(); err != nil { // saves digest to f's metadata (func.yaml) + return + } + + // Metadata generated from pushing (ImageDigest) should not trigger a rebuild + // through a staleness check, so update the build stamp we checked earlier. + return updateBuildStamp(f) } // Built returns true if the given path contains a function which has been // built without any filesystem modifications since (is not stale). -func (c *Client) Built(path string) bool { +func Built(path string) bool { f, err := NewFunction(path) if err != nil { return false } - // Missing a build image always means !Built (but does not satisfy staleness + // If there is no build stamp, it is not built. + // This case should be redundant with the below check for an image, but is + // temporarily necessary (see the long-winded caviat note below). + stamp := buildStamp(path) + if stamp == "" { + return false + } + + // Missing an image name always means !Built (but does not satisfy staleness // checks). // NOTE: This will be updated in the future such that a build does not // automatically update the function's serialized, source-controlled state, @@ -989,16 +1015,7 @@ func (c *Client) Built(path string) bool { // An example of how this bug manifests is that every rebuild of a function // registers the func.yaml as being dirty for source-control purposes, when // this should only happen on deploy. - if !f.HasImage() { - return false - } - - buildstampPath := filepath.Join(path, RunDataDir, buildstamp) - - // If there is no build stamp, it is also not built. - // This case should be redundant with the above check for an image, but is - // temporarily necessary (see the long-winded caviat note above). - if _, err := os.Stat(buildstampPath); err != nil { + if f.Image == "" { return false } @@ -1008,14 +1025,10 @@ func (c *Client) Built(path string) bool { fmt.Fprintf(os.Stderr, "error calculating function's fingerprint: %v\n", err) return false } - b, err := os.ReadFile(buildstampPath) - if err != nil { - fmt.Fprintf(os.Stderr, "error reading function's fingerprint: %v\n", err) + + if stamp != hash { return false } - if string(b) != hash { - return false // changes detected - } // Function has a populated image, existing buildstamp, and the calculated // fingerprint has not changed. @@ -1025,6 +1038,20 @@ func (c *Client) Built(path string) bool { return true } +// buildStamp returns the current (last) build stamp for the function +// at the given path, if it can be found. +func buildStamp(path string) string { + buildstampPath := filepath.Join(path, RunDataDir, buildstamp) + if _, err := os.Stat(buildstampPath); err != nil { + return "" + } + b, err := os.ReadFile(buildstampPath) + if err != nil { + return "" + } + return string(b) +} + // fingerprint returns a hash of the filenames and modification timestamps of // the files within a function's root. func fingerprint(f Function) (string, error) { diff --git a/client_int_test.go b/client_int_test.go index 89e83a442..3788f6b0f 100644 --- a/client_int_test.go +++ b/client_int_test.go @@ -37,7 +37,6 @@ import ( // ## Cluster Cleanup // // The test cluster and most resources can be removed with: - // ./hack/delete.sh // // NOTE: Downloaded images are not removed. diff --git a/client_test.go b/client_test.go index c7bea7d00..745454b05 100644 --- a/client_test.go +++ b/client_test.go @@ -1530,7 +1530,7 @@ func TestClient_BuiltStamps(t *testing.T) { client := fn.New(fn.WithBuilder(builder), fn.WithRegistry(TestRegistry)) // paths that do not contain a function are !Built - Degenerate case - if client.Built(root) { + if fn.Built(root) { t.Fatal("path not containing a function returned as being built") } @@ -1538,7 +1538,7 @@ func TestClient_BuiltStamps(t *testing.T) { if err := client.Init(fn.Function{Runtime: TestRuntime, Root: root}); err != nil { t.Fatal(err) } - if client.Built(root) { + if fn.Built(root) { t.Fatal("newly created function returned Built==true") } @@ -1546,7 +1546,7 @@ func TestClient_BuiltStamps(t *testing.T) { if err := client.Build(context.Background(), root); err != nil { t.Fatal(err) } - if !client.Built(root) { + if !fn.Built(root) { t.Fatal("freshly built function should return Built==true") } } @@ -1597,7 +1597,7 @@ func TestClient_BuiltDetects(t *testing.T) { } // Prior to a filesystem edit, it will be Built. - if !client.Built(root) { + if !fn.Built(root) { t.Fatal("freshly built function reported Built==false (1)") } @@ -1612,7 +1612,7 @@ func TestClient_BuiltDetects(t *testing.T) { // Release thread and wait to ensure that the clock advances even in constrained CI environments time.Sleep(100 * time.Millisecond) - if client.Built(root) { + if fn.Built(root) { t.Fatal("client did not detect file timestamp change as indicating build staleness") } @@ -1620,7 +1620,7 @@ func TestClient_BuiltDetects(t *testing.T) { if err := client.Build(ctx, root); err != nil { t.Fatal(err) } - if !client.Built(root) { + if !fn.Built(root) { t.Fatal("freshly built function reported Built==false (2)") } @@ -1632,7 +1632,7 @@ func TestClient_BuiltDetects(t *testing.T) { f.Close() // The system should now detect the function is stale - if client.Built(root) { + if fn.Built(root) { t.Fatal("client did not detect an added file as indicating build staleness") } @@ -1640,7 +1640,7 @@ func TestClient_BuiltDetects(t *testing.T) { if err := client.Build(ctx, root); err != nil { t.Fatal(err) } - if !client.Built(root) { + if !fn.Built(root) { t.Fatal("freshly built function reported Built==false (3)") } @@ -1649,7 +1649,7 @@ func TestClient_BuiltDetects(t *testing.T) { if err := os.Remove(filepath.Join(root, testfile)); err != nil { t.Fatal(err) } - if client.Built(root) { + if fn.Built(root) { t.Fatal("client did not detect a removed file as indicating build staleness") } } diff --git a/cmd/build.go b/cmd/build.go index 55c43ed86..0eccade10 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -94,11 +94,11 @@ EXAMPLES // Options whose value may be defined globally may also exist on the // contextually relevant function; sets are flattened above via cfg.Apply(f) cmd.Flags().StringP("builder", "b", cfg.Builder, - fmt.Sprintf("build strategy to use when creating the underlying image. Currently supported build strategies are %s.", KnownBuilders())) + fmt.Sprintf("Builder to use when creating the function's container. Currently supported builders are %s. (Env: $FUNC_BUILDER)", KnownBuilders())) cmd.Flags().BoolP("confirm", "c", cfg.Confirm, "Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)") cmd.Flags().StringP("registry", "r", cfg.Registry, - "Registry + namespace part of the image to build, ex 'quay.io/myuser'. The full image name is automatically determined (Env: $FUNC_REGISTRY)") + "Container registry + registry namespace. (ex 'ghcr.io/myuser'). The full image name is automatically determined using this along with function name. (Env: $FUNC_REGISTRY)") // Function-Context Flags: // Options whose value is available on the function with context only @@ -130,25 +130,26 @@ EXAMPLES } func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err error) { - if err = config.CreatePaths(); err != nil { - return // see docker/creds potential mutation of auth.json + var ( + cfg buildConfig + f fn.Function + ) + if err = config.CreatePaths(); err != nil { // for possible auth.json usage + return } - - cfg, err := newBuildConfig().Prompt() - if err != nil { + if cfg, err = newBuildConfig().Prompt(); err != nil { return } - if err = cfg.Validate(); err != nil { return } - - f, err := fn.NewFunction(cfg.Path) - if err != nil { + if f, err = fn.NewFunction(cfg.Path); err != nil { return } f = cfg.Configure(f) // Updates f at path to include build request values + // TODO: this logic is duplicated with runDeploy. Shouild be in buildConfig + // constructor. // Checks if there is a difference between defined registry and its value used as a prefix in the image tag // In case of a mismatch a new image tag is created and used for build // Do not react if image tag has been changed outside configuration @@ -249,7 +250,7 @@ func newBuildConfig() buildConfig { // Configure the given function. Updates a function struct with all // configurable values. Note that buildConfig already includes function's -// current values, as they were passed through vi flag defaults, so overwriting +// current values, as they were passed through via flag defaults, so overwriting // is a noop. func (c buildConfig) Configure(f fn.Function) fn.Function { f = c.Global.Configure(f) diff --git a/cmd/build_test.go b/cmd/build_test.go index a69f557f3..d65335d09 100644 --- a/cmd/build_test.go +++ b/cmd/build_test.go @@ -13,9 +13,14 @@ import ( // TestBuild_ConfigApplied ensures that the build command applies config // settings at each level (static, global, function, envs, flags). 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/TestBuild_ConfigApplied", cwd()) + home = fmt.Sprintf("%s/testdata/TestX_ConfigApplied", cwd()) root = fromTempDirectory(t) f = fn.Function{Runtime: "go", Root: root, Name: "f"} pusher = mock.NewPusher() @@ -29,7 +34,7 @@ func TestBuild_ConfigApplied(t *testing.T) { // Ensure the global config setting was loaded: Registry // global config in ./testdata/TestBuild_ConfigApplied sets registry - if err = NewBuildCmd(clientFn).Execute(); err != nil { + if err = cmdFn(clientFn).Execute(); err != nil { t.Fatal(err) } if f, err = fn.NewFunction(root); err != nil { @@ -40,7 +45,7 @@ func TestBuild_ConfigApplied(t *testing.T) { } // Ensure flags are evaluated - cmd := NewBuildCmd(clientFn) + cmd := cmdFn(clientFn) cmd.SetArgs([]string{"--builder-image", "example.com/builder/image:v1.2.3"}) if err = cmd.Execute(); err != nil { t.Fatal(err) @@ -59,7 +64,7 @@ func TestBuild_ConfigApplied(t *testing.T) { if err := f.Write(); err != nil { t.Fatal(err) } - if err := NewBuildCmd(clientFn).Execute(); err != nil { + if err := cmdFn(clientFn).Execute(); err != nil { t.Fatal(err) } if f, err = fn.NewFunction(root); err != nil { @@ -72,7 +77,7 @@ func TestBuild_ConfigApplied(t *testing.T) { // Ensure environment variables loaded: Push // Test environment variable evaluation using FUNC_PUSH t.Setenv("FUNC_PUSH", "true") - if err := NewBuildCmd(clientFn).Execute(); err != nil { + if err := cmdFn(clientFn).Execute(); err != nil { t.Fatal(err) } if f, err = fn.NewFunction(root); err != nil { @@ -81,15 +86,19 @@ func TestBuild_ConfigApplied(t *testing.T) { if !pusher.PushInvoked { t.Fatalf("push was not invoked when FUNC_PUSH=true") } - } -// TestBuild_ConfigPrecidence ensures that the correct precidence for config +// TestBuild_ConfigPrecedence ensures that the correct precidence for config // are applied: static < global < function context < envs < flags -func TestBuild_ConfigPrecidence(t *testing.T) { +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/TestBuild_ConfigPrecidence", cwd()) + home = fmt.Sprintf("%s/testdata/TestX_ConfigPrecedence", cwd()) builder = mock.NewBuilder() clientFn = NewTestClient(fn.WithBuilder(builder)) ) @@ -103,7 +112,7 @@ func TestBuild_ConfigPrecidence(t *testing.T) { if err = fn.New().Init(f); err != nil { t.Fatal(err) } - if err := NewBuildCmd(clientFn).Execute(); err != nil { + if err := cmdFn(clientFn).Execute(); err != nil { t.Fatal(err) } if f, err = fn.NewFunction(root); err != nil { @@ -120,7 +129,7 @@ func TestBuild_ConfigPrecidence(t *testing.T) { if err := fn.New().Init(f); err != nil { t.Fatal(err) } - if err = NewBuildCmd(clientFn).Execute(); err != nil { + if err = cmdFn(clientFn).Execute(); err != nil { t.Fatal(err) } if f, err = fn.NewFunction(root); err != nil { @@ -140,7 +149,7 @@ func TestBuild_ConfigPrecidence(t *testing.T) { if err := fn.New().Init(f); err != nil { t.Fatal(err) } - if err = NewBuildCmd(clientFn).Execute(); err != nil { + if err = cmdFn(clientFn).Execute(); err != nil { t.Fatal(err) } if f, err = fn.NewFunction(root); err != nil { @@ -159,7 +168,7 @@ func TestBuild_ConfigPrecidence(t *testing.T) { if err := fn.New().Init(f); err != nil { t.Fatal(err) } - if err := NewBuildCmd(clientFn).Execute(); err != nil { + if err := cmdFn(clientFn).Execute(); err != nil { t.Fatal(err) } if f, err = fn.NewFunction(root); err != nil { @@ -178,7 +187,7 @@ func TestBuild_ConfigPrecidence(t *testing.T) { if err := fn.New().Init(f); err != nil { t.Fatal(err) } - cmd := NewBuildCmd(clientFn) + cmd := cmdFn(clientFn) cmd.SetArgs([]string{"--registry=example.com/flag"}) if err := cmd.Execute(); err != nil { t.Fatal(err) @@ -417,6 +426,11 @@ func TestBuild_Registry(t *testing.T) { // to the current command execution is loaded and used for flag defaults by // spot-checking the builder setting. 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().Init(fn.Function{Runtime: "go", Root: root, Registry: TestRegistry}); err != nil { @@ -424,7 +438,7 @@ func TestBuild_FunctionContext(t *testing.T) { } // Build the function explicitly setting the builder to !builders.Default - cmd := NewBuildCmd(NewTestClient()) + cmd := cmdFn(NewTestClient()) dflt := cmd.Flags().Lookup("builder").DefValue // The initial default value should be builders.Default (see global config) @@ -458,7 +472,7 @@ func TestBuild_FunctionContext(t *testing.T) { // The command default should now take into account the function when // determining the flag default - cmd = NewBuildCmd(NewTestClient()) + cmd = cmdFn(NewTestClient()) dflt = cmd.Flags().Lookup("builder").DefValue if dflt != builder { diff --git a/cmd/delete_test.go b/cmd/delete_test.go index b3efacbf7..3ff81e90a 100644 --- a/cmd/delete_test.go +++ b/cmd/delete_test.go @@ -21,8 +21,8 @@ func TestDelete_Namespace(t *testing.T) { t.Setenv("KUBECONFIG", filepath.Join(cwd(), "nonexistent")) t.Setenv("KUBERNETES_SERVICE_HOST", "") cmd := NewDeleteCmd(func(cc ClientConfig, options ...fn.Option) (*fn.Client, func()) { - if cc.Namespace != "default" { - t.Fatalf("expected 'default', got '%v'", cc.Namespace) + if cc.Namespace != "" { + t.Fatalf("expected '', got '%v'", cc.Namespace) } return fn.New(), func() {} }) diff --git a/cmd/deploy.go b/cmd/deploy.go index e6f1a1372..d2c7c4b62 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -85,6 +85,9 @@ DESCRIPTION EXAMPLES + o Deploy the function + $ {{.Name}} deploy + o Deploy the function using interactive prompts. This is useful for the first deployment, since most settings will be remembered for future deployments. $ {{rootCmdUse}} deploy -c @@ -121,36 +124,70 @@ EXAMPLES `, SuggestFor: []string{"delpoy", "deplyo"}, PreRunE: bindEnv("confirm", "env", "git-url", "git-branch", "git-dir", "remote", "build", "builder", "builder-image", "image", "registry", "push", "platform", "path", "namespace"), + RunE: func(cmd *cobra.Command, args []string) error { + return runDeploy(cmd, newClient) + }, } - // Config + // Global Config cfg, err := config.NewDefault() if err != nil { fmt.Fprintf(cmd.OutOrStdout(), "error loading config at '%v'. %v\n", config.File(), err) } + // Function Context + f, _ := fn.NewFunction(effectivePath()) + if f.Initialized() { + cfg = cfg.Apply(f) + } + // Flags - cmd.Flags().BoolP("confirm", "c", cfg.Confirm, "Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)") + // + // Globally-Configurable Flags: + // Options whose value may be defined globally may also exist on the + // contextually relevant function; but sets are flattened via cfg.Apply(f) + cmd.Flags().StringP("builder", "b", cfg.Builder, + fmt.Sprintf("Builder to use when creating the function's container. Currently supported builders are %s.", KnownBuilders())) + cmd.Flags().BoolP("confirm", "c", cfg.Confirm, + "Prompt to confirm all configuration options (Env: $FUNC_CONFIRM)") + cmd.Flags().StringP("registry", "r", cfg.Registry, + "Container registry + registry namespace. (ex 'ghcr.io/myuser'). The full image name is automatically determined using this along with function name. (Env: $FUNC_REGISTRY)") + cmd.Flags().StringP("namespace", "n", cfg.Namespace, + "Deploy into a specific namespace. Will use function's current namespace by default if already deployed, and the currently active namespace if it can be determined. (Env: $FUNC_NAMESPACE)") + + // Function-Context Flags: + // Options whose value is avaolable on the function with context only + // (persisted but not globally configurable) + builderImage := f.Build.BuilderImages[f.Build.Builder] + cmd.Flags().StringP("builder-image", "", builderImage, + "Specify a custom builder image for use by the builder other than its default. ($FUNC_BUILDER_IMAGE)") + cmd.Flags().StringP("image", "i", f.Image, "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().StringArrayP("env", "e", []string{}, "Environment variable to set in the form NAME=VALUE. "+ "You may provide this flag multiple times for setting multiple environment variables. "+ "To unset, specify the environment variable name followed by a \"-\" (e.g., NAME-).") - cmd.Flags().StringP("git-url", "g", "", "Repo url to push the code to be built (Env: $FUNC_GIT_URL)") - cmd.Flags().StringP("git-branch", "t", "", "Git branch to be used for remote builds (Env: $FUNC_GIT_BRANCH)") - cmd.Flags().StringP("git-dir", "d", "", "Directory in the repo where the function is located (Env: $FUNC_GIT_DIR)") - cmd.Flags().BoolP("remote", "", false, "Trigger a remote deployment. Default is to deploy and build from the local system: $FUNC_REMOTE)") - - // Flags shared with Build (specifically related to the build step): - cmd.Flags().StringP("build", "", "auto", "Build the function. [auto|true|false]. [Env: $FUNC_BUILD]") - cmd.Flags().Lookup("build").NoOptDefVal = "true" // --build is equivalient to --build=true - cmd.Flags().StringP("builder", "b", cfg.Builder, fmt.Sprintf("builder to use when creating the underlying image. Currently supported builders are %s.", KnownBuilders())) - cmd.Flags().StringP("builder-image", "", "", "The image the specified builder should use; either an as an image name or a mapping. ($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", "", "Registry + namespace part of the image to build, ex 'ghcr.io/myuser'. The full image name is automatically determined. (Env: $FUNC_REGISTRY)") - cmd.Flags().BoolP("push", "u", true, "Push the function image to registry before deploying (Env: $FUNC_PUSH)") - cmd.Flags().StringP("platform", "", "", "Target platform to build (e.g. linux/amd64).") - cmd.Flags().StringP("namespace", "n", cfg.Namespace, "Deploy into a specific namespace. Will use function's current namespace by default if already deployed. (Env: $FUNC_NAMESPACE)") + cmd.Flags().StringP("git-url", "g", f.Build.Git.URL, + "Repo url to push the code to be built (Env: $FUNC_GIT_URL)") + cmd.Flags().StringP("git-branch", "t", f.Build.Git.Revision, + "Git revision (branch) to be used when deploying via a git repository (Env: $FUNC_GIT_BRANCH)") + cmd.Flags().StringP("git-dir", "d", f.Build.Git.ContextDir, + "Directory in the repo to find the function (default is the root) (Env: $FUNC_GIT_DIR)") + cmd.Flags().BoolP("remote", "", f.Deploy.Remote, + "Trigger a remote deployment. Default is to deploy and build from the local system (Env: $FUNC_REMOTE)") + + // Static Flags: + // Options which have statc defaults only (not globally configurable nor + // persisted with the function) + cmd.Flags().StringP("build", "", "auto", + "Build the function. [auto|true|false]. (Env: $FUNC_BUILD)") + cmd.Flags().Lookup("build").NoOptDefVal = "true" // register `--build` as equivalient to `--build=true` + cmd.Flags().BoolP("push", "u", true, + "Push the function image to registry before deploying. (Env: $FUNC_PUSH)") + cmd.Flags().StringP("platform", "", "", + "Optionally specify a specific platform to build for (e.g. linux/amd64). (Env: $FUNC_PLATFORM)") setPathFlag(cmd) + // Tab Completion if err := cmd.RegisterFlagCompletionFunc("builder", CompleteBuilderList); err != nil { fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err) } @@ -159,109 +196,54 @@ EXAMPLES fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err) } - cmd.RunE = func(cmd *cobra.Command, args []string) error { - return runDeploy(cmd, args, newClient) - } - return cmd } -// runDeploy gathers configuration from environment, flags and the user, -// merges these into the function requested, and triggers either a remote or -// local build-and-deploy. -func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err error) { - if err = config.CreatePaths(); err != nil { - return // see docker/creds potential mutation of auth.json - } - - // Create a deploy config from environment variables and flags - cfg, err := newDeployConfig(cmd) - if err != nil { +func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { + var ( + cfg deployConfig + f fn.Function + ) + if err = config.CreatePaths(); err != nil { // for possible auth.json usage return } - - // Prompt the user to potentially change config interactively. - cfg, err = cfg.Prompt() - if err != nil { + if cfg, err = newDeployConfig(cmd).Prompt(); err != nil { return } - - // Validate the config - if err = cfg.Validate(); err != nil { + if err = cfg.Validate(cmd); err != nil { return } - - // Print warnings regarding namespace target - namespaceWarnings(cfg, cmd) - - // Load the function, and if it exists (path initialized as a function), merge - // in any updates from flags/env vars (namespace, explicit image name, envs). - f, err := fn.NewFunction(cfg.Path) - if err != nil { + if f, err = fn.NewFunction(cfg.Path); err != nil { return } - if !f.Initialized() { - return fmt.Errorf("'%v' does not contain an initialized function", cfg.Path) - } - if f.Registry == "" || cmd.Flags().Changed("registry") { - // Sets default AND accepts any user-provided overrides - f.Registry = cfg.Registry - } - if f.Build.Builder == "" || cmd.Flags().Changed("builder") { - // Sets default AND accepts any user-provided overrides - f.Build.Builder = cfg.Builder - } - if f.Deploy.Namespace == "" || cmd.Flags().Changed("namespace") { - // Sets default AND accepts andy user-provided overrides - f.Deploy.Namespace = cfg.Namespace + if f, err = cfg.Configure(f); err != nil { // Updates f with deploy cfg + return } - if cmd.Flags().Changed("remote") { - f.Deploy.Remote = cfg.Remote - } else { - cfg.Remote = f.Deploy.Remote - } - if cfg.Image != "" { - f.Image = cfg.Image - } - if cfg.ImageDigest != "" { - fmt.Fprintf(cmd.OutOrStdout(), "Deploying image '%v' with digest '%s'. Build and push are disabled.\n", f.Image, f.ImageDigest) - f.ImageDigest = cfg.ImageDigest - } - if cfg.Builder != "" { - f.Build.Builder = cfg.Builder - } - if cfg.BuilderImage != "" { - f.Build.BuilderImages[cfg.Builder] = cfg.BuilderImage - } - if cfg.GitURL != "" { - parts := strings.Split(cfg.GitURL, "#") - f.Build.Git.URL = parts[0] - if len(parts) == 2 { // see Validate() where len enforced to be <= 2 - f.Build.Git.Revision = parts[1] + // TODO: this is duplicate logic with runBuild. + // Refactor both to have this logic part of creating the buildConfig and thus + // shared because newDeployConfig uses newBuildConfig for its embedded struct. + if f.Registry != "" && !cmd.Flags().Changed("image") && strings.Index(f.Image, "/") > 0 && !strings.HasPrefix(f.Image, f.Registry) { + prfx := f.Registry + if prfx[len(prfx)-1:] != "/" { + prfx = prfx + "/" } - } - if cfg.GitBranch != "" { - f.Build.Git.Revision = cfg.GitBranch - } - if cfg.GitDir != "" { - f.Build.Git.ContextDir = cfg.GitDir + sps := strings.Split(f.Image, "/") + updImg := prfx + sps[len(sps)-1] + fmt.Fprintf(cmd.ErrOrStderr(), "Warning: function has current image '%s' which has a different registry than the currently configured registry '%s'. The new image tag will be '%s'. To use an explicit image, use --image.\n", f.Image, f.Registry, updImg) + f.Image = updImg } - f.Run.Envs, _, err = mergeEnvs(f.Run.Envs, cfg.EnvToUpdate, cfg.EnvToRemove) - if err != nil { - return - } + // Informative non-error messages regarding the final deployment request + printDeployMessages(cmd.OutOrStdout(), cfg) - // Validate that a builder short-name was obtained, whether that be from - // the function's prior state, or the value of flags/environment. - if err = ValidateBuilder(f.Build.Builder); err != nil { + // Save the function which has now been updated with flags/config + if err = f.Write(); err != nil { // TODO: remove when client API uses 'f' return } - // Choose a builder based on the value of the --builder flag and a possible - // override for the build image for that builder to use from the optional - // builder-image flag. + // Client + // Concrete implementations (ex builder) vary based on final effective cfg. var builder fn.Builder if f.Build.Builder == builders.Pack { builder = buildpacks.NewBuilder( @@ -273,8 +255,7 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err s2i.WithPlatform(cfg.Platform), s2i.WithVerbose(cfg.Verbose)) } else { - err = fmt.Errorf("builder '%v' is not recognized", f.Build.Builder) - return + return builders.ErrUnknownBuilder{Name: f.Build.Builder, Known: KnownBuilders()} } client, done := newClient(ClientConfig{Namespace: f.Deploy.Namespace, Verbose: cfg.Verbose}, @@ -282,25 +263,7 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err fn.WithBuilder(builder)) defer done() - // Default Client Registry, Function Registry or explicit Image required - if client.Registry() == "" && f.Registry == "" && f.Image == "" { - if interactiveTerminal() { - // to be consistent, this should throw an error, with the registry - // prompting code placed within cfg.Prompt and triggered with --confirm - fmt.Println("Please choose a registry for the function image. For example, 'docker.io/tigerteam'.") - if err = survey.AskOne( - &survey.Input{Message: "Registry for function images:"}, - &cfg.Registry, survey.WithValidator( - NewRegistryValidator(cfg.Path))); err != nil { - return fn.ErrRegistryRequired - } - fmt.Println("Note: building a function the first time will take longer than subsequent builds") - } - - return fn.ErrRegistryRequired - } - - // Perform the deployment either remote or local. + // Deploy if cfg.Remote { // Invoke a remote build/push/deploy pipeline // Returned is the function with fields like Registry and Image populated. @@ -308,10 +271,7 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err return } } else { - if err = f.Write(); err != nil { // TODO: remove when client API uses 'f' - return - } - if build(cfg.Build, f, client) { // --build or "auto" with FS changes + if shouldBuild(cfg.Build, f, client) { // --build or "auto" with FS changes if err = client.Build(cmd.Context(), f.Root); err != nil { return } @@ -336,13 +296,13 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err return f.Write() } -// build returns true if the value of buildStr is a truthy value, or if -// it is the literal "auto" and the function reports as being currently +// shouldBuild returns true if the value of the build option is a truthy value, +// or if it is the literal "auto" and the function reports as being currently // unbuilt. Invalid errors are not reported as this is the purview of // deployConfig.Validate -func build(buildCfg string, f fn.Function, client *fn.Client) bool { +func shouldBuild(buildCfg string, f fn.Function, client *fn.Client) bool { if buildCfg == "auto" { - return !client.Built(f.Root) // first build or modified filesystem + return !fn.Built(f.Root) // first build or modified filesystem } build, _ := strconv.ParseBool(buildCfg) return build @@ -513,16 +473,24 @@ you can install docker credential helper https://github.com/docker/docker-creden } type deployConfig struct { - buildConfig + buildConfig // further embeds config.Global // Perform build using the settings from the embedded buildConfig struct. // Acceptable values are the keyword 'auto', or a truthy value such as // 'true', 'false, '1' or '0'. Build string - // Remote indicates the deployment (and possibly build) process are to - // be triggered in a remote environment rather than run locally. - Remote bool + // Env variables. May include removals using a "-" + Env []string + + // Git branch for remote builds + GitBranch string + + // Directory in the git repo where the function is located + GitDir string + + // Git repo url for remote builds + GitURL string // Namespace override for the deployed function. If provided, the // underlying platform will be instructed to deploy the function to the given @@ -532,75 +500,110 @@ type deployConfig struct { // (~/.kube/config) in the case of Kubernetes. Namespace string - // Envs passed via cmd to be added/updated - EnvToUpdate *util.OrderedMap - - // Envs passed via cmd to removed - EnvToRemove []string - - // Git repo url for remote builds - GitURL string - - // Git branch for remote builds - GitBranch string - - // Directory in the git repo where the function is located - GitDir string - - // ImageDigest is automatically split off an --image tag - ImageDigest string + // Remote indicates the deployment (and possibly build) process are to + // be triggered in a remote environment rather than run locally. + Remote bool } // newDeployConfig creates a buildConfig populated from command flags and // environment variables; in that precedence. -func newDeployConfig(cmd *cobra.Command) (deployConfig, error) { - envToUpdate, envToRemove, err := envFromCmd(cmd) - if err != nil { - return deployConfig{}, err - } - - c := deployConfig{ +func newDeployConfig(cmd *cobra.Command) (c deployConfig) { + c = deployConfig{ buildConfig: newBuildConfig(), Build: viper.GetString("build"), - Remote: viper.GetBool("remote"), - Namespace: viper.GetString("namespace"), - EnvToUpdate: envToUpdate, - EnvToRemove: envToRemove, - GitURL: viper.GetString("git-url"), + Env: viper.GetStringSlice("env"), GitBranch: viper.GetString("git-branch"), GitDir: viper.GetString("git-dir"), - ImageDigest: "", // automatically split off --image if provided below + GitURL: viper.GetString("git-url"), + Namespace: viper.GetString("namespace"), + Remote: viper.GetBool("remote"), } - if c.Image, c.ImageDigest, err = parseImage(c.Image); err != nil { - return c, err + // NOTE: .Env shold be viper.GetStringSlice, but this returns unparsed + // results and appears to be an open issue since 2017: + // https://github.com/spf13/viper/issues/380 + var err error + if c.Env, err = cmd.Flags().GetStringArray("env"); err != nil { + fmt.Fprintf(cmd.OutOrStdout(), "error reading envs: %v", err) } + return +} - return c, nil +// Configure the given function. Updates a function struct with all +// configurable values. Note that the config already includes function's +// current values, as they were passed through via flag defaults. +func (c deployConfig) Configure(f fn.Function) (fn.Function, error) { + var err error + + // Bubble configure request + // + // The member values on the config object now take absolute precidence + // because they include 1) static config 2) user's global config + // 3) Environment variables and 4) flag values (which were set with their + // default being 1-3). + f = c.buildConfig.Configure(f) // also configures .buildConfig.Global + + // Configure basic members + f.Build.Git.URL = c.GitURL + f.Build.Git.ContextDir = c.GitDir + f.Build.Git.Revision = c.GitBranch // TODO: shouild match; perhaps "refSpec" + f.Deploy.Namespace = c.Namespace + f.Deploy.Remote = c.Remote + + // ImageDigest + // Parsed off f.Image if provided. Deploying adds the ability to specify a + // digest on the associated image (not available on build as nonsensical). + f.ImageDigest, err = imageDigest(f.Image) + if err != nil { + return f, err + } + + // Envs + // Preprocesses any Envs provided (which may include removals) into a final + // set + f.Run.Envs, err = applyEnvs(f.Run.Envs, c.Env) + if err != nil { + return f, err + } + + // .Revision + // TODO: the system should support specifying revision (refSpec) as a URL + // fragment ([#]) throughout, which, when implemented, removes + // the need for the below split into separate members: + if parts := strings.SplitN(c.GitURL, "#", 2); len(parts) == 2 { + f.Build.Git.URL = parts[0] + f.Build.Git.Revision = parts[1] + } + return f, nil +} + +// Apply Env additions/removals to a set of extant envs, returning the final +// merged list. +func applyEnvs(current []fn.Env, args []string) (final []fn.Env, err error) { + // TODO: validate env test cases completely validate this functionality + + // Parse and Merge + inserts, removals, err := util.OrderedMapAndRemovalListFromArray(args, "=") + if err != nil { + return + } + final, _, err = mergeEnvs(current, inserts, removals) + return } // Prompt the user with value of config members, allowing for interaractive changes. // Skipped if not in an interactive terminal (non-TTY), or if --yes (agree to // all prompts) was explicitly set. func (c deployConfig) Prompt() (deployConfig, error) { + var err error + if c.buildConfig, err = c.buildConfig.Prompt(); err != nil { + return c, err + } + if !interactiveTerminal() || !c.Confirm { return c, nil } var qs = []*survey.Question{ - { - Name: "remote", - Prompt: &survey.Confirm{ - Message: "Trigger a remote (on-cluster) build?", - Default: c.Remote, - }, - }, - { - Name: "GitURL", - Prompt: &survey.Input{ - Message: "Git URL", - Default: c.GitURL, - }, - }, { Name: "namespace", Prompt: &survey.Input{ @@ -609,72 +612,79 @@ func (c deployConfig) Prompt() (deployConfig, error) { }, }, { - Name: "path", - Prompt: &survey.Input{ - Message: "Function source path:", - Default: c.Path, - }, - }, - { - Name: "registry", - Prompt: &survey.Input{ - Message: "Registry for function images:", - Default: c.Registry, + Name: "remote", + Prompt: &survey.Confirm{ + Message: "Trigger a remote (on-cluster) build?", + Default: c.Remote, }, }, } - if err := survey.Ask(qs, &c); err != nil { + if err = survey.Ask(qs, &c); err != nil { return c, err } - // calculate imageName with potentially new registry/path - imageName := deriveImage(c.Image, c.Registry, c.Path) - - qs = []*survey.Question{ - { - Name: "image", - Prompt: &survey.Input{ - Message: "Full image name (e.g. quay.io/boson/node-sample):", - Default: imageName, - }, - }, - { - Name: "namespace", - Prompt: &survey.Input{ - Message: "Namespace into which the function is (re)deployed", - Default: c.Namespace, + if c.Remote { + qs = []*survey.Question{ + { + Name: "GitURL", + Prompt: &survey.Input{ + Message: "URL to Git Repository for the remote to use (default is to send local source code)", + Default: c.GitURL, + }, }, - }, + } + if err = survey.Ask(qs, &c); err != nil { + return c, err + } } - err := survey.Ask(qs, &c) + + // TODO: prompt for optional additional git settings here: + // if c.GitURL != "" { + // } return c, err } // Validate the config passes an initial consistency check -func (c deployConfig) Validate() (err error) { +func (c deployConfig) Validate(cmd *cobra.Command) (err error) { // Bubble validation if err = c.buildConfig.Validate(); err != nil { return } - // Can not enable build when specifying an --image + // Check Image Digest was included + // (will be set on the function during .Configure + var digest string + if digest, err = imageDigest(c.Image); err != nil { + return + } + + // --build can be "auto"|true|false + if c.Build != "auto" { + if _, err := strconv.ParseBool(c.Build); err != nil { + return fmt.Errorf("unrecognized value for --build '%v'. accepts 'auto', 'true' or 'false' (or similarly truthy value)", c.Build) + } + } + + // Can not enable build when specifying an --image with digest (already built) truthy := func(s string) bool { v, _ := strconv.ParseBool(s) return v } - if c.ImageDigest != "" && truthy(c.Build) { + if digest != "" && truthy(c.Build) { return errors.New("building can not be enabled when using an image with digest") } - // Can not push when specifying an --image - if c.ImageDigest != "" && c.Push { + // Can not push when specifying an --image with digest + if digest != "" && c.Push { return errors.New("pushing is not valid when specifying an image with digest") } - // Git settings are only avaolabe with --remote - if (c.GitURL != "" || c.GitDir != "" || c.GitBranch != "") && !c.Remote { - return errors.New("git settings (--git-url --git-dir and --git-branch) are currently only available when triggering remote deployments (--remote)") + // Git references can only be supplied explicitly when coupled with --remote + // See `printDeployMessages` which issues informative messages to the user + // regarding this potentially confusing nuance. + if !c.Remote && (cmd.Flags().Changed("git-url") || cmd.Flags().Changed("git-dir") || cmd.Flags().Changed("git-branch")) { + return errors.New("git settings (--git-url --git-dir and --git-branch) are only applicable when triggering remote deployments (--remote)") } // Git URL can contain at maximum one '#' @@ -683,72 +693,92 @@ func (c deployConfig) Validate() (err error) { return fmt.Errorf("invalid --git-url '%v'", c.GitURL) } - // --build can be "auto"|true|false - if c.Build != "auto" { - if _, err := strconv.ParseBool(c.Build); err != nil { - return fmt.Errorf("unrecognized value for --build '%v'. accepts 'auto', 'true' or 'false' (or similarly truthy value)", c.Build) - } - } + // NOTE: There is no explicit check for --registry or --image here, because + // this logic is baked into core, which will validate the cases and return + // an fn.ErrNameRequired, fn.ErrImageRequired etc as needed. return } -func parseImage(v string) (name, digest string, err error) { +// imageDigest returns the image digest from a full image string if it exists, +// and includes basic validation that a provided digest is correctly formatted. +func imageDigest(v string) (digest string, err error) { vv := strings.Split(v, "@") if len(vv) < 2 { - name = v + return // has no digest + } else if len(vv) > 2 { + err = fmt.Errorf("image '%v' contains an invalid digest (extra '@')", v) return } - name = vv[0] digest = vv[1] if !strings.HasPrefix(digest, "sha256:") { - return v, "", fmt.Errorf("image '%s' has an invalid prefix syntax for digest (should be 'sha256:')", v) + err = fmt.Errorf("image digest '%s' requires 'sha256:' prefix", digest) + return } if len(digest[7:]) != 64 { - return v, "", fmt.Errorf("sha256 hash in '%s' has the wrong length (%d), should be 64", digest, len(digest[7:])) + err = fmt.Errorf("image digest '%v' has an invalid sha256 hash length of %v when it should be 64", digest, len(digest[7:])) } return } -// Warnings are printed to the output when the evaluation of effective namespace -// may be confusing to the user. -// -// active = the curently active kube cluster namespace (or "default") -// current = the namespace in which the function is currently deployed (or "") -func namespaceWarnings(cfg deployConfig, cmd *cobra.Command) { - // NOTE(lkingland): This function can largely be removed when Namespace is - // gathered from the Global Config struct, because this logic will implicitly - // exist in the way it is instantiated. - - active, err := k8s.GetNamespace("") - if err != nil { - active = "default" - } - var ( - f, _ = fn.NewFunction(cfg.Path) - current = f.Deploy.Namespace // Current - target = current // Target Current by default - flagValue = cfg.Namespace // Flag Value - flagProvided = cmd.Flags().Changed("namespace") // Flag Provided - out = cmd.ErrOrStderr() - ) - if current == "" { - target = active - } - if flagProvided { - target = flagValue - } - - // Warn if deploying to a namespace other than the active (user might not see it): - if target != active { - fmt.Fprintf(out, "Warning: target namespace '%s' is not the currently active namespace '%s'. Continuing with deployment to '%s'.\n", target, active, target) +// printDeployMessages to the output. Non-error deployment messages. +func printDeployMessages(out io.Writer, cfg deployConfig) { + // Digest + // ------ + // If providing an image digest, print this, and note that the values + // of push and build are ignored. + // TODO: perhaps just error if either --push or --build were actually + // provided (using the cobra .Changed accessor) + digest, err := imageDigest(cfg.Image) + if err != nil && digest != "" { + fmt.Fprintf(out, "Deploying image '%v' with digest '%s'. Build and push are disabled.\n", cfg.Image, digest) + } + + // Namespace + // --------- + f, _ := fn.NewFunction(cfg.Path) + currentNamespace := f.Deploy.Namespace // will be "" if no initialed f at path. + targetNamespace := cfg.Namespace + + // If potentially creating a duplicate deployed function in a different + // namespace. TODO: perhaps add a --delete or --force flag which will + // automagically delete the deployment in the "old" namespace. + if targetNamespace != currentNamespace && currentNamespace != "" { + fmt.Fprintf(out, "Warning: function is in namespace '%s', but requested namespace is '%s'. Continuing with deployment to '%v'.\n", currentNamespace, targetNamespace, targetNamespace) + } + + // Namespace Changing + // ------------------- + // If the target namespace is provided but differs from active, warn because + // the function wont be visible to other commands such as kubectl unless + // context namespace is switched. + activeNamespace, err := k8s.GetNamespace("") + if err == nil && targetNamespace != "" && targetNamespace != activeNamespace { + fmt.Fprintf(out, "Warning: namespace chosen is '%s', but currently active namespace is '%s'. Continuing with deployment to '%s'.\n", cfg.Namespace, activeNamespace, cfg.Namespace) + } + + // Git Args + // ----------------- + // Print a warning if the function already contains Git attributes, but the + // current invocation is not remote. (providing Git attributes directly + // via flags without --remote will error elsewhere). + // + // When invoking a remote build with --remote, the --git-X arguments + // are persisted to the local function's source code such that the reference + // is retained. Subsequent runs of deploy then need not have these arguments + // present. + // + // However, when building _locally_ thereafter, the deploy command should + // prefer the local source code, ignoring the values for --git-url etc. + // Since this might be confusing, a warning is issued below that the local + // function source does include a reference to a git reposotiry, but that it + // 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 code will be used.") } - // Warn if deploying to a different namespace than it already exists within (creates orphan): - if target != current && current != "" { - fmt.Fprintf(out, "Warning: function is in namespace '%s', but requested namespace is '%s'. Continuing with deployment to '%v'.\n", current, target, target) - } } diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index b65fcc544..732a8dd9c 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "path/filepath" + "reflect" "strings" "testing" "time" @@ -13,13 +14,34 @@ import ( "github.com/spf13/cobra" fn "knative.dev/func" "knative.dev/func/builders" + "knative.dev/func/config" "knative.dev/func/mock" ) -const TestRegistry = "example.com/alice" - +// commandConstructor is used to share test implementations between commands +// which only differ in the command being tested (ex: build and deploy share +// a large overlap in tests because building is a subset of the deploy task) type commandConstructor func(ClientFactory) *cobra.Command +// TestDeploy_ConfigApplied ensures that the deploy command applies config +// settings at each level (static, global, function, envs, flags) +func TestDeploy_ConfigApplied(t *testing.T) { + testConfigApplied(NewDeployCmd, t) +} + +// 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) +} + +// 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. +func TestDeploy_FunctionContext(t *testing.T) { + testFunctionContext(NewDeployCmd, t) +} + // 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) { @@ -355,11 +377,13 @@ func TestDeploy_RemoteBuildURLPermutations(t *testing.T) { // Valid flag permutations (empty indicates flag should be omitted) // and a functon which will convert a permutation into flags for use // by the subtests. + // The empty string indicates the case in which the flag is not provided. var ( remoteValues = []string{"", "true", "false"} buildValues = []string{"", "true", "false", "auto"} urlValues = []string{"", "https://example.com/user/repo"} + // toArgs converts one permutaton of the values into command arguments toArgs = func(remote, build, url string) []string { args := []string{} if remote != "" { @@ -385,7 +409,7 @@ func TestDeploy_RemoteBuildURLPermutations(t *testing.T) { t.Fatal(err) } - // deploy it using the deploy commnand with flags set to the currently + // deploy it using the deploy command with flags set to the currently // effective flag permutation. var ( deployer = mock.NewDeployer() @@ -402,9 +426,9 @@ func TestDeploy_RemoteBuildURLPermutations(t *testing.T) { err := cmd.Execute() // err is checked below // Assertions - if remote != "" && remote != "false" { // default "" is == false. - // REMOTE Assertions + if remote != "" && remote != "false" { // the default of "" is == false + // REMOTE Assertions if !pipeliner.RunInvoked { // Remote deployer should be triggered t.Error("remote was not invoked") } @@ -526,12 +550,12 @@ func Test_ImageWithDigestErrors(t *testing.T) { { name: "invalid digest prefix 'Xsha256', expect error", image: "example.com/myNamespace/myFunction:latest@Xsha256:7d66645b0add6de7af77ef332ecd4728649a2f03b9a2716422a054805b595c4e", - errString: "image 'example.com/myNamespace/myFunction:latest@Xsha256:7d66645b0add6de7af77ef332ecd4728649a2f03b9a2716422a054805b595c4e' has an invalid prefix syntax for digest (should be 'sha256:')", + errString: "image digest 'Xsha256:7d66645b0add6de7af77ef332ecd4728649a2f03b9a2716422a054805b595c4e' requires 'sha256:' prefix", }, { name: "invalid sha hash length(added X at the end), expect error", image: "example.com/myNamespace/myFunction:latest@sha256:7d66645b0add6de7af77ef332ecd4728649a2f03b9a2716422a054805b595c4eX", - errString: "sha256 hash in 'sha256:7d66645b0add6de7af77ef332ecd4728649a2f03b9a2716422a054805b595c4eX' has the wrong length (65), should be 64", + errString: "image digest 'sha256:7d66645b0add6de7af77ef332ecd4728649a2f03b9a2716422a054805b595c4eX' has an invalid sha256 hash length of 65 when it should be 64", }, } @@ -590,51 +614,58 @@ func Test_ImageWithDigestErrors(t *testing.T) { func TestDeploy_Namespace(t *testing.T) { root := fromTempDirectory(t) - var expectedNamespace string - - // A mock deployer which validates the namespace received is that expected - deployer := mock.NewDeployer() - deployer.DeployFn = func(_ context.Context, f fn.Function) (res fn.DeploymentResult, err error) { - e := &expectedNamespace // closure - if f.Deploy.Namespace != *e { - t.Fatalf("expected namespace '%v', got '%v'", *e, f.Deploy.Namespace) - } - return - } - // A function which will be repeatedly, mockingly deployed - if err := fn.New().Init(fn.Function{Root: root, Runtime: "go", Registry: TestRegistry}); err != nil { + f := fn.Function{Root: root, Runtime: "go", Registry: TestRegistry} + if err := fn.New().Init(f); err != nil { t.Fatal(err) } - // Ensure the default "func" which is gathered from the default kubeconfig - // path of ./tesdata/default_kubeconfig - expectedNamespace = "func" // see ./testdata/default_kubeconfig + // The mock deployer responds that the given function was deployed + // to the namespace indicated in f.Deploy.Namespace or "default" if empty + // (it does not actually consider the current kubernetes context) + deployer := mock.NewDeployer() + cmd := NewDeployCmd(NewTestClient(fn.WithDeployer(deployer))) cmd.SetArgs([]string{}) if err := cmd.Execute(); err != nil { t.Fatal(err) } + f, _ = fn.NewFunction(root) + if f.Deploy.Namespace != "default" { + t.Fatalf("expected namespace 'default', got '%v'", f.Deploy.Namespace) + } // Change the function's active namespace and ensure it is used, preempting - // the 'func' namespace gathered from kubeconfig. - expectedNamespace = "alreadyDeployedNamespace" + // the 'default' namespace from the mock f, err := fn.NewFunction(root) if err != nil { t.Fatal(err) } - f.Deploy.Namespace = expectedNamespace + f.Deploy.Namespace = "alreadyDeployed" if err := f.Write(); err != nil { t.Fatal(err) } + cmd = NewDeployCmd(NewTestClient(fn.WithDeployer(deployer))) + cmd.SetArgs([]string{}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + f, _ = fn.NewFunction(root) + if f.Deploy.Namespace != "alreadyDeployed" { + t.Fatalf("expected namespace 'alreadyDeployed', got '%v'", f.Deploy.Namespace) + } // Ensure an explicit name (a flag) is taken with highest precedence - expectedNamespace = "flagValueNamespace" cmd = NewDeployCmd(NewTestClient(fn.WithDeployer(deployer))) - cmd.SetArgs([]string{"--namespace", expectedNamespace}) + cmd.SetArgs([]string{"--namespace=newNamespace"}) if err := cmd.Execute(); err != nil { t.Fatal(err) } + f, _ = fn.NewFunction(root) + if f.Deploy.Namespace != "newNamespace" { + t.Fatalf("expected namespace 'newNamespace', got '%v'", f.Deploy.Namespace) + } + } // TestDeploy_GitArgsPersist ensures that the git flags, if provided, are @@ -782,9 +813,27 @@ func TestDeploy_NamespaceDefaults(t *testing.T) { t.Fatalf("newly created functions should not have a namespace set until deployed. Got '%v'", f.Deploy.Namespace) } + // a deployer which actually uses config.DefaultNamespace + // This is not the default implementation of mock.NewDeployer as this would + // be likely to be confusing, since it would vary on developer machines + // unless they remember to clear local envs, such as is done here within + // fromTempDirectory(t). To avert this potential confusion, the use of + // config.DefaultNamespace() is kept local to this test. + deployer := mock.NewDeployer() + deployer.DeployFn = func(_ context.Context, f fn.Function) (result fn.DeploymentResult, err error) { + // deployer implementations shuld have integration tests which confirm + // this logic: + if f.Deploy.Namespace != "" { + result.Namespace = f.Deploy.Namespace + } else { + result.Namespace = config.DefaultNamespace() + } + return + } + // New deploy command that will not actually deploy or build (mocked) cmd := NewDeployCmd(NewTestClient( - fn.WithDeployer(mock.NewDeployer()), + fn.WithDeployer(deployer), fn.WithBuilder(mock.NewBuilder()), fn.WithPipelinesProvider(mock.NewPipelinesProvider()), fn.WithRegistry(TestRegistry), @@ -896,17 +945,17 @@ func TestDeploy_NamespaceRedeployWarning(t *testing.T) { fn.WithRegistry(TestRegistry), )) cmd.SetArgs([]string{}) - stderr := strings.Builder{} - cmd.SetErr(&stderr) + stdout := strings.Builder{} + cmd.SetOut(&stdout) if err := cmd.Execute(); err != nil { t.Fatal(err) } - expected := "Warning: target namespace 'funcns' is not the currently active namespace 'mynamespace'. Continuing with deployment to 'funcns'." + expected := "Warning: namespace chosen is 'funcns', but currently active namespace is 'mynamespace'. Continuing with deployment to 'funcns'." // Ensure output contained warning if changing namespace - if !strings.Contains(stderr.String(), expected) { - t.Log("STDERR:\n" + stderr.String()) + if !strings.Contains(stdout.String(), expected) { + t.Log("STDOUT:\n" + stdout.String()) t.Fatalf("Expected warning not found:\n%v", expected) } @@ -923,14 +972,12 @@ func TestDeploy_NamespaceRedeployWarning(t *testing.T) { // TestDeploy_RemotePersists ensures that the remote field is read from // the function by default, and is able to be overridden by flags/env vars. func TestDeploy_RemotePersists(t *testing.T) { - t.Helper() root := fromTempDirectory(t) - cmdFn := NewDeployCmd if err := fn.New().Init(fn.Function{Runtime: "node", Root: root}); err != nil { t.Fatal(err) } - cmd := cmdFn(NewTestClient(fn.WithRegistry(TestRegistry))) + cmd := NewDeployCmd(NewTestClient(fn.WithRegistry(TestRegistry))) cmd.SetArgs([]string{}) if err := cmd.Execute(); err != nil { t.Fatal(err) @@ -940,7 +987,7 @@ func TestDeploy_RemotePersists(t *testing.T) { var f fn.Function // Deploy the function, specifying remote deployment(on-cluster) - cmd = cmdFn(NewTestClient(fn.WithRegistry(TestRegistry))) + cmd = NewDeployCmd(NewTestClient(fn.WithRegistry(TestRegistry))) cmd.SetArgs([]string{"--remote"}) if err := cmd.Execute(); err != nil { t.Fatal(err) @@ -954,7 +1001,7 @@ func TestDeploy_RemotePersists(t *testing.T) { } // Deploy the function again without specifying remote - cmd = cmdFn(NewTestClient(fn.WithRegistry(TestRegistry))) + cmd = NewDeployCmd(NewTestClient(fn.WithRegistry(TestRegistry))) cmd.SetArgs([]string{}) if err := cmd.Execute(); err != nil { t.Fatal(err) @@ -984,3 +1031,137 @@ func TestDeploy_RemotePersists(t *testing.T) { t.Fatalf("value of remote flag not persisted") } } + +// TestDeploy_Envs ensures that environment variable for the function, provided +// as arguments, are correctly evaluated. This includes: +// - Multiple Envs are supported (flag can be provided multiple times) +// - Existing Envs on the function are retained +// - Flags provided with the minus '-' suffix are treated as a removal +func TestDeploy_Envs(t *testing.T) { + var ( + root = fromTempDirectory(t) + ptr = func(s string) *string { return &s } // TODO: remove pointers from Envs. + f fn.Function + cmd *cobra.Command + err error + expected []fn.Env + ) + + if err = fn.New().Init(fn.Function{Runtime: "go", Root: root, Registry: TestRegistry}); err != nil { + t.Fatal(err) + } + + // Deploy the function with two environment variables specified. + cmd = NewDeployCmd(NewTestClient()) + cmd.SetArgs([]string{"--env=ENV1=VAL1", "--env=ENV2=VAL2"}) + if err = cmd.Execute(); err != nil { + t.Fatal(err) + } + // Assert it contains the two environment variables + if f, err = fn.NewFunction(root); err != nil { + t.Fatal(err) + } + expected = []fn.Env{ + {Name: ptr("ENV1"), Value: ptr("VAL1")}, + {Name: ptr("ENV2"), Value: ptr("VAL2")}, + } + if !reflect.DeepEqual(f.Run.Envs, expected) { + t.Fatalf("Expected envs '%v', got '%v'", expected, f.Run.Envs) + } + + // Deploy the function with an additinal environment variable. + cmd = NewDeployCmd(NewTestClient()) + cmd.SetArgs([]string{"--env=ENV3=VAL3"}) + if err = cmd.Execute(); err != nil { + t.Fatal(err) + } + // Assert the original envs were retained and the new env was added. + if f, err = fn.NewFunction(root); err != nil { + t.Fatal(err) + } + expected = []fn.Env{ + {Name: ptr("ENV1"), Value: ptr("VAL1")}, + {Name: ptr("ENV2"), Value: ptr("VAL2")}, + {Name: ptr("ENV3"), Value: ptr("VAL3")}, + } + if !reflect.DeepEqual(f.Run.Envs, expected) { + t.Fatalf("Expected envs '%v', got '%v'", expected, f.Run.Envs) + } + + // Deploy the function with a removal instruction + cmd = NewDeployCmd(NewTestClient()) + cmd.SetArgs([]string{"--env=ENV1-"}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + if f, err = fn.NewFunction(root); err != nil { + t.Fatal(err) + } + expected = []fn.Env{ + {Name: ptr("ENV2"), Value: ptr("VAL2")}, + {Name: ptr("ENV3"), Value: ptr("VAL3")}, + } + if !reflect.DeepEqual(f.Run.Envs, expected) { + t.Fatalf("Expected envs '%v', got '%v'", expected, f.Run.Envs) + } + + // Try removing the rest for good measure + cmd = NewDeployCmd(NewTestClient()) + cmd.SetArgs([]string{"--env=ENV2-", "--env=ENV3-"}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + if f, err = fn.NewFunction(root); err != nil { + t.Fatal(err) + } + if len(f.Run.Envs) != 0 { + t.Fatalf("Expected no envs to remain, got '%v'", f.Run.Envs) + } + + // TODO: create and test typed errors for ErrEnvNotExist etc. +} + +// TestDeploy_UnsetFlag ensures that unsetting a flag on the command +// line causes the pertinent value to be zeroed out. +func TestDeploy_UnsetFlag(t *testing.T) { + // From a temp directory + root := fromTempDirectory(t) + + // Create a function + f := fn.Function{Runtime: "go", Root: root, Registry: TestRegistry} + if err := fn.New().Init(f); err != nil { + t.Fatal(err) + } + + // Deploy it, specifying a Git URL + cmd := NewDeployCmd(NewTestClient()) + cmd.SetArgs([]string{"--remote", "--git-url=https://git.example.com/alice/f"}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + + // Load the function and confirm the URL was persisted + f, err := fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + if f.Build.Git.URL != "https://git.example.com/alice/f" { + t.Fatalf("url not persisted") + } + + // Deploy it again, unsetting the value + cmd = NewDeployCmd(NewTestClient()) + cmd.SetArgs([]string{"--git-url="}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + + // Load the function and confirm the URL was unset + f, err = fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + if f.Build.Git.URL != "" { + t.Fatalf("url not cleared") + } +} diff --git a/cmd/describe_test.go b/cmd/describe_test.go index d3da82222..a673119b5 100644 --- a/cmd/describe_test.go +++ b/cmd/describe_test.go @@ -1,7 +1,6 @@ package cmd import ( - "path/filepath" "testing" fn "knative.dev/func" @@ -81,20 +80,20 @@ func TestDescribe_NameAndPathExclusivity(t *testing.T) { // TestDescribe_Namespace ensures that the namespace provided to the client // for use when describing a function is set -// 1. The flag /env variable if provided -// 2. The namespace of the function at path if provided -// 3. The user's current active namespace +// 1. Blank when not provided nor available (delegate to the describer impl to +// choose current kube context) +// 2. The namespace of the contextually active function +// 3. The flag /env variable if provided func TestDescribe_Namespace(t *testing.T) { root := fromTempDirectory(t) client := fn.New(fn.WithDescriber(mock.NewDescriber())) - // Ensure that the default is "default" when no context can be identified - t.Setenv("KUBECONFIG", filepath.Join(cwd(), "nonexistent")) - t.Setenv("KUBERNETES_SERVICE_HOST", "") + // Ensure that the default is "", indicating the describer should use + // config.DefaultNamespace cmd := NewDescribeCmd(func(cc ClientConfig, _ ...fn.Option) (*fn.Client, func()) { - if cc.Namespace != "default" { - t.Fatalf("expected 'default', got '%v'", cc.Namespace) + if cc.Namespace != "" { + t.Fatalf("expected '', got '%v'", cc.Namespace) } return client, func() {} }) diff --git a/cmd/list.go b/cmd/list.go index d57b78c62..633e1ea7c 100644 --- a/cmd/list.go +++ b/cmd/list.go @@ -39,15 +39,19 @@ Lists all deployed functions in a given namespace. PreRunE: bindEnv("all-namespaces", "output", "namespace"), } - // Config - cfg, err := config.NewDefault() - if err != nil { - fmt.Fprintf(cmd.OutOrStdout(), "error loading config at '%v'. %v\n", config.File(), err) - } + // Namespace Config + // Differing from other commands, the default namespace for the list + // command is always the currently active namespace as returned by + // config.DefaultNamespace(). The -A flag clears this value indicating + // the lister implementation should not filter by namespace and instead + // list from all namespaces. This logic is sligtly inverse to the other + // namespace-sensitive commands which default to the currently active + // function if available, and delegate to the implementation to use + // the config default otherwise. // Flags cmd.Flags().BoolP("all-namespaces", "A", false, "List functions in all namespaces. If set, the --namespace flag is ignored.") - cmd.Flags().StringP("namespace", "n", cfg.Namespace, "The namespace for which to list functions. (Env: $FUNC_NAMESPACE)") + cmd.Flags().StringP("namespace", "n", config.DefaultNamespace(), "The namespace for which to list functions. (Env: $FUNC_NAMESPACE)") cmd.Flags().StringP("output", "o", "human", "Output format (human|plain|json|xml|yaml) (Env: $FUNC_OUTPUT)") if err := cmd.RegisterFlagCompletionFunc("output", CompleteOutputFormatList); err != nil { diff --git a/cmd/list_test.go b/cmd/list_test.go index 50cf1efa1..4f3448c20 100644 --- a/cmd/list_test.go +++ b/cmd/list_test.go @@ -8,7 +8,7 @@ import ( ) // TestList_Namespace ensures that list command options for specifying a -// namespace (--namespace) or all namespaces (--all-namespacs) are properly +// namespace (--namespace) or all namespaces (--all-namespaces) are properly // evaluated. func TestList_Namespace(t *testing.T) { _ = fromTempDirectory(t) diff --git a/cmd/root.go b/cmd/root.go index b8bd1d3c7..1dcc83a9d 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -6,7 +6,6 @@ import ( "os" "path/filepath" "strings" - "testing" "time" "knative.dev/func/cmd/templates" @@ -143,17 +142,21 @@ func registry() string { // definition (prior to parsing). func effectivePath() (path string) { var ( - env = os.Getenv("FUNC_PATH") - fs = pflag.NewFlagSet("", pflag.ContinueOnError) - long = fs.StringP("path", "p", "", "") + env = os.Getenv("FUNC_PATH") + fs = pflag.NewFlagSet("", pflag.ContinueOnError) + p = fs.StringP("path", "p", "", "") ) fs.SetOutput(io.Discard) - _ = fs.Parse(os.Args[1:]) + fs.ParseErrorsWhitelist.UnknownFlags = true // wokeignore:rule=whitelist + err := fs.Parse(os.Args[1:]) + if err != nil { + fmt.Fprintf(os.Stderr, "error preparsing flags: %v\n", err) + } if env != "" { path = env } - if *long != "" { - path = *long + if *p != "" { + path = *p } return path } @@ -448,15 +451,3 @@ func surveySelectDefault(value string, options []string) string { // which should fail proper validation return "" } - -// clearEnvs sets all environment variables with the prefix of FUNC_ to -// empty (unsets) for the duration of the test t. -func clearEnvs(t *testing.T) { - t.Helper() - for _, v := range os.Environ() { - if strings.HasPrefix(v, "FUNC_") { - parts := strings.SplitN(v, "=", 2) - t.Setenv(parts[0], "") - } - } -} diff --git a/cmd/root_test.go b/cmd/root_test.go index 651acb7cd..9f630de60 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -17,6 +17,8 @@ import ( . "knative.dev/func/testing" ) +const TestRegistry = "example.com/alice" + func TestRoot_PersistentFlags(t *testing.T) { tests := []struct { name string @@ -320,6 +322,13 @@ func TestRoot_effectivePath(t *testing.T) { } }) + t.Run("continues on unrecognized flags", func(t *testing.T) { + os.Args = []string{"test", "-r=repo.example.com/bob", "-p=p3"} + if effectivePath() != "p3" { + t.Fatalf("the effective path did not evaluate when unexpected flags were present") + } + }) + } // Helpers @@ -361,11 +370,11 @@ func piped(t *testing.T) func() string { } } -// fromTempDirectory is a cli-specific test helper which endeavors to create +// fromTempDirectory is a test helper which endeavors to create // an environment clean of developer's settings for use during CLI testing. func fromTempDirectory(t *testing.T) string { t.Helper() - clearEnvs(t) + ClearEnvs(t) // We have to define KUBECONFIG, or the file at ~/.kube/config (if extant) // will be used (disrupting tests by using the current user's environment). diff --git a/cmd/run.go b/cmd/run.go index 77f7c0a4e..0b01c6929 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -98,7 +98,7 @@ func runRun(cmd *cobra.Command, args []string, newClient ClientFactory) (err err // is stale (has either never been built or has had filesystem modifications // since the last build). if cfg.Build == "auto" { - if !client.Built(function.Root) { + if !fn.Built(function.Root) { if err = client.Build(cmd.Context(), cfg.Path); err != nil { return } diff --git a/cmd/testdata/TestBuild_ConfigApplied/func/config.yaml b/cmd/testdata/TestX_ConfigApplied/func/config.yaml similarity index 100% rename from cmd/testdata/TestBuild_ConfigApplied/func/config.yaml rename to cmd/testdata/TestX_ConfigApplied/func/config.yaml diff --git a/cmd/testdata/TestBuild_ConfigPrecidence/func/config.yaml b/cmd/testdata/TestX_ConfigPrecedence/func/config.yaml similarity index 100% rename from cmd/testdata/TestBuild_ConfigPrecidence/func/config.yaml rename to cmd/testdata/TestX_ConfigPrecedence/func/config.yaml diff --git a/config/config.go b/config/config.go index 91fa74349..8fbd2a06b 100644 --- a/config/config.go +++ b/config/config.go @@ -28,6 +28,11 @@ const ( // DefaultNamespace for remote operations is the currently active // context namespace (if available) or the fallback "default". +// Namespace, when left blank on the function itself, indicates this +// value should be used when deploying for the first time. Subsequelty +// the value will be populated, indicating the namespace in which the +// function is currently deployed. Changes to this value will issue warnings +// to the user. func DefaultNamespace() (namespace string) { var err error if namespace, err = k8s.GetNamespace(""); err != nil { @@ -50,9 +55,8 @@ type Global struct { // for one which further takes into account the optional config file. func New() Global { return Global{ - Builder: DefaultBuilder, - Language: DefaultLanguage, - Namespace: DefaultNamespace(), + Builder: DefaultBuilder, + Language: DefaultLanguage, // ... } } diff --git a/docs/reference/func_build.md b/docs/reference/func_build.md index bf4c18320..7edb98138 100644 --- a/docs/reference/func_build.md +++ b/docs/reference/func_build.md @@ -55,7 +55,7 @@ func build ### Options ``` - -b, --builder string build strategy to use when creating the underlying image. Currently supported build strategies are "pack" and "s2i". (default "pack") + -b, --builder string Builder to use when creating the function's container. Currently supported builders are "pack" and "s2i". (Env: $FUNC_BUILDER) (default "pack") --builder-image string Specify a custom builder image for use by the builder other than its default. (Env: $FUNC_BUILDER_IMAGE) -c, --confirm Prompt to confirm all configuration options (Env: $FUNC_CONFIRM) -h, --help help for build @@ -63,7 +63,7 @@ func build -p, --path string Path to the project directory. Default is current working directory (Env: $FUNC_PATH) --platform string Optionally specify a target platform, for example "linux/amd64" when using the s2i build strategy -u, --push Attempt to push the function image to the configured registry after being successfully built - -r, --registry string Registry + namespace part of the image to build, ex 'quay.io/myuser'. The full image name is automatically determined (Env: $FUNC_REGISTRY) + -r, --registry string Container registry + registry namespace. (ex 'ghcr.io/myuser'). The full image name is automatically determined using this along with function name. (Env: $FUNC_REGISTRY) ``` ### Options inherited from parent commands diff --git a/docs/reference/func_delete.md b/docs/reference/func_delete.md index a6128a7ba..c07591ff6 100644 --- a/docs/reference/func_delete.md +++ b/docs/reference/func_delete.md @@ -35,7 +35,7 @@ func delete -n apps myfunc -a, --all string Delete all resources created for a function, eg. Pipelines, Secrets, etc. (Env: $FUNC_ALL) (allowed values: "true", "false") (default "true") -c, --confirm Prompt to confirm all configuration options (Env: $FUNC_CONFIRM) -h, --help help for delete - -n, --namespace string The namespace in which to delete. (Env: $FUNC_NAMESPACE) (default "default") + -n, --namespace string The namespace in which to delete. (Env: $FUNC_NAMESPACE) -p, --path string Path to the project directory. Default is current working directory (Env: $FUNC_PATH) ``` diff --git a/docs/reference/func_deploy.md b/docs/reference/func_deploy.md index a330d050f..0b008a5e6 100644 --- a/docs/reference/func_deploy.md +++ b/docs/reference/func_deploy.md @@ -58,6 +58,9 @@ DESCRIPTION EXAMPLES + o Deploy the function + $ func deploy + o Deploy the function using interactive prompts. This is useful for the first deployment, since most settings will be remembered for future deployments. $ func deploy -c @@ -100,22 +103,22 @@ func deploy ### Options ``` - --build string[="true"] Build the function. [auto|true|false]. [Env: $FUNC_BUILD] (default "auto") - -b, --builder string builder to use when creating the underlying image. Currently supported builders are "pack" and "s2i". (default "pack") - --builder-image string The image the specified builder should use; either an as an image name or a mapping. ($FUNC_BUILDER_IMAGE) + --build string[="true"] Build the function. [auto|true|false]. (Env: $FUNC_BUILD) (default "auto") + -b, --builder string Builder to use when creating the function's container. Currently supported builders are "pack" and "s2i". (default "pack") + --builder-image string Specify a custom builder image for use by the builder other than its default. ($FUNC_BUILDER_IMAGE) -c, --confirm Prompt to confirm all configuration options (Env: $FUNC_CONFIRM) -e, --env stringArray Environment variable to set in the form NAME=VALUE. You may provide this flag multiple times for setting multiple environment variables. To unset, specify the environment variable name followed by a "-" (e.g., NAME-). - -t, --git-branch string Git branch to be used for remote builds (Env: $FUNC_GIT_BRANCH) - -d, --git-dir string Directory in the repo where the function is located (Env: $FUNC_GIT_DIR) + -t, --git-branch string Git revision (branch) to be used when deploying via a git repository (Env: $FUNC_GIT_BRANCH) + -d, --git-dir string Directory in the repo to find the function (default is the root) (Env: $FUNC_GIT_DIR) -g, --git-url string Repo url to push the code to be built (Env: $FUNC_GIT_URL) -h, --help help for deploy -i, --image string 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) - -n, --namespace string Deploy into a specific namespace. Will use function's current namespace by default if already deployed. (Env: $FUNC_NAMESPACE) (default "default") + -n, --namespace string Deploy into a specific namespace. Will use function's current namespace by default if already deployed, and the currently active namespace if it can be determined. (Env: $FUNC_NAMESPACE) -p, --path string Path to the project directory. Default is current working directory (Env: $FUNC_PATH) - --platform string Target platform to build (e.g. linux/amd64). - -u, --push Push the function image to registry before deploying (Env: $FUNC_PUSH) (default true) - -r, --registry string Registry + namespace part of the image to build, ex 'ghcr.io/myuser'. The full image name is automatically determined. (Env: $FUNC_REGISTRY) - --remote Trigger a remote deployment. Default is to deploy and build from the local system: $FUNC_REMOTE) + --platform string Optionally specify a specific platform to build for (e.g. linux/amd64). (Env: $FUNC_PLATFORM) + -u, --push Push the function image to registry before deploying. (Env: $FUNC_PUSH) (default true) + -r, --registry string Container registry + registry namespace. (ex 'ghcr.io/myuser'). The full image name is automatically determined using this along with function name. (Env: $FUNC_REGISTRY) + --remote Trigger a remote deployment. Default is to deploy and build from the local system (Env: $FUNC_REMOTE) ``` ### Options inherited from parent commands diff --git a/docs/reference/func_describe.md b/docs/reference/func_describe.md index db158dfdd..5c06603c8 100644 --- a/docs/reference/func_describe.md +++ b/docs/reference/func_describe.md @@ -30,7 +30,7 @@ func describe --output yaml --path myotherfunc ``` -h, --help help for describe - -n, --namespace string The namespace in which to look for the named function. (Env: $FUNC_NAMESPACE) (default "default") + -n, --namespace string The namespace in which to look for the named function. (Env: $FUNC_NAMESPACE) -o, --output string Output format (human|plain|json|xml|yaml|url) (Env: $FUNC_OUTPUT) (default "human") -p, --path string Path to the project directory. Default is current working directory (Env: $FUNC_PATH) ``` diff --git a/function.go b/function.go index 28e90504e..65885f571 100644 --- a/function.go +++ b/function.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "reflect" "regexp" "strings" "time" @@ -325,7 +326,15 @@ func nameFromPath(path string) string { // Write aka (save, serialize, marshal) the function to disk at its path. // Only valid functions can be written. +// In order to retain built status (staleness checks), the file is only +// modified if the structure actually changes. func (f Function) Write() (err error) { + // Skip writing (and dirtying the work tree) if there were no modifications. + f1, _ := NewFunction(f.Root) + if reflect.DeepEqual(f, f1) { + return + } + if err = f.Validate(); err != nil { return } @@ -345,17 +354,6 @@ func (f Function) Initialized() bool { return !f.Created.IsZero() } -// Built indicates the function has been built. Does not guarantee the -// image indicated actually exists, just that it _should_ exist based off -// the current state of the Function object, in particular the value of -// the Image and ImageDigest fields. -func (f Function) HasImage() bool { - // If Image (the override) and ImageDigest (the most recent build stamp) are - // both empty, the function is considered unbuilt. - // TODO: upgrade to a "build complete" timestamp. - return f.Image != "" || f.ImageDigest != "" -} - // ImageWithDigest returns the full reference to the image including SHA256 Digest. // If Digest is empty, image:tag is returned. // TODO: Populate this only on a successful deploy, as this results on a dirty diff --git a/mock/deployer.go b/mock/deployer.go index 1dea80ef5..c3ddde3ed 100644 --- a/mock/deployer.go +++ b/mock/deployer.go @@ -6,6 +6,13 @@ import ( fn "knative.dev/func" ) +// DefaultNamespace for mock deployments +// See deployer implementations for tests which ensure the currently +// active kube namespace is chosen when no explicit namespace is provided. +// This mock emulates a deployer which responds that the function was deployed +// to that defined on the function, or "default" if not defined. +const DefaultNamespace = "default" + type Deployer struct { DeployInvoked bool DeployFn func(context.Context, fn.Function) (fn.DeploymentResult, error) @@ -13,7 +20,13 @@ type Deployer struct { func NewDeployer() *Deployer { return &Deployer{ - DeployFn: func(context.Context, fn.Function) (fn.DeploymentResult, error) { return fn.DeploymentResult{}, nil }, + DeployFn: func(_ context.Context, f fn.Function) (fn.DeploymentResult, error) { + result := fn.DeploymentResult{Namespace: DefaultNamespace} + if f.Deploy.Namespace != "" { + result.Namespace = f.Deploy.Namespace + } + return result, nil + }, } } diff --git a/testing/testing.go b/testing/testing.go index dbc388239..61e89d1f3 100644 --- a/testing/testing.go +++ b/testing/testing.go @@ -218,3 +218,16 @@ func Cwd() (cwd string) { } return cwd } + +// ClearEnvs sets all environment variables with the prefix of FUNC_ to +// empty (unsets) for the duration of the test t and is used when +// a test needs to completely clear func-releated envs prior to running. +func ClearEnvs(t *testing.T) { + t.Helper() + for _, v := range os.Environ() { + if strings.HasPrefix(v, "FUNC_") { + parts := strings.SplitN(v, "=", 2) + t.Setenv(parts[0], "") + } + } +}