Comment out hard coded opensearch.yml #504
Conversation
Signed-off-by: Derek Ho <[email protected]>
| config: | ||
| # Values must be YAML literal style scalar / YAML multiline string. | ||
| # <filename>: | | ||
| # <formatted-value(s)> | ||
| # log4j2.properties: | | ||
| # status = error | ||
| # | ||
| # appender.console.type = Console | ||
| # appender.console.name = console | ||
| # appender.console.layout.type = PatternLayout | ||
| # appender.console.layout.pattern = [%d{ISO8601}][%-5p][%-25c{1.}] [%node_name]%marker %m%n | ||
| # | ||
| # rootLogger.level = info | ||
| # rootLogger.appenderRef.console.ref = console | ||
| opensearch.yml: | | ||
| cluster.name: opensearch-cluster | ||
|
|
||
| # Bind to all interfaces because we don't know what IP address Docker will assign to us. | ||
| network.host: 0.0.0.0 | ||
|
|
||
| # Setting network.host to a non-loopback address enables the annoying bootstrap checks. "Single-node" mode disables them again. | ||
| # Implicitly done if ".singleNode" is set to "true". | ||
| # discovery.type: single-node | ||
|
|
||
| # Start OpenSearch Security Demo Configuration | ||
| # WARNING: revise all the lines below before you go into production | ||
| plugins: |
There was a problem hiding this comment.
This file should be generated via the demo configuration script, if it is hard coded we will need to maintain this in two places.
derek-ho
requested review from
bbarani,
TheAlgo,
DandyDeveloper,
peterzhuamazon,
prudhvigodithi and
gaiksaya
as code owners
|
I don't understand this change. This is a helm chart, they are configured via values files.. how are you generating your configuration otherwise? |
|
@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. |
Signed-off-by: Derek Ho <[email protected]>
Signed-off-by: Derek Ho <[email protected]>
Signed-off-by: Derek Ho <[email protected]>
Signed-off-by: Derek Ho <[email protected]>
derek-ho
changed the title
|
@prudhvigodithi Could you please review this? |
|
@prudhvigodithi Could you please add 1 more review? |
|
Hey Just tested this and works fine with removing the Adding @TheAlgo @smlx @bbarani @peterzhuamazon |
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. |
|
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:
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. |
|
Instead of commenting out @TheAlgo @DarshitChanpura @peterzhuamazon @bbarani @derek-ho @smlx |
|
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. |
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
For any changes to files within Helm chart directories:
CHANGELOG.mdupdated to reflect changeBy 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.