Adding support for agent authenticate server by using kubeapi-server ca by dberkov · Pull Request #47 · kubernetes-sigs/apiserver-network-proxy

dberkov · 2019-12-18T22:16:06Z

The PR allows to reuse certificate generated for kubelet by proxy/agent/test-client, instead of generating dedicated set of certificates for proxy / agent / test-agent.

Server

Proxy-server runs as a static pod on master and initializes a grpc server with /etc/srv/kubernetes/pki/apiserver.crt & /etc/srv/kubernetes/pki/apiserver.key (no CA certificate)

Agent

Agent runs on node as a regular pod. It uses existing /var/run/secrets/kubernetes.io/serviceaccount/ca.crt (ca.crt bounds in every pod by k8s) to validate proxy-server's certificate returns during tls handshake.

Test-Clinet

As part of the tests, test-client runs on master VM as a static pod and communicates with proxy-server via UDS.

k8s-ci-robot · 2019-12-18T22:16:09Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: login-issues@jira.linuxfoundation.org

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

k8s-ci-robot · 2019-12-18T22:16:14Z

Welcome @dberkov!

It looks like this is your first PR to kubernetes-sigs/apiserver-network-proxy 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/apiserver-network-proxy has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2019-12-18T22:16:14Z

Hi @dberkov. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

caesarxuchao · 2019-12-18T22:17:01Z

/ok-to-test

caesarxuchao · 2019-12-18T22:17:27Z

cc @cheftako @Jefftree

README.md

dberkov · 2019-12-18T23:59:13Z

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

CLA signature completed

caesarxuchao · 2020-01-05T21:59:57Z

/assign

caesarxuchao

Dima, can you move the code refactor into another PR?

If I understand correctly, with this PR, only the proxy agent validates the identity of the proxy server, but not the other way. Will there be follow-up PRs that implement the other direction of verification?

I haven't reviewed the test changes.

cmd/client/main.go

cmd/proxy/main.go

artifacts/k8s-templates/konnectivity-proxy.yaml

dberkov · 2020-01-06T18:22:25Z

Dima,
[1] can you move the code refactor into another PR?

[2] If I understand correctly, with this PR, only the proxy agent validates the identity of the proxy server, but not the other way. Will there be follow-up PRs that implement the other direction of verification?

[3] I haven't reviewed the test changes.

[1] - renaming udsName has been undo
[2] - yes. This PR covers the flow when agent validates proxy's identity. The opposite direction will be in separate PR.

README.md

artifacts/k8s-templates/konnectivity-agent.yaml

pkg/util/certificates.go

caesarxuchao · 2020-01-08T00:39:32Z

Makefile

 	${DOCKER_CMD} manifest push --purge $(SERVER_FULL_IMAGE):$(TAG)

+.PHONY: docker-push-manifest/proxy-test-client
+docker-push-manifest/proxy-test-client: ## Push the fat manifest docker image.


I know it's copied from somewhere, but what does "fat manifest" mean?

https://developer.ibm.com/linuxonpower/2017/07/27/create-multi-architecture-docker-image/ suggest a "fat manifest" is a mutli-architecture manifest.

README.md

cmd/client/main.go

pkg/util/certificates.go

cmd/proxy/main.go

artifacts/k8s-templates/konnectivity-agent.yaml

caesarxuchao · 2020-01-08T01:17:52Z

Thank you @dberkov. The refactor looks good. I have some comments on the details.

caesarxuchao

/lgtm

caesarxuchao · 2020-01-10T22:57:25Z

examples/kubernetes/konnectivity-agent.yaml

+      timeoutSeconds: 15
+    resources:
+      limits:
+        cpu: 50m


is this accurate?

I assume so, it's a copy and paste of what we have in k/k (https://github.com/kubernetes/kubernetes/blob/master/cluster/gce/addons/konnectivity-agent/daemonset.yaml#L53). cc: @cheftako

A very good question. Its sufficient for the server to come up. It will be interesting to see if its sufficient to proxy something like a cluster with high admission webhook load.

caesarxuchao · 2020-01-10T23:00:57Z

Can you squash the commits? At least the "Do not make unnecessary call to kubeapi-server to pull certificates" commit is redundant.

Jefftree · 2020-01-11T00:17:17Z

/lgtm

caesarxuchao · 2020-01-11T00:32:35Z

/lgtm

cheftako · 2020-01-14T00:01:15Z

cmd/client/main.go

 	flags.IntVar(&o.requestPort, "request-port", o.requestPort, "The port the request server is listening on.")
 	flags.StringVar(&o.proxyHost, "proxy-host", o.proxyHost, "The host of the proxy server.")
 	flags.IntVar(&o.proxyPort, "proxy-port", o.proxyPort, "The port the proxy server is listening on.")
-	flags.StringVar(&o.proxyUdsName, "proxy-uds", o.proxyHost, "The UDS name to connect to.")


cmd/proxy/main.go

examples/kubernetes/konnectivity-server.yaml

cheftako

/lgtm

certificate.

dberkov · 2020-01-14T04:57:31Z

/assign @cheftako

Jefftree · 2020-01-14T05:01:03Z

/lgtm

cheftako · 2020-01-14T17:51:57Z

/approve

k8s-ci-robot · 2020-01-14T17:52:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cheftako, dberkov

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [cheftako]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…odules/github.com/stretchr/testify-1.7.3 Build(deps): bump github.com/stretchr/testify from 1.7.2 to 1.7.3

…ncy-openshift-4.16-ose-apiserver-network-proxy OCPBUGS-27932: Updating ose-apiserver-network-proxy-container image to be consistent with ART for 4.16

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Dec 18, 2019

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 18, 2019

k8s-ci-robot requested review from anfernee and cheftako December 18, 2019 22:16

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 18, 2019

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 18, 2019

dberkov force-pushed the auth branch from e438431 to dda4748 Compare December 18, 2019 22:23

anfernee reviewed Dec 18, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

dberkov force-pushed the auth branch from dda4748 to 0f8a936 Compare December 18, 2019 23:43

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Dec 18, 2019

dberkov force-pushed the auth branch 2 times, most recently from 080653c to a669644 Compare December 28, 2019 00:26

k8s-ci-robot assigned caesarxuchao Jan 5, 2020

caesarxuchao reviewed Jan 5, 2020

View reviewed changes

cmd/client/main.go Outdated Show resolved Hide resolved

cmd/proxy/main.go Outdated Show resolved Hide resolved

cmd/proxy/main.go Show resolved Hide resolved

caesarxuchao reviewed Jan 6, 2020

View reviewed changes

artifacts/k8s-templates/konnectivity-proxy.yaml Outdated Show resolved Hide resolved

artifacts/k8s-templates/konnectivity-proxy.yaml Show resolved Hide resolved

Jefftree reviewed Jan 6, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

Jefftree reviewed Jan 6, 2020

View reviewed changes

artifacts/k8s-templates/konnectivity-agent.yaml Show resolved Hide resolved

caesarxuchao reviewed Jan 6, 2020

View reviewed changes

pkg/util/certificates.go Outdated Show resolved Hide resolved

caesarxuchao reviewed Jan 8, 2020

View reviewed changes

caesarxuchao reviewed Jan 10, 2020

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 10, 2020

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 10, 2020

dberkov force-pushed the auth branch from 878992c to ca97d33 Compare January 11, 2020 00:00

dberkov changed the title ~~Add support for authentication via kube-api-server~~ Adding support for agent authenticate server by using kubeapi-server ca Jan 11, 2020

k8s-ci-robot assigned Jefftree Jan 11, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 11, 2020

cheftako reviewed Jan 14, 2020

View reviewed changes

cmd/proxy/main.go Outdated Show resolved Hide resolved

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 14, 2020

cheftako reviewed Jan 14, 2020

View reviewed changes

examples/kubernetes/konnectivity-server.yaml Outdated Show resolved Hide resolved

cheftako approved these changes Jan 14, 2020

View reviewed changes

k8s-ci-robot assigned cheftako Jan 14, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 14, 2020

Adding support for agent authenticate server by using kubeapi-server ca

166faaf

certificate.

dberkov force-pushed the auth branch from ab81c73 to 166faaf Compare January 14, 2020 04:56

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 14, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 14, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 14, 2020

k8s-ci-robot merged commit b03c198 into kubernetes-sigs:master Jan 14, 2020

dberkov mentioned this pull request Jan 22, 2020

Adding support for server to authenticate agent #51

Merged

Conversation

dberkov commented Dec 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Server

Agent

Test-Clinet

Uh oh!

k8s-ci-robot commented Dec 18, 2019

Uh oh!

k8s-ci-robot commented Dec 18, 2019

Uh oh!

k8s-ci-robot commented Dec 18, 2019

Uh oh!

caesarxuchao commented Dec 18, 2019

Uh oh!

caesarxuchao commented Dec 18, 2019

Uh oh!

Uh oh!

dberkov commented Dec 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

caesarxuchao commented Jan 5, 2020

Uh oh!

caesarxuchao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dberkov commented Jan 6, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

caesarxuchao Jan 8, 2020

Choose a reason for hiding this comment

Uh oh!

cheftako Jan 8, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

caesarxuchao commented Jan 8, 2020

Uh oh!

caesarxuchao left a comment

Choose a reason for hiding this comment

Uh oh!

caesarxuchao Jan 10, 2020

Choose a reason for hiding this comment

Uh oh!

Jefftree Jan 10, 2020

Choose a reason for hiding this comment

Uh oh!

cheftako Jan 14, 2020

Choose a reason for hiding this comment

Uh oh!

caesarxuchao commented Jan 10, 2020

Uh oh!

Jefftree commented Jan 11, 2020

Uh oh!

caesarxuchao commented Jan 11, 2020

Uh oh!

cheftako Jan 14, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cheftako left a comment

Choose a reason for hiding this comment

Uh oh!

dberkov commented Jan 14, 2020

dberkov commented Dec 18, 2019 •

edited

Loading

dberkov commented Dec 18, 2019 •

edited

Loading