Output a warning for label image labels instead of erroring

sambhav · ktock · commit b988fc918ad3 · 2021-10-31T14:44:35.000+09:00
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 <sambhavs.email@gmail.com> (cherry picked from commit 2a8dac1) Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
diff --git 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"
@@ -252,7 +253,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
@@ -26,6 +26,7 @@ import (
 	runhcsoptions "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options"
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/containers"
+	clabels "github.com/containerd/containerd/labels"
 	"github.com/containerd/containerd/plugin"
 	"github.com/containerd/containerd/reference/docker"
 	"github.com/containerd/containerd/runtime/linux/runctypes"
@@ -34,6 +35,7 @@ import (
 	imagedigest "github.com/opencontainers/go-digest"
 	"github.com/pelletier/go-toml"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 	"golang.org/x/net/context"
 	runtime "k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
 
@@ -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
@@ -19,6 +19,7 @@ package server
 import (
 	"context"
 	"io/ioutil"
+	"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")
