Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Truncate annotations when generating kubernetes yaml files #14023

Merged
merged 1 commit into from
Apr 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion cmd/podman/play/kube.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 == "-" {
Expand Down
2 changes: 2 additions & 0 deletions libpod/define/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 28 additions & 10 deletions libpod/kube.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"strconv"
"strings"
"time"
"unicode/utf8"

"github.com/containers/common/libnetwork/types"
"github.com/containers/common/pkg/config"
Expand Down Expand Up @@ -288,6 +289,16 @@ func newServicePortState() servicePortState {
}
}

func TruncateKubeAnnotation(str string) string {
str = strings.TrimSpace(str)
if utf8.RuneCountInString(str) < define.MaxKubeAnnotation {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, one more. The only spec I can find (I didn't look too hard) mentions 63 characters but doesn't actually indicate if we're talking new-style Unicode or old-style ASCII C chars. Does anyone know how to figure this one out?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IDK, but I"m interested in seeing the answer....

Copy link
Member

@Luap99 Luap99 Apr 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is also inconsistent since in cmd/podman/play/kube.go the byte len is checked and not utf8 len.

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) {
Expand Down Expand Up @@ -348,19 +359,21 @@ 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)
if err != nil {
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.
Expand Down Expand Up @@ -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()
Expand All @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
}

Expand Down
6 changes: 5 additions & 1 deletion pkg/domain/infra/abi/play.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/machine/e2e/config_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/machine/e2e/machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
45 changes: 45 additions & 0 deletions test/system/700-play.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}