-
Notifications
You must be signed in to change notification settings - Fork 120
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 Kyma Access command to CLI #2067
Conversation
Skipping CI for Draft Pull Request. |
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.
In general, looks really ok. The only doubt I have is if it's not better to extract most functions that are creating Kubeconfig to the separate cluster and test it. But I don't want to force it in this PR. Maybe it would be better to create a separate PR.
internal/cmd/access/access.go
Outdated
cmd.Flags().StringVar(&cfg.name, "name", "", "Name of the Service Account to be created") | ||
cmd.Flags().StringVar(&cfg.clusterrole, "clusterrole", "", "Name of the cluster role to bind the Service Account") | ||
cmd.Flags().StringVar(&cfg.output, "output", "", "Path to the output kubeconfig file") | ||
cmd.Flags().StringVar(&cfg.namespace, "namespace", "default", "Namespace ") |
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.
I would suggest to improve the --namespace
flag :)
internal/cmd/access/access.go
Outdated
}, | ||
} | ||
_, err := cfg.KubeClient.Static().RbacV1().ClusterRoles().Create(cfg.Ctx, &cRole, metav1.CreateOptions{}) | ||
if client.IgnoreNotFound(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.
I would suggest to replace client.IngoreNotFound
with errors.IsAlreadyExists
from the k8s.io/apimachinery/pkg/api/errors
. I think the Create
func will never return a not found
error because in this case this func will just create it
* v1 * v12 * print * workingv1 * accesscommand * cut function into pieces this is my last resort * typo * cleanup * long * gomodtidy * review --------- Co-authored-by: Piotr Halama <[email protected]>
* v1 * v12 * print * workingv1 * accesscommand * cut function into pieces this is my last resort * typo * cleanup * long * gomodtidy * review --------- Co-authored-by: Piotr Halama <[email protected]>
Description
Adds a command that creates a kubeconfig that includes token and certificate for specially crafted service account with admin access rights
Related issue(s)