Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Enable Elasticsearch running as non-root without any extra capabilities or privileges #703

Merged
merged 3 commits into from
Mar 26, 2021

Conversation

timricese
Copy link
Contributor

@timricese timricese commented Mar 18, 2021

Issue #, if available:
#555 partially

Description of changes:

Implemented helm varaibles for setting a securityContext for Elasticsearch pods, disabling fixmount initContainers and not giving SYS_CHROOT cap (both of which might not even be required anyway), in order to make it possible to run as non-root.

The "fixmount" initContainer is not required if a fsGroup is set via
securityContext.

The "SYS_CHROOT" cap does not appear to be required any more.

Example parameters for this setup are given in values-nonroot.yaml

Default values will not change the previous behavior of the helm chart.

Test Results:

Tested on production grade cluster, both via gitops/flux/helm-operator and helm-cli install

Note: If this PR is related to Helm, please also update the README for related documentation changes. Thanks.
https://github.com/opendistro-for-elasticsearch/opendistro-build/blob/master/helm/README.md

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on signing off your commits, please check here.

@peterzhuamazon
Copy link
Contributor

Hi @timricese, thanks for the PR, we have a few questions hope you can address:

  1. Could you use another name for .Values.elasticsearch.securityContext since securityContext is preserved in Kube? (ex: use .Values.elasticsearch.securityContextCustom)
  2. Could you attach log or screenshot of the test results? Since this is a big change.

Thanks.

@timricese
Copy link
Contributor Author

Hi @timricese, thanks for the PR, we have a few questions hope you can address:

  1. Could you use another name for .Values.elasticsearch.securityContext since securityContext is preserved in Kube? (ex: use .Values.elasticsearch.securityContextCustom)
  2. Could you attach log or screenshot of the test results? Since this is a big change.

Thanks.

1: absolutely, ill change that shortly

regarding 2:, would a console log of install via helm and then showing the pods are actually running and healthy be enough? Or do you have any unit tests for the helm package that i might not be aware of?

@timricese
Copy link
Contributor Author

timricese commented Mar 19, 2021

Incase a cconsole log of installing the helm chart + showing the pods work is enough, here it is.

~/git/opendistro-build/helm/opendistro-es ⑂main* $ helm template test . -f values-nonroot.yaml -n opendistro-unprivileged | kubectl apply -f - -n opendistro-unprivileged
serviceaccount/test-opendistro-es-es created
serviceaccount/test-opendistro-es-kibana created
secret/test-opendistro-es-es-config created
role.rbac.authorization.k8s.io/test-opendistro-es-es created
role.rbac.authorization.k8s.io/test-opendistro-es-kibana created
rolebinding.rbac.authorization.k8s.io/test-opendistro-es-elastic-rolebinding created
rolebinding.rbac.authorization.k8s.io/test-opendistro-es-kibana-rolebinding created
service/test-opendistro-es-data-svc created
service/test-opendistro-es-client-service created
service/test-opendistro-es-discovery created
service/test-opendistro-es-kibana-svc created
deployment.apps/test-opendistro-es-client created
deployment.apps/test-opendistro-es-kibana created
statefulset.apps/test-opendistro-es-data created
statefulset.apps/test-opendistro-es-master created

a screenshot of the pods running immediately after, in k9s
image

and a screenshot of a simple query via dev tools in kibana, accessed via portforwarding to the kibana pod

image

Again, if there are any specific unit tests that I should be running, please let me know. I couldnt find any.

Tim Rice added 3 commits March 19, 2021 21:20
By setting a securityContext for the pod, disabling initContainers and not
giving SYS_CHROOT cap (both of which might not even be required anyway) it
is possible to run as non-root.

The "fixmount" initContainer is not required if a fsGroup is set via
securityContext.

The "SYS_CHROOT" cap does not appear to be required any more.

Example parameters for this setup are given in values-nonroot.yaml

Signed-off-by: Tim Rice <[email protected]>
Signed-off-by: Tim Rice <[email protected]>
@peterzhuamazon
Copy link
Contributor

Thanks @timricese for the test results and logs.
We have approved and merged this PR.
Thanks.

@peterzhuamazon peterzhuamazon merged commit 260edbd into opendistro-for-elasticsearch:main Mar 26, 2021
mmguero added a commit to mmguero-dev/Malcolm that referenced this pull request Apr 8, 2021
…search container can now be run as non-root. This commit uses Malcolm's normal "drop privileges" pattern so that by the time the docker entrypoint for the ODFE container is called we are already a non-root user.
@vijeswari
Copy link

Hi,
I tried creating ODFE pods running as non-root user using ODFE 1.13.2 docker image and helm chart. The pods creation fail with the following error:

[xxxx]$ kubectl logs -f test-opendistro-es-client-6bbb7dd9fd-przsc elasticsearch
OpenDistro for Elasticsearch Security Demo Installer
** Warning: Do not use on production or public reachable systems **
Basedir: /usr/share/elasticsearch
Elasticsearch install type: rpm/deb on CentOS Linux release 7.9.2009 (Core)
Elasticsearch config dir: /usr/share/elasticsearch/config
Elasticsearch config file: /usr/share/elasticsearch/config/elasticsearch.yml
Elasticsearch bin dir: /usr/share/elasticsearch/bin
Elasticsearch plugins dir: /usr/share/elasticsearch/plugins
Elasticsearch lib dir: /usr/share/elasticsearch/lib
Detected Elasticsearch Version: x-content-7.10.2
Detected Open Distro Security Version: 1.13.1.0

Success

Execute this script now on all your nodes and then start all nodes

tee: securityadmin_demo.sh: Permission denied

Helm install command:
helm install test . -f values-nonroot.yaml

Can you please help me understand what could be missing here. The same test seemed to have passed as part of this change.

Thanks.

@getvasanth
Copy link

Is there any update on this issue?

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.

5 participants