Add --no-trunc flag to maintain original annotation length

[jakecorrenti]

Adds a --no-trunc flag to podman kube generate preventing the
annotations from being trimmed at 63 characters. However, due to
the fact the annotations will not be trimmed, any annotation that is
longer than 63 characters means this YAML will no longer be Kubernetes
compatible. However, these YAML files can still be used with podman kube play due to the addition of the new flag below.

Adds a --no-trunc flag to podman kube play supporting YAML files with
annotations that were not truncated to the Kubernetes maximum length of
63 characters.

Does this PR introduce a user-facing change?

Adds a `--no-trunc` flag to `podman kube generate` that does not truncate > 63 char annotations. Adds a `--no-trunc` flag to `podman kube play` that suppors YAML files with annotations that are longer than 63 characters. Warning: if an annotation is longer than 63 chars, then the generated yaml file is not Kubernetes compatible. 

[jakecorrenti]

@mheon @TomSweeneyRedHat ptal

[mheon]

Code LGTM

[mheon]

@edsantiago Mind looking at the tests?

[edsantiago]

Tests need work. If they pass, it's only by sheer coincidence. Please use something other than bind-mound-options. I'm sorry, I don't know enough about this to be able to offer a suggestion.

And, this is super super super important, never create files or directories under pwd.

[mheon]

You can add an annotation with --annotation to completely control length

[jakecorrenti]

On my end if I add an annotation with --annotation that is 64 characters it still gets truncated, if I'm understanding you correctly

[mheon]

That sounds like it could be a bug, I'd expect user-added annotations to survive untruncated

[edsantiago]

I haven't tested user annotations, but here's one possible way to do it with a tmpdir:

diff --git a/test/e2e/generate_kube_test.go b/test/e2e/generate_kube_test.go
index a938043e2..8732ecc48 100644
--- a/test/e2e/generate_kube_test.go
+++ b/test/e2e/generate_kube_test.go
@@ -1564,13 +1564,11 @@ USER test1`
 	})
 
 	It("podman generate kube --no-trunc on container", func() {
-		dirName := "demoDir"
 		ctrName := "demo"
-		err := os.Mkdir(dirName, os.ModePerm)
+		vol1 := filepath.Join(podmanTest.TempDir, RandomString(99))
+		err := os.MkdirAll(vol1, 0755)
 		Expect(err).ToNot(HaveOccurred())
-		defer os.RemoveAll(dirName)
-
-		session := podmanTest.Podman([]string{"run", "-v", "./" + dirName + ":/" + dirName + ":Z", "--name", ctrName, ALPINE})
+		session := podmanTest.Podman([]string{"run", "-v", vol1 + ":/tmp/foo:Z", "--name", ctrName, ALPINE})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
 
@@ -1582,7 +1580,7 @@ USER test1`
 		err = yaml.Unmarshal(kube.Out.Contents(), pod)
 		Expect(err).ToNot(HaveOccurred())
 
-		Expect(len(pod.Annotations["bind-mount-options"])).To(BeNumerically(">", 63))
+		Expect(pod.Annotations["bind-mount-options"]).To(Equal(vol1 + ":Z"))
 	})
 
 	It("podman generate kube --no-trunc on pod", func() {

[jakecorrenti]

I haven't tested user annotations, but here's one possible way to do it with a tmpdir:

diff --git a/test/e2e/generate_kube_test.go b/test/e2e/generate_kube_test.go
index a938043e2..8732ecc48 100644
--- a/test/e2e/generate_kube_test.go
+++ b/test/e2e/generate_kube_test.go
@@ -1564,13 +1564,11 @@ USER test1`
 	})
 
 	It("podman generate kube --no-trunc on container", func() {
-		dirName := "demoDir"
 		ctrName := "demo"
-		err := os.Mkdir(dirName, os.ModePerm)
+		vol1 := filepath.Join(podmanTest.TempDir, RandomString(99))
+		err := os.MkdirAll(vol1, 0755)
 		Expect(err).ToNot(HaveOccurred())
-		defer os.RemoveAll(dirName)
-
-		session := podmanTest.Podman([]string{"run", "-v", "./" + dirName + ":/" + dirName + ":Z", "--name", ctrName, ALPINE})
+		session := podmanTest.Podman([]string{"run", "-v", vol1 + ":/tmp/foo:Z", "--name", ctrName, ALPINE})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
 
@@ -1582,7 +1580,7 @@ USER test1`
 		err = yaml.Unmarshal(kube.Out.Contents(), pod)
 		Expect(err).ToNot(HaveOccurred())
 
-		Expect(len(pod.Annotations["bind-mount-options"])).To(BeNumerically(">", 63))
+		Expect(pod.Annotations["bind-mount-options"]).To(Equal(vol1 + ":Z"))
 	})
 
 	It("podman generate kube --no-trunc on pod", func() {

Thank you! This is miles better than anything I would have come up with

[TomSweeneyRedHat]

LGTM
once the tests and @edsantiago are hip

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago, jakecorrenti

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [edsantiago]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[edsantiago]

System tests are failing.

Also, before you repush, could you rebase? It's a really good habit to get into, your colleagues will silently thank you for it.

[edsantiago]

This gets the tests passing (and fixes a typo), but brings up the question of this being a behavior change. Possibly a desired one, as discussed above, but it merits a comment in the release note.

diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go
index fefaa8987..3aee0e6cf 100644
--- a/pkg/domain/infra/abi/play.go
+++ b/pkg/domain/infra/abi/play.go
@@ -230,7 +230,7 @@ func (ic *ContainerEngine) PlayKube(ctx context.Context, body io.Reader, options
 			podTemplateSpec.Spec = podYAML.Spec
 			for name, val := range podYAML.Annotations {
 				if len(val) > define.MaxKubeAnnotation {
-					logrus.Warnf("annotation %q=%q value length exceeds Kubernetetes max %d", name, val, define.MaxKubeAnnotation)
+					logrus.Warnf("annotation %q=%q value length exceeds Kubernetes max %d", name, val, define.MaxKubeAnnotation)
 				}
 			}
 			for name, val := range options.Annotations {
diff --git a/test/system/700-play.bats b/test/system/700-play.bats
index 62bfe2478..51ece0818 100644
--- a/test/system/700-play.bats
+++ b/test/system/700-play.bats
@@ -427,16 +427,16 @@ _EOF
     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"
+    run_podman play kube --annotation "name=$RANDOMSTRING" $PODMAN_TMPDIR/test.yaml
+    assert "$output" =~ "annotation exceeds maximum size, 63, of kubernetes annotation:" "Expected to warn with Length greater than 63"
 }
 
 @test "podman play Yaml with annotation > Max" {
    RANDOMSTRING=$(random_string 65)
 
    _write_test_yaml "annotations=test: ${RANDOMSTRING}" command=id
-    run_podman 125 play kube - < $PODMAN_TMPDIR/test.yaml
-    assert "$output" =~ "invalid annotation \"test\"=\"$RANDOMSTRING\"" "Expected to fail with annotation length greater than 63"
+    run_podman play kube - < $PODMAN_TMPDIR/test.yaml
+    assert "$output" =~ "warning.*annotation .*test.*=.*$RANDOMSTRING.* value length exceeds Kubernetes max 63" "Expected to warn with annotation length greater than 63"
 }
 
 @test "podman kube play - default log driver" {

[vrothberg]

Nice work, @jakecorrenti !

Final polishing and we're good to go.

@edsantiago PTAL

[umohnani8]

typo: s/maxmum/maximum

[umohnani8]

one typo otherwise LGTM

[TomSweeneyRedHat]

Ditto Urvashi's comment. Fix the typo, then LGTM

[jakecorrenti]

fixed and rebased

[TomSweeneyRedHat]

LGTM

[umohnani8]

/lgtm