metrics-server assumes same TLS config for kube-apiserver and kubelets

[clhodapp]

Presently, metrics-server re-uses the TLS config that it constructs for communication with kube-apiserver in its configuration for talking with the kubelets. This is bad because kube-apiserver and kubelet are supposed to (or at least can) use separate CAs. As it stands, bringing metrics-server into the mix requires you to use the same CA for kube-apiserver and your kubelets.

Problem line: https://github.com/kubernetes-incubator/metrics-server/blob/251f7b578894d3f9adfccd9b0cc2127321819fba/metrics/sources/kubelet/configs.go#L67

[ghost]

I guess this configuration will cause the swaggerui can not work correctly.

My k8s cluster was built with kubeadmin, and when I try to visit the swaggerui, I get these errors.

itaas@kvm-013487:~/k8s$ kubectl logs metrics-server-859cb9bd4b-tnhtx -n=kube-system
I1122 06:41:18.721787       1 heapster.go:71] /metrics-server --source=kubernetes.summary_api:''
I1122 06:41:18.721867       1 heapster.go:72] Metrics Server version v0.2.0
I1122 06:41:18.722052       1 configs.go:61] Using Kubernetes client with master "https://10.96.0.1:443" and version 
I1122 06:41:18.722076       1 configs.go:62] Using kubelet port 10255
I1122 06:41:18.723357       1 heapster.go:128] Starting with Metric Sink
I1122 06:41:19.349881       1 serving.go:308] Generated self-signed cert (apiserver.local.config/certificates/apiserver.crt, apiserver.local.config/certificates/apiserver.key)
I1122 06:41:19.661749       1 heapster.go:101] Starting Heapster API server...
[restful] 2017/11/22 06:41:19 log.go:33: [restful/swagger] listing is available at https:///swaggerapi
[restful] 2017/11/22 06:41:19 log.go:33: [restful/swagger] https:///swaggerui/ is mapped to folder /swagger-ui/
I1122 06:41:19.663614       1 serve.go:85] Serving securely on 0.0.0.0:443
I1123 01:27:49.900802       1 logs.go:41] http: TLS handshake error from 192.168.222.192:47540: remote error: tls: bad certificate
I1123 01:27:50.116378       1 logs.go:41] http: TLS handshake error from 192.168.222.192:47542: remote error: tls: bad certificate
I1123 01:27:56.031159       1 logs.go:41] http: TLS handshake error from 192.168.222.192:47556: remote error: tls: bad certificate

[DirectXMan12]

I don't think this would cause issues with the swagger UI. Please file a separate bug for that.

[patrickf55places]

The master API server receives the kubelet CA from the --kubelet-certificate-authority CLI option when it boots. AFAIK, the API server does not expose this via an API or ConfigMap.

[clhodapp]

I believe one possible option is to augment metrics-server to accept an additional set of arguments to specify the kubelet CA & client credentials.

[DirectXMan12]

that seems fairly reasonable. PRs are welcome (and/or one of @piosz or I will get to it eventually)

[SpComb]

The TLSClientConfig also includes the InsecureSkipTLSVerify => Insecure - for the default case of the kubelet using a self-signed cert, you need to disable TLS cert verification for the kubelet API. Currently you can't do that without also disabling verification of the kube API cert.

[aurcioli-handy]

Does anyone have a solution to this? How can we get metrics-server to talk to kubelet when kubelet has tls client auth?

[clhodapp]

@aurcioli-handy You can do it but you have to use the same CA for both Kubelet and the main API, which feels slightly dirty but should not in and of itself be a security issue if done properly.

[aurcioli-handy]

Okay, I was able to get this working with the following setup:

kubelet

ca.pem is same CA as kube-apiserver

  --client-ca-file=/etc/kubernetes/ssl/ca.pem \
  --read-only-port=0

metrics-server

- command:
        - /metrics-server
        - --source=kubernetes.summary_api:https://kubernetes.default.svc?inClusterConfig=false&kubeletHttps=true&kubeletPort=10250&auth=/etc/kubernetes/ssl/kubeconfig&insecure=true

with kubeconfig

apiVersion: v1
kind: Config
clusters:
- name: local
  cluster:
    server: https://kubernetes.default
    insecure-skip-tls-verify: true
users:
- name: kubelet
  user:
    client-certificate: /etc/kubernetes/ssl/kubelet.pem
    client-key: /etc/kubernetes/ssl/kubelet-key.pem
contexts:
- context:
    cluster: local
    user: kubelet
  name: kubelet-context
current-context: kubelet-context

And the rest was just massaging RBAC to get the permissions right.

[igelkotten]

@aurcioli-handy

Wouldn't this line in your deployment of metric-server:

• --source=kubernetes.summary_api:https://kubernetes.default.svc?inClusterConfig=false&kubeletHttps=true&kubeletPort=10250&auth=/etc/kubernetes/ssl/kubeconfig &insecure=true

Result in it accepting 'insecure' certificates? and that way it doesn't matter what certificates's you put there?

[DirectXMan12]

We should probably have a new --kubelet-kubeconfig option. I'll try and tackle post-refactor.

[caitong93]

@DirectXMan12 @clhodapp

How about add kubelet-client-certificate, --kubelet-client-key, --kubelet-certificate-authority like kube-apiserver does ?
For those who use separate ca for kubelet, this may be a easier approach because they may not have a kubeconfig file containing client certs.

[DirectXMan12]

that's an option too (although probably just the kubelet-certificate-authority is necessary. If we have the other 2, we should to a kubeconfig, since it's easier to manage and inspect than having to set three separate flags. At any rate, I probably won't have time to work on this any time soon, so it's a good issue for a community member to pick up.

/good-first-issue

[k8s-ci-robot]

@DirectXMan12:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

that's an option too (although probably just the kubelet-certificate-authority is necessary. If we have the other 2, we should to a kubeconfig, since it's easier to manage and inspect than having to set three separate flags. At any rate, I probably won't have time to work on this any time soon, so it's a good issue for a community member to pick up.

/good-first-issue

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.

[caitong93]

I would like to try fix this.

If we have the other 2, we should to a kubeconfig, since it's easier to manage and inspect than having to set three separate flags

Do you mean a file in format of kubeconfig for combine things together? If so, it is reasonable, by this way, client certificate and bearer tokens can be both supported.

@DirectXMan12

[DirectXMan12]

yeah, that was my suggestion -- if you use a file in kubeconfig format, you can just re-use the logic for loading a kubeconfig, etc. On the other hand, I've learned (recently) that some people find this method confusing, so flags are probably fine too. Let's start with a kubelet-certificate-authority flag, since that's the most pressing.

[DirectXMan12]

/assign @caitong93

[k8s-ci-robot]

@DirectXMan12: GitHub didn't allow me to assign the following users: caitong93.

Note that only kubernetes-incubator members and repo collaborators can be assigned.
For more information please see the contributor guide

In response to this:

/assign @caitong93

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.