Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

chore: remove apiserver /etc/kubernetes/certs mount #3808

Merged
merged 1 commit into from
Sep 9, 2020

Conversation

jackfrancis
Copy link
Member

Reason for Change:

In our haste to unblock AAD scenarios, we added an unnecessary /etc/kubernetes/certs mountPath to the apiserver spec. In fact, the needed mount was at /etc/ssl/certs, added in #3800.

We are already mounting /etc/kubernetes, and thus have access to /etc/kubernetes/certs recursively. E.g. from a cluster built with this changeset:

azureuser@k8s-master-20163450-0:~$ kubectl exec -it kube-apiserver-k8s-master-20163450-0 -n kube-system -- /bin/sh
# ls -la /etc/kubernetes/certs
total 80
drwxr-xr-x 2 root root 4096 Sep  9 16:50 .
drwxr-xr-x 6 root root 4096 Sep  9 16:50 ..
-rw-r--r-- 1 root root 6404 Sep  9 16:50 apiserver.crt
-rw------- 1 root root 3243 Sep  9 16:49 apiserver.key
-rw-r--r-- 1 root root 1720 Sep  9 16:48 ca.crt
-rw------- 1 root root 3243 Sep  9 16:49 ca.key
-rw-r--r-- 1 root root 1785 Sep  9 16:48 client.crt
-rw------- 1 root root 3243 Sep  9 16:50 client.key
-rw-r--r-- 1 root root 1818 Sep  9 16:49 etcdclient.crt
-rw------- 1 root root 3243 Sep  9 16:49 etcdclient.key
-rw-r--r-- 1 root root 1814 Sep  9 16:49 etcdpeer0.crt
-rw------- 1 1001 1001 3243 Sep  9 16:49 etcdpeer0.key
-rw-r--r-- 1 root root 1818 Sep  9 16:49 etcdserver.crt
-rw------- 1 1001 1001 3243 Sep  9 16:49 etcdserver.key
-rw-r--r-- 1 root root 1147 Sep  9 16:50 kubeletserver.crt
-rw------- 1 root root 1675 Sep  9 16:50 kubeletserver.key
-rw-r--r-- 1 root root 1123 Sep  9 16:50 proxy-ca.crt
-rw-r--r-- 1 root root 1005 Sep  9 16:50 proxy.crt
-rw------- 1 root root 1679 Sep  9 16:50 proxy.key
# cat /etc/kubernetes/manifests/kube-apiserver.yaml
apiVersion: v1
kind: Pod
metadata:
  name: kube-apiserver
  namespace: kube-system
  labels:
    tier: control-plane
    component: kube-apiserver
