allow digested images to be 'run'

cmd/build.go

@@ -192,7 +192,7 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro
 		return
 	}
 	if cfg.Push {
-		if f, err = client.Push(cmd.Context(), f); err != nil {
+		if f, _, err = client.Push(cmd.Context(), f); err != nil {
 			return
 		}
 	}

cmd/deploy.go

@@ -298,16 +298,24 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) {
 			return
 		}
 
-		// Preprocess image name. Validate the image and check whether its digested
-		// This might alter f.Deploy.Image.
-		var digested bool
-		f, digested, err = processImageName(f, cfg.Image)
-		if err != nil {
-			return
+		var (
+			digested   bool
+			justBuilt  bool
+			justPushed bool
+		)
+
+		// Validate the image and check whether its digested or not
+		if cfg.Image != "" {
+			digested, err = isDigested(cfg.Image)
+			if err != nil {
+				return
+			}
+			// image is valid and undigested
+			if !digested {
+				f.Deploy.Image = cfg.Image
+			}
 		}
 
-		var justBuilt bool
-
 		// If user provided --image with digest, they are requesting that specific
 		// image to be used which means building phase should be skipped and image
 		// should be deployed as is
@@ -319,19 +327,18 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) {
 				return
 			}
 			if cfg.Push {
-				if f, err = client.Push(cmd.Context(), f); err != nil {
+				if f, justPushed, err = client.Push(cmd.Context(), f); err != nil {
 					return
 				}
 			}
-			// TODO: gauron99 - temporary fix for undigested image direct deploy (w/out
-			// build) I think we will be able to remove this after we clean up the
-			// building process - move the setting of built image in building phase?
-			if justBuilt && f.Build.Image != "" {
+			// TODO: gauron99 - temporary fix for undigested image direct deploy
+			// (w/out build) This might be more complex to do than leaving like this
+			// image digests are created via the registry on push.
+			if (justBuilt || justPushed) && f.Build.Image != "" {
 				// f.Build.Image is set in Push for now, just set it as a deployed image
 				f.Deploy.Image = f.Build.Image
 			}
 		}
-
 		if f, err = client.Deploy(cmd.Context(), f, fn.WithDeploySkipBuildCheck(cfg.Build == "false")); err != nil {
 			return
 		}
@@ -372,7 +379,8 @@ func build(cmd *cobra.Command, flag string, f fn.Function, client *fn.Client, bu
 		}
 	} else if _, err = strconv.ParseBool(flag); err != nil {
 		return f, false, fmt.Errorf("--build ($FUNC_BUILD) %q not recognized.  Should be 'auto' or a truthy value such as 'true', 'false', '0', or '1'.", flag)
-
+	} else if !build {
+		return f, false, nil
 	}
 	return f, true, nil
 }
@@ -671,10 +679,11 @@ func (c deployConfig) Validate(cmd *cobra.Command) (err error) {
 	}
 
 	// Check Image Digest was included
-	// (will be set on the function during .Configure)
 	var digest bool
-	if digest, err = isDigested(c.Image); err != nil {
-		return
+	if c.Image != "" {
+		if digest, err = isDigested(c.Image); err != nil {
+			return
+		}
 	}
 
 	// --build can be "auto"|true|false
@@ -775,44 +784,40 @@ func printDeployMessages(out io.Writer, f fn.Function) {
 	}
 }
 
-// isUndigested returns true if provided image string 'v' has valid tag and false if
-// not. It is lenient in validating - does not always throw an error, just
-// returning false in some scenarios.
-func isUndigested(v string) (validTag bool, err error) {
-	if strings.Contains(v, "@") {
-		// digest has been processed separately
-		return
-	}
-	vv := strings.Split(v, ":")
-	if len(vv) < 2 {
-		// assume user knows what hes doing
-		validTag = true
-		return
-	} else if len(vv) > 2 {
-		err = fmt.Errorf("image '%v' contains an invalid tag (extra ':')", v)
-		return
-	}
-	tag := vv[1]
-	if tag == "" {
-		err = fmt.Errorf("image '%v' has an empty tag", v)
-		return
-	}
-
-	validTag = true
-	return
-}
-
 // isDigested returns true if provided image string 'v' has digest and false if not.
 // Includes basic validation that a provided digest is correctly formatted.
