From 7259a6315c7f7d97665d928de6357fc3cbcae136 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Tue, 26 Apr 2022 15:53:36 -0400 Subject: [PATCH] Truncate annotations when generating kubernetes yaml files Kubernetes only allows 63 characters in an annotation. Make sure that we only add 63 or less charaters when generating kube. Warn if containers or pods have longer length and truncate. Discussion: https://github.com/containers/podman/discussions/13901 Fixes: https://github.com/containers/podman/issues/13962 Signed-off-by: Daniel J Walsh --- cmd/podman/play/kube.go | 6 ++++- libpod/define/annotations.go | 2 ++ libpod/kube.go | 38 ++++++++++++++++++++-------- pkg/domain/infra/abi/play.go | 6 ++++- pkg/machine/e2e/config_init.go | 2 +- pkg/machine/e2e/machine_test.go | 2 +- test/system/700-play.bats | 45 +++++++++++++++++++++++++++++++++ 7 files changed, 87 insertions(+), 14 deletions(-) diff --git a/cmd/podman/play/kube.go b/cmd/podman/play/kube.go index e92516eb46..40d14a609b 100644 --- a/cmd/podman/play/kube.go +++ b/cmd/podman/play/kube.go @@ -178,7 +178,11 @@ func kube(cmd *cobra.Command, args []string) error { if kubeOptions.Annotations == nil { kubeOptions.Annotations = make(map[string]string) } - kubeOptions.Annotations[splitN[0]] = splitN[1] + annotation := splitN[1] + if len(annotation) > define.MaxKubeAnnotation { + return errors.Errorf("annotation exceeds maximum size, %d, of kubernetes annotation: %s", define.MaxKubeAnnotation, annotation) + } + kubeOptions.Annotations[splitN[0]] = annotation } yamlfile := args[0] if yamlfile == "-" { diff --git a/libpod/define/annotations.go b/libpod/define/annotations.go index a83fbff0ba..8f52799812 100644 --- a/libpod/define/annotations.go +++ b/libpod/define/annotations.go @@ -135,6 +135,8 @@ const ( // creating a checkpoint image to specify the name of host distribution on // which the checkpoint was created. CheckpointAnnotationDistributionName = "io.podman.annotations.checkpoint.distribution.name" + // MaxKubeAnnotation is the max length of annotations allowed by Kubernetes. + MaxKubeAnnotation = 63 ) // IsReservedAnnotation returns true if the specified value corresponds to an diff --git a/libpod/kube.go b/libpod/kube.go index eb62643fe2..2e2b956d92 100644 --- a/libpod/kube.go +++ b/libpod/kube.go @@ -10,6 +10,7 @@ import ( "strconv" "strings" "time" + "unicode/utf8" "github.com/containers/common/libnetwork/types" "github.com/containers/common/pkg/config" @@ -288,6 +289,16 @@ func newServicePortState() servicePortState { } } +func TruncateKubeAnnotation(str string) string { + str = strings.TrimSpace(str) + if utf8.RuneCountInString(str) < define.MaxKubeAnnotation { + return str + } + trunc := string([]rune(str)[:define.MaxKubeAnnotation]) + logrus.Warnf("Truncation Annotation: %q to %q: Kubernetes only allows %d characters", str, trunc, define.MaxKubeAnnotation) + return trunc +} + // containerPortsToServicePorts takes a slice of containerports and generates a // slice of service ports func (state *servicePortState) containerPortsToServicePorts(containerPorts []v1.ContainerPort) ([]v1.ServicePort, error) { @@ -348,11 +359,13 @@ func (p *Pod) podWithContainers(ctx context.Context, containers []*Container, po for _, ctr := range containers { if !ctr.IsInfra() { + for k, v := range ctr.config.Spec.Annotations { + podAnnotations[fmt.Sprintf("%s/%s", k, removeUnderscores(ctr.Name()))] = TruncateKubeAnnotation(v) + } // Convert auto-update labels into kube annotations - for k, v := range getAutoUpdateAnnotations(removeUnderscores(ctr.Name()), ctr.Labels()) { - podAnnotations[k] = v + for k, v := range getAutoUpdateAnnotations(ctr.Name(), ctr.Labels()) { + podAnnotations[k] = TruncateKubeAnnotation(v) } - isInit := ctr.IsInitCtr() ctr, volumes, _, annotations, err := containerToV1Container(ctx, ctr) @@ -360,7 +373,7 @@ func (p *Pod) podWithContainers(ctx context.Context, containers []*Container, po return nil, err } for k, v := range annotations { - podAnnotations[define.BindMountPrefix+k] = strings.TrimSpace(v) + podAnnotations[define.BindMountPrefix+k] = TruncateKubeAnnotation(v) } // Since port bindings for the pod are handled by the // infra container, wipe them here. @@ -466,10 +479,14 @@ func simplePodWithV1Containers(ctx context.Context, ctrs []*Container) (*v1.Pod, kubeAnnotations := make(map[string]string) ctrNames := make([]string, 0, len(ctrs)) for _, ctr := range ctrs { - ctrNames = append(ctrNames, strings.ReplaceAll(ctr.Name(), "_", "")) + ctrNames = append(ctrNames, removeUnderscores(ctr.Name())) + for k, v := range ctr.config.Spec.Annotations { + kubeAnnotations[fmt.Sprintf("%s/%s", k, removeUnderscores(ctr.Name()))] = TruncateKubeAnnotation(v) + } + // Convert auto-update labels into kube annotations - for k, v := range getAutoUpdateAnnotations(removeUnderscores(ctr.Name()), ctr.Labels()) { - kubeAnnotations[k] = v + for k, v := range getAutoUpdateAnnotations(ctr.Name(), ctr.Labels()) { + kubeAnnotations[k] = TruncateKubeAnnotation(v) } isInit := ctr.IsInitCtr() @@ -482,7 +499,7 @@ func simplePodWithV1Containers(ctx context.Context, ctrs []*Container) (*v1.Pod, return nil, err } for k, v := range annotations { - kubeAnnotations[define.BindMountPrefix+k] = strings.TrimSpace(v) + kubeAnnotations[define.BindMountPrefix+k] = TruncateKubeAnnotation(v) } if isInit { kubeInitCtrs = append(kubeInitCtrs, kubeCtr) @@ -523,7 +540,7 @@ func simplePodWithV1Containers(ctx context.Context, ctrs []*Container) (*v1.Pod, } } // end if ctrDNS } - podName := strings.ReplaceAll(ctrs[0].Name(), "_", "") + podName := removeUnderscores(ctrs[0].Name()) // Check if the pod name and container name will end up conflicting // Append -pod if so if util.StringInSlice(podName, ctrNames) { @@ -1051,12 +1068,13 @@ func getAutoUpdateAnnotations(ctrName string, ctrLabels map[string]string) map[s autoUpdateLabel := "io.containers.autoupdate" annotations := make(map[string]string) + ctrName = removeUnderscores(ctrName) for k, v := range ctrLabels { if strings.Contains(k, autoUpdateLabel) { // since labels can variate between containers within a pod, they will be // identified with the container name when converted into kube annotations kc := fmt.Sprintf("%s/%s", k, ctrName) - annotations[kc] = v + annotations[kc] = TruncateKubeAnnotation(v) } } diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index 1d347ed8cc..c5c31c4889 100644 --- a/pkg/domain/infra/abi/play.go +++ b/pkg/domain/infra/abi/play.go @@ -78,7 +78,11 @@ func (ic *ContainerEngine) PlayKube(ctx context.Context, body io.Reader, options podTemplateSpec.ObjectMeta = podYAML.ObjectMeta podTemplateSpec.Spec = podYAML.Spec - + for name, val := range podYAML.Annotations { + if len(val) > define.MaxKubeAnnotation { + return nil, errors.Errorf("invalid annotation %q=%q value length exceeds Kubernetetes max %d", name, val, define.MaxKubeAnnotation) + } + } for name, val := range options.Annotations { if podYAML.Annotations == nil { podYAML.Annotations = make(map[string]string) diff --git a/pkg/machine/e2e/config_init.go b/pkg/machine/e2e/config_init.go index 55218221d8..2340a11331 100644 --- a/pkg/machine/e2e/config_init.go +++ b/pkg/machine/e2e/config_init.go @@ -12,7 +12,7 @@ type initMachine struct { --image-path string Path to qcow image (default "testing") -m, --memory uint Memory in MB (default 2048) --now Start machine now - --rootful Whether this machine should prefer rootful container exectution + --rootful Whether this machine should prefer rootful container execution --timezone string Set timezone (default "local") -v, --volume stringArray Volumes to mount, source:target --volume-driver string Optional volume driver diff --git a/pkg/machine/e2e/machine_test.go b/pkg/machine/e2e/machine_test.go index 46fe180698..2b3b60b2b3 100644 --- a/pkg/machine/e2e/machine_test.go +++ b/pkg/machine/e2e/machine_test.go @@ -116,7 +116,7 @@ func teardown(origHomeDir string, testDir string, mb *machineTestBuilder) { s := new(stopMachine) for _, name := range mb.names { if _, err := mb.setName(name).setCmd(s).run(); err != nil { - fmt.Printf("error occured rm'ing machine: %q\n", err) + fmt.Printf("error occurred rm'ing machine: %q\n", err) } } if err := os.RemoveAll(testDir); err != nil { diff --git a/test/system/700-play.bats b/test/system/700-play.bats index 8af4cd25b2..b0624cbf22 100644 --- a/test/system/700-play.bats +++ b/test/system/700-play.bats @@ -233,3 +233,48 @@ _EOF run_podman stop -a -t 0 run_podman pod rm -t 0 -f test_pod } + +@test "podman play --annotation > Max" { + TESTDIR=$PODMAN_TMPDIR/testdir + RANDOMSTRING=$(random_string 65) + mkdir -p $TESTDIR + echo "$testYaml" | sed "s|TESTDIR|${TESTDIR}|g" > $PODMAN_TMPDIR/test.yaml + run_podman 125 play kube --annotation "name=$RANDOMSTRING" $PODMAN_TMPDIR/test.yaml + assert "$output" =~ "annotation exceeds maximum size, 63, of kubernetes annotation:" "Expected to fail with Length greater than 63" +} + +@test "podman play Yaml with annotation > Max" { + RANDOMSTRING=$(random_string 65) + testBadYaml=" +apiVersion: v1 +kind: Pod +metadata: + annotations: + test: ${RANDOMSTRING} + labels: + app: test + name: test_pod +spec: + containers: + - command: + - id + env: + - name: PATH + value: /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin + - name: TERM + value: xterm + - name: container + + value: podman + image: quay.io/libpod/userimage + name: test + resources: {} +status: {} +" + TESTDIR=$PODMAN_TMPDIR/testdir + mkdir -p $TESTDIR + echo "$testBadYaml" | sed "s|TESTDIR|${TESTDIR}|g" > $PODMAN_TMPDIR/test.yaml + + run_podman 125 play kube - < $PODMAN_TMPDIR/test.yaml + assert "$output" =~ "invalid annotation \"test\"=\"$RANDOMSTRING\"" "Expected to fail with annotation length greater than 63" +}