-
Notifications
You must be signed in to change notification settings - Fork 21
fix(kubeconfig): remove unused arg [local-path] from help text #89
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -29,6 +29,12 @@ func wrapKubeconfigCommand(wrappedCmd *cobra.Command, originalRunE func(*cobra.C | |||||||||||||||||||||||||||||||||||||||||||
| // Add --login flag to update system kubeconfig file instead of local one | ||||||||||||||||||||||||||||||||||||||||||||
| wrappedCmd.Flags().BoolP("login", "l", false, "update system kubeconfig file, not local one") | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| // Fix help text for unused arg [local-path] from talosctl kubeconfig command | ||||||||||||||||||||||||||||||||||||||||||||
| wrappedCmd.Use = "kubeconfig" | ||||||||||||||||||||||||||||||||||||||||||||
| wrappedCmd.Long = `Download the admin kubeconfig from the node. | ||||||||||||||||||||||||||||||||||||||||||||
| If merge flag is true, config will be merged with ~/.kube/config. | ||||||||||||||||||||||||||||||||||||||||||||
| Otherwise, kubeconfig will be written to PWD.` | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+32
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update help text to accurately reflect wrapped command behavior. The
Consider updating the description to accurately reflect the wrapped command's behavior: 🔎 Suggested help text revision- wrappedCmd.Long = `Download the admin kubeconfig from the node.
+ wrappedCmd.Long = `Download the admin kubeconfig from the node.
-If merge flag is true, config will be merged with ~/.kube/config.
-Otherwise, kubeconfig will be written to PWD.`
+By default, kubeconfig is written to the path specified in Chart.yaml
+globalOptions.Kubeconfig (relative to project root). If merge flag is
+true, config will be merged with ~/.kube/config instead.
+
+Use --login flag to write to a custom location (system kubeconfig)
+rather than the project's configured path.
+
+The command automatically:
+- Sets secure permissions (600) on the kubeconfig file
+- Adds 'kubeconfig' to .gitignore if written to project root
+- Updates the server endpoint if --endpoints is specified
+- Encrypts to kubeconfig.encrypted if talm.key exists`📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| wrappedCmd.RunE = func(cmd *cobra.Command, args []string) error { | ||||||||||||||||||||||||||||||||||||||||||||
| // Ensure project root is detected | ||||||||||||||||||||||||||||||||||||||||||||
| if !Config.RootDirExplicit { | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
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.
The
Longdescription for thekubeconfigcommand is inaccurate and could be misleading to users:PWD. However, the implementation writes it to a path relative to the project root (Config.RootDir), which may differ from the current working directory (PWD). The path is determined byglobalOptions.kubeconfiginChart.yamlor defaults to akubeconfigfile in the project root.--mergeisfalseby default by describing theif merge flag is truecase. The underlyingtalosctl kubeconfigcommand defaults--mergetotrue. The help text should describe the default behavior first to avoid confusion.A more accurate description would improve user experience.