-
Notifications
You must be signed in to change notification settings - Fork 8
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
Make the binary dynamically determine whether it is a kubectl plugin #61
Conversation
Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
Looks to work for me, help prints the correct binary and autocomplete works (assuming you have that set up as per cert-manager/cert-manager#4657) One thing we could do though, is if we detect the binary name, could we detect |
I've tried it, I had to rename the binary after running go install . && mv $(go env GOPATH)/bin/{cmctl,kubectl-cert_manager} Installing the "completion" is... weird. It is a shim rather than a completion script. I think I prefer Adam's idea. With Adam's idea, instead of:
It would be: ln -s $(which kubectl-cert_manager) $(dirname $(which kubectl-cert_manager))/kubectl_complete-cert-manager No script needs to be created, and What do you think? |
I'll investigate if this is easily doable. |
I propose that we improve the completion logic in a separate PR. |
Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
Use: "convert", | ||
Short: "Convert cert-manager config files between different API versions", | ||
Long: templates.LongDesc(` |
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.
Does this have to do with "making the binary dynamically determine whether it is a kubectl plugin"?
Inlining these Long
paragraphs is a good idea, but it makes it extra hard to review the actual changes.
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.
Yes, I inlined these paragraphs because sometimes we call build.WithTemplate(setupCtx,
which takes the setupCtx
argument to determine if it is running as cmctl
or kubectl cert-manager
.
Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
Looks good! We will probably need some docs ready for the release telling people how to install |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: inteon 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 |
fixes #38
See https://krew.sigs.k8s.io/docs/developer-guide/develop/best-practices/
TODO: I would like to get some feedback on the
kubectl cert-manager
autocomplete functionality.UPDATE: autocomplete works as expected, additional improvements can be made in a separate PR
This PR adds a new completion command based on @maelvls' instructions to make kubectl completion work (cert-manager/cert-manager#4657):
./kubectl-cert-manager completion kubectl --help