Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: make image digest check more permissive #2510

Merged
merged 2 commits into from
Sep 24, 2024
Merged

fix: make image digest check more permissive #2510

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9ea4844
fix: make image digest check more permissive
lkingland Sep 20, 2024
1be1e14
use extant implementation for digest check
lkingland Sep 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 8 additions & 45 deletions cmd/deploy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
// 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:]))
ref, err := name.ParseReference(v)
if err != nil {
return false, err
}

validDigest = true
return
_, ok := ref.(name.Digest)
return ok, nil
}
61 changes: 46 additions & 15 deletions cmd/deploy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
}

Expand Down Expand Up @@ -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
}
Expand All @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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")
}
}
Loading