containers
/
image
mirror of https://github.com/containers/image.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix ValidatePolicyConfigurationScope 

Re-enable the failing tests, and rewrite the implementation to match expected inputs.

Notably this re-enables the @ID form with no name.

Like docker.Transport, this does no validation of the name part; using
the docker/reference parses is not correct because they are not intended to
accept e.g. hostname-only "docker.io"

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This commit is contained in:
Miloslav Trmač 2018-03-09 09:14:22 +01:00
parent b8969b7aba
commit 04e1018848
2 changed files with 41 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
storage/storage_transport.go
View File

@ -3,6 +3,7 @@
package storage
import (
"fmt"
"path/filepath"
"strings"
@ -399,27 +400,30 @@ func (s storageTransport) ValidatePolicyConfigurationScope(scope string) error {
if scope == "" {
return nil
}
// But if there is anything left, it has to be a name, with or without
// a tag, with or without an ID, since we don't return namespace values
// that are just bare IDs.
scopeInfo := strings.SplitN(scope, "@", 2)
if len(scopeInfo) == 1 && scopeInfo[0] != "" {
_, err := reference.ParseNormalizedNamed(scopeInfo[0])
if err != nil {
fields := strings.SplitN(scope, "@", 3)
switch len(fields) {
case 1: // name only
case 2: // name:tag@ID or name[:tag]@digest
if _, idErr := digest.Parse("sha256:" + fields[1]); idErr != nil {
if _, digestErr := digest.Parse(fields[1]); digestErr != nil {
return fmt.Errorf("%v is neither a valid digest(%s) nor a valid ID(%s)", fields[1], digestErr.Error(), idErr.Error())
}
}
case 3: // name[:tag]@digest@ID
if _, err := digest.Parse(fields[1]); err != nil {
return err
}
} else if len(scopeInfo) == 2 && scopeInfo[0] != "" && scopeInfo[1] != "" {
_, err := reference.ParseNormalizedNamed(scopeInfo[0])
if err != nil {
if _, err := digest.Parse("sha256:" + fields[2]); err != nil {
return err
}
_, err = digest.Parse("sha256:" + scopeInfo[1])
if err != nil {
return err
}
} else {
return ErrInvalidReference
default: // Coverage: This should never happen
return errors.New("Internal error: unexpected number of fields form strings.SplitN")
}
// As for field[0], if it is non-empty at all:
// FIXME? We could be verifying the various character set and length restrictions
// from docker/distribution/reference.regexp.go, but other than that there
// are few semantically invalid strings.
return nil
}

39
storage/storage_transport_test.go
View File

@ -139,18 +139,18 @@ func TestTransportValidatePolicyConfigurationScope(t *testing.T) {
// Valid inputs
for _, scope := range []string{
"[" + root + "suffix1]", // driverlessStoreSpec in PolicyConfigurationNamespaces
"[" + driver + "@" + root + "suffix3]", // storeSpec in PolicyConfigurationNamespaces
// FIXME storeSpec + "@" + sha256digestHex, // ID only
storeSpec + "docker.io", // Host name only
storeSpec + "docker.io/library", // A repository namespace
storeSpec + "docker.io/library/busybox", // A repository name
storeSpec + "docker.io/library/busybox:notlatest", // name:tag
storeSpec + "docker.io/library/busybox:notlatest@" + sha256digestHex, // name@ID
// FIXME storeSpec + "docker.io/library/busybox@" + sha256Digest2, // name@digest
// FIXME storeSpec + "docker.io/library/busybox@" + sha256Digest2 + "@" + sha256digestHex, // name@digest@ID
// FIXME storeSpec + "docker.io/library/busybox:notlatest@" + sha256Digest2, // name:tag@digest
// FIXME storeSpec + "docker.io/library/busybox:notlatest@" + sha256Digest2 + "@" + sha256digestHex, // name:tag@digest@ID
"[" + root + "suffix1]", // driverlessStoreSpec in PolicyConfigurationNamespaces
"[" + driver + "@" + root + "suffix3]", // storeSpec in PolicyConfigurationNamespaces
storeSpec + "@" + sha256digestHex, // ID only
storeSpec + "docker.io", // Host name only
storeSpec + "docker.io/library", // A repository namespace
storeSpec + "docker.io/library/busybox", // A repository name
storeSpec + "docker.io/library/busybox:notlatest", // name:tag
storeSpec + "docker.io/library/busybox:notlatest@" + sha256digestHex, // name@ID
storeSpec + "docker.io/library/busybox@" + sha256Digest2, // name@digest
storeSpec + "docker.io/library/busybox@" + sha256Digest2 + "@" + sha256digestHex, // name@digest@ID
storeSpec + "docker.io/library/busybox:notlatest@" + sha256Digest2, // name:tag@digest
storeSpec + "docker.io/library/busybox:notlatest@" + sha256Digest2 + "@" + sha256digestHex, // name:tag@digest@ID
} {
err := Transport.ValidatePolicyConfigurationScope(scope)
assert.NoError(t, err, scope)
@ -164,13 +164,16 @@ func TestTransportValidatePolicyConfigurationScope(t *testing.T) {
"[relative/path]", // Non-absolute graph root path
"[" + driver + "@relative/path]", // Non-absolute graph root path
// "[thisisunknown@" + root + "suffix2]", // Unknown graph driver FIXME: validate against storage.ListGraphDrivers() once that's available
storeSpec + sha256digestHex, // Almost a valid single-component name, but rejected because it looks like an ID that's missing its "@" prefix
storeSpec + "@", // An incomplete two-component name
storeSpec + "@", // An incomplete two-component name
storeSpec + "UPPERCASEISINVALID", // Invalid single-component name
storeSpec + "UPPERCASEISINVALID@" + sha256digestHex, // Invalid name in name@ID
storeSpec + "docker.io/library/busybox@ab", // Invalid ID in name@ID
storeSpec + "docker.io/library/busybox@", // Empty ID in name@ID
storeSpec + "docker.io/library/busybox@sha256:ab", // Invalid digest in name@digest
storeSpec + "docker.io/library/busybox@ab", // Invalid ID in name@ID
storeSpec + "docker.io/library/busybox@", // Empty ID/digest in name@ID
storeSpec + "docker.io/library/busybox@@" + sha256digestHex, // Empty digest in name@digest@ID
storeSpec + "docker.io/library/busybox@ab@" + sha256digestHex, // Invalid digest in name@digest@ID
storeSpec + "docker.io/library/busybox@sha256:ab@" + sha256digestHex, // Invalid digest in name@digest@ID
storeSpec + "docker.io/library/busybox@" + sha256Digest2 + "@", // Empty ID in name@digest@ID
storeSpec + "docker.io/library/busybox@" + sha256Digest2 + "@ab", // Invalid ID in name@digest@ID
} {
err := Transport.ValidatePolicyConfigurationScope(scope)
assert.Error(t, err, scope)