8000 rkt: skip parsing in case of an empty string by dongsupark · Pull Request #3822 · rkt/rkt · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Feb 24, 2020. It is now read-only.

rkt: skip parsing in case of an empty string #3822

Merged
merged 1 commit into from
Oct 11, 2017

Conversation

dongsupark
Copy link

When any string in []args is empty, "", then the al.Count() gets still incremented, even if there's nothing to parse. As a result, rkt app add fails with an error message like must give only one app, even when only one app name is given.

For example, rktlet runs the following command:

rkt app --insecure-options=image,ondisk add 58992c0a-540d-40a4-bf39-4bb193d74f94 \
docker://busybox:1.26 --name=2-seccomp-container-docker-default-d25eafce-ad02-11e7-a670-2eecc98c9398d25eaff2-ad02-11e7-a670-2eecc98c9398 \
--user-annotation=k8s.io/reserved/image-name=busybox:1.26  --exec=top

Then the last part, " --exec=top" is parsed to a list of 2 separate strings: " " and "--exec=top", That's why al.Count() becomes 2.

See also kubernetes-retired/rktlet#146 (comment)

When any string in []args is empty, "", then the `al.Count()` gets
still incremented, even if there's nothing to parse. As a result, `rkt
app add` fails with an error message like `must give only one app`,
even when only one app name is given.

For example, rktlet runs the following command:

```
rkt app --insecure-options=image,ondisk add 58992c0a-540d-40a4-bf39-4bb193d74f94 \
docker://busybox:1.26 --name=2-seccomp-container-docker-default-d25eafce-ad02-11e7-a670-2eecc98c9398d25eaff2-ad02-11e7-a670-2eecc98c9398 \
--user-annotation=k8s.io/reserved/image-name=busybox:1.26  --exec=top
```

Then the last part, "  --exec=top" is parsed a list of 2 separate
strings: " " and "--exec=top", That's why `al.Count()` becomes 2.

See also kubernetes-retired/rktlet#146 (comment)
Copy link
Member
@iaguis iaguis left a comment

Choose a reason for hiding this comment

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

lgtm

@iaguis iaguis merged commit ac9ecac into rkt:master Oct 11, 2017
@iaguis iaguis added this to the 1.30.0 milestone Apr 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0