From 2a8dac12a7629c1c9f1be66c40e6cd5d25d9c70b Mon Sep 17 00:00:00 2001 From: Sambhav Kothari Date: Thu, 14 Oct 2021 10:46:04 +0100 Subject: [PATCH] Output a warning for label image labels instead of erroring This change ignore errors during container runtime due to large image labels and instead outputs warning. This is necessary as certain image building tools like buildpacks may have large labels in the images which need not be passed to the container. Signed-off-by: Sambhav Kothari --- cmd/ctr/commands/run/run.go | 9 ++++++++- pkg/cri/server/helpers.go | 11 ++++++++++- pkg/cri/server/helpers_test.go | 7 +++++-- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/cmd/ctr/commands/run/run.go b/cmd/ctr/commands/run/run.go index 1e5191324..4a5fd56dd 100644 --- a/cmd/ctr/commands/run/run.go +++ b/cmd/ctr/commands/run/run.go @@ -29,6 +29,7 @@ import ( "github.com/containerd/containerd/cmd/ctr/commands" "github.com/containerd/containerd/cmd/ctr/commands/tasks" "github.com/containerd/containerd/containers" + clabels "github.com/containerd/containerd/labels" "github.com/containerd/containerd/namespaces" "github.com/containerd/containerd/oci" gocni "github.com/containerd/go-cni" @@ -254,7 +255,13 @@ func fullID(ctx context.Context, c containerd.Container) string { func buildLabels(cmdLabels, imageLabels map[string]string) map[string]string { labels := make(map[string]string) for k, v := range imageLabels { - labels[k] = v + if err := clabels.Validate(k, v); err == nil { + labels[k] = v + } else { + // In case the image label is invalid, we output a warning and skip adding it to the + // container. + logrus.WithError(err).Warnf("unable to add image label with key %s to the container", k) + } } // labels from the command line will override image and the initial image config labels for k, v := range cmdLabels { diff --git a/pkg/cri/server/helpers.go b/pkg/cri/server/helpers.go index d29a42bf8..42d186380 100644 --- a/pkg/cri/server/helpers.go +++ b/pkg/cri/server/helpers.go @@ -26,6 +26,7 @@ import ( "github.com/containerd/containerd" "github.com/containerd/containerd/containers" "github.com/containerd/containerd/errdefs" + clabels "github.com/containerd/containerd/labels" criconfig "github.com/containerd/containerd/pkg/cri/config" containerstore "github.com/containerd/containerd/pkg/cri/store/container" imagestore "github.com/containerd/containerd/pkg/cri/store/image" @@ -36,6 +37,7 @@ import ( "github.com/containerd/containerd/runtime/linux/runctypes" runcoptions "github.com/containerd/containerd/runtime/v2/runc/options" "github.com/containerd/typeurl" + "github.com/sirupsen/logrus" runhcsoptions "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options" imagedigest "github.com/opencontainers/go-digest" @@ -285,8 +287,15 @@ func filterLabel(k, v string) string { // buildLabel builds the labels from config to be passed to containerd func buildLabels(configLabels, imageConfigLabels map[string]string, containerType string) map[string]string { labels := make(map[string]string) + for k, v := range imageConfigLabels { - labels[k] = v + if err := clabels.Validate(k, v); err == nil { + labels[k] = v + } else { + // In case the image label is invalid, we output a warning and skip adding it to the + // container. + logrus.WithError(err).Warnf("unable to add image label with key %s to the container", k) + } } // labels from the CRI request (config) will override labels in the image config for k, v := range configLabels { diff --git a/pkg/cri/server/helpers_test.go b/pkg/cri/server/helpers_test.go index aacb14586..c0c1c6e75 100644 --- a/pkg/cri/server/helpers_test.go +++ b/pkg/cri/server/helpers_test.go @@ -19,6 +19,7 @@ package server import ( "context" "os" + "strings" "testing" "time" @@ -119,8 +120,9 @@ func TestGetRepoDigestAndTag(t *testing.T) { func TestBuildLabels(t *testing.T) { imageConfigLabels := map[string]string{ - "a": "z", - "d": "y", + "a": "z", + "d": "y", + "long-label": strings.Repeat("example", 10000), } configLabels := map[string]string{ "a": "b", @@ -132,6 +134,7 @@ func TestBuildLabels(t *testing.T) { assert.Equal(t, "d", newLabels["c"]) assert.Equal(t, "y", newLabels["d"]) assert.Equal(t, containerKindSandbox, newLabels[containerKindLabel]) + assert.NotContains(t, newLabels, "long-label") newLabels["a"] = "e" assert.Empty(t, configLabels[containerKindLabel], "should not add new labels into original label")