Skip to content

Add a --short flag to kubectl config view #7007

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 23, 2015
Merged

Add a --short flag to kubectl config view #7007

merged 1 commit into from
Apr 23, 2015

Conversation

j3ffml
Copy link
Contributor

@j3ffml j3ffml commented Apr 17, 2015

Defaults to false, when enabled, redacts raw data for a shorter,
human-readable view. Useful for inspecting files that have embeded
data. We may actually want to make the default be --short=true, and
disable it for --flatten.

cc @deads2k, @satnam6502

@j3ffml j3ffml changed the title Add a --sort flag to kubectl config view Add a --short flag to kubectl config view Apr 17, 2015
@j3ffml j3ffml force-pushed the kubeconfig-verbose branch from 7da9c93 to 994cc9b Compare April 17, 2015 22:41
deads2k
deads2k reviewed Apr 20, 2015
View reviewed changes
pkg/kubectl/cmd/config/view.go
@@ -83,6 +85,7 @@ func NewCmdConfigView(out io.Writer, ConfigAccess ConfigAccess) *cobra.Command {

options.Merge.Default(true)
cmd.Flags().Var(&options.Merge, "merge", "merge together the full hierarchy of kubeconfig files")
cmd.Flags().BoolVar(&options.Short, "short", false, "redact raw data for a shorter, human-readable view")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redact raw byte data?

@deads2k
Copy link
Contributor

deads2k commented Apr 20, 2015

Optional nit, the code lgtm.

@liggitt Any thoughts on the defaulting. It'd be nice to be able to read the output by default, but changing the default value of short based on the value of other flags might be confusing.

liggitt
liggitt reviewed Apr 20, 2015
View reviewed changes
pkg/kubectl/cmd/config/view.go
@@ -106,6 +109,11 @@ func (o ViewOptions) Run(out io.Writer, printer kubectl.ResourcePrinter) error {
}
}

if o.Short {
fmt.Printf("short\n")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the extra output? won't this mess up anything expecting structured output, or anything using templated output?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's intentional. For templates, you won't it shortened. If you're shortening, then the output is no longer a useable kubeconfig, so you don't want it to parse.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it wasn't intentional, just debugging cruft. If we want to make --short incompatible with -o template, that should be done explicitly.

@satnam6502
Copy link
Contributor

Rather than having --short which defaults to true, how about --full or --raw or --authinfo which defaults to false? I feel that short is not quite the right word here since what you want to do is to express the idea of just an abstraction or sub-set of the original information being passed through.

@deads2k
Copy link
Contributor

deads2k commented Apr 20, 2015

You could try going one step further and actually having --redacted that substitutes the certs and replaces the tokens and passwords. It would be removing two non-secrets as well, but would make it easy for people to shared troublesome kubeconfigs for help.

@j3ffml j3ffml force-pushed the kubeconfig-verbose branch from 994cc9b to 499404d Compare April 20, 2015 17:48
@j3ffml
Copy link
Contributor Author

j3ffml commented Apr 20, 2015

@deads2k, @liggitt what do you think of Satnam's idea of using --raw for full output, and redacting byte data by default? The other flags (--flatten, --minify, --output) would still output raw byte data, and we can fail the command if users specify incompatible flags (e.g. both --raw=false and --flatten).

@j3ffml j3ffml force-pushed the kubeconfig-verbose branch 3 times, most recently from d0a3b1e to b60fdd3 Compare April 23, 2015 00:50
@j3ffml
Copy link
Contributor Author

j3ffml commented Apr 23, 2015

@deads2k, PTAL
Changed flag name to --raw, enabled by default, except when --flatten is true. Let me know if you have any objections.

deads2k
deads2k reviewed Apr 23, 2015
View reviewed changes
pkg/client/clientcmd/api/helpers_test.go
caData := "ca"

config := newMergedConfig(certFile.Name(), certData, keyFile.Name(), keyData, caFile.Name(), caData, nil)
//config.CurrentContext = "shaker-context"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete (looks like leftovers)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@deads2k
Copy link
Contributor

deads2k commented Apr 23, 2015

lgtm

@j3ffml
Add a --raw flag to kubectl config view
fa6ce7b 
Defaults to false, unless --flatten is specified. Default behavior
(--raw=false) is that byte data (Client{Certificate,Key}data,
CertificateAuthorityData) is redacted for a more human-readable view.
Useful for manually inspecting files that have embeded data.
@j3ffml j3ffml force-pushed the kubeconfig-verbose branch from b60fdd3 to fa6ce7b Compare April 23, 2015 16:21
@satnam6502
Copy link
Contributor

Re-running Shippable.

@satnam6502 satnam6502 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 23, 2015
@j3ffml
Copy link
Contributor Author

j3ffml commented Apr 23, 2015

Can haz merge?

satnam6502 added a commit that referenced this pull request Apr 23, 2015
@satnam6502
Merge pull request #7007 from jlowdermilk/kubeconfig-verbose
bb478a9 
Add a --short flag to kubectl config view
@satnam6502 satnam6502 merged commit bb478a9 into kubernetes:master Apr 23, 2015
@j3ffml j3ffml deleted the kubeconfig-verbose branch April 28, 2015 16:35
@karlkfi karlkfi mentioned this pull request Jun 20, 2015
Closed
@liggitt liggitt mentioned this pull request Jul 9, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@satnam6502 satnam6502

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@j3ffml @deads2k @satnam6502 @liggitt @googlebot