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

Alpha JWS Discovery API for locating an apiserver securely #32203

Merged
merged 3 commits into from
Sep 23, 2016

Conversation

dgoodwin
Copy link
Contributor

@dgoodwin dgoodwin commented Sep 7, 2016

This PR contains an early alpha prototype of the JWS discovery API outlined in proposal #30707.

CA certificate, API endpoints, and the token to be used to authenticate to this discovery API are currently passed in as secrets. If the caller provides a valid token ID, a JWS signed blob of ClusterInfo containing the API endpoints and the CA cert to use will be returned to the caller. This is used by the alpha kubeadm to allow seamless, very quick cluster setup with simple commands well suited for copy paste.

Current TODO list:

  • Allow the use of arbitrary strings as token ID/token, we're currently treating them as raw keys.
  • Integrate the building of the pod container, move to cluster/images/kube-discovery.
    • Build for: amd64, arm, arm64 and ppc64le. (just replace GOARCH=)
    • Rename to gcr.io/google_containers/kube-discovery-ARCH:1.0
    • Cleanup rogue files in discovery sub-dir.
    • Move pkg/discovery/ to cmd/discovery/app.

There is additional pending work to return a kubeconfig rather than ClusterInfo, however I believe this is slated for post-alpha.


This change is Reviewable

@luxas
Copy link
Member

luxas commented Sep 7, 2016

@kubernetes/sig-cluster-lifecycle

@luxas luxas added sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. release-note-experimental labels Sep 7, 2016
@luxas luxas added this to the v1.5 milestone Sep 7, 2016
@k8s-github-robot k8s-github-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. release-note-label-needed labels Sep 7, 2016
@luxas luxas assigned jbeda, errordeveloper, mikedanese and luxas and unassigned thockin Sep 7, 2016
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

@@ -0,0 +1,6 @@
FROM golang
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be in the discovery/ dir, but cluster/images/jws-discovery or something like that.

Also, it should be

FROM busybox
COPY kube-discovery /usr/local/bin
ENTRYPOINT ["/usr/local/bin/kube-discovery"]

in order to be consistent with the other server images

@dgoodwin
Copy link
Contributor Author

dgoodwin commented Sep 9, 2016

Introduced a Makefile, is this on track for building the container for appropriate arches @luxas ?

# See the License for the specific language governing permissions and
# limitations under the License.

# Build the hyperkube image.
Copy link
Member

Choose a reason for hiding this comment

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

kube-discovery

@luxas
Copy link
Member

luxas commented Sep 13, 2016

Please nuke the Makefile and cluster/images/hyperkube/Makefile changes...

@luxas
Copy link
Member

luxas commented Sep 15, 2016

Lastly, we should apply cla: human-approved I think.
I guess @thockin has the legal power to do that :)

@dgoodwin
Copy link
Contributor Author

Boilerplate updated.

Hmm, I should be CLA approved, at least I was for this: kubernetes/release#50, via being added to the kubernetes-redhat-contributors group.

@luxas
Copy link
Member

luxas commented Sep 15, 2016

Yes, I just think the issue is that the CLA bot doesn't allow two committers

@luxas
Copy link
Member

luxas commented Sep 15, 2016

Ping @jbeda @errordeveloper @mikedanese @smarterclayton
This PR is ready for final review.

@mikedanese Should build and push the images as soon as it's LGTM'd (which I hope is today)

@taimir
Copy link
Contributor

taimir commented Sep 16, 2016

CC @cheld @kenan435 @floreks

all: build

build:
cp -r ./* ${TEMP_DIR}
Copy link
Contributor

Choose a reason for hiding this comment

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

@dgoodwin I think you need a line similar to this here:

cd ${TEMP_DIR} && sed -i.back "s|BASEIMAGE|${BASEIMAGE}|g" Dockerfile

At least that's how it's done in the hyperkube image :)

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, thanks!

@k8s-bot
Copy link

k8s-bot commented Sep 16, 2016

GCE e2e build/test passed for commit baebd7c.

@errordeveloper
Copy link
Member

LGTM, would be great to get this in as soon as we can!

@luxas
Copy link
Member

luxas commented Sep 19, 2016

LGTM to me as well, I hope we can get this in today or tomorrow.

Waiting for @mikedanese's and/or @jbeda's review.
Also we need someone with the legal power to add cla: human-approved

@lukemarsden
Copy link
Contributor

Friendly ping @mikedanese :)

@dgoodwin
Copy link
Contributor Author

@mikedanese I have no preference on how that image gets built so say the word and I can apply https://storage.googleapis.com/mikedanese/patches/0001-build-kube-discovery-image-with-release.patch if you like. It does look simpler.

@luxas
Copy link
Member

luxas commented Sep 21, 2016

I still think we should follow the kube-dns approach with tagged 1.x releases that is built/pushed whenever it wants to, instead of following the 2 week alpha schedule (which in the worst case, v1.4.0-alpha.3, got two more weeks delayed)

Everything for that is done. @mikedanese I vote for merging this now.

We can take this discussion again when we start thinking about promoting it to beta.

@mikedanese mikedanese added lgtm "Looks good to me", indicates that a PR is ready to be merged. cla: human-approved and removed cla: no labels Sep 22, 2016
@luxas luxas added the priority/backlog Higher priority than priority/awaiting-more-evidence. label Sep 22, 2016
@luxas
Copy link
Member

luxas commented Sep 23, 2016

@k8s-bot test this please github issue: #IGNORE

@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 1834039 into kubernetes:master Sep 23, 2016
@luxas
Copy link
Member

luxas commented Sep 23, 2016

@mikedanese Please push these images for all architectures today!
I think that would make many sig-cluster-lifecycle guys happy :)

@errordeveloper
Copy link
Member

Amazing to see this merged! Please ping me whenever we have images on GCR, and I'll update #33262 accordingly.

@lukemarsden
Copy link
Contributor

@mikedanese do we have images on GCR for this?

@luxas
Copy link
Member

luxas commented Sep 25, 2016

Yes

@dgoodwin dgoodwin deleted the kubediscovery branch September 27, 2016 11:41
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. priority/backlog Higher priority than priority/awaiting-more-evidence. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet