kube-ops-view
kube-ops-view copied to clipboard
Refactor code to support running on secure environments
I did the following to support running in secure/unprivileged environments (namely OpenShift)
- Explicitly define the namespace for all objects. In the event where one wants to change the namespace of the deployment, there needs to be a change to the ClusterRoleBinding of the kube-ops-view service account. This change makes it easier to understand and change the namespace of everything systematically.
- Add a service account for the redis pod. This is a bit more of an opinionated change, but I think explicitly defining a service account to consume allows more flexibility if permissions need to be changed at a later date.
- Change serviceAccount to serviceAccountName in pod spec. Kubernetes 1.15 docs use serviceAccountName (https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account) so I'm updating this field to conform to that. I've noticed that when I deploy on OpenShift 3.11 using serviceAccountName instead of serviceAccount, the object actually created contains both fields and behaves as intended.
- Update resource requests and limits for kube-ops-view deployment. I frequently ran into issues with the kube-ops-view pod getting OOMKilled due to lack of resources in an environment with over 15 nodes. After bumping these resources, I no longer noticed any issues.
- Removed uid 1000 from security context in kube-ops-view pod. I removed this line and I don't see any error messages being thrown, so I think it's safe to remove.
- Refactored redis deployment. This is the big one for this commit. Running privileged containers is strongly discouraged in my environments, so I think I found a good solution to this. The container image is switched to bitnami/redis to support a rootless redis. Volumes are explicitly defined to support read-only containers. An init container is added to the bitnami image to support having the conf directory of the redis pod being read-write without mounting over its contents.
I'm looking for feedback and suggestions before this is merged. As I mentioned, I've tested this on OpenShift 3.11. This is a large PR, so if you need data from other Kubernetes environments (GKE, AKS) let me know I can test against those environments as well. Finally, if there is any specific data you need from these environments, let me know and I can get started on adding the data to the PR.
Thanks for your PR!
- I don't agree with adding
namespace
to all manifests as this is redundant (kubectl apply ... -n myns
is enough), IMHO it also has nothing to do with "security". -
serviceAccountName
: makes sense - resource limits: OK
- I will check out the proposed Redis changes, I guess they make sense :smile:
Jacob,
One of the main reasons why I explicitly define the namespace for all objects is that the roleBinding for the kube-ops-view serviceaccount binds to a subject in an explicitly defined namespace, so if a user wanted to deploy this in another namespace, there would still need to be modifications to a file before successful deployment. I agree that my approach is a bit heavy-handed though... Should I instead add documentation that the rolebinding needs to be modified before deployment into other namespaces?
I tried applying this to a hardened k8s environment and i guess the image need to be updated to go along with these changes?
Normal Pulled 21s (x8 over 97s) kubelet Successfully pulled image "hjacobs/kube-ops-view:0.12"
Warning Failed 21s (x8 over 97s) kubelet Error: container has runAsNonRoot and image will run as root
Normal Pulling 8s (x9 over 101s) kubelet pulling image "hjacobs/kube-ops-view:0.12"
Basically I did the same in my Openshift deployment. I removed runAsNonRoot and runAsUser so the container will start with a dynamically created non-root uid. But I kept the original redis container and simply added an emptyDir for /data:
volumeMounts:
- name: redis-data
mountPath: /data
volumes:
- name: redis-data
emptyDir: {}
Any dynamicaly created user is able to write to /data now. I don't know if there are other reasons to change the container. But this deployment looks simpler to me... and works fine.