diff options
author | OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com> | 2022-04-27 09:42:31 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-04-27 09:42:31 -0400 |
commit | bbe419ef9ffda699aa87ab0fcb2bdf50c0ff5058 (patch) | |
tree | 85bf123d2a7e1c3439d7a2da9e4656eabfbf1fb7 | |
parent | c44efb011003942adedb6ac74457be0123d23a02 (diff) | |
parent | 7259a6315c7f7d97665d928de6357fc3cbcae136 (diff) | |
download | podman-bbe419ef9ffda699aa87ab0fcb2bdf50c0ff5058.tar.gz podman-bbe419ef9ffda699aa87ab0fcb2bdf50c0ff5058.tar.bz2 podman-bbe419ef9ffda699aa87ab0fcb2bdf50c0ff5058.zip |
Merge pull request #14023 from rhatdan/kube
Truncate annotations when generating kubernetes yaml files
-rw-r--r-- | cmd/podman/play/kube.go | 6 | ||||
-rw-r--r-- | libpod/define/annotations.go | 2 | ||||
-rw-r--r-- | libpod/kube.go | 38 | ||||
-rw-r--r-- | pkg/domain/infra/abi/play.go | 6 | ||||
-rw-r--r-- | pkg/machine/e2e/config_init.go | 2 | ||||
-rw-r--r-- | pkg/machine/e2e/machine_test.go | 2 | ||||
-rw-r--r-- | 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 e92516eb4..40d14a609 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 a83fbff0b..8f5279981 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 cbb1a8799..8b75a0c44 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) { @@ -1057,12 +1074,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 5de475480..0da07bab8 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 55218221d..2340a1133 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 46fe18069..2b3b60b2b 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 8af4cd25b..b0624cbf2 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" +} |