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.

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

Implement package triple with utilities to generate certificate-key pairs for CA, server and clients. #35593

Conversation

madhusudancs
Copy link
Contributor

@madhusudancs madhusudancs commented Oct 26, 2016

Please review only the last commit here. This is based on PR #35592 which will be reviewed independently.

Design Doc: PR #34484

cc @kubernetes/sig-cluster-federation @quinton-hoole @mwielgus


This change is Reviewable

@madhusudancs madhusudancs added area/cluster-federation release-note-none Denotes a PR that doesn't merit a release note. labels Oct 26, 2016
@madhusudancs madhusudancs added this to the v1.5 milestone Oct 26, 2016
@k8s-github-robot k8s-github-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 26, 2016
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Add comment here explaining what is this package for. It is not obvious what a package named "triple" will do :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the package doc.

limitations under the License.
*/

package triple
Copy link
Contributor

Choose a reason for hiding this comment

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

How about keypairgenerator as package name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

keypairgenerator is too long. Also, that's what the parent package does. This package uses the parent package to generate the keypairs, but generates the pairs for the (CA, server, client) triple.

@madhusudancs
Copy link
Contributor Author

@nikhiljindal addressed the comment. PTAL.

@k8s-ci-robot
Copy link
Contributor

Jenkins unit/integration failed for commit 2a7be55ff258787cefc9d9538d432f6fa28df586. Full PR test history.

The magic incantation to run this job again is @k8s-bot unit test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins verification failed for commit 2a7be55ff258787cefc9d9538d432f6fa28df586. Full PR test history.

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@nikhiljindal
Copy link
Contributor

I still dont like triple but dont have a better suggestion than keypairgenerator :)
Dont have any other comment so fine to merge this as is.

@madhusudancs madhusudancs force-pushed the federation-kubefed-init-certutil-triple branch from 2a7be55 to a9544b2 Compare October 28, 2016 07:39
@madhusudancs
Copy link
Contributor Author

@nikhiljindal leaving it as is. Rebased on top of master and squashed the commits. Applying the LGTM label.

@madhusudancs madhusudancs force-pushed the federation-kubefed-init-certutil-triple branch from a9544b2 to 47e53f2 Compare October 28, 2016 07:40
@madhusudancs madhusudancs added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 28, 2016
@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 28, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins Kubemark GCE e2e failed for commit 47e53f2. Full PR test history.

The magic incantation to run this job again is @k8s-bot kubemark e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GCE e2e failed for commit 47e53f2. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@madhusudancs
Copy link
Contributor Author

@k8s-bot gci gce e2e test this

@madhusudancs
Copy link
Contributor Author

@k8s-bot kubemark e2e test this

@k8s-github-robot
Copy link

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

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 962a152 into kubernetes:master Oct 29, 2016
@madhusudancs madhusudancs deleted the federation-kubefed-init-certutil-triple branch October 31, 2016 06:06
k8s-github-robot pushed a commit that referenced this pull request Nov 3, 2016
Automatic merge from submit-queue

[Federation][init] Implement `kubefed init` command that performs federation control plane bootstrap.

Please review only the last commit here. This is based on PR #35593 which will be reviewed independently.

I am intentionally not including the unit tests in this PR to better distribute and parallelize reviews. This PR is already big.

I will add a release note separately for this entire feature, so please don't worry too much about the release note here in the PR.

Design Doc: PR #34484

cc @kubernetes/sig-cluster-federation @quinton-hoole @nikhiljindal
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants