[bitnami/elasticsearch] do not fail if asked to chown read-only files

[ianroberts]

Description of the change

Add || true to chown commands that operate recursively on folders that are expected to be mounted into the container from volumes or bind mounts, so that the chown does not cause a fatal error if any of the target directories or their children are mounted read-only.

Benefits

Currently when running as root the elasticsearch container will fail to start if any folder under /opt/bitnami/elasticsearch/config is mounted from a read-only filesystem. In particular this happens in the corresponding bitnami Helm chart, where the TLS certificates for Elasticsearch are mounted in from a Kubernetes secret. This change means that such cases are no longer fatal errors - the files in the read-only filesystems will not of course have their ownership changed, but there's not really anything better we can do if the filesystem is read-only.

Possible drawbacks

If a mounted filesystem should have been read-write but was mounted read-only by mistake, it would previously have caused a fatal error, prompting the user to change their configuration; this change will make that a no-op so they might not notice.

Applicable issues

• fixes [bitnami/elasticsearch] crash when running as root when TLS certificates are read-only #77525

Additional information

This fix should probably be applied before bitnami/charts#31960 is merged.

[github-actions]

This Pull Request has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thank you for your contribution.

[ianroberts]

Still relevant.

[migruiz4]

Thank you very much for your contribution @ianroberts. I'm sorry for the late response.

I would like to suggest two changes to your contribution:

• You used || :, but we prefer using || true.
• You added both || : and -f option for chown, which is redundant. The option -f suppresses both error exit-code and messages, while the || : would supress the exit code. I would suggest not using -f, since hiding error message could make troubleshooting really complicated in some cases.

[ianroberts]

Very sensible suggestions, I've implemented them and rebased on the latest main branch.

[ianroberts]

• You added both || : and -f option for chown, which is redundant. The option -f suppresses both error exit-code and messages, while the || : would supress the exit code.

My first attempt just used the -f but it turns out that -f suppresses the messages but does not on its own affect the exit code. So I added the || : (now || true) as well to actually fix the issue.

[ianroberts]

Still waiting for final review.

[github-actions]

This Pull Request has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thank you for your contribution.

[ianroberts]

Not stale, still waiting for final approval by @migruiz4

[github-actions]

This Pull Request has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thank you for your contribution.

[ianroberts]

Not stale. I addressed the review comments six weeks ago on 11th March but my fixes are still awaiting final approval. Please could someone look at this soon as I have another PR on the helm chart that is blocked waiting for this one.

[github-actions]

This Pull Request has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thank you for your contribution.

[ianroberts]

Not stale. I addressed the review comments six weeks ago on 11th March but my fixes are still awaiting final approval. Please could someone look at this soon as I have another PR on the helm chart that is blocked waiting for this one.

Eight weeks.

@carrodher if @migruiz4 is no longer available to handle this, would you be able to assign someone else?

[carrodher]

Sorry for the delay, this PR was somehow out of our radar. Could you please move the changes to the elasticsearch/9/debian-12 folder, which is the one available in Bitnami at this moment? Thanks

[ianroberts]

Rebased, but the delay is slightly unfortunate if it means there will never be a fixed container released for any Elasticsearch in the 8.x series 😞