From 18c4322bb3ddcb8f8b4eea2f3c027a06194041b4 Mon Sep 17 00:00:00 2001 From: Jess Valarezo Date: Thu, 21 Sep 2017 15:11:46 -0700 Subject: [PATCH] Labels are consistently validated across services * The combined size of a key/value pair cannot exceed 4096 bytes Signed-off-by: Jess Valarezo --- api/services/containers/v1/containers.pb.go | 2 ++ api/services/containers/v1/containers.proto | 2 ++ api/services/content/v1/content.pb.go | 8 +++-- api/services/content/v1/content.proto | 8 +++-- api/services/images/v1/images.pb.go | 1 + api/services/images/v1/images.proto | 1 + api/services/namespaces/v1/namespace.pb.go | 2 ++ api/services/namespaces/v1/namespace.proto | 2 ++ api/services/snapshot/v1/snapshots.pb.go | 8 +++++ api/services/snapshot/v1/snapshots.proto | 8 +++++ cmd/ctr/images.go | 14 ++++++-- labels/validate.go | 23 +++++++++++++ labels/validate_test.go | 37 +++++++++++++++++++++ metadata/containers.go | 9 ++++- metadata/content.go | 18 ++++++++++ metadata/images.go | 19 +++++++++++ metadata/namespaces.go | 11 ++++++ metadata/snapshot.go | 22 ++++++++++++ services/content/service.go | 2 +- 19 files changed, 189 insertions(+), 8 deletions(-) create mode 100644 labels/validate.go create mode 100644 labels/validate_test.go diff --git a/api/services/containers/v1/containers.pb.go b/api/services/containers/v1/containers.pb.go index edcd93c6b..90d309903 100644 --- a/api/services/containers/v1/containers.pb.go +++ b/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"` diff --git a/api/services/containers/v1/containers.proto b/api/services/containers/v1/containers.proto index cfc7ebdea..0a2311c4c 100644 --- a/api/services/containers/v1/containers.proto +++ b/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 labels = 2; diff --git a/api/services/content/v1/content.pb.go b/api/services/content/v1/content.pb.go index 40cfc66d1..c9b76b5a7 100644 --- a/api/services/content/v1/content.pb.go +++ b/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"` } diff --git a/api/services/content/v1/content.proto b/api/services/content/v1/content.proto index 9538408bd..a0a41c447 100644 --- a/api/services/content/v1/content.proto +++ b/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 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 labels = 7; } diff --git a/api/services/images/v1/images.pb.go b/api/services/images/v1/images.pb.go index 70003cc12..41b97dfdd 100644 --- a/api/services/images/v1/images.pb.go +++ b/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"` diff --git a/api/services/images/v1/images.proto b/api/services/images/v1/images.proto index 49a6cc42e..d6b6b8378 100644 --- a/api/services/images/v1/images.proto +++ b/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 labels = 2; // Target describes the content entry point of the image. diff --git a/api/services/namespaces/v1/namespace.pb.go b/api/services/namespaces/v1/namespace.pb.go index 9854ec16e..e98330b6c 100644 --- a/api/services/namespaces/v1/namespace.pb.go +++ b/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"` diff --git a/api/services/namespaces/v1/namespace.proto b/api/services/namespaces/v1/namespace.proto index bbbd85054..d58a8c2ad 100644 --- a/api/services/namespaces/v1/namespace.proto +++ b/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 labels = 2; diff --git a/api/services/snapshot/v1/snapshots.pb.go b/api/services/snapshot/v1/snapshots.pb.go index 2758bf490..ae3c7927e 100644 --- a/api/services/snapshot/v1/snapshots.pb.go +++ b/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"` } diff --git a/api/services/snapshot/v1/snapshots.proto b/api/services/snapshot/v1/snapshots.proto index af3930ffb..f8c089520 100644 --- a/api/services/snapshot/v1/snapshots.proto +++ b/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 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 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 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 labels = 6; } diff --git a/cmd/ctr/images.go b/cmd/ctr/images.go index f64fbaeb4..19a450751 100644 --- a/cmd/ctr/images.go +++ b/cmd/ctr/images.go @@ -108,10 +108,16 @@ var imagesSetLabelsCommand = cli.Command{ Usage: "Set and clear labels for an image.", ArgsUsage: "[flags] [=, ...]", 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,7 +137,11 @@ var imagesSetLabelsCommand = cli.Command{ ) for k := range labels { - fieldpaths = append(fieldpaths, strings.Join([]string{"labels", k}, ".")) + if replaceAll { + fieldpaths = append(fieldpaths, "labels") + } else { + fieldpaths = append(fieldpaths, strings.Join([]string{"labels", k}, ".")) + } } image := images.Image{ diff --git a/labels/validate.go b/labels/validate.go new file mode 100644 index 000000000..16cf26f82 --- /dev/null +++ b/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 +} diff --git a/labels/validate_test.go b/labels/validate_test.go new file mode 100644 index 000000000..7f08768e0 --- /dev/null +++ b/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") + } + } +} diff --git a/metadata/containers.go b/metadata/containers.go index 90a33e553..e0f047a5f 100644 --- a/metadata/containers.go +++ b/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") } diff --git a/metadata/content.go b/metadata/content.go index 744cf2538..e29d07d74 100644 --- a/metadata/content.go +++ b/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 diff --git a/metadata/images.go b/metadata/images.go index a44b3a7f4..fee5dac0b 100644 --- a/metadata/images.go +++ b/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 diff --git a/metadata/namespaces.go b/metadata/namespaces.go index 4053733c5..f822c33c7 100644 --- a/metadata/namespaces.go +++ b/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)) diff --git a/metadata/snapshot.go b/metadata/snapshot.go index 5ca4a6fa1..254bc1f0f 100644 --- a/metadata/snapshot.go +++ b/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 +} diff --git a/services/content/service.go b/services/content/service.go index 7a5e6d6d0..eeff5d1ef 100644 --- a/services/content/service.go +++ b/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 {