Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upzsh completion: complete capabilities after --cap-{add,drop} #2485
Conversation
c4b3de5
to
038b129
Possible values and descriptions are taken from https://docs.docker.com/engine/reference/run/#runtime-privilege-and-linux-capabilities The distinction of default-on and default-off capabilities is inspired by the bash completion. A small check if --cap-{add,drop}=ALL is present in the command line is done, if so all capabilities (regardless of their default) are suggested. No other clever behaviour is added (i.e. a capability that's already added will be suggested again in --cap-add=). Signed-off-by: Paul Seyfert <pseyfert.mathphys@gmail.com>
Signed-off-by: Paul Seyfert <pseyfert.mathphys@gmail.com>
|
Thanks! I left a comment with some suggestions |
| # list and description taken from https://docs.docker.com/engine/reference/run/#runtime-privilege-and-linux-capabilities | ||
| local -a default_off_capabilities default_on_capabilities all_capabilities | ||
| default_off_capabilities=( | ||
| "SYS_MODULE:Load and unload kernel modules." |
thaJeztah
May 7, 2020
Member
Could you
- sort these lists alphabetically,
- also add
ALL to the list?
- double check if this list matches the Bash completion; https://github.com/docker/cli/blob/master/contrib/completion/bash/docker#L834-L882
- Perhaps change the names of
default_off_capabilities and default_on_capabilities to capabilities_droppable / capabilities_droppable / capabilities_addable (to match the bash completion)?
Could you
- sort these lists alphabetically,
- also add
ALLto the list? - double check if this list matches the Bash completion; https://github.com/docker/cli/blob/master/contrib/completion/bash/docker#L834-L882
- Perhaps change the names of
default_off_capabilitiesanddefault_on_capabilitiestocapabilities_droppable/capabilities_droppable/capabilities_addable(to match the bash completion)?
pseyfert
May 7, 2020
•
Author
sort alphabetically
sure
add ALL to the list
not sure i understand the suggestion. in line 264 and 265 ALL gets added to the suggestions in any case, so I handle this special capability separate instead of adding it to both lists. Did you overlook that or do you suggest dropping the special treatment and adding ALL to the default_off and default_on lists?
double check against bash
will do
change the names
I like your suggestion
sort alphabetically
sure
add
ALLto the list
not sure i understand the suggestion. in line 264 and 265 ALL gets added to the suggestions in any case, so I handle this special capability separate instead of adding it to both lists. Did you overlook that or do you suggest dropping the special treatment and adding ALL to the default_off and default_on lists?
double check against bash
will do
change the names
I like your suggestion
pseyfert
May 7, 2020
Author
✓ sorted alphabetically
✓ compared against bash: the only difference I spotted is that my alphabetic sorting put SYSLOG into a different position
✓ renamed the arrays (and rephrased the labels)
→ waiting for clarification on ALL. I thought back why I treated ALL separately in the first draft. I had decided in favor of that when I was considering to obtain the list of capabilities in an automated way rather than hardcoding them. Since I didn't go for that, the reason for three instead of two arrays is gone. Having a "suggest for drop" "suggest for add" and "suggest always" list kind of eases things up when completing docker run --cap-add=ALL --cap-drop= (because then ALL doesn't appear twice in the suggestions (in the "droppable (on by default)" and the "droppable (enabled through ALL)") but if you prefer two lists for other reasons that's fine with me
✓ sorted alphabetically
✓ compared against bash: the only difference I spotted is that my alphabetic sorting put SYSLOG into a different position
✓ renamed the arrays (and rephrased the labels)
→ waiting for clarification on ALL. I thought back why I treated ALL separately in the first draft. I had decided in favor of that when I was considering to obtain the list of capabilities in an automated way rather than hardcoding them. Since I didn't go for that, the reason for three instead of two arrays is gone. Having a "suggest for drop" "suggest for add" and "suggest always" list kind of eases things up when completing docker run --cap-add=ALL --cap-drop= (because then ALL doesn't appear twice in the suggestions (in the "droppable (on by default)" and the "droppable (enabled through ALL)") but if you prefer two lists for other reasons that's fine with me
Signed-off-by: Paul Seyfert <pseyfert.mathphys@gmail.com>
|
ping? |
601eeb5
to
44328d8
- What I did
Add a new zsh function to the completion file
contrib/completion/zsh/_docker.The bulk of the function is copy and paste from the documentation of
--cap-add(Subject to review: I barely read through them, assuming the docs on the web are a good initial version. I did not investigate if the list of capabilities can be obtained in an automated manner.).The function is called when tab completing
docker run --cap-add=(ordrop).The suggested completion should look like that (depending on a user's configuration of zsh's tab completion):
The function checks:
if
--cap-addor--cap-dropis completed. In the former case, off-by-default capabilities are suggested. In the latter case on-by-default capabilities are suggested (I took that idea from the bash completion).if
--cap-add=ALLor--cap-drop=ALLare present on the current command line. If--cap-add=ALLis present in the command line, then--cap-drop=will also suggest off-by-default capabilities (and vice versa). This should look something like that:The function does not check any other already-present settings of
--cap-add. I.e.docker run --cap-add=BLOCK_SUSPEND --cap-add=<TAB>will still suggestBLOCK_SUSPENDas option.(Subject for discussion. My impression is it's not worth increasing the complexity of the completion function for such a corner case.)
- How I did it
Sticking to zsh's builtin features. I use the
opt_argsassociative array todetect what's already specified in
--cap-addon the command line. If theoption is given multiple times,
$opt_args[--cap-add]will be colon separated.I didn't add any logic to handle that case but rely on
*ALL*matching exactlythe
ALLvalue andALLdoesn't occur in any other capability.- How to verify it
Typed interactively a few examples
- Description for the changelog
Add suggestions for
docker run --cap-{add,drop}=<TAB>in the zsh completion- A picture of a cute animal (not mandatory but encouraged)