rootless: not require userns for help/version #1268
Conversation
giuseppe
force-pushed
the
no-rootless-for-version
branch
from
aa902b5 to
97cf485
Compare
|
|
|
@giuseppe Can we add integration tests for those three cases? Just to avoid any potential regressions. |
giuseppe
force-pushed
the
no-rootless-for-version
branch
3 times, most recently
from
a7d9342 to
f4e4ab1
Compare
cmd/podman/main.go
Outdated
| "version", | ||
| } | ||
|
|
||
| func sliceContains(slice []string, s string) bool { |
There was a problem hiding this comment.
This function probably should live in utils.go.
giuseppe
force-pushed
the
no-rootless-for-version
branch
from
f4e4ab1 to
0ec54b6
Compare
|
@giuseppe |
cmd/podman/main.go
Outdated
| args := c.Args() | ||
| if args.Present() { | ||
| name := args.First() | ||
| if name != "" { |
There was a problem hiding this comment.
Isn't it impossible that if the args.Present() was true then args.First() == ""?
cmd/podman/main.go
Outdated
| if args.Present() { | ||
| name := args.First() | ||
| if name != "" { | ||
| requiresRootless := !sliceContains(cmdsNotRequiringRootless, name) |
There was a problem hiding this comment.
Why create a variable for this?
cmd/podman/main.go
Outdated
| @@ -25,19 +25,15 @@ var ( | |||
| exitCode = 125 | |||
| ) | |||
|
|
|||
| var cmdsNotRequiringRootless = []string{ | |||
| "help", | |||
There was a problem hiding this comment.
Why not make this a map, then you don't need to the string compare stuff below,
There was a problem hiding this comment.
yes that is better. I've pushed a new version
giuseppe
force-pushed
the
no-rootless-for-version
branch
from
0ec54b6 to
66ef382
Compare
these commands do not require to be root in an userns Closes: containers#1263 Signed-off-by: Giuseppe Scrivano <[email protected]>
giuseppe
force-pushed
the
no-rootless-for-version
branch
from
66ef382 to
a8b04a2
Compare
|
This LGTM, but you should update the title to remove info |
giuseppe
changed the title
|
done, I've updated the PR title |
|
📌 Commit a8b04a2 has been approved by |
|
☀️ Test successful - status-papr |
github-actions
bot
added
the
locked - please file new issue/PR
these commands do not require to be root in an userns
Closes: #1263
Signed-off-by: Giuseppe Scrivano [email protected]