fix: make image digest check more permissive (#2510)

* fix: make image digest check more permissive

* use extant implementation for digest check

cmd/deploy.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 
 	"github.com/AlecAivazis/survey/v2"
+	"github.com/google/go-containerregistry/pkg/name"
 	"github.com/ory/viper"
 	"github.com/spf13/cobra"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -785,51 +786,13 @@ func printDeployMessages(out io.Writer, f fn.Function) {
 	}
 }
 
-// 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".
+// isDigested checks that the given image reference has a digest. Invalid
+// reference return error.
 func isDigested(v string) (validDigest bool, err error) {
-	var digest string
-	vv := strings.Split(v, "@")
-	if len(vv) < 2 {
-		// 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
+	ref, err := name.ParseReference(v)
+	if err != nil {
+		return false, err
 	}
-	// image has a digest, validate further
-	digest = vv[1]
-
-	if !strings.HasPrefix(digest, "sha256:") {
-		err = fmt.Errorf("image digest '%s' requires 'sha256:' prefix", digest)
-		return
-	}
-
-	if len(digest[7:]) != 64 {
-		err = fmt.Errorf("image digest '%v' has an invalid sha256 hash length of %v when it should be 64", digest, len(digest[7:]))
-	}
-
-	validDigest = true
-	return
+	_, ok := ref.(name.Digest)
+	return ok, nil
 }

cmd/deploy_test.go

@@ -756,34 +756,34 @@ func TestDeploy_ImageWithDigestErrors(t *testing.T) {
 		image     string // value to provide as --image
 		build     string // If provided, the value of the build flag
 		push      bool   // if true, explicitly set argument --push=true
-		errString string // the string value of an expected error
+		errPrefix string // the string value of an expected error
 	}{
 		{
 			name:  "correctly formatted full image with digest yields no error (degen case)",
-			image: "example.com/myNamespace/myFunction:latest@sha256:7d66645b0add6de7af77ef332ecd4728649a2f03b9a2716422a054805b595c4e",
+			image: "example.com/namespace/myfunction@sha256:7d66645b0add6de7af77ef332ecd4728649a2f03b9a2716422a054805b595c4e",
 			build: "false",
 		},
 		{
 			name:      "--build forced on yields error",
-			image:     "example.com/myNamespace/myFunction:latest@sha256:7d66645b0add6de7af77ef332ecd4728649a2f03b9a2716422a054805b595c4e",
+			image:     "example.com/mynamespace/myfunction@sha256:7d66645b0add6de7af77ef332ecd4728649a2f03b9a2716422a054805b595c4e",
 			build:     "true",
-			errString: "building can not be enabled when using an image with digest",
+			errPrefix: "building can not be enabled when using an image with digest",
 		},
 		{
 			name:      "push flag explicitly set with digest should error",
-			image:     "example.com/myNamespace/myFunction:latest@sha256:7d66645b0add6de7af77ef332ecd4728649a2f03b9a2716422a054805b595c4e",
+			image:     "example.com/mynamespace/myfunction@sha256:7d66645b0add6de7af77ef332ecd4728649a2f03b9a2716422a054805b595c4e",
 			push:      true,
-			errString: "pushing is not valid when specifying an image with digest",
+			errPrefix: "pushing is not valid when specifying an image with digest",
 		},
 		{
 			name:      "invalid digest prefix 'Xsha256', expect error",
-			image:     "example.com/myNamespace/myFunction:latest@Xsha256:7d66645b0add6de7af77ef332ecd4728649a2f03b9a2716422a054805b595c4e",
-			errString: "image digest 'Xsha256:7d66645b0add6de7af77ef332ecd4728649a2f03b9a2716422a054805b595c4e' requires 'sha256:' prefix",
+			image:     "example.com/mynamespace/myfunction@Xsha256:7d66645b0add6de7af77ef332ecd4728649a2f03b9a2716422a054805b595c4e",
+			errPrefix: "could not parse reference",
 		},
 		{
 			name:      "invalid sha hash length(added X at the end), expect error",
-			image:     "example.com/myNamespace/myFunction:latest@sha256:7d66645b0add6de7af77ef332ecd4728649a2f03b9a2716422a054805b595c4eX",
-			errString: "image digest 'sha256:7d66645b0add6de7af77ef332ecd4728649a2f03b9a2716422a054805b595c4eX' has an invalid sha256 hash length of 65 when it should be 64",
+			image:     "example.com/mynamespace/myfunction@sha256:7d66645b0add6de7af77ef332ecd4728649a2f03b9a2716422a054805b595c4eX",
+			errPrefix: "could not parse reference",
 		},
 	}
 
@@ -823,11 +823,11 @@ func TestDeploy_ImageWithDigestErrors(t *testing.T) {
 			cmd.SetArgs(args)
 			err = cmd.Execute()
 			if err != nil {
-				if tt.errString == "" {
+				if tt.errPrefix == "" {
 					t.Fatal(err) // no error was expected.  fail
 				}
-				if tt.errString != err.Error() {
-					t.Fatalf("expected error '%v' not received. got '%v'", tt.errString, err.Error())
+				if !strings.HasPrefix(err.Error(), tt.errPrefix) {
+					t.Fatalf("expected error prefix '%v' not received. got '%v'", tt.errPrefix, err.Error())
 				}
 				// There was an error, but it was expected
 			}
@@ -842,7 +842,7 @@ func TestDeploy_ImageWithDigestErrors(t *testing.T) {
 func TestDeploy_ImageWithDigestDoesntPopulateBuild(t *testing.T) {
 	root := FromTempDirectory(t)
 	// image with digest (well almost, atleast in length and syntax)
-	const img = "docker.io/4141gauron3268@sha256:XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"
+	const img = "docker.io/4141gauron3268@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
 	// Create a new Function in the temp directory
 	_, err := fn.New().Init(fn.Function{Runtime: "go", Root: root})
 	if err != nil {
@@ -869,7 +869,7 @@ func TestDeploy_ImageWithDigestDoesntPopulateBuild(t *testing.T) {
 // TestDeploy_WithoutDigest ensures that images specified with --image and
 // without digest are correctly processed and propagated to .Deploy.Image
 func TestDeploy_WithoutDigest(t *testing.T) {
-	const sha = "sha256:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+	const sha = "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
 
 	tests := []struct {
 		name            string   //name of the test
@@ -2140,3 +2140,34 @@ func TestDeploy_CorrectImageDeployed(t *testing.T) {
 		})
 	}
 }
+
+// Test_isDigested ensures that the function is properly delegating to
+// by checking both that it will fail on an invalid reference and will
+// return true if the image contains a digest and false otherwise.
+// See the delegate from the implementation for comprehensive tests.
+func Test_isDigested(t *testing.T) {
+	var digested bool
+	var err error
+
+	// Ensure validation
+	_, err = isDigested("invalid&image@sha256:12345")
+	if err == nil {
+		t.Fatal("did not validate image reference")
+	}
+
+	// Ensure positive case
+	if digested, err = isDigested("alpine"); err != nil {
+		t.Fatal(err)
+	}
+	if digested {
+		t.Fatal("reported digested on undigested image reference")
+	}
+
+	// Ensure negative case
+	if digested, err = isDigested("alpine@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"); err != nil {
+		t.Fatal(err)
+	}
+	if !digested {
+		t.Fatal("did not report image reference has digest")
+	}
+}