Skip to content
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

Add a kubectl create secret tls command #24719

Merged
merged 1 commit into from
May 22, 2016
Merged

Add a kubectl create secret tls command #24719

merged 1 commit into from
May 22, 2016

Conversation

bprashanth
Copy link
Contributor

@bprashanth bprashanth commented Apr 24, 2016 •
edited by mikedanese
Loading

A somewhat hasty implementation that enables progress along: #20176 (comment), #24669, #20176 (comment) if associated parties have spare cycles. @kubernetes/kubectl

This change is Reviewable

@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Apr 24, 2016
deads2k
deads2k reviewed Apr 25, 2016
View reviewed changes
pkg/kubectl/cmd/create_secret.go
The public/private key pair must exist before hand. The public key certificate must be .PEM encoded and match the given private key.`

secretForTLSExample = ` # You can create a TLS secret using:
$ kubectl create secret tls tls-secret-name --cert=path to tls.cert --key=path to tls.key`
Copy link
Contributor

Choose a reason for hiding this comment

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

path/to/tls.cert

deads2k
deads2k reviewed Apr 25, 2016
View reviewed changes
pkg/kubectl/secret_for_tls.go
return fmt.Errorf("certificate must be specified.")
}
// TODO: Add more validation.
// 1. Public/private key pair match.
Copy link
Contributor

Choose a reason for hiding this comment

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

I see this one as required for merge. We really want to be sure that what people give us for these isn't borked.

@deads2k
Copy link
Contributor

deads2k commented Apr 25, 2016

@Kargakis generators!

@0xmichalis
Copy link
Contributor

whatcanyoudo

janetkuo
janetkuo reviewed Apr 26, 2016
View reviewed changes
docs/user-guide/kubectl/kubectl_create_secret_tls.md
The public/private key pair must exist before hand. The public key certificate must be .PEM encoded and match the given private key.

```
kubectl create secret tls NAME --cert=path to cert file --key=path to key file [--dry-run]
Copy link
Member

@janetkuo janetkuo Apr 26, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Suggest not using space in flag arguments. How about --cert=path/to/cert/file or --cert=CERTFILE

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

Copy link
Contributor

Choose a reason for hiding this comment

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

done

Need to regenerate?

@bprashanth
Copy link
Contributor Author

I'm now loading the key pair and throwing errors back, also added a unittest for the same. PTAL.

0xmichalis
0xmichalis reviewed Apr 26, 2016
View reviewed changes
pkg/kubectl/cmd/create_secret.go
The public/private key pair must exist before hand. The public key certificate must be .PEM encoded and match the given private key.`

secretForTLSExample = ` # Create a new TLS secret named tls-secret with the given key pair:
$ kubectl create secret tls tls-secret --cert=path/to/tls.cert --key=path/to/tls.key`
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we have stopped using the dollar sign in kubectl examples

@deads2k
Copy link
Contributor

deads2k commented Apr 26, 2016

I'd like to see a usage example in test-cmd. Using a static (checked-in) cert/key pair would be fine.

@deads2k
Copy link
Contributor

deads2k commented Apr 26, 2016

A couple more tests (noted in comments) and this lgtm.

@deads2k deads2k added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Apr 28, 2016
@bprashanth bprashanth force-pushed the kubectl_tls branch 2 times, most recently from bb02fb9 to c93aaac Compare May 15, 2016 01:21
@bprashanth
Copy link
Contributor Author

PTAL, sorry for the huge delay in updating this. Got sidetracked with 1.3 stuff.

@k8s-github-robot k8s-github-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 15, 2016
@deads2k
Copy link
Contributor

deads2k commented May 16, 2016

@bprashanth I think it just needs an update to the help (some comments outstanding) and its ready.

@deads2k deads2k added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 16, 2016
@deads2k
Copy link
Contributor

deads2k commented May 16, 2016

lgtm

@bprashanth
Copy link
Contributor Author

Forgot to git add file, inheriting LGTM

@bprashanth bprashanth added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels May 17, 2016
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented May 22, 2016

GCE e2e build/test passed for commit daa8e29.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 39f0c6b into kubernetes:master May 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@deads2k deads2k

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@bprashanth @deads2k @0xmichalis @k8s-github-robot @k8s-bot @ramr @smarterclayton @janetkuo @derekwaynecarr @googlebot