Skip to content

Commit

Permalink
identifiers: use common package for identifier validation
Browse files Browse the repository at this point in the history
A few days ago, we added validation for namespaces. We've decided to
expand these naming rules to include containers. To facilitate this, a
common package `identifiers` now provides a common validation area.
These rules will be extended to apply to task identifiers, snapshot keys
and other areas where user-provided identifiers may be used.

Signed-off-by: Stephen J Day <stephen.day@docker.com>
  • Loading branch information
stevvooe committed Jun 23, 2017
1 parent e69423f commit 70815af
Show file tree
Hide file tree
Showing 9 changed files with 119 additions and 136 deletions.
58 changes: 58 additions & 0 deletions identifiers/validate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Package identifiers provides common validation for identifiers, keys and ids
// across containerd.
//
// To allow such identifiers to be used across various contexts safely, the character
// set has been restricted to that defined for domains in RFC 1035, section
// 2.3.1. This will make identifiers safe for use across networks, filesystems
// and other media.
//
// While the character set may expand in the future, we guarantee that the
// identifiers will be safe for use as filesystem path components.
package identifiers

import (
"regexp"

"github.com/pkg/errors"
)

const (
label = `[A-Za-z][A-Za-z0-9]+(?:[-]+[A-Za-z0-9]+)*`
)

var (
// identifierRe validates that a identifier matches valid identifiers.
//
// Rules for domains, defined in RFC 1035, section 2.3.1, are used for
// identifiers.
identifierRe = regexp.MustCompile(reAnchor(label + reGroup("[.]"+reGroup(label)) + "*"))

errIdentifierInvalid = errors.Errorf("invalid, must match %v", identifierRe)
)

// IsInvalid return true if the error was due to an invalid identifer.
func IsInvalid(err error) bool {
return errors.Cause(err) == errIdentifierInvalid
}

// Validate return nil if the string s is a valid identifier.
//
// identifiers must be valid domain identifiers according to RFC 1035, section 2.3.1. To
// enforce case insensitvity, all characters must be lower case.
//
// In general, identifiers that pass this validation, should be safe for use as
// a domain identifier or filesystem path component.
func Validate(s string) error {
if !identifierRe.MatchString(s) {
return errors.Wrapf(errIdentifierInvalid, "identifier %q", s)
}
return nil
}

func reGroup(s string) string {
return `(?:` + s + `)`
}

func reAnchor(s string) string {
return `^` + s + `$`
}
47 changes: 47 additions & 0 deletions identifiers/validate_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package identifiers

import (
"testing"
)

func TestValidIdentifiers(t *testing.T) {
for _, input := range []string{
"default",
"Default",
t.Name(),
"default-default",
"default--default",
"containerd.io",
"foo.boo",
"swarmkit.docker.io",
"zn--e9.org", // or something like it!
} {
t.Run(input, func(t *testing.T) {
if err := Validate(input); err != nil {
t.Fatalf("unexpected error: %v != nil", err)
}
})
}
}

func TestInvalidIdentifiers(t *testing.T) {
for _, input := range []string{
".foo..foo",
"foo/foo",
"foo/..",
"foo..foo",
"foo.-boo",
"-foo.boo",
"foo.boo-",
"foo_foo.boo_underscores", // boo-urns?
} {

t.Run(input, func(t *testing.T) {
if err := Validate(input); err == nil {
t.Fatal("expected invalid error")
} else if !IsInvalid(err) {
t.Fatal("error should be an invalid identifier error")
}
})
}
}
5 changes: 5 additions & 0 deletions metadata/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/boltdb/bolt"
"github.com/containerd/containerd/containers"
"github.com/containerd/containerd/identifiers"
"github.com/containerd/containerd/namespaces"
"github.com/pkg/errors"
)
Expand Down Expand Up @@ -77,6 +78,10 @@ func (s *containerStore) Create(ctx context.Context, container containers.Contai
return containers.Container{}, err
}

if err := identifiers.Validate(container.ID); err != nil {
return containers.Container{}, err
}

bkt, err := createContainersBucket(s.tx, namespace)
if err != nil {
return containers.Container{}, err
Expand Down
3 changes: 2 additions & 1 deletion metadata/namespaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"

"github.com/boltdb/bolt"
"github.com/containerd/containerd/identifiers"
"github.com/containerd/containerd/namespaces"
)

Expand All @@ -21,7 +22,7 @@ func (s *namespaceStore) Create(ctx context.Context, namespace string, labels ma
return err
}

if err := namespaces.Validate(namespace); err != nil {
if err := identifiers.Validate(namespace); err != nil {
return err
}

Expand Down
5 changes: 3 additions & 2 deletions namespaces/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package namespaces
import (
"os"

"github.com/containerd/containerd/identifiers"
"github.com/pkg/errors"
"golang.org/x/net/context"
)
Expand Down Expand Up @@ -61,8 +62,8 @@ func NamespaceRequired(ctx context.Context) (string, error) {
return "", errNamespaceRequired
}

if err := Validate(namespace); err != nil {
return "", err
if err := identifiers.Validate(namespace); err != nil {
return "", errors.Wrap(err, "namespace validation")
}

return namespace, nil
Expand Down
52 changes: 0 additions & 52 deletions namespaces/validate.go

This file was deleted.

79 changes: 0 additions & 79 deletions namespaces/validate_test.go

This file was deleted.

3 changes: 2 additions & 1 deletion services/containers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package containers
import (
api "github.com/containerd/containerd/api/services/containers/v1"
"github.com/containerd/containerd/containers"
"github.com/containerd/containerd/identifiers"
"github.com/containerd/containerd/metadata"
"github.com/containerd/containerd/namespaces"
"github.com/gogo/protobuf/types"
Expand Down Expand Up @@ -60,7 +61,7 @@ func mapGRPCError(err error, id string) error {
return grpc.Errorf(codes.AlreadyExists, "container %v already exists", id)
case namespaces.IsNamespaceRequired(err):
return grpc.Errorf(codes.InvalidArgument, "namespace required, please set %q header", namespaces.GRPCHeader)
case namespaces.IsNamespaceInvalid(err):
case identifiers.IsInvalid(err):
return grpc.Errorf(codes.InvalidArgument, err.Error())
}

Expand Down
3 changes: 2 additions & 1 deletion services/images/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package images
import (
imagesapi "github.com/containerd/containerd/api/services/images/v1"
"github.com/containerd/containerd/api/types"
"github.com/containerd/containerd/identifiers"
"github.com/containerd/containerd/images"
"github.com/containerd/containerd/metadata"
"github.com/containerd/containerd/namespaces"
Expand Down Expand Up @@ -85,7 +86,7 @@ func mapGRPCError(err error, id string) error {
return grpc.Errorf(codes.AlreadyExists, "image %v already exists", id)
case namespaces.IsNamespaceRequired(err):
return grpc.Errorf(codes.InvalidArgument, "namespace required, please set %q header", namespaces.GRPCHeader)
case namespaces.IsNamespaceInvalid(err):
case identifiers.IsInvalid(err):
return grpc.Errorf(codes.InvalidArgument, err.Error())
}

Expand Down

0 comments on commit 70815af

Please sign in to comment.