spec:
  priorityClassName: system-node-critical
  hostNetwork: true
  containers:
    - name: kube-apiserver
      image: mcr.microsoft.com/oss/kubernetes/kube-apiserver:v1.18.8
      imagePullPolicy: IfNotPresent
      command: ["kube-apiserver"]
      args: ["--advertise-address=10.255.255.5", "--allow-privileged=true", "--anonymous-auth=false", "--audit-log-maxage=30", "--audit-log-maxbackup=10", "--audit-log-maxsize=100", "--audit-log-path=/var/log/kubeaudit/audit.log", "--audit-policy-file=/etc/kubernetes/addons/audit-policy.yaml", "--authorization-mode=Node,RBAC", "--bind-address=0.0.0.0", "--client-ca-file=/etc/kubernetes/certs/ca.crt", "--cloud-config=/etc/kubernetes/azure.json", "--cloud-provider=azure", "--enable-admission-plugins=NamespaceLifecycle,LimitRanger,ServiceAccount,DefaultStorageClass,DefaultTolerationSeconds,ValidatingAdmissionWebhook,ResourceQuota,ExtendedResourceToleration,PodSecurityPolicy", "--enable-bootstrap-token-auth=true", "--etcd-cafile=/etc/kubernetes/certs/ca.crt", "--etcd-certfile=/etc/kubernetes/certs/etcdclient.crt", "--etcd-keyfile=/etc/kubernetes/certs/etcdclient.key", "--etcd-servers=https://127.0.0.1:2379", "--insecure-port=0", "--kubelet-client-certificate=/etc/kubernetes/certs/client.crt", "--kubelet-client-key=/etc/kubernetes/certs/client.key", "--profiling=false", "--proxy-client-cert-file=/etc/kubernetes/certs/proxy.crt", "--proxy-client-key-file=/etc/kubernetes/certs/proxy.key", "--requestheader-allowed-names=", "--requestheader-client-ca-file=/etc/kubernetes/certs/proxy-ca.crt", "--requestheader-extra-headers-prefix=X-Remote-Extra-", "--requestheader-group-headers=X-Remote-Group", "--requestheader-username-headers=X-Remote-User", "--secure-port=443", "--service-account-key-file=/etc/kubernetes/certs/apiserver.key", "--service-account-lookup=true", "--service-cluster-ip-range=10.0.0.0/16", "--storage-backend=etcd3", "--tls-cert-file=/etc/kubernetes/certs/apiserver.crt", "--tls-cipher-suites=TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA", "--tls-private-key-file=/etc/kubernetes/certs/apiserver.key", "--v=4"]
      volumeMounts:
        - name: etc-kubernetes
          mountPath: /etc/kubernetes
        - name: etc-ssl-certs
          mountPath: /etc/ssl/certs
        - name: var-lib-kubelet
          mountPath: /var/lib/kubelet
        - name: msi
          mountPath: /var/lib/waagent/ManagedIdentity-Settings
          readOnly: true
        - name: sock
          mountPath: /opt
        - name: auditlog
          mountPath: /var/log/kubeaudit
  volumes:
    - name: etc-kubernetes
      hostPath:
        path: /etc/kubernetes
    - name: etc-ssl-certs
      hostPath:
        path: /etc/ssl/certs
    - name: var-lib-kubelet
      hostPath:
        path: /var/lib/kubelet
    - name: msi
      hostPath:
        path: /var/lib/waagent/ManagedIdentity-Settings
    - name: sock
      hostPath:
        path: /opt
    - name: auditlog
      hostPath:
        path: /var/log/kubeaudit

Observe above that we have local access from the apiserver container to /etc/kubernetes/certs without that explicit mountPath, because we inherit that access by having a mountPath to the parent directory.

Issue Fixed:

Requirements:

Notes:

@codecov
Copy link

codecov bot commented Sep 9, 2020

Codecov Report

Merging #3808 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3808   +/-   ##
=======================================
  Coverage   73.20%   73.20%           
=======================================
  Files         148      148           
  Lines       25385    25385           
=======================================
  Hits        18583    18583           
  Misses       5666     5666           
  Partials     1136     1136           
Impacted Files Coverage Δ
pkg/engine/templates_generated.go 53.42% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0807b15...3a91897. Read the comment docs.

mboersma
mboersma previously approved these changes Sep 9, 2020
Copy link
Member

@mboersma mboersma left a comment

Choose a reason for hiding this comment

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

/lgtm

@acs-bot acs-bot added the lgtm label Sep 9, 2020
sozercan
sozercan previously approved these changes Sep 9, 2020
Copy link
Member

@sozercan sozercan left a comment

Choose a reason for hiding this comment

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

LGTM, /etc/ssl host mount should be removable for v1.19+ clusters for kube-proxy and kube-apiserver too (needs to be tested).

@acs-bot
Copy link

acs-bot commented Sep 9, 2020

New changes are detected. LGTM label has been removed.

@acs-bot acs-bot removed the lgtm label Sep 9, 2020
@acs-bot
Copy link

acs-bot commented Sep 9, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jackfrancis, mboersma

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

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [jackfrancis,mboersma]

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

@jackfrancis jackfrancis merged commit 5bad19f into Azure:master Sep 9, 2020
@jackfrancis jackfrancis deleted the apiserver-etc-kubernetes branch September 9, 2020 23:37
penggu pushed a commit to penggu/aks-engine that referenced this pull request Oct 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants