Merge pull request #1528 from jessvalarezo/labels-validate

Labels are consistently validated across services.

api/services/containers/v1/containers.pb.go

@@ -65,6 +65,8 @@ type Container struct {
 	ID string `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"`
 	// Labels provides an area to include arbitrary data on containers.
 	//
+	// The combined size of a key/value pair cannot exceed 4096 bytes.
+	//
 	// Note that to add a new value to this field, read the existing set and
 	// include the entire result in the update call.
 	Labels map[string]string `protobuf:"bytes,2,rep,name=labels" json:"labels,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"`

api/services/containers/v1/containers.proto

@@ -42,6 +42,8 @@ message Container {
 
 	// Labels provides an area to include arbitrary data on containers.
 	//
+	// The combined size of a key/value pair cannot exceed 4096 bytes.
+	//
 	// Note that to add a new value to this field, read the existing set and
 	// include the entire result in the update call.
 	map<string, string> labels  = 2;

api/services/content/v1/content.pb.go

@@ -115,7 +115,9 @@ type Info struct {
 	CreatedAt time.Time `protobuf:"bytes,3,opt,name=created_at,json=createdAt,stdtime" json:"created_at"`
 	// UpdatedAt provides the time the info was last updated.
 	UpdatedAt time.Time `protobuf:"bytes,4,opt,name=updated_at,json=updatedAt,stdtime" json:"updated_at"`
-	// Labels are arbitrary data on content.
+	// Labels are arbitrary data on snapshots.
+	//
+	// The combined size of a key/value pair cannot exceed 4096 bytes.
 	Labels map[string]string `protobuf:"bytes,5,rep,name=labels" json:"labels,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"`
 }
 
@@ -321,7 +323,9 @@ type WriteContentRequest struct {
 	// If this is empty and the message is not a commit, a response will be
 	// returned with the current write state.
 	Data []byte `protobuf:"bytes,6,opt,name=data,proto3" json:"data,omitempty"`
-	// Labels are arbitrary data to set on commit.
+	// Labels are arbitrary data on snapshots.
+	//
+	// The combined size of a key/value pair cannot exceed 4096 bytes.
 	Labels map[string]string `protobuf:"bytes,7,rep,name=labels" json:"labels,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"`
 }
 

api/services/content/v1/content.proto

@@ -87,7 +87,9 @@ message Info {
 	// UpdatedAt provides the time the info was last updated.
 	google.protobuf.Timestamp updated_at = 4 [(gogoproto.stdtime) = true, (gogoproto.nullable) = false];
 
-	// Labels are arbitrary data on content.
+	// Labels are arbitrary data on snapshots.
+	//
+	// The combined size of a key/value pair cannot exceed 4096 bytes.
 	map<string, string> labels  = 5;
 }
 
@@ -270,7 +272,9 @@ message WriteContentRequest {
 	// returned with the current write state.
 	bytes data = 6;
 
-	// Labels are arbitrary data to set on commit.
+	// Labels are arbitrary data on snapshots.
+	//
+	// The combined size of a key/value pair cannot exceed 4096 bytes.
 	map<string, string> labels  = 7;
 }
 

api/services/images/v1/images.pb.go

@@ -67,6 +67,7 @@ type Image struct {
 	// and do not get inherited into the package image in any way.
 	//
 	// Labels may be updated using the field mask.
+	// The combined size of a key/value pair cannot exceed 4096 bytes.
 	Labels map[string]string `protobuf:"bytes,2,rep,name=labels" json:"labels,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"`
 	// Target describes the content entry point of the image.
 	Target containerd_types.Descriptor `protobuf:"bytes,3,opt,name=target" json:"target"`

api/services/images/v1/images.proto

@@ -51,6 +51,7 @@ message Image {
 	// and do not get inherited into the package image in any way.
 	//
 	// Labels may be updated using the field mask.
+	// The combined size of a key/value pair cannot exceed 4096 bytes.
 	map<string, string> labels = 2;
 
 	// Target describes the content entry point of the image.

api/services/namespaces/v1/namespace.pb.go

@@ -55,6 +55,8 @@ type Namespace struct {
 	Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"`
 	// Labels provides an area to include arbitrary data on namespaces.
 	//
+	// The combined size of a key/value pair cannot exceed 4096 bytes.
+	//
 	// Note that to add a new value to this field, read the existing set and
 	// include the entire result in the update call.
 	Labels map[string]string `protobuf:"bytes,2,rep,name=labels" json:"labels,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"`

api/services/namespaces/v1/namespace.proto

@@ -32,6 +32,8 @@ message Namespace {
 
 	// Labels provides an area to include arbitrary data on namespaces.
 	//
+	// The combined size of a key/value pair cannot exceed 4096 bytes.
+	//
 	// Note that to add a new value to this field, read the existing set and
 	// include the entire result in the update call.
 	map<string, string> labels  = 2;

api/services/snapshot/v1/snapshots.pb.go

@@ -97,6 +97,8 @@ type PrepareSnapshotRequest struct {
 	Key         string `protobuf:"bytes,2,opt,name=key,proto3" json:"key,omitempty"`
 	Parent      string `protobuf:"bytes,3,opt,name=parent,proto3" json:"parent,omitempty"`
 	// Labels are arbitrary data on snapshots.
+	//
+	// The combined size of a key/value pair cannot exceed 4096 bytes.
 	Labels map[string]string `protobuf:"bytes,4,rep,name=labels" json:"labels,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"`
 }
 
@@ -117,6 +119,8 @@ type ViewSnapshotRequest struct {
 	Key         string `protobuf:"bytes,2,opt,name=key,proto3" json:"key,omitempty"`
 	Parent      string `protobuf:"bytes,3,opt,name=parent,proto3" json:"parent,omitempty"`
 	// Labels are arbitrary data on snapshots.
+	//
+	// The combined size of a key/value pair cannot exceed 4096 bytes.
 	Labels map[string]string `protobuf:"bytes,4,rep,name=labels" json:"labels,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"`
 }
 
@@ -163,6 +167,8 @@ type CommitSnapshotRequest struct {
 	Name        string `protobuf:"bytes,2,opt,name=name,proto3" json:"name,omitempty"`
 	Key         string `protobuf:"bytes,3,opt,name=key,proto3" json:"key,omitempty"`
 	// Labels are arbitrary data on snapshots.
+	//
+	// The combined size of a key/value pair cannot exceed 4096 bytes.
 	Labels map[string]string `protobuf:"bytes,4,rep,name=labels" json:"labels,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"`
 }
 
@@ -188,6 +194,8 @@ type Info struct {
 	// UpdatedAt provides the time the info was last updated.
 	UpdatedAt time.Time `protobuf:"bytes,5,opt,name=updated_at,json=updatedAt,stdtime" json:"updated_at"`
 	// Labels are arbitrary data on snapshots.
+	//
+	// The combined size of a key/value pair cannot exceed 4096 bytes.
 	Labels map[string]string `protobuf:"bytes,6,rep,name=labels" json:"labels,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"`
 }
 

api/services/snapshot/v1/snapshots.proto

@@ -29,6 +29,8 @@ message PrepareSnapshotRequest {
 	string parent = 3;
 
 	// Labels are arbitrary data on snapshots.
+	//
+	// The combined size of a key/value pair cannot exceed 4096 bytes.
 	map<string, string> labels  = 4;
 }
 
@@ -42,6 +44,8 @@ message ViewSnapshotRequest {
 	string parent = 3;
 
 	// Labels are arbitrary data on snapshots.
+	//
+	// The combined size of a key/value pair cannot exceed 4096 bytes.
 	map<string, string> labels  = 4;
 }
 
@@ -69,6 +73,8 @@ message CommitSnapshotRequest {
 	string key = 3;
 
 	// Labels are arbitrary data on snapshots.
+	//
+	// The combined size of a key/value pair cannot exceed 4096 bytes.
 	map<string, string> labels  = 4;
 }
 
@@ -99,6 +105,8 @@ message Info {
 	google.protobuf.Timestamp updated_at = 5 [(gogoproto.stdtime) = true, (gogoproto.nullable) = false];
 
 	// Labels are arbitrary data on snapshots.
+	//
+	// The combined size of a key/value pair cannot exceed 4096 bytes.
 	map<string, string> labels  = 6;
 }
 

cmd/ctr/images.go

@@ -108,10 +108,16 @@ var imagesSetLabelsCommand = cli.Command{
 	Usage:       "set and clear labels for an image.",
 	ArgsUsage:   "[flags] <name> [<key>=<value>, ...]",
 	Description: "Set and clear labels for an image.",
-	Flags:       []cli.Flag{},
+	Flags: []cli.Flag{
+		cli.BoolFlag{
+			Name:  "replace-all, r",
+			Usage: "replace all labels",
+		},
+	},
 	Action: func(clicontext *cli.Context) error {
 		var (
 			ctx, cancel  = appContext(clicontext)
+			replaceAll   = clicontext.Bool("replace-all")
 			name, labels = objectWithLabelArgs(clicontext)
 		)
 		defer cancel()
@@ -131,8 +137,12 @@ var imagesSetLabelsCommand = cli.Command{
 		)
 
 		for k := range labels {
+			if replaceAll {
+				fieldpaths = append(fieldpaths, "labels")
+			} else {
 				fieldpaths = append(fieldpaths, strings.Join([]string{"labels", k}, "."))
 			}
+		}
 
 		image := images.Image{
 			Name:   name,

labels/validate.go

@@ -0,0 +1,23 @@
+package labels
+
+import (
+	"github.com/containerd/containerd/errdefs"
+	"github.com/pkg/errors"
+)
+
+const (
+	maxSize = 4096
+)
+
+func Validate(k, v string) error {
+	// A label key and value should be under 4096 bytes
+	if (len(k) + len(v)) > maxSize {
+		if len(k) > 10 {
+			k = k[:10]
+		}
+
+		return errors.Wrapf(errdefs.ErrInvalidArgument, "label key and value greater than maximum size (%d bytes), key: %s", maxSize, k)
+	}
+
+	return nil
+}

labels/validate_test.go

@@ -0,0 +1,37 @@
+package labels
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/containerd/containerd/errdefs"
+)
+
+func TestValidLabels(t *testing.T) {
+	shortStr := "s"
+	longStr := strings.Repeat("s", maxSize-1)
+
+	for key, value := range map[string]string{
+		"some":   "value",
+		shortStr: longStr,
+	} {
+		if err := Validate(key, value); err != nil {
+			t.Fatalf("unexpected error: %v != nil", err)
+		}
+	}
+}
+
+func TestInvalidLabels(t *testing.T) {
+	addOneStr := "s"
+	maxSizeStr := strings.Repeat("s", maxSize)
+
+	for key, value := range map[string]string{
+		maxSizeStr: addOneStr,
+	} {
+		if err := Validate(key, value); err == nil {
+			t.Fatal("expected invalid error")
+		} else if !errdefs.IsInvalidArgument(err) {
+			t.Fatal("error should be an invalid label error")
+		}
+	}
+}

metadata/containers.go

@@ -10,6 +10,7 @@ import (
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/filters"
 	"github.com/containerd/containerd/identifiers"
+	"github.com/containerd/containerd/labels"
 	"github.com/containerd/containerd/metadata/boltutil"
 	"github.com/containerd/containerd/namespaces"
 	"github.com/gogo/protobuf/proto"
@@ -243,7 +244,13 @@ func validateContainer(container *containers.Container) error {
 		}
 	}
 
-	// labels and image have no validation
+	// image has no validation
+	for k, v := range container.Labels {
+		if err := labels.Validate(k, v); err == nil {
+			return errors.Wrapf(err, "containers.Labels")
+		}
+	}
+
 	if container.Runtime.Name == "" {
 		return errors.Wrapf(errdefs.ErrInvalidArgument, "container.Runtime.Name must be set")
 	}

metadata/content.go

@@ -10,6 +10,7 @@ import (
 	"github.com/containerd/containerd/content"
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/filters"
+	"github.com/containerd/containerd/labels"
 	"github.com/containerd/containerd/metadata/boltutil"
 	"github.com/containerd/containerd/namespaces"
 	digest "github.com/opencontainers/go-digest"
@@ -94,6 +95,9 @@ func (cs *contentStore) Update(ctx context.Context, info content.Info, fieldpath
 			// Set mutable fields
 			updated.Labels = info.Labels
 		}
+		if err := validateInfo(&updated); err != nil {
+			return err
+		}
 
 		updated.UpdatedAt = time.Now().UTC()
 		return writeInfo(&updated, bkt)
@@ -371,6 +375,10 @@ func (nw *namespacedWriter) commit(ctx context.Context, tx *bolt.Tx, size int64,
 			return err
 		}
 	}
+	if err := validateInfo(&base); err != nil {
+		return err
+	}
+
 	status, err := nw.Writer.Status()
 	if err != nil {
 		return err
@@ -446,6 +454,16 @@ func (cs *contentStore) checkAccess(ctx context.Context, dgst digest.Digest) err
 	})
 }
 
+func validateInfo(info *content.Info) error {
+	for k, v := range info.Labels {
+		if err := labels.Validate(k, v); err == nil {
+			return errors.Wrapf(err, "info.Labels")
+		}
+	}
+
+	return nil
+}
+
 func readInfo(info *content.Info, bkt *bolt.Bucket) error {
 	if err := boltutil.ReadTimestamps(bkt, &info.CreatedAt, &info.UpdatedAt); err != nil {
 		return err

metadata/images.go

@@ -11,6 +11,7 @@ import (
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/filters"
 	"github.com/containerd/containerd/images"
+	"github.com/containerd/containerd/labels"
 	"github.com/containerd/containerd/metadata/boltutil"
 	"github.com/containerd/containerd/namespaces"
 	digest "github.com/opencontainers/go-digest"
@@ -101,6 +102,10 @@ func (s *imageStore) Create(ctx context.Context, image images.Image) (images.Ima
 		return images.Image{}, errors.Wrapf(errdefs.ErrInvalidArgument, "image name is required for create")
 	}
 
+	if err := validateImage(&image); err != nil {
+		return images.Image{}, err
+	}
+
 	return image, withImagesBucket(s.tx, namespace, func(bkt *bolt.Bucket) error {
 		ibkt, err := bkt.CreateBucket([]byte(image.Name))
 		if err != nil {
@@ -170,6 +175,10 @@ func (s *imageStore) Update(ctx context.Context, image images.Image, fieldpaths
 			updated = image
 		}
 
+		if err := validateImage(&image); err != nil {
+			return err
+		}
+
 		updated.CreatedAt = createdat
 		updated.UpdatedAt = time.Now().UTC()
 		return writeImage(ibkt, &updated)
@@ -191,6 +200,16 @@ func (s *imageStore) Delete(ctx context.Context, name string) error {
 	})
 }
 
+func validateImage(image *images.Image) error {
+	for k, v := range image.Labels {
+		if err := labels.Validate(k, v); err != nil {
+			return errors.Wrapf(err, "image.Labels")
+		}
+	}
+
+	return nil
+}
+
 func readImage(image *images.Image, bkt *bolt.Bucket) error {
 	if err := boltutil.ReadTimestamps(bkt, &image.CreatedAt, &image.UpdatedAt); err != nil {
 		return err

metadata/namespaces.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/boltdb/bolt"
 	"github.com/containerd/containerd/errdefs"
+	l "github.com/containerd/containerd/labels"
 	"github.com/containerd/containerd/namespaces"
 	"github.com/pkg/errors"
 )
@@ -27,6 +28,12 @@ func (s *namespaceStore) Create(ctx context.Context, namespace string, labels ma
 		return err
 	}
 
+	for k, v := range labels {
+		if err := l.Validate(k, v); err != nil {
+			return errors.Wrapf(err, "namespace.Labels")
+		}
+	}
+
 	// provides the already exists error.
 	bkt, err := topbkt.CreateBucket([]byte(namespace))
 	if err != nil {
@@ -70,6 +77,10 @@ func (s *namespaceStore) Labels(ctx context.Context, namespace string) (map[stri
 }
 
 func (s *namespaceStore) SetLabel(ctx context.Context, namespace, key, value string) error {
+	if err := l.Validate(key, value); err != nil {
+		return errors.Wrapf(err, "namespace.Labels")
+	}
+
 	return withNamespacesLabelsBucket(s.tx, namespace, func(bkt *bolt.Bucket) error {
 		if value == "" {
 			return bkt.Delete([]byte(key))

metadata/snapshot.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/boltdb/bolt"
 	"github.com/containerd/containerd/errdefs"
+	"github.com/containerd/containerd/labels"
 	"github.com/containerd/containerd/metadata/boltutil"
 	"github.com/containerd/containerd/mount"
 	"github.com/containerd/containerd/namespaces"
@@ -180,6 +181,9 @@ func (s *snapshotter) Update(ctx context.Context, info snapshot.Info, fieldpaths
 		} else {
 			local.Labels = info.Labels
 		}
+		if err := validateSnapshot(&local); err != nil {
+			return err
+		}
 		local.Updated = time.Now().UTC()
 
 		if err := boltutil.WriteTimestamps(sbkt, local.Created, local.Updated); err != nil {
@@ -257,6 +261,10 @@ func (s *snapshotter) createSnapshot(ctx context.Context, key, parent string, re
 		}
 	}
 
+	if err := validateSnapshot(&base); err != nil {
+		return nil, err
+	}
+
 	var m []mount.Mount
 	if err := update(ctx, s.db, func(tx *bolt.Tx) error {
 		bkt, err := createSnapshotterBucket(tx, ns, s.name)
@@ -328,6 +336,10 @@ func (s *snapshotter) Commit(ctx context.Context, name, key string, opts ...snap
 		}
 	}
 
+	if err := validateSnapshot(&base); err != nil {
+		return err
+	}
+
 	return update(ctx, s.db, func(tx *bolt.Tx) error {
 		bkt := getSnapshotterBucket(tx, ns, s.name)
 		if bkt == nil {
@@ -502,3 +514,13 @@ func (s *snapshotter) Walk(ctx context.Context, fn func(context.Context, snapsho
 
 	return nil
 }
+
+func validateSnapshot(info *snapshot.Info) error {
+	for k, v := range info.Labels {
+		if err := labels.Validate(k, v); err != nil {
+			return errors.Wrapf(err, "info.Labels")
+		}
+	}
+
+	return nil
+}

services/content/service.go

@@ -354,7 +354,7 @@ func (s *Service) Write(session api.Content_WriteServer) (err error) {
 		// 	2. Compress inline.
 		// 	3. Validate digest and size (maybe).
 		//
-		// Supporting these two paths is quite awkward but it let's both API
+		// Supporting these two paths is quite awkward but it lets both API
 		// users use the same writer style for each with a minimum of overhead.
 		if req.Expected != "" {
 			if expected != "" && expected != req.Expected {