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
Shell completion for plugins #105867
Shell completion for plugins #105867
Conversation
Hi @marckhouzam. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Thanks for taking this forward, is there any reasons we would not be able to use a conventional flag across plugins, i.e: |
I agree such an approach would be a more elegant solution. It was discussed in #74178 early on but then we realized that it would create a backwards-compatibility issue. If a plugin does not support shell completion but So we need a solution that also indicates to
I agree there too. We could hide the files using a dot prefix: We could instead use an agreed upon location that is not on PATH, e.g., For this draft I mimicked the current plugin approach for simplicity, but I'd appreciate some guidance on what would be the best approach. |
@marccampbell I'm sure y'all will have some thoughts here. |
/assign |
@marckhouzam Hey Marc! Also, in your example - I don't see what list of values would be suggested as completions for your plugin, could you explain how it'd work? |
Hi @Drugoy. Currently, if you want to try the proposed solution, then you need to rebuild kubectl using this PR and then you need to provide an executable file called So if you take the
This is a simple example, but you would have to also handle flags if the user typed a Maybe the helm documentation can help? I'm proposing to do the same thing for kubectl plugins: https://helm.sh/docs/topics/plugins/#providing-shell-auto-completion |
@marckhouzam, thanks for explaining in details. 2 simple kubectl plugin example ideas: say, I don't like to remember how to use kubectl to do a few things, so, instead, I'd like to call:
In the first example kubectl parses kube-config to list existing contexts. p.s.: IMO such completion cases should be treated like specials and there shouldn't be necessity to provide any extra |
For dynamic completions your
Such a script will provide all namespace names as completions for that plugin.
Where would you reference such completions function? You need the |
@marckhouzam IMO, kubectl plugins completions should follow this approach and also use functions. As for your bash scripts illustrating how to provide completions for kubectl plugins - it looks like the only requisite for completion scripts/functions is that they return a list of items, where each item should be on its own line. But that was an illustration that provided only a single completion list, while kubectl has multiple completion lists: some sub-commands have arguments (like What'd you suggest to do here? |
This is an interesting point. IIUC, with such an approach, for a plugin named
You would have to write a single smart script or program that would check the arguments passed to it and print the proper completions based on that. Here is an example of a helm plugin for which I wrote this completion script: https://github.com/marckhouzam/helm-fullstatus/blob/master/plugin.complete I realize it can be quite complicated to write a proper completion script depending on the complexity of the plugin, but I don't think there is anything Another way to make plugin completion very easy is to use a framework that provides support for completions. For example, plugins written in Go using the Cobra project, can provide completions much more easily; I give such an example for |
Something like that, yeah.
I don't really have an explanation: I tried to understand how I see that if I call
Well, yeah, since that's what the currently documented approach of configuring shell completions for
In the source code of kubectl I actually see instructions for fish and powershell as well.
Maybe you are right. I see that |
I know how you feel, it took me a very long time to understand it and I'm still learning 😉
This has changed with kubectl 1.22 which introduces (through the Cobra project), the use of the
You are right. The way to do that is to call |
761f71a
to
3963fbf
Compare
Alright, I've finally rebased this PR and posted a PR to update the plugin KEP to describe the proposed shell-completion solution for plugins: kubernetes/enhancements#3496 I've also added to this PR support for shell completion for the Hopefully we can get this in to 1.26 🤞 |
/remove-lifecycle stale |
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor nits here and there, but all looks great - thank you! And apologies for the long delays 😅
/lgtm
/approve
// plugin. This is only done when performing shell completion that relate | ||
// to plugins. | ||
func SetupPluginCompletion(cmd *cobra.Command, args []string) { | ||
if len(args) > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: if you reverse this condition, the code will be easier to read without unnecessary indentation.
if len(args) == 0 {
return
}
// the rest of the code goes here...
|
||
var comps []string | ||
directive := cobra.ShellCompDirectiveDefault | ||
if err := prog.Run(); err == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same nit as above, fail fast to limit the indentation 😉
if err := prog.Run(); err != nil {
return comps, directive
}
// the rest of the code goes here...
/triage accepted |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: marckhouzam, soltysh 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:
Approvers can indicate their approval by writing |
@marckhouzam Great work! 👏 🎉 |
This proposal was discussed in kubernetes/kubernetes#105867 Signed-off-by: Marc Khouzam <marc.khouzam@gmail.com>
For this to work, you will need kubectl 1.26 and a shim that you can create with the following command: cat <<'EOF' >~/bin/kubectl_complete-view_secret && chmod u+x ~/bin/kubectl_complete-view_secret kubectl view-secret __complete "$@" EOF See: kubernetes/kubernetes#105867
What type of PR is this?
/kind feature
/sig cli
What this PR does / why we need it:
This PR does three things:
1- includes plugin names during kubectl shell completion (cherry-pick of @superbrothers's commit from #76561)
2- allows a plugin to provide completions through an executable script named
kubectl_complete-<pluginName>
present on$PATH
3- adds support for shell completion to the
sample-cli-plugin
Which issue(s) this PR fixes:
Fixes #74178
Special notes for your reviewer:
Tests are missing at the moment.
When doing completion for arguments or flags for a plugin, kubectl will call
kubectl_complete-<plugin>
to obtain the list of completions. For example, for thekrew
plugin, when the user triggers completion by doing:kubectl will look on
$PATH
for an executable file calledkubectl_complete-krew
. This file should print the list of valid completions for the plugin to stdout. kubectl will then present these completion choices to the user. This approach is the one used by the Helm project for its plugins: https://helm.sh/docs/topics/plugins/#dynamic-completionBackwards-compatibility:
The solution is backwards-compatible with plugins in both cases:
kubectl
using this solutionkubectl
Krew:
From what I can see from
krew
(I'm no expert), there is no current way to deliver a separate executablekubectl_complete-<plugin>
completion file. Therefore, if this solution is chosen, it may be interesting to add support for it inkrew
.Explanation:
krew
installs one plugin binary on $PATH using the plugin naming convention. That means that the only way for a plugin completion solution to work out-of-the-box withkrew
would be to call the binary plugin itself to get completions (using a special sub-command or flag). This is not a viable solution because if a plugin ignores arguments and flags, calling it to obtain completions will mistakenly run the actual plugin logic.How to test
Manually creating a completion script:
For zsh (but very similar for bash, fish or powershell).
We need a plugin to provide completion through a new
kubectl_complete-<pluginName>
file. Let's create one ourselves. Let's do it for thekrew
plugin, which I assume many in the community have installed. Sincekrew
uses Cobra, it has the__complete
command, which makes it very easy to obtain completions, as will become clear below:Please put the
kubectl_complete-krew
file somewhere on your $PATH. Then:Automatically generating completion scripts:
I've written a new plugin, "plugin-completion", which automatically generates the proper shell completion script for installed plugins that use Cobra. It works with more than half the plugins registered with krew. It can help test this PR quite easily.
NOTE: Flag completion for plugins works with Cobra 1.3 which is now part of the master branch.
Does this PR introduce a user-facing change?