Repository: knative-sandbox/kn-plugin-func (go)

Issue:
func build with -i does not use given image name
Using `func build -i docker.io/gauron/app:latest` doesnt create image named `docker.io/gauron/app:latest` but instead 
`image-registry.openshift-image-registry.svc:5000/default/app` (name gets overridden)

* **Reproduce**: 
`func create -l go`
`func build -i docker.io/name/app`


it looks to me like in `Build` function (client.go) the registry is populated with the openshift default registry and used in `DerivedImage` (look at image below)

![image](https://user-images.githubusercontent.com/49119790/179743827-58f04a9f-7764-4b55-a5a0-6aaa368191a3.png)
`f.Registry` is empty but `registry` is the default openshift registry right before the if statement (as indicated by the LOOK DEEP print) therefore the if is skipped and image is built without image flag

/kind bug

Pull request:
fix: image name derivation and flag precidence
:broom: clean up image name derivation logic
:bug: image flag now correctly respected
:gift: tests in commands and core codifying `registry` vs `image` treatement
:gift: prompts on deploy should now correctly calculate image default
:broom: clean up namespace derivation logic and add a test

Clean up logic used to derive and update image name based on registry, function name etc.  
Adds requisite tests to both `deploy` and `build`.

/kind cleanup

Patch:
diff --git a/OWNERS_ALIASES b/OWNERS_ALIASES
index dac3811c..e0d806b7 100644
--- a/OWNERS_ALIASES
+++ b/OWNERS_ALIASES
@@ -142,6 +142,7 @@ aliases:
   - cardil
   - rhuss
   kn-plugin-func-approvers:
+  - jrangelramos
   - lance
   - lkingland
   - matejvasek
diff --git a/client.go b/client.go
index 686bc6d8..55e95692 100644
--- a/client.go
+++ b/client.go
@@ -368,10 +368,10 @@ func WithRepository(uri string) Option {
 	}
 }
 
