Comment out hard coded opensearch.yml

[derek-ho]

Description

This PR stops feeding in a hard coded opensearch.yml into the pods. We are expecting to generate this output from running the install demo configuration script from the security plugin side.

Issues Resolved

[List any issues this PR will resolve. You should likely open an issue if one does not already exist.]

Check List

• [x] Commits are signed per the DCO using --signoff

For any changes to files within Helm chart directories:

• [x] Helm chart version bumped
• [x] Helm chart CHANGELOG.md updated to reflect change

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 following Developer Certificate of Origin and signing off your commits, please check here.

[smlx]

I don't understand this change. This is a helm chart, they are configured via values files.. how are you generating your configuration otherwise?

[DarshitChanpura]

@smlx @TheAlgo When an OpenSearch image is downloaded and cluster is spun up, the Security plugin runs install demo configuration by default as part of install. During this phase, opensearch.yml is loaded with security related values. Hence we do not need to hardcode opensearch.yml. Feel free to run this locally and lmk if otherwise.

[DarshitChanpura]

@prudhvigodithi Could you please review this?

[DarshitChanpura]

@prudhvigodithi Could you please add 1 more review?

[prudhvigodithi]

Hey Just tested this and works fine with removing the .Values.config , basically when user adds the opensearch.yml through .Values.config, it gets added to /usr/share/opensearch/config/opensearch.yml overriding the default. Also one advantage is next if there is any change in opensearch.yml and added through .Values.config, the pods get auto restarted and loads the new values from opensearch.yml. So @DarshitChanpura and @derek-ho any specific reason we are removing, we can leave it as well right ?

Adding @TheAlgo @smlx @bbarani @peterzhuamazon

[DarshitChanpura]

So @DarshitChanpura and @derek-ho any specific reason we are removing, we can leave it as well right ?

From what I understand, the opensearch.yml always gets created when demo configuration script is run (which is default). If in case the user decides to pass in custom configuration, they can always pass it in via .values.yml. In either of these two cases, the hardcoded opensearch.yml serves no purpose. Which is why we are removing this? @derek-ho Please add if I missed anything here.

[smlx]

I don't know if it is on the roadmap of the Opensearch image to avoid generating the demo config at runtime but IMO it should be. Generating config at runtime has the following consequences for docker and k8s:

• it slows down container startup
• root filesystem can't be read only
• can't run the container as an arbitrary user (has to have permissions to write the config file)

IMO it makes more sense to aim for immutable images with defined injection points for configuration rather than generating config at runtime. Therefore I think this PR is a move in the wrong direction.

[prudhvigodithi]

Instead of commenting out opensearch.yml, I have raised a PR https://github.com/opensearch-project/helm-charts/pull/516, that address configMap Read-only file system error when passed via .Values.config, with this the user can either use the default opensearch.yml or pass the opensearch.yml via .Values.config. The file will be created as -rw-r--r-- 1 opensearch opensearch 3646 Feb 7 03:22 opensearch.yml, having RW access within the cluster, please check.

@TheAlgo @DarshitChanpura @peterzhuamazon @bbarani @derek-ho @smlx

[derek-ho]

I can close this out, since I tested out the alternative also works and it seems the direction of the community is not towards removing this.