-
Notifications
You must be signed in to change notification settings - Fork 41
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
Add CRUD support for spaces groups #466
Conversation
cmd/up/group/cmd.go
Outdated
Get getCmd `cmd:"" help:"Get a group."` | ||
|
||
Short bool `short:"s" env:"UP_SHORT" name:"short" help:"Short output."` | ||
KubeContext string `env:"UP_CONTEXT" default:"upbound" name:"context" help:"Kubernetes context to operate on."` |
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.
is this one used?
I think we have to uniformely wire this through into GetCurrentContext(ctx)
.
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.
Maybe also an idea, as we get more and more CRUD commands, we could create some common set of flags for CRUD.
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.
Actually neither of these other CLI args are used - I was just looking over that myself. What is the purpose of this arg, anyway? Just another way to override the current context of the kubeconfig?
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, another way to override the kubeconfig/context.
a22ca00
to
ca18076
Compare
Description of your changes
Rebased on top of #457
Add support for
up group create/list/get/delete
using the selected spaces kubeconfig.I have:
make reviewable
to ensure this PR is ready for review.backport release-x.y
labels to auto-backport this PR, as appropriate.How has this code been tested