-// WithRegistry sets the default registry which is consulted when an image name/tag
-// is not explocitly provided.  Can be fully qualified, including the registry
-// (ex: 'quay.io/myname') or simply the namespace 'myname' which indicates the
-// the use of the default registry.
+// WithRegistry sets the default registry which is consulted when an image
+// name is not explicitly provided.  Can be fully qualified, including the
+// registry and namespace (ex: 'quay.io/myname') or simply the namespace
+// (ex: 'myname').
 func WithRegistry(registry string) Option {
 	return func(c *Client) {
 		c.registry = registry
@@ -410,6 +410,12 @@ func (c *Client) Instances() *Instances {
 	return c.instances
 }
 
+// Repository accessor returns the default registry for use when building
+// Functions which do not specify Registry or Image name explicitly.
+func (c *Client) Registry() string {
+	return c.registry
+}
+
 // Runtimes available in totality.
 // Not all repository/template combinations necessarily exist,
 // and further validation is performed when a template+runtime is chosen.
@@ -622,9 +628,19 @@ func (c *Client) Build(ctx context.Context, path string) (err error) {
 		return
 	}
 
-	// Derive Image from the path (precedence is given to extant config)
-	if f.Image, err = DerivedImage(path, c.registry); err != nil {
-		return
+	// Default function registry to the client's global registry
+	if f.Registry == "" {
+		f.Registry = c.registry
+	}
+
+	// If no image name has been yet defined (not yet built/deployed), calculate.
+	// Image name is stored on the function for later use by deploy, etc.
+	// TODO: write this to .func/build instead, and populate f.Image on deploy
+	// such that local builds do not dirty the work tree.
+	if f.Image == "" {
+		if f.Image, err = f.ImageName(); err != nil {
+			return
+		}
 	}
 
 	if err = c.builder.Build(ctx, f); err != nil {
diff --git a/cmd/build.go b/cmd/build.go
index 8950e673..25d13c93 100644
--- a/cmd/build.go
+++ b/cmd/build.go
@@ -76,45 +76,6 @@ and the image name is stored in the configuration file.
 	return cmd
 }
 
-// ValidateBuilder ensures that the given builder is one that the CLI
-// knows how to instantiate, returning a builkder.ErrUnknownBuilder otherwise.
-func ValidateBuilder(name string) (err error) {
-	for _, known := range KnownBuilders() {
-		if name == known {
-			return
-		}
-	}
-	return builders.ErrUnknownBuilder{Name: name, Known: KnownBuilders()}
-}
-
-func ValidNamespaceAndRegistry(path string) survey.Validator {
-	return func(val interface{}) error {
-
-		// if the value passed in is the zero value of the appropriate type
-		if len(val.(string)) == 0 {
-			return errors.New("Value is required")
-		}
-
-		_, err := fn.DerivedImage(path, val.(string)) //image can be derived without any error
-
-		if err != nil {
-			return fmt.Errorf("invalid registry [%q]: %w", val.(string), err)
-		}
-		return nil
-	}
-}
-
-// KnownBuilders are a typed string slice of builder short names which this
-// CLI understands.  Includes a customized String() representation intended
-// for use in flags and help text.
-func KnownBuilders() builders.Known {
-	// The set of builders supported by this CLI will likely always equate to
-	// the set of builders enumerated in the builders pacakage.
-	// However, future third-party integrations may support less than, or more
-	// builders, and certain environmental considerations may alter this list.
-	return builders.All()
-}
-
 func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err error) {
 	config, err := newBuildConfig().Prompt()
 	if err != nil {
@@ -124,63 +85,31 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro
 		return
 	}
 
-	function, err := functionWithOverrides(config.Path, functionOverrides{Image: config.Image})
+	// Load the Function at path, and if it is initialized, update it with
+	// pertinent values from the config.
+	f, err := fn.NewFunction(config.Path)
 	if err != nil {
 		return
 	}
-
-	// Check if the function has been initialized
-	if !function.Initialized() {
+	if !f.Initialized() {
 		return fmt.Errorf("'%v' does not contain an initialized function", config.Path)
 	}
-
-	// If a registry name was provided as a command line flag, it should be validated
 	if config.Registry != "" {
-		err = ValidNamespaceAndRegistry(config.Path)(config.Registry)
-		if err != nil {
-			return
-		}
-	} else if function.Registry != "" {
-		// Otherwise, if a registry name was provided in func.config, set that value
-		// for the command configuration and validate it
-		config.Registry = function.Registry
-		err = ValidNamespaceAndRegistry(config.Path)(config.Registry)
-		if err != nil {
-			return
-		}
+		f.Registry = config.Registry
 	}
-
-	// If the function does not yet have an image name and one was not provided on the command line
-	if function.Image == "" && config.Image == "" {
-		//  AND a --registry was not provided, then we need to
-		// prompt for a registry from which we can derive an image name.
-		if config.Registry == "" {
-			fmt.Println("A registry for function images is required. For example, 'docker.io/tigerteam'.")
-
-			err = survey.AskOne(
-				&survey.Input{Message: "Registry for function images:"},
-				&config.Registry, survey.WithValidator(ValidNamespaceAndRegistry(config.Path)))
-			if err != nil {
-				if errors.Is(err, terminal.InterruptErr) {
-					return nil
-				}
-				return
-			}
-			fmt.Println("Note: building a function the first time will take longer than subsequent builds")
-		}
-
-		// We have the registry, so let's use it to derive the function image name
-		config.Image = deriveImage(config.Image, config.Registry, config.Path)
-
-		function.Image = config.Image
+	if config.Image != "" {
+		f.Image = config.Image
+	}
+	if config.Builder != "" {
+		f.Builder = config.Builder
 	}
 
 	// Choose a builder based on the value of the --builder flag
 	var builder fn.Builder
-	if function.Builder == "" || cmd.Flags().Changed("builder") {
-		function.Builder = config.Builder
+	if f.Builder == "" || cmd.Flags().Changed("builder") {
+		f.Builder = config.Builder
 	} else {
-		config.Builder = function.Builder
+		config.Builder = f.Builder
 	}
 	if err = ValidateBuilder(config.Builder); err != nil {
 		return err
@@ -200,28 +129,38 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro
 			s2i.WithPlatform(config.Platform))
 	}
 
-	// All set, let's write changes in the config to the disk
-	err = function.Write()
-	if err != nil {
-		return
-	}
-
 	// Use the user-provided builder image, if supplied
 	if config.BuilderImage != "" {
-		function.BuilderImages[config.Builder] = config.BuilderImage
+		f.BuilderImages[config.Builder] = config.BuilderImage
 	}
 
-	// Create a client using the registry defined in config plus any additional
-	// options provided (such as mocks for testing)
 	client, done := newClient(ClientConfig{Verbose: config.Verbose},
 		fn.WithRegistry(config.Registry),
 		fn.WithBuilder(builder))
 	defer done()
 
-	err = client.Build(cmd.Context(), config.Path)
-	if err == nil && config.Push {
+	// Default Client Registry, Function Registry or explicit Image is required
+	if client.Registry() == "" && f.Registry == "" && f.Image == "" {
+		// It is not necessary that we validate here, since the client API has
+		// its own validation, but it does give us the opportunity to show a very
+		// cli-specific and detailed error message.
+		return ErrRegistryRequired
+	}
+
+	// This preemptive write call will be unnecessary when the API is updated
+	// to use Function instances rather than file paths. For now it must write
+	// even if the command fails later.  Not ideal.
+	if err = f.Write(); err != nil {
+		return
+	}
+
+	if err = client.Build(cmd.Context(), config.Path); err != nil {
+		return
+	}
+	if config.Push {
 		err = client.Push(cmd.Context(), config.Path)
 	}
+
 	return
 }
 
@@ -278,12 +217,11 @@ func newBuildConfig() buildConfig {
 // Skipped if not in an interactive terminal (non-TTY), or if --confirm false (agree to
 // all prompts) was set (default).
 func (c buildConfig) Prompt() (buildConfig, error) {
-	imageName := deriveImage(c.Image, c.Registry, c.Path)
 	if !interactiveTerminal() || !c.Confirm {
 		return c, nil
 	}
 
-	bc := buildConfig{Verbose: c.Verbose}
+	imageName := deriveImage(c.Image, c.Registry, c.Path)
 
 	var qs = []*survey.Question{
 		{
@@ -292,7 +230,6 @@ func (c buildConfig) Prompt() (buildConfig, error) {
 				Message: "Project path:",
 				Default: c.Path,
 			},
-			Validate: survey.Required,
 		},
 		{
 			Name: "image",
@@ -300,10 +237,28 @@ func (c buildConfig) Prompt() (buildConfig, error) {
 				Message: "Full image name (e.g. quay.io/boson/node-sample):",
 				Default: imageName,
 			},
-			Validate: survey.Required,
 		},
 	}
-	err := survey.Ask(qs, &bc)
+	err := survey.Ask(qs, &c)
+	if err != nil {
+		return c, err
+	}
 
-	return bc, err
+	// if the result of imageName is empty (unset, underivable),
+	// and the value of c.Image is empty (none provided explicitly by flag, env
+	// variable or prompt), then try one more time to get enough to to derive an
+	// image name: explicitly ask for registry.
+	if imageName == "" && c.Image == "" {
+		qs = []*survey.Question{
+			{
+				Name: "registry",
+				Prompt: &survey.Input{
+					Message: "Registry for function image:",
+					Default: c.Registry, // This should be innefectual
+				},
+			},
+		}
+	}
+	err = survey.Ask(qs, &c)
+	return c, err
 }
diff --git a/cmd/deploy.go b/cmd/deploy.go
index e05efbb8..44bc14d6 100644
--- a/cmd/deploy.go
+++ b/cmd/deploy.go
@@ -107,6 +107,8 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err
 	}
 
 	//if --image contains '@', validate image digest and disable build and push if not set, otherwise return an error
+	// TODO(lkingland): this logic could be assimilated into the Deploy Config
+	// struct and it's constructor.
 	imageSplit := strings.Split(config.Image, "@")
 	imageDigestProvided := false
 
@@ -117,23 +119,30 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err
 		imageDigestProvided = true
 	}
 
-	function, err := functionWithOverrides(config.Path, functionOverrides{Namespace: config.Namespace, Image: config.Image})
+	// 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(config.Path)
 	if err != nil {
 		return
 	}
-
-	// save image digest if provided in --image
-	if imageDigestProvided {
-		function.ImageDigest = imageSplit[1]
+	if !f.Initialized() {
+		return fmt.Errorf("'%v' does not contain an initialized function", config.Path)
+	}
+	if config.Registry != "" {
+		f.Registry = config.Registry
+	}
+	if config.Image != "" {
+		f.Image = config.Image
+	}
+	if config.Builder != "" {
+		f.Builder = config.Builder
 	}
 
-	// add ns to func.yaml on first deploy and warn if current context differs from func.yaml
-	function.Namespace, err = checkNamespaceDeploy(function.Namespace, config.Namespace)
+	f.Namespace, err = checkNamespaceDeploy(f.Namespace, config.Namespace)
 	if err != nil {
 		return
 	}
-
-	function.Envs, _, err = mergeEnvs(function.Envs, config.EnvToUpdate, config.EnvToRemove)
+	f.Envs, _, err = mergeEnvs(f.Envs, config.EnvToUpdate, config.EnvToRemove)
 	if err != nil {
 		return
 	}
@@ -147,43 +156,21 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err
 			return err
 		}
 	} else {
-		currentBuildType = function.BuildType
-	}
-
-	// Check if the function has been initialized
-	if !function.Initialized() {
-		return fmt.Errorf("the given path '%v' does not contain an initialized function. Please create one at this path before deploying", config.Path)
+		currentBuildType = f.BuildType
 	}
 
-	// If the function does not yet have an image name and one was not provided on the command line
-	if function.Image == "" && currentBuildType != "disabled" {
-		//  AND a --registry was not provided, then we need to
-		// prompt for a registry from which we can derive an image name.
-		if config.Registry == "" {
-			fmt.Println("A registry for function images is required. For example, 'docker.io/tigerteam'.")
-
-			err = survey.AskOne(
-				&survey.Input{Message: "Registry for function images:"},
-				&config.Registry, survey.WithValidator(survey.Required))
-			if err != nil {
-				if err == terminal.InterruptErr {
-					return nil
-				}
-				return
-			}
-		}
-
-		// We have the registry, so let's use it to derive the function image name
-		config.Image = deriveImage(config.Image, config.Registry, config.Path)
-		function.Image = config.Image
+	if imageDigestProvided {
+		// TODO(lkingland):  This could instead be part of the config, relying on
+		// zero values rather than a flag indicating "Image Digest was Provided"
+		f.ImageDigest = imageSplit[1] // save image digest if provided in --image
 	}
 
 	// Choose a builder based on the value of the --builder flag
 	var builder fn.Builder
-	if function.Builder == "" || cmd.Flags().Changed("builder") {
-		function.Builder = config.Builder
+	if f.Builder == "" || cmd.Flags().Changed("builder") {
+		f.Builder = config.Builder
 	} else {
-		config.Builder = function.Builder
+		config.Builder = f.Builder
 	}
 	if err = ValidateBuilder(config.Builder); err != nil {
 		return err
@@ -198,34 +185,28 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err
 		builder = s2i.NewBuilder(s2i.WithVerbose(config.Verbose), s2i.WithPlatform(config.Platform))
 	}
 
-	// All set, let's write changes in the config to the disk
-	err = function.Write()
-	if err != nil {
-		return
-	}
-
-	// Default config namespace is the function's namespace
-	if config.Namespace == "" {
-		config.Namespace = function.Namespace
-	}
-
-	// if registry was not changed via command line flag meaning it's empty
-	// keep the same registry by setting the config.registry to empty otherwise
-	// trust viper to override the env variable with the given flag if both are specified
-	if regFlag, _ := cmd.Flags().GetString("registry"); regFlag == "" {
-		config.Registry = ""
-	}
-
 	// Use the user-provided builder image, if supplied
 	if config.BuilderImage != "" {
-		function.BuilderImages[config.Builder] = config.BuilderImage
+		f.BuilderImages[config.Builder] = config.BuilderImage
 	}
 
-	client, done := newClient(ClientConfig{Namespace: config.Namespace, Verbose: config.Verbose},
+	client, done := newClient(ClientConfig{Namespace: f.Namespace, Verbose: config.Verbose},
 		fn.WithRegistry(config.Registry),
 		fn.WithBuilder(builder))
 	defer done()
 
+	// Default Client Registry, Function Registry or explicit Image required
+	if client.Registry() == "" && f.Registry == "" && f.Image == "" {
+		return ErrRegistryRequired
+	}
+
+	// NOTE: curently need to preemptively write out function state until
+	// the API is updated to use instances, at which point we will only
+	// write on success.
+	if err = f.Write(); err != nil {
+		return
+	}
+
 	switch currentBuildType {
 	case fn.BuildTypeLocal, "":
 		if config.GitURL != "" || config.GitDir != "" || config.GitBranch != "" {
@@ -235,7 +216,7 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err
 			return err
 		}
 	case fn.BuildTypeGit:
-		git := function.Git
+		git := f.Git
 
 		if config.GitURL != "" {
 			git.URL = &config.GitURL
@@ -270,6 +251,28 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err
 	return client.Deploy(cmd.Context(), config.Path)
 }
 
+// ValidateBuilder ensures that the given builder is one that the CLI
+// knows how to instantiate, returning a builkder.ErrUnknownBuilder otherwise.
+func ValidateBuilder(name string) (err error) {
+	for _, known := range KnownBuilders() {
+		if name == known {
+			return
+		}
+	}
+	return builders.ErrUnknownBuilder{Name: name, Known: KnownBuilders()}
+}
+
+// KnownBuilders are a typed string slice of builder short names which this
+// CLI understands.  Includes a customized String() representation intended
+// for use in flags and help text.
+func KnownBuilders() builders.Known {
+	// The set of builders supported by this CLI will likely always equate to
+	// the set of builders enumerated in the builders pacakage.
+	// However, future third-party integrations may support less than, or more
+	// builders, and certain environmental considerations may alter this list.
+	return builders.All()
+}
+
 func newPromptForCredentials(in io.Reader, out, errOut io.Writer) func(registry string) (docker.Credentials, error) {
 	firstTime := true
 	return func(registry string) (docker.Credentials, error) {
@@ -399,23 +402,9 @@ type deployConfig struct {
 	// (~/.kube/config) in the case of Kubernetes.
 	Namespace string
 
-	// Path of the function implementation on local disk. Defaults to current
-	// working directory of the process.
-	Path string
-
-	// Verbose logging.
-	Verbose bool
-
-	// Confirm: confirm values arrived upon from environment plus flags plus defaults,
-	// with interactive prompting (only applicable when attached to a TTY).
-	Confirm bool
-
 	// Build the associated function before deploying.
 	BuildType string
 
-	// Push function image to the registry before deploying.
-	Push bool
-
 	// Envs passed via cmd to be added/updated
 	EnvToUpdate *util.OrderedMap
 
@@ -450,11 +439,7 @@ func newDeployConfig(cmd *cobra.Command) (deployConfig, error) {
 	return deployConfig{
 		buildConfig: newBuildConfig(),
 		Namespace:   viper.GetString("namespace"),
-		Path:        getPathFlag(),
-		Verbose:     viper.GetBool("verbose"), // defined on root
-		Confirm:     viper.GetBool("confirm"),
 		BuildType:   buildType,
-		Push:        viper.GetBool("push"),
 		EnvToUpdate: envToUpdate,
 		EnvToRemove: envToRemove,
 		GitURL:      viper.GetString("git-url"),
@@ -472,52 +457,47 @@ func (c deployConfig) Prompt() (deployConfig, error) {
 	}
 
 	var qs = []*survey.Question{
+		{
+			Name: "path",
+			Prompt: &survey.Input{
+				Message: "Project path:",
+				Default: c.Path,
+			},
+		},
 		{
 			Name: "registry",
 			Prompt: &survey.Input{
 				Message: "Registry for function images:",
-				Default: c.buildConfig.Registry,
+				Default: c.Registry,
 			},
-			Validate: survey.Required,
 		},
+	}
+	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: "namespace",
+			Name: "image",
 			Prompt: &survey.Input{
-				Message: "Namespace:",
-				Default: c.Namespace,
+				Message: "Full image name (e.g. quay.io/boson/node-sample):",
+				Default: imageName,
 			},
 		},
 		{
-			Name: "path",
+			Name: "namespace",
 			Prompt: &survey.Input{
-				Message: "Project path:",
-				Default: c.Path,
+				Message: "Namespace into which the function is (re)deployed",
+				Default: c.Namespace,
 			},
-			Validate: survey.Required,
 		},
 	}
-	answers := struct {
-		Registry  string
-		Namespace string
-		Path      string
-	}{}
-	err := survey.Ask(qs, &answers)
-	if err != nil {
-		return deployConfig{}, err
-	}
+	err := survey.Ask(qs, &c)
 
-	dc := deployConfig{
-		buildConfig: buildConfig{
-			Registry: answers.Registry,
-		},
-		Namespace: answers.Namespace,
-		Path:      answers.Path,
-		Verbose:   c.Verbose,
-	}
-
-	dc.Image = deriveImage(dc.Image, dc.Registry, dc.Path)
-
-	return dc, nil
+	return c, err
 }
 
 // ErrInvalidBuildType indicates that the passed build type was invalid.
@@ -563,20 +543,39 @@ func parseImageDigest(imageSplit []string, config deployConfig, cmd *cobra.Comma
 // checkNamespaceDeploy checks current namespace against func.yaml and warns if its different
 // or sets namespace to be written in func.yaml if its the first deployment
 func checkNamespaceDeploy(funcNamespace string, confNamespace string) (string, error) {
-	currNamespace, err := k8s.GetNamespace("")
-	if err != nil {
-		return funcNamespace, err
+	var namespace string
+	var err error
+
+	// Choose target namespace
+	if confNamespace != "" {
+		namespace = confNamespace // --namespace takes precidence
+	} else if funcNamespace != "" {
+		namespace = funcNamespace // value from previous deployment (func.yaml) 2nd priority
+	} else {
+		// Try to derive a default from the current k8s context, if any.
+		namespace, err = k8s.GetNamespace("")
+		if err != nil {
+			return "", nil // configured k8s environment not required
+		}
 	}
 
-	// If ns exists in func.yaml & NOT given via CLI (--namespace flag) & current ns does NOT match func.yaml ns
-	if funcNamespace != "" && confNamespace == "" && (currNamespace != funcNamespace) {
-		fmt.Fprintf(os.Stderr, "Warning: Current namespace '%s' does not match namespace '%s' in func.yaml. Function is deployed at '%s' namespace\n", currNamespace, funcNamespace, funcNamespace)
+	currNamespace, err := k8s.GetNamespace("")
+	if err == nil && namespace != currNamespace {
+		fmt.Fprintf(os.Stderr, "Warning: Current namespace '%s' does not match function which is deployed at '%s' namespace\n", currNamespace, funcNamespace)
 	}
 
-	// Add current namespace to func.yaml if it is NOT set yet & NOT given via --namespace.
-	if funcNamespace == "" {
-		funcNamespace = currNamespace
+	if funcNamespace != "" && namespace != funcNamespace {
+		fmt.Fprintf(os.Stderr, "Warning: New namespace '%s' does not match current namespace '%s'\n", namespace, funcNamespace)
 	}
 
-	return funcNamespace, nil
+	return namespace, nil
 }
+
+var ErrRegistryRequired = errors.New(`A container registry is required.  For example:
+--registry docker.io/myusername
+
+For more advanced usage, it is also possible to specify the exact image to use. For example:
+
+--image docker.io/myusername/myfunc:latest
+
+To run the command in an interactive mode, use --confirm (-c)`)
diff --git a/cmd/root.go b/cmd/root.go
index b45b1594..102d688f 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -160,38 +160,6 @@ func bindEnv(flags ...string) bindFunc {
 	}
 }
 
-type functionOverrides struct {
-	Image     string
-	Namespace string
-}
-
-// functionWithOverrides sets the namespace and image strings for the
-// function project at root, if provided, and returns the function
-// configuration values.
-// Please note that When this function is called, the overrides are not persisted.
-func functionWithOverrides(root string, overrides functionOverrides) (f fn.Function, err error) {
-	f, err = fn.NewFunction(root)
-	if err != nil {
-		return
-	}
-
-	overrideMapping := []struct {
-		src  string
-		dest *string
-	}{
-		{overrides.Image, &f.Image},
-		{overrides.Namespace, &f.Namespace},
-	}
-
-	for _, m := range overrideMapping {
-		if m.src != "" {
-			*m.dest = m.src
-		}
-	}
-
-	return
-}
-
 // deriveName returns the explicit value (if provided) or attempts to derive
 // from the given path.  Path is defaulted to current working directory, where
 // a function configuration, if it exists and contains a name, is used.
@@ -232,9 +200,9 @@ func deriveNameAndAbsolutePathFromPath(path string) (string, string) {
 	return pathParts[len(pathParts)-1], absPath
 }
 
-// deriveImage returns the same image name which will be used if no explicit
-// image is provided.  I.e. derived from the configured registry (registry
-// plus username) and the function's name.
+// deriveImage returns the same image name which will be used.
+// I.e. if the explicit name is empty, derive one from the configured registry
+// (registry plus username) and the function's name.
 //
 // This is calculated preemptively here in the CLI (prior to invoking the
 // client), only in order to provide information to the user via the prompt.
@@ -265,8 +233,10 @@ func deriveImage(explicitImage, defaultRegistry, path string) string {
 	if f.Image != "" {
 		return f.Image // use value previously provided or derived.
 	}
-	derivedValue, _ := fn.DerivedImage(path, defaultRegistry)
-	return derivedValue // Use the func system's derivation logic.
+	// Use the func system's derivation logic.
+	// Errors deriving result in an empty return
+	derivedValue, _ := f.ImageName()
+	return derivedValue
 }
 
 func envFromCmd(cmd *cobra.Command) (*util.OrderedMap, []string, error) {
diff --git a/cmd/run.go b/cmd/run.go
index 55669c46..0847e38d 100644
--- a/cmd/run.go
+++ b/cmd/run.go
@@ -70,10 +70,15 @@ func runRun(cmd *cobra.Command, args []string, newClient ClientFactory) (err err
 		return
 	}
 
+	// Load the Function, and if it exists (path initialized as a Function), merge
+	// in any env vars from the context and save.
 	function, err := fn.NewFunction(config.Path)
 	if err != nil {
 		return
 	}
+	if !function.Initialized() {
+		return fmt.Errorf("the given path '%v' does not contain an initialized function", config.Path)
+	}
 
 	var updated int
 	function.Envs, updated, err = mergeEnvs(function.Envs, config.EnvToUpdate, config.EnvToRemove)
@@ -87,11 +92,6 @@ func runRun(cmd *cobra.Command, args []string, newClient ClientFactory) (err err
 		}
 	}
 
-	// Check if the function has been initialized
-	if !function.Initialized() {
-		return fmt.Errorf("the given path '%v' does not contain an initialized function", config.Path)
-	}
-
 	// Client for use running (and potentially building), using the config
 	// gathered plus any additional option overrieds (such as for providing
 	// mocks when testing for builder and runner)
diff --git a/function.go b/function.go
index 802fa8eb..c1558e96 100644
--- a/function.go
+++ b/function.go
@@ -344,73 +344,6 @@ func (f Function) ImageWithDigest() string {
 	return part1 + strings.Split(part2, ":")[0] + "@" + f.ImageDigest
 }
 
-// DerivedImage returns the derived image name (OCI container tag) of the
-// function whose source is at root, with the default registry for when
-// the image has to be calculated (derived).
-// The following are equivalent due to the use of DefaultRegistry:
-// registry:  docker.io/myname
-//            myname
-// A full image name consists of registry, image name and tag.
-// in form [registry]/[image-name]:[tag]
-// example docker.io/alice/my.example.func:latest
-// Default if not provided is --registry (a required global setting)
-// followed by the provided (or derived) image name.
-// TODO: this calculated field should probably be generated on instantiation
-// to avoid confusion.
-func DerivedImage(root, registry string) (image string, err error) {
-	f, err := NewFunction(root)
-	if err != nil {
-		// an inability to load the function means it is not yet initialized
-		// We could try to be smart here and fall through to the function name
-		// deriviation logic, but that's likely to be confusing.  Better to
-		// stay simple and say that derivation of Image depends on first having
-		// the function initialized.
-		return
-	}
-
-	// If the function has already had image populated
-	// and a new registry hasn't been provided, use this pre-calculated value.
-	if f.Image != "" && f.Registry == registry {
-		image = f.Image
-		return
-	}
-
-	// registry is currently required until such time as we support
-	// pushing to an implicitly-available in-cluster registry by default.
-	if registry == "" {
-		err = errors.New("registry name is required")
-		return
-	}
-
-	// If the function loaded, and there is not yet an Image set, then this is
-	// the first build and no explicit image override was specified.  We should
-	// therefore derive the image tag from the defined registry and name.
-	// form:    [registry]/[user]/[function]:latest
-	// example: quay.io/alice/my.function.name:latest
-	// Also nested namespaces should be supported:
-	// form:    [registry]/[parent]/[user]/[function]:latest
-	// example: quay.io/project/alice/my.function.name:latest
-	registry = strings.Trim(registry, "/") // too defensive?
-	registryTokens := strings.Split(registry, "/")
-	if len(registryTokens) == 1 {
-		//namespace provided only 'alice'
-		image = DefaultRegistry + "/" + registry + "/" + f.Name
-	} else if len(registryTokens) == 2 || len(registryTokens) == 3 {
-		// registry/namespace provided `quay.io/alice` or registry/parent-namespace/namespace provided `quay.io/project/alice`
-		image = registry + "/" + f.Name
-	} else if len(registryTokens) > 3 { // the name of the image is also provided `quay.io/alice/my.function.name`
-		err = fmt.Errorf("registry should be either 'namespace', 'registry/namespace' or 'registry/parent/namespace', the name of the image will be derived from the function name.")
-		return
-	}
-
-	// Explicitly append :latest.  We currently expect source control to drive
-	// versioning, rather than rely on Docker Hub tags with explicit version
-	// numbers, as is seen in many serverless solutions.  This will be updated
-	// to branch name when we add source-driven canary/ bluegreen deployments.
-	image = image + ":latest"
-	return
-}
-
 // assertEmptyRoot ensures that the directory is empty enough to be used for
 // initializing a new function.
 func assertEmptyRoot(path string) (err error) {
@@ -433,6 +366,50 @@ func assertEmptyRoot(path string) (err error) {
 	return
 }
 
+// ImageName returns a full image name (OCI container tag) for the
+// Function based off of the Function's `Registry` member plus `Name`.
+// Used to calculate the final value for .Image when none is provided
+// explicitly.
+//
+// form:    [registry]/[user]/[function]:latest
+// example: quay.io/alice/my.function.name:latest
+//
+// Also nested namespaces should be supported:
+// form:    [registry]/[parent]/[user]/[function]:latest
+// example: quay.io/project/alice/my.function.name:latest
+//
+// Registry values which only contain a single token are presumed to
+// indicate the namespace at the default registry.
+func (f Function) ImageName() (image string, err error) {
+	if f.Registry == "" {
+		return "", errors.New("registry is required")
+	}
+	if f.Name == "" {
+		return "", errors.New("name is required")
+	}
+
+	f.Registry = strings.Trim(f.Registry, "/") // too defensive?
+
+	registryTokens := strings.Split(f.Registry, "/")
+	if len(registryTokens) == 1 { // only namespace provided: ex. 'alice'
+		image = DefaultRegistry + "/" + f.Registry + "/" + f.Name
+	} else if len(registryTokens) == 2 || len(registryTokens) == 3 {
+		// registry/namespace ('quay.io/alice') or
+		// registry/parent-namespace/namespace ('quay.io/project/alice') provided
+		image = f.Registry + "/" + f.Name
+	} else if len(registryTokens) > 3 { // the name of the image is also provided `quay.io/alice/my.function.name`
+		return "", fmt.Errorf("registry should be either 'namespace', 'registry/namespace' or 'registry/parent/namespace', the name of the image will be derived from the function name.")
+	}
+
+	// Explicitly append :latest tag.  We expect source control to drive
+	// versioning, rather than rely on image tags with explicitly pinned version
+	// numbers, as is seen in many serverless solutions.  This will be updated
+	// to branch name when we add source-driven canary/ bluegreen deployments.
+
+	// For pinning to an exact container image, see ImageWithDigest
+	return image + ":latest", nil
+}
+
 // contentiousFiles are files which, if extant, preclude the creation of a
 // function rooted in the given directory.
 var contentiousFiles = []string{


Test patch:
diff --git a/client_test.go b/client_test.go
index 26566cea..f61ac95e 100644
--- a/client_test.go
+++ b/client_test.go
@@ -365,10 +365,9 @@ func TestClient_New_RegistryRequired(t *testing.T) {
 	}
 }
 
-// TestClient_New_ImageNameDerived ensures that the full image (tag) of the resultant OCI
-// container is populated based of a derivation using configured registry
-// plus the service name.
-func TestClient_New_ImageNameDerived(t *testing.T) {
+// TestClient_New_ImageNamePopulated ensures that the full image (tag) of the
+// resultant OCI container is populated.
+func TestClient_New_ImageNamePopulated(t *testing.T) {
 	// Create the root function directory
 	root := "testdata/example.com/testDeriveImage"
 	defer Using(t, root)()
@@ -385,10 +384,16 @@ func TestClient_New_ImageNameDerived(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// In form: [Default Registry]/[Registry Namespace]/[Service Name]:latest
-	expected := TestRegistry + "/" + f.Name + ":latest"
-	if f.Image != expected {
-		t.Fatalf("expected image '%v' got '%v'", expected, f.Image)
+	// This opaque-box unit test ensures NewImageTag is invoked and applied.
+	// See the Test_NewImageTag clear-box unit test for an in-depth exploration of
+	// how the values of image and registry are treated to create, by default:
+	//   [Default Registry]/[Registry Namespace]/[Service Name]:latest
+	imageTag, err := f.ImageName()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if f.Image != imageTag {
+		t.Fatalf("expected image '%v' got '%v'", imageTag, f.Image)
 	}
 }
 
@@ -680,6 +685,71 @@ func TestClient_Update(t *testing.T) {
 	}
 }
 
+// TestClient_Deploy_RegistryUpdate ensures that deploying a Function updates
+// its image member on initial deploy, and on subsequent deploys only
+// if reset to it zero value.
+func TestClient_Deploy_RegistryUpdate(t *testing.T) {
+	root, rm := Mktemp(t)
+	defer rm()
+	client := fn.New(fn.WithRegistry("example.com/alice"))
+
+	// New runs build and deploy, thus the initial instantiation should result in
+	// the member being populated from the client's registry and function name.
+	if err := client.New(context.Background(), fn.Function{Runtime: "go", Name: "f", Root: root}); err != nil {
+		t.Fatal(err)
+	}
+	f, err := fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if f.Image != "example.com/alice/f:latest" {
+		t.Error("image name was not initially set")
+	}
+
+	// Updating the registry and performing a subsequent update should not result
+	// in the image member being updated to the new value: registry is only used
+	// when calculating a nonexistent value
+	f.Registry = "example.com/bob"
+	if err := f.Write(); err != nil {
+		t.Fatal(err)
+	}
+	if err := client.Build(context.Background(), root); err != nil {
+		t.Fatal(err)
+	}
+	if err := client.Deploy(context.Background(), root); err != nil {
+		t.Fatal(err)
+	}
+	expected := "example.com/alice/f:latest"
+	f, err = fn.NewFunction(root) // reload and check
+	if err != nil {
+		t.Fatal(err)
+	}
+	if f.Image != expected { // NOT changed to bob
+		t.Errorf("expected image name to stay '%v' and not be updated, but got '%v'", expected, f.Image)
+	}
+
+	// Reset the value of .Image to default "" and ensure this triggers recalc.
+	f.Image = ""
+	f.Registry = "example.com/bob"
+	if err := f.Write(); err != nil {
+		t.Fatal(err)
+	}
+	if err := client.Build(context.Background(), root); err != nil {
+		t.Fatal(err)
+	}
+	if err := client.Deploy(context.Background(), root); err != nil {
+		t.Fatal(err)
+	}
+	expected = "example.com/bob/f:latest"
+	f, err = fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if f.Image != expected { // DOES change to bob
+		t.Errorf("expected image name to stay '%v' and not be updated, but got '%v'", expected, f.Image)
+	}
+}
+
 // TestClient_Remove_ByPath ensures that the remover is invoked to remove
 // the function with the name of the function at the provided root.
 func TestClient_Remove_ByPath(t *testing.T) {
@@ -906,6 +976,83 @@ func TestClient_List_OutsideRoot(t *testing.T) {
 	}
 }
 
+// TestClient_Deploy_Image ensures that initilally the function's image
+// member has no value (not initially deployed); the value is populated
+// upon deployment with a value derived from the function's name and currently
+// effective client registry; that the value of f.Image will take precidence
+// over .Registry, which is used to calculate a default value for image.
+func TestClient_Deploy_Image(t *testing.T) {
+	root, rm := Mktemp(t)
+	defer rm()
+
+	client := fn.New(
+		fn.WithBuilder(mock.NewBuilder()),
+		fn.WithDeployer(mock.NewDeployer()),
+		fn.WithRegistry("example.com/alice"))
+
+	err := client.Create(fn.Function{Name: "myfunc", Runtime: "go", Root: root})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	f, err := fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Upon initial creation, the value of .Image is empty
+	if f.Image != "" {
+		t.Fatalf("new function should have no image, got '%v'", f.Image)
+	}
+
+	// Upon deployment, the function should be populated;
+	if err = client.Build(context.Background(), root); err != nil {
+		t.Fatal(err)
+	}
+	if err = client.Deploy(context.Background(), root); err != nil {
+		t.Fatal(err)
+	}
+	f, err = fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	expected := "example.com/alice/myfunc:latest"
+	if f.Image != expected {
+		t.Fatalf("expected image '%v', got '%v'", expected, f.Image)
+	}
+	expected = "example.com/alice"
+	if f.Registry != "example.com/alice" {
+		t.Fatalf("expected registry '%v', got '%v'", expected, f.Registry)
+	}
+
+	// The value of .Image always takes precidence
+	f.Image = "registry2.example.com/bob/myfunc:latest"
+	if err = f.Write(); err != nil {
+		t.Fatal(err)
+	}
+	if err = client.Build(context.Background(), root); err != nil {
+		t.Fatal(err)
+	}
+	if err = client.Deploy(context.Background(), root); err != nil {
+		t.Fatal(err)
+	}
+	expected = "registry2.example.com/bob/myfunc:latest"
+	if f.Image != expected {
+		t.Fatalf("expected image '%v', got '%v'", expected, f.Image)
+	}
+	expected = "example.com/alice"
+	if f.Registry != "example.com/alice" {
+		// Note that according to current logic, the function's defined registry
+		// may be inaccurate.  Consider an initial deploy to registryA, followed by
+		// an explicit mutaiton of the function's .Image member.
+		// This could either remain as a documented nuance:
+		//   'The value of f.Registry is only used in the event an image name
+		//    need be derived (f.Image =="")
+		// Or we could update .Registry to always be in sync by parsing the .Image
+		t.Fatalf("expected registry '%v', got '%v'", expected, f.Registry)
+	}
+}
+
 // TestClient_Deploy_UnbuiltErrors ensures that a call to deploy a function which was not
 // fully created (ie. was only initialized, not actually built and deploys)
 // yields an expected, and informative, error.
diff --git a/cmd/build_test.go b/cmd/build_test.go
index 78af8fe9..ee6a53be 100644
--- a/cmd/build_test.go
+++ b/cmd/build_test.go
@@ -1,15 +1,11 @@
 package cmd
 
 import (
-	"context"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"testing"
 
-	"github.com/spf13/cobra"
 	fn "knative.dev/kn-plugin-func"
-	"knative.dev/kn-plugin-func/builders"
 	"knative.dev/kn-plugin-func/mock"
 	. "knative.dev/kn-plugin-func/testing"
 )
@@ -50,79 +46,41 @@ func TestBuild_ImageFlag(t *testing.T) {
 	}
 }
 
-// TestBuild_InvalidRegistry ensures that running build specifying the name of the
-// registry explicitly as an argument invokes the registry validation code.
-func TestBuild_InvalidRegistry(t *testing.T) {
-	var (
-		args    = []string{"--registry", "foo/bar/foobar/boofar"} // provide an invalid registry name
-		builder = mock.NewBuilder()                               // with a mock builder
-	)
-
-	// Run this test in a temporary directory
-	defer Fromtemp(t)()
-	// Write a func.yaml config which does not specify an image
-	funcYaml := `name: testymctestface
-namespace: ""
-runtime: go
-image: ""
-imageDigest: ""
-builder: quay.io/boson/faas-go-builder
-builders:
-  default: quay.io/boson/faas-go-builder
-envs: []
-annotations: {}
-labels: []
-created: 2021-01-01T00:00:00+00:00
-`
-	if err := ioutil.WriteFile("func.yaml", []byte(funcYaml), 0600); err != nil {
-		t.Fatal(err)
-	}
-
-	// Create build command that will use a mock builder.
-	cmd := NewBuildCmd(NewClientFactory(func() *fn.Client {
-		return fn.New(fn.WithBuilder(builder))
-	}))
+// TestBuild_RegistryOrImageRequired ensures that when no registry or image are
+// provided, and the client has not been instantiated with a default registry,
+// an ErrRegistryRequired is received.
+func TestBuild_RegistryOrImageRequired(t *testing.T) {
+	testRegistryOrImageRequired(NewBuildCmd, t)
+}
 
-	// Execute the command
-	cmd.SetArgs(args)
-	err := cmd.Execute()
-	if err == nil {
-		t.Fatal("Expected error")
-	}
+// TestBuild_ImageAndRegistry
+func TestBuild_ImageAndRegistry(t *testing.T) {
+	testRegistryOrImageRequired(NewBuildCmd, t)
 }
 
-// TestBuild_registryConfigurationInYaml tests that a build will execute successfully
-// when there is no registry provided on the command line, but one exists in func.yaml
-func TestBuild_registryConfigurationInYaml(t *testing.T) {
-	var (
-		builder = mock.NewBuilder() // with a mock builder
-	)
+// TestBuild_InvalidRegistry ensures that providing an invalid resitry
+// fails with the expected error.
+func TestBuild_InvalidRegistry(t *testing.T) {
+	testInvalidRegistry(NewBuildCmd, t)
+}
 
-	// Run this test in a temporary directory
-	defer Fromtemp(t)()
-	// Write a func.yaml config which does not specify an image
-	// but does specify a registry
-	funcYaml := `name: registrytest
-namespace: ""
-runtime: go
-image: ""
-registry: quay.io/boson/foo
-created: 2021-01-01T00:00:00+00:00
-`
-	if err := ioutil.WriteFile("func.yaml", []byte(funcYaml), 0600); err != nil {
-		t.Fatal(err)
-	}
+// TestBuild_RegistryLoads ensures that a function with a defined registry
+// will use this when recalculating .Image on build when no --image is
+// explicitly provided.
+func TestBuild_RegistryLoads(t *testing.T) {
+	testRegistryLoads(NewBuildCmd, t)
+}
 
-	// Create build command that will use a mock builder.
-	cmd := NewBuildCmd(NewClientFactory(func() *fn.Client {
-		return fn.New(fn.WithBuilder(builder), fn.WithRegistry("quay.io/boson/foo"))
-	}))
+// TestBuild_BuilderPersists ensures that the builder chosen is read from
+// the function by default, and is able to be overridden by flags/env vars.
+func TestBuild_BuilderPersists(t *testing.T) {
+	testBuilderPersists(NewBuildCmd, t)
+}
 
-	// Execute the command
-	err := cmd.Execute()
-	if err != nil {
-		t.Fatal(err)
-	}
+// TestBuild_ValidateBuilder ensures that the validation function correctly
+// identifies valid and invalid builder short names.
+func TestBuild_BuilderValidated(t *testing.T) {
+	testBuilderValidated(NewBuildCmd, t)
 }
 
 func TestBuild_runBuild(t *testing.T) {
@@ -223,119 +181,3 @@ created: 2009-11-10 23:00:00`,
 		})
 	}
 }
-
-func testBuilderPersistence(t *testing.T, testRegistry string, cmdBuilder func(ClientFactory) *cobra.Command) {
-	//add this to work with all other tests in deploy_test.go
-	defer WithEnvVar(t, "KUBECONFIG", fmt.Sprintf("%s/testdata/kubeconfig_deploy_namespace", cwd()))()
-
-	root, rm := Mktemp(t)
-	defer rm()
-
-	client := fn.New(fn.WithRegistry(testRegistry))
-
-	f := fn.Function{Runtime: "go", Root: root, Name: "myfunc", Registry: testRegistry}
-
-	if err := client.New(context.Background(), f); err != nil {
-		t.Fatal(err)
-	}
-
-	cmd := cmdBuilder(NewClientFactory(func() *fn.Client {
-		return client
-	}))
-
-	cmd.SetArgs([]string{"--registry", testRegistry})
-	if err := cmd.Execute(); err != nil {
-		t.Fatal(err)
-	}
-
-	var err error
-	// Assert the function has persisted a value of builder (has a default)
-	if f, err = fn.NewFunction(root); err != nil {
-		t.Fatal(err)
-	}
-	if f.Builder == "" {
-		t.Fatal("value of builder not persisted using a flag default")
-	}
-
-	// Build the function, specifying a Builder
-	cmd.SetArgs([]string{"--builder=s2i"})
-	if err := cmd.Execute(); err != nil {
-		t.Fatal(err)
-	}
-	// Assert the function has persisted the value of builder
-	if f, err = fn.NewFunction(root); err != nil {
-		t.Fatal(err)
-	}
-	if f.Builder != builders.S2I {
-		t.Fatal("value of builder flag not persisted when provided")
-	}
-	// Build the function without specifying a Builder
-	cmd = cmdBuilder(NewClientFactory(func() *fn.Client {
-		return client
-	}))
-	cmd.SetArgs([]string{"--registry", testRegistry})
-
-	if err := cmd.Execute(); err != nil {
-		t.Fatal(err)
-	}
-
-	// Assert the function has retained its original value
-	if f, err = fn.NewFunction(root); err != nil {
-		t.Fatal(err)
-	}
-
-	if f.Builder != builders.S2I {
-		t.Fatal("value of builder updated when not provided")
-	}
-
-	// Build the function again using a different builder
-	cmd.SetArgs([]string{"--builder=pack"})
-	if err := cmd.Execute(); err != nil {
-		t.Fatal(err)
-	}
-
-	// Assert the function has persisted the new value
-	if f, err = fn.NewFunction(root); err != nil {
-		t.Fatal(err)
-	}
-	if f.Builder != builders.Pack {
-		t.Fatal("value of builder flag not persisted on subsequent build")
-	}
-
-	// Build the function, specifying a platform with "pack" Builder
-	cmd.SetArgs([]string{"--platform", "linux"})
-	if err := cmd.Execute(); err == nil {
-		t.Fatal("Expected error")
-	}
-
-	// Set an invalid builder
-	cmd.SetArgs([]string{"--builder", "invalid"})
-	if err := cmd.Execute(); err == nil {
-		t.Fatal("Expected error")
-	}
-}
-
-// TestBuild_BuilderPersistence ensures that the builder chosen is read from
-// the function by default, and is able to be overridden by flags/env vars.
-func TestBuild_BuilderPersistence(t *testing.T) {
-	testBuilderPersistence(t, "docker.io/tigerteam", NewBuildCmd)
-}
-
-// TestBuild_ValidateBuilder ensures that the validation function correctly
-// identifies valid and invalid builder short names.
-func Test_ValidateBuilder(t *testing.T) {
-	for _, name := range builders.All() {
-		if err := ValidateBuilder(name); err != nil {
-			t.Fatalf("expected builder '%v' to be valid, but got error: %v", name, err)
-		}
-	}
-
-	// This CLI creates no builders beyond those in the core reposiory.  Other
-	// users of the client library may provide their own named implementation of
-	// the fn.Builder interface. Those would have a different set of valid
-	// builders.
-
-	if err := ValidateBuilder("invalid"); err == nil {
-		t.Fatalf("did not get expected error validating an invalid builder name")
-	}
-}
diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go
index 996fb346..a4c01cb7 100644
--- a/cmd/deploy_test.go
+++ b/cmd/deploy_test.go
@@ -2,17 +2,350 @@ package cmd
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"testing"
 
 	"github.com/ory/viper"
+	"github.com/spf13/cobra"
 	"k8s.io/utils/pointer"
 	fn "knative.dev/kn-plugin-func"
+	"knative.dev/kn-plugin-func/builders"
 	"knative.dev/kn-plugin-func/mock"
 	. "knative.dev/kn-plugin-func/testing"
 )
 
+const TestRegistry = "example.com/alice"
+
+type commandConstructor func(ClientFactory) *cobra.Command
+
+// TestDeploy_RegistryOrImageRequired ensures that when no registry or image are
+// provided (or exist on the function already), and the client has not been
+// instantiated with a default registry, an ErrRegistryRequired is received.
+func TestDeploy_RegistryOrImageRequired(t *testing.T) {
+	testRegistryOrImageRequired(NewDeployCmd, t) // shared with build
+}
+
+func testRegistryOrImageRequired(cmdFn commandConstructor, t *testing.T) {
+	t.Helper()
+	root, rm := Mktemp(t)
+	defer rm()
+
+	if err := fn.New().Create(fn.Function{Runtime: "go", Root: root}); err != nil {
+		t.Fatal(err)
+	}
+
+	cmd := cmdFn(NewClientFactory(func() *fn.Client {
+		return fn.New()
+	}))
+
+	// If neither --registry nor --image are provided, and the client was not
+	// instantiated with a default registry, a ErrRegistryRequired is expected.
+	cmd.SetArgs([]string{}) // this explicit clearing of args may not be necessary
+	if err := cmd.Execute(); err != nil {
+		if !errors.Is(err, ErrRegistryRequired) {
+			t.Fatalf("expected ErrRegistryRequired, got error: %v", err)
+		}
+	}
+
+	// earlire test covers the --registry only case, test here that --image
+	// also succeeds.
+	cmd.SetArgs([]string{"--image=example.com/alice/myfunc"})
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+}
+
+// TestBuild_ImageAndRegistry ensures that image is derived when --registry
+// is provided without --image; that --image is used if provided; that when
+// both are provided, they are both passed to the deployer; and that these
+// values are persisted.
+func TestDeploy_ImageAndRegistry(t *testing.T) {
+	testImageAndRegistry(NewDeployCmd, t)
+}
+
+func testImageAndRegistry(cmdFn commandConstructor, t *testing.T) {
+	t.Helper()
+	root, rm := Mktemp(t)
+	defer rm()
+
+	if err := fn.New().Create(fn.Function{Runtime: "go", Root: root}); err != nil {
+		t.Fatal(err)
+	}
+
+	var (
+		builder  = mock.NewBuilder()
+		deployer = mock.NewDeployer()
+		cmd      = cmdFn(NewClientFactory(func() *fn.Client {
+			return fn.New(
+				fn.WithBuilder(builder),
+				fn.WithDeployer(deployer),
+				fn.WithRegistry(TestRegistry))
+		}))
+	)
+
+	// If only --registry is provided:
+	// the resultant Function should have the registry populated and image
+	// derived from the name.
+	cmd.SetArgs([]string{"--registry=example.com/alice"})
+	deployer.DeployFn = func(f fn.Function) error {
+		if f.Registry != "example.com/alice" {
+			t.Fatal("registry flag not provided to deployer")
+		}
+		return nil
+	}
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+
+	// If only --image is provided:
+	// the deploy should not fail, and the resultant Function should have the
+	// Image member set to what was explicitly provided via the --image flag
+	// (not a derived name)
+	cmd.SetArgs([]string{"--image=example.com/alice/myfunc"})
+	deployer.DeployFn = func(f fn.Function) error {
+		if f.Image != "example.com/alice/myfunc" {
+			t.Fatalf("deployer expected f.Image 'example.com/alice/myfunc', got '%v'", f.Image)
+		}
+		return nil
+	}
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+
+	// If both --registry and --image are provided:
+	// they should both be plumbed through such that downstream agents (deployer
+	// in this case) see them set on the Function and can act accordingly.
+	cmd.SetArgs([]string{"--registry=example.com/alice", "--image=example.com/alice/subnamespace/myfunc"})
+	deployer.DeployFn = func(f fn.Function) error {
+		if f.Registry != "example.com/alice" {
+			t.Fatal("registry flag value not seen on the Function by the deployer")
+		}
+		if f.Image != "example.com/alice/subnamespace/myfunc" {
+			t.Fatal("image flag value not seen on the Function by deployer")
+		}
+		return nil
+	}
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+
+	// TODO it may be cleaner to error if both registry and image are provided,
+	// allowing deployer implementations to avoid arbitration logic.
+}
+
+// TestDepoy_InvalidRegistry ensures that providing an invalid registry
+// fails with the expected error.
+func TestDeploy_InvalidRegistry(t *testing.T) {
+	testInvalidRegistry(NewDeployCmd, t)
+}
+
+func testInvalidRegistry(cmdFn commandConstructor, t *testing.T) {
+	t.Helper()
+	root, rm := Mktemp(t)
+	defer rm()
+
+	f := fn.Function{
+		Root:    root,
+		Name:    "myFunc",
+		Runtime: "go",
+	}
+	if err := fn.New().Create(f); err != nil {
+		t.Fatal(err)
+	}
+
+	cmd := cmdFn(NewClientFactory(func() *fn.Client {
+		return fn.New()
+	}))
+
+	cmd.SetArgs([]string{"--registry=foo/bar/invald/myfunc"})
+
+	if err := cmd.Execute(); err == nil {
+		// TODO: typed ErrInvalidRegistry
+		t.Fatal("invalid registry did not generate expected error")
+	}
+}
+
+// TestDeploy_RegistryLoads ensures that a function with a defined registry
+// will use this when recalculating .Image on deploy when no --image is
+// explicitly provided.
+func TestDeploy_RegistryLoads(t *testing.T) {
+	testRegistryLoads(NewDeployCmd, t)
+}
+
+func testRegistryLoads(cmdFn commandConstructor, t *testing.T) {
+	t.Helper()
+	root, rm := Mktemp(t)
+	defer rm()
+
+	f := fn.Function{
+		Root:     root,
+		Name:     "myFunc",
+		Runtime:  "go",
+		Registry: "example.com/alice",
+	}
+	if err := fn.New().Create(f); err != nil {
+		t.Fatal(err)
+	}
+
+	cmd := cmdFn(NewClientFactory(func() *fn.Client {
+		return fn.New(
+			fn.WithBuilder(mock.NewBuilder()),
+			fn.WithDeployer(mock.NewDeployer()))
+	}))
+	cmd.SetArgs([]string{})
+
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+
+	f, err := fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if f.Image != "example.com/alice/myFunc:latest" {
+		t.Fatalf("unexpected image name: %v", f.Image)
+	}
+}
+
+// TestDeploy_BuilderPersists ensures that the builder chosen is read from
+// the function by default, and is able to be overridden by flags/env vars.
+func TestDeploy_BuilderPersists(t *testing.T) {
+	testBuilderPersists(NewDeployCmd, t)
+}
+
+func testBuilderPersists(cmdFn commandConstructor, t *testing.T) {
+	t.Helper()
+	root, rm := Mktemp(t)
+	defer rm()
+
+	if err := fn.New().Create(fn.Function{Runtime: "go", Root: root}); err != nil {
+		t.Fatal(err)
+	}
+	cmd := cmdFn(NewClientFactory(func() *fn.Client {
+		return fn.New(fn.WithRegistry(TestRegistry))
+	}))
+	cmd.SetArgs([]string{})
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+
+	var err error
+	var f fn.Function
+
+	// Assert the function has persisted a value of builder (has a default)
+	if f, err = fn.NewFunction(root); err != nil {
+		t.Fatal(err)
+	}
+	if f.Builder == "" {
+		t.Fatal("value of builder not persisted using a flag default")
+	}
+
+	// Build the function, specifying a Builder
+	viper.Reset()
+	cmd.SetArgs([]string{"--builder=s2i"})
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+	// Assert the function has persisted the value of builder
+	if f, err = fn.NewFunction(root); err != nil {
+		t.Fatal(err)
+	}
+	if f.Builder != builders.S2I {
+		t.Fatal("value of builder flag not persisted when provided")
+	}
+
+	// Build the function again without specifying a Builder
+	viper.Reset()
+	cmd.SetArgs([]string{})
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+
+	// Assert the function has retained its original value
+	// (was not reset back to a flag default)
+	if f, err = fn.NewFunction(root); err != nil {
+		t.Fatal(err)
+	}
+	if f.Builder != builders.S2I {
+		t.Fatal("value of builder updated when not provided")
+	}
+
+	// Build the function again using a different builder
+	viper.Reset()
+	cmd.SetArgs([]string{"--builder=pack"})
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+
+	// Assert the function has persisted the new value
+	if f, err = fn.NewFunction(root); err != nil {
+		t.Fatal(err)
+	}
+	if f.Builder != builders.Pack {
+		t.Fatal("value of builder flag not persisted on subsequent build")
+	}
+
+	// Build the function, specifying a platform with "pack" Builder
+	cmd.SetArgs([]string{"--platform", "linux"})
+	if err := cmd.Execute(); err == nil {
+		t.Fatal("Expected error")
+	}
+
+	// Set an invalid builder
+	cmd.SetArgs([]string{"--builder", "invalid"})
+	if err := cmd.Execute(); err == nil {
+		t.Fatal("Expected error")
+	}
+}
+
+// TestDeploy_BuilderValidated ensures that the validation function correctly
+// identifies valid and invalid builder short names.
+func TestDeploy_BuilderValidated(t *testing.T) {
+	testBuilderValidated(NewDeployCmd, t)
+}
+
+func testBuilderValidated(cmdFn commandConstructor, t *testing.T) {
+	t.Helper()
+	root, rm := Mktemp(t)
+	defer rm()
+
+	if err := fn.New().Create(fn.Function{Runtime: "go", Root: root}); err != nil {
+		t.Fatal(err)
+	}
+
+	cmd := cmdFn(NewClientFactory(func() *fn.Client {
+		return fn.New()
+	}))
+
+	cmd.SetArgs([]string{"--builder=invalid"})
+	if err := cmd.Execute(); err == nil {
+		t.Fatalf("expected error string '%v', got '%v'", "expected", err.Error())
+	}
+
+}
+
+// TestDeploy_ValidateBuilder tests that the ValidateBuilder validator
+// accepts === the set of known builders.
+func Test_ValidateBuilder(t *testing.T) {
+	for _, name := range builders.All() {
+		if err := ValidateBuilder(name); err != nil {
+			t.Fatalf("expected builder '%v' to be valid, but got error: %v", name, err)
+		}
+	}
+
+	// This CLI creates no builders beyond those in the core reposiory.  Other
+	// users of the client library may provide their own named implementation of
+	// the fn.Builder interface. Those would have a different set of valid
+	// builders.
+
+	if err := ValidateBuilder("invalid"); err == nil {
+		t.Fatalf("did not get expected error validating an invalid builder name")
+	}
+}
+
 func Test_runDeploy(t *testing.T) {
 	tests := []struct {
 		name                 string
@@ -275,10 +608,76 @@ runtime: go`,
 	}
 }
 
-// TestDeploy_BuilderPersistence ensures that the builder chosen is read from
-// the function by default, and is able to be overridden by flags/env vars.
-func TestDeploy_BuilderPersistence(t *testing.T) {
-	testBuilderPersistence(t, "docker.io/tigerteam", NewDeployCmd)
+func Test_checkNamespaceDeploy(t *testing.T) {
+	tests := []struct {
+		name          string
+		confNamespace string
+		funcNamespace string
+		context       bool
+		expected      string
+		expectedError string
+	}{
+		{
+			name: "defaults and no context returns empty with no error",
+		},
+		{
+			name:     "defaults with a context returns the context",
+			context:  true,
+			expected: "test-ns-deploy", // from ./testdata
+		},
+		{
+			name:          "extant value takes precidence when none provided",
+			confNamespace: "",
+			funcNamespace: "last-deploy-value",
+			context:       true,
+			expected:      "last-deploy-value",
+		},
+		{
+			name:          "config values take precidence",
+			confNamespace: "flag-value",
+			funcNamespace: "last-deploy-value",
+			context:       true,
+			expected:      "flag-value",
+		},
+	}
+
+	// contains a kube config with active namespace "test-ns-deploy"
+	contextPath := fmt.Sprintf("%s/testdata/kubeconfig_deploy_namespace", cwd())
+
+	defer WithEnvVar(t, "KUBECONFIG", contextPath)()
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+
+			root, rm := Mktemp(t)
+			defer rm()
+
+			if test.context {
+				defer WithEnvVar(t, "KUBECONFIG", contextPath)()
+			} else {
+				defer WithEnvVar(t, "KUBECONFIG", cwd())()
+			}
+
+			f := fn.Function{
+				Runtime:   "go",
+				Root:      root,
+				Namespace: test.funcNamespace,
+			}
+
+			if err := fn.New().Create(f); err != nil {
+				t.Fatal(err)
+			}
+
+			ns, err := checkNamespaceDeploy(test.funcNamespace, test.confNamespace)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if ns != test.expected {
+				t.Errorf("expected namespace '%v' got '%v'", test.expected, ns)
+			}
+
+		})
+	}
 }
 
 func Test_namespaceCheck(t *testing.T) {
diff --git a/function_unit_test.go b/function_unit_test.go
index b18ed56c..b433ac1c 100644
--- a/function_unit_test.go
+++ b/function_unit_test.go
@@ -5,8 +5,6 @@ package function
 
 import (
 	"testing"
-
-	. "knative.dev/kn-plugin-func/testing"
 )
 
 func TestFunction_ImageWithDigest(t *testing.T) {
@@ -48,91 +46,39 @@ func TestFunction_ImageWithDigest(t *testing.T) {
 	}
 }
 
-func Test_DerivedImage(t *testing.T) {
+// TestFunction_ImageName ensures that the full image name is
+// returned for a Function, based on the Function's Registry and Name,
+// including utilizing the DefaultRegistry if the Function's defined
+// registry is a single token (just the namespace).
+func TestFunction_ImageName(t *testing.T) {
+	var (
+		f   Function
+		got string
+		err error
+	)
 	tests := []struct {
-		name     string
-		fnName   string
-		image    string
-		registry string
-		want     string
+		registry      string
+		name          string
+		expectedImage string
+		expectError   bool
 	}{
-		{
-			name:     "No change",
-			fnName:   "testDerivedImage",
-			image:    "docker.io/foo/testDerivedImage:latest",
-			registry: "docker.io/foo",
-			want:     "docker.io/foo/testDerivedImage:latest",
-		},
-		{
-			name:     "Same registry without docker.io/, original with docker.io/",
-			fnName:   "testDerivedImage0",
-			image:    "docker.io/foo/testDerivedImage0:latest",
-			registry: "foo",
-			want:     "docker.io/foo/testDerivedImage0:latest",
-		},
-		{
-			name:     "Same registry, original without docker.io/",
-			fnName:   "testDerivedImage1",
-			image:    "foo/testDerivedImage1:latest",
-			registry: "foo",
-			want:     "docker.io/foo/testDerivedImage1:latest",
-		},
-		{
-			name:     "Different registry without docker.io/, original without docker.io/",
-			fnName:   "testDerivedImage2",
-			image:    "foo/testDerivedImage2:latest",
-			registry: "bar",
-			want:     "docker.io/bar/testDerivedImage2:latest",
-		},
-		{
-			name:     "Different registry with docker.io/, original without docker.io/",
-			fnName:   "testDerivedImage3",
-			image:    "foo/testDerivedImage3:latest",
-			registry: "docker.io/bar",
-			want:     "docker.io/bar/testDerivedImage3:latest",
-		},
-		{
-			name:     "Different registry with docker.io/, original with docker.io/",
-			fnName:   "testDerivedImage4",
-			image:    "docker.io/foo/testDerivedImage4:latest",
-			registry: "docker.io/bar",
-			want:     "docker.io/bar/testDerivedImage4:latest",
-		},
-		{
-			name:     "Different registry with quay.io/, original without docker.io/",
-			fnName:   "testDerivedImage5",
-			image:    "foo/testDerivedImage5:latest",
-			registry: "quay.io/foo",
-			want:     "quay.io/foo/testDerivedImage5:latest",
-		},
-		{
-			name:     "Different registry with quay.io/, original with docker.io/",
-			fnName:   "testDerivedImage6",
-			image:    "docker.io/foo/testDerivedImage6:latest",
-			registry: "quay.io/foo",
-			want:     "quay.io/foo/testDerivedImage6:latest",
-		},
+		{"alice", "myfunc", DefaultRegistry + "/alice/myfunc:latest", false},
+		{"quay.io/alice", "myfunc", "quay.io/alice/myfunc:latest", false},
+		{"docker.io/alice", "myfunc", "docker.io/alice/myfunc:latest", false},
+		{"docker.io/alice/sub", "myfunc", "docker.io/alice/sub/myfunc:latest", false},
+		{"alice", "", "", true},
+		{"", "myfunc", "", true},
 	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-
-			root := "testdata/" + tt.fnName
-			defer Using(t, root)()
-
-			// write out the function
-			client := New()
-			err := client.Create(Function{Runtime: "go", Name: tt.fnName, Root: root})
-			if err != nil {
-				t.Fatal(err)
-			}
-
-			got, err := DerivedImage(root, tt.registry)
-			if err != nil {
-				t.Errorf("DerivedImage() for image %v and registry %v; got error %v", tt.image, tt.registry, err)
-			}
-			if got != tt.want {
-				t.Errorf("DerivedImage() for image %v and registry %v; got %v, want %v", tt.image, tt.registry, got, tt.want)
-			}
-		})
+	for _, test := range tests {
+		f = Function{Registry: test.registry, Name: test.name}
+		got, err = f.ImageName()
+		if test.expectError && err == nil {
+			t.Errorf("registry '%v' and name '%v' did not yield the expected error",
+				test.registry, test.name)
+		}
+		if got != test.expectedImage {
+			t.Errorf("expected registry '%v' name '%v' to yield image '%v', got '%v'",
+				test.registry, test.name, test.expectedImage, got)
+		}
 	}
 }