Merge pull request #6187 from ktock/1.5-fix-config-label

[release/1.5] Output a warning for label image labels instead of erroring

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 {

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 {

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")