Default to running Elasticsearch as ReadOnlyRootFilesystem: true

[naemono]

Primary Change

This enables running Elasticsearch with readOnlyRootFilesystem: true as this aligns with security best practices.

Todo

• Full set of e2e tests on both 7.x and 8.x
• Documentation

related #6126

[naemono]

buildkite test this -f p=gke,s=8.7.0,s=7.17.8

[thbkrkr]

buildkite test this -f p=gke,t=TestVolumeEmptyDir -m s=8.7.0,s=7.17.8

[barkbay]

resolves #6126

nit: I think this change is a good thing, but unless I'm missing something it does not solve #6126. The requirement is to be able to run the whole Pod with readOnlyRootFilesystem: true, which is for now not possible for the init container elastic-internal-init-filesystem (this is something I missed during my initial review of the issue).

I'm also wondering if it would be a good thing to include other security settings like:

securityContext:
  allowPrivilegeEscalation: false
  capabilities:
    drop:
      - ALL
  privileged: false
  readOnlyRootFilesystem: true
  runAsNonRoot: true

[naemono]

Thanks for the heads up about wanting to run the whole pod as read-only @barkbay .

which is for now not possible for the init container

So I was reviewing this and am curious about the history of the approach we take. From what I see happening in this init container that would violate having it mounted as read-only are these copy operations:

# these are all from the xpack secret, and are separate keys within the secret itself:
Linking /mnt/elastic-internal/xpack-file-realm/users to /usr/share/elasticsearch/config/users
Linking /mnt/elastic-internal/xpack-file-realm/roles.yml to /usr/share/elasticsearch/config/roles.yml
Linking /mnt/elastic-internal/xpack-file-realm/users_roles to /usr/share/elasticsearch/config/users_roles
# This is from the 'config' secret, and is a key within the secret
Linking /mnt/elastic-internal/elasticsearch-config/elasticsearch.yml to /usr/share/elasticsearch/config/elasticsearch.yml
# This is a key within the unicast configmap
Linking /mnt/elastic-internal/unicast-hosts/unicast_hosts.txt to /usr/share/elasticsearch/config/unicast_hosts.txt
# This is another from the xpack secret
Linking /mnt/elastic-internal/xpack-file-realm/service_tokens to /usr/share/elasticsearch/config/service_tokens
File linking duration: 0 sec.

Why are we not simply mounting these keys as files from the secret/cm into this location?

For example (untested atm):

volumeMounts:
- mountPath: /usr/share/elasticsearch/config
   name: xpack-users
   readOnly: true
volumes:
  - name: xpack-users
     secret:
        secretName: elasticsearch-es-xpack-file-realm
        items:
        - key: users
          path: users

[barkbay]

Why are we not simply mounting these keys as files from the secret/cm into this location?

mountPath must be unique, and all the original content of /usr/share/elasticsearch/config would be wiped out. I think the idea is to keep the original content of /usr/share/elasticsearch/config, as it is provided by the Elasticsearch image, and update only what is necessary.

Also note that subPath cannot be used as the content would not be updated:

Note: A container using a Secret as a subPath volume mount does not receive automated Secret updates.

[naemono]

Also note that subPath cannot be used as the content would not be updated:

So the solution to one of the 2 issues (that we know of) is having a separate secret for each of these files that are needed to be propagated into the config directory and cannot be renamed or pointed to a different file in the elasticsearch.yml.

As for the issue of the configuration directory we can adjust this via ES_PATH_CONF environment variable and use a mount path outside of root but that would involve a full redesign of how we're handling ES configuration. These are the files I see inside of existing config directory btw:

config/role_mapping.yml
config/service_tokens
config/transport-certs/
config/log4j2.properties
config/jvm.options
config/jvm.options.d
config/elasticsearch-plugins.example.yml
config/transport-remote-certs/
config/node-transport-cert/
config/elasticsearch.yml
config/elasticsearch.keystore
config/unicast_hosts.txt
config/users_roles
config/roles.yml
config/http-certs/
config/users
config/log4j2.file.properties

How would you feel about this proposal @barkbay :

1. solving the existing issues seen in the e2e test run with just running the main container as readOnlyRootFilesystem: true.
2. Investigating adding the additional security features you mentioned above.
3. adjusting the existing issue to be an issue about redesigning how we do ES configuration to allow readOnlyRootFilesystem: true
4. merging this as an incremental step in the direction of running the whole pod as readonly?

[Jorricks]

I'm also wondering if it would be a good thing to include other security settings like:

securityContext:
  allowPrivilegeEscalation: false
  capabilities:
    drop:
      - ALL
  privileged: false
  readOnlyRootFilesystem: true
  runAsNonRoot: true

Just jumping in here (hope you don't mind). That would be awesome. Currently we are enforcing our namespaces to have a restricted pod security standard. However, this operator breaks under this enforcement without the above.

[barkbay]

Currently we are enforcing our namespaces to have a restricted pod security standard.

Unfortunately runAsNonRoot is not that straightforward as the Elasticsearch image is running as a non numeric user:

elasticsearch-sample-es-default-2   0/1     Init:CreateContainerConfigError   0          3m37s

Warning  Failed     2m10s (x12 over 3m48s)  kubelet            Error: container has runAsNonRoot and image has non-numeric user (elasticsearch), cannot verify user is non-root

But maybe this is something we can discuss with the Elasticsearch team.

[naemono]

But maybe this is something we can discuss with the Elasticsearch team.

I'll discuss with the Elasticsearch team and see what can be done. Thanks

[naemono]

@barkbay elastic/elasticsearch#95390 opened against Elasticsearch for this change after speaking with this and seeing no reason to not make this change.

[naemono]

@barkbay elastic/elasticsearch#95390 opened against Elasticsearch for this change after speaking with this and seeing no reason to not make this change.

And the PR was already merged so we can enable and test these against 8.8-SNAPSHOT when it becomes available.

How would you feel about the above proposal @barkbay against 8.8+ only?

[thbkrkr]

Superseeded by #6703.