+// Given that image is not digested, image will still be validated and return
+// a combination of bool (img has valid digest) and err (img is in valid format)
+// Therefore returned combination of [false,nil] means "valid undigested image".
 func isDigested(v string) (validDigest bool, err error) {
 	var digest string
 	vv := strings.Split(v, "@")
 	if len(vv) < 2 {
-		return // has no digest
+		// image does NOT have a digest, validate further
+		if v == "" {
+			err = fmt.Errorf("provided image is empty, cannot validate")
+			return
+		}
+		vvv := strings.Split(v, ":")
+		if len(vvv) < 2 {
+			// assume user knows what hes doing
+			return
+		} else if len(vvv) > 2 {
+			err = fmt.Errorf("image '%v' contains an invalid tag (extra ':')", v)
+			return
+		}
+		tag := vvv[1]
+		if tag == "" {
+			err = fmt.Errorf("image '%v' has an empty tag", v)
+			return
+		}
+		return
 	} else if len(vv) > 2 {
+		// image is invalid
 		err = fmt.Errorf("image '%v' contains an invalid digest (extra '@')", v)
 		return
 	}
+	// image has a digest, validate further
 	digest = vv[1]
 
 	if !strings.HasPrefix(digest, "sha256:") {
@@ -827,35 +832,3 @@ func isDigested(v string) (validDigest bool, err error) {
 	validDigest = true
 	return
 }
-
-// processImageName processes the image name for deployment. It ensures that
-// image string is validated if --image was given and ensures that proper
-// fields of Function structure are populated if needed.
-// Returns a Function structure(1), bool indicating if image was given with
-// digest(2) and error(3)
-func processImageName(fin fn.Function, configImage string) (f fn.Function, digested bool, err error) {
-	f = fin
-	// check if --image was provided with a digest. 'digested' bool indicates if
-	// image contains a digest or not (image is "digested").
-	digested, err = isDigested(configImage)
-	if err != nil {
-		return
-	}
-	// if image is digested, no need to process further
-	if digested {
-		return
-	}
-	// digested = false here
-
-	// valid image can be with/without a tag and might be/not be built next
-	valid, err := isUndigested(configImage)
-	if err != nil {
-		return
-	}
-	if valid {
-		// this can be overridden when build&push=enabled with freshly built
-		// (digested) image OR directly deployed when build&push=disabled
-		f.Deploy.Image = configImage
-	}
-	return
-}

cmd/deploy_test.go

@@ -2017,3 +2017,126 @@ func TestDeploy_WithoutHome(t *testing.T) {
 		t.Fatal(err)
 	}
 }
+
+// TestDeploy_CorrectImageDeployed ensures that deploying will always pass
+// the correct image name to the deployer (populating the f.Deploy.Image value)
+// in various scenarios.
+func TestDeploy_CorrectImageDeployed(t *testing.T) {
+	const sha = "sha256:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+	// dataset
+	tests := []struct {
+		name         string
+		image        string
+		buildArgs    []string
+		deployArgs   []string
+		shouldFail   bool
+		shouldBuild  bool
+		pusherActive bool
+	}{
+		{
+			name:       "basic test to create and deploy",
+			image:      "myimage",
+			deployArgs: []string{"--image", "myimage"},
+		},
+		{
+			name:  "test to deploy with prebuild",
+			image: "myimage",
+			buildArgs: []string{
+				"--image=myimage",
+			},
+			deployArgs: []string{
+				"--build=false",
+			},
+			shouldBuild: true,
+		},
+		{
+			name:  "test to build and deploy",
+			image: "myimage",
+			buildArgs: []string{
+				"--image=myimage",
+			},
+			shouldBuild: true,
+		},
+		{
+			name:  "test to deploy without build should fail",
+			image: "myimage",
+			deployArgs: []string{
+				"--build=false",
+			},
+			shouldFail: true,
+		},
+		{
+			name:  "test to build then deploy with push",
+			image: "myimage" + "@" + sha,
+			buildArgs: []string{
+				"--image=myimage",
+			},
+			deployArgs: []string{
+				"--build=false",
+				"--push=true",
+			},
+			shouldBuild:  true,
+			pusherActive: true,
+		},
+	}
+
+	// run tests
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			root := FromTempDirectory(t)
+			f := fn.Function{
+				Runtime: "go",
+				Root:    root,
+			}
+			_, err := fn.New().Init(f)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			// prebuild function if desired
+			if tt.shouldBuild {
+				cmd := NewBuildCmd(NewTestClient(fn.WithRegistry(TestRegistry)))
+				cmd.SetArgs(tt.buildArgs)
+				if err = cmd.Execute(); err != nil {
+					t.Fatal(err)
+				}
+			}
+
+			pusher := mock.NewPusher()
+			if tt.pusherActive {
+				pusher.PushFn = func(_ context.Context, _ fn.Function) (string, error) {
+					return sha, nil
+				}
+			}
+
+			deployer := mock.NewDeployer()
+			deployer.DeployFn = func(_ context.Context, f fn.Function) (result fn.DeploymentResult, err error) {
+				// verify the image passed to the deployer
+				if f.Deploy.Image != tt.image {
+					return fn.DeploymentResult{}, fmt.Errorf("image '%v' does not match the expected image '%v'\n", f.Deploy.Image, tt.image)
+				}
+				return
+			}
+
+			// Deploy the function
+			cmd := NewDeployCmd(NewTestClient(
+				fn.WithDeployer(deployer), //is always specified
+				fn.WithPusher(pusher)))    // if specified, will return sha for testing
+
+			cmd.SetArgs(tt.deployArgs)
+
+			// assert
+			err = cmd.Execute()
+			if tt.shouldFail {
+				if err == nil {
+					t.Fatal("expected an error but got none")
+				}
+			} else {
+				// should not fail
+				if err != nil {
+					t.Fatal(err)
+				}
+			}
+		})
+	}
+}

cmd/run.go

@@ -177,12 +177,45 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) {
 	// If requesting to run via the container, build the container if it is
 	// either out-of-date or a build was explicitly requested.
 	if cfg.Container {
+		var digested bool
+
 		buildOptions, err := cfg.buildOptions()
 		if err != nil {
 			return err
 		}
-		if f, _, err = build(cmd, cfg.Build, f, client, buildOptions); err != nil {
-			return err
+
+		// if image was specified, check if its digested and do basic validation
+		if cfg.Image != "" {
+			digested, err = isDigested(cfg.Image)
+			if err != nil {
+				return err
+			}
+			if !digested {
+				// assign valid undigested image
+				f.Build.Image = cfg.Image
+			}
+		}
+
+		if digested {
+			// run cmd takes f.Build.Image - see newContainerConfig in docker/runner.go
+			// it doesnt get saved, just runtime image
+			f.Build.Image = cfg.Image
+		} else {
+
+			if f, _, err = build(cmd, cfg.Build, f, client, buildOptions); err != nil {
+				return err
+			}
+		}
+	} else {
+		// dont run digested image without a container
+		if cfg.Image != "" {
+			digested, err := isDigested(cfg.Image)
+			if err != nil {
+				return err
+			}
+			if digested {
+				return fmt.Errorf("cannot use digested image with --container=false")
+			}
 		}
 	}