kube-ops-view icon indicating copy to clipboard operation
kube-ops-view copied to clipboard

Refactor code to support running on secure environments

Open geoffmore opened this issue 5 years ago • 4 comments

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.

geoffmore avatar Jun 24 '19 13:06 geoffmore

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:

hjacobs avatar Jul 18 '19 20:07 hjacobs

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?

geoffmore avatar Jul 19 '19 15:07 geoffmore

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"

gregswift avatar Sep 10 '19 23:09 gregswift

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.

megabreit avatar Nov 28 '19 17:11 megabreit