opensearch-project / helm-charts

:wheel_of_dharma: A community repository for Helm Charts of OpenSearch Project.
https://opensearch.org/docs/latest/opensearch/install/helm/
Apache License 2.0
173 stars 234 forks source link

[BUG][OpenSearch] global.dockerRegistry does not support the value "null" #510

Open gsmith-sas opened 11 months ago

gsmith-sas commented 11 months ago

Describe the bug Helm charts keys can use the value "null" to indicate that the key should not be set at all. I tried to set the global.dockerRegistry key in the OpenSearch Helm chart to 'null' and the Helm deployment fails with the message "invalid value; expected string".

To Reproduce Steps to reproduce the behavior:

  1. Deploy the OpenSearch Helm chart and include --set global.dockerRegistry=null (e.g. helm install my-release --set global.dockerRegistry=null opensearch/opensearch)
  2. See error: Error: INSTALLATION FAILED: template: opensearch/templates/_helpers.tpl:110:49: executing "opensearch.dockerRegistry" at <"/">: invalid value; expected string

Expected behavior This should have deployed the Helm chart without errors.

Chart Name OpenSearch

Screenshots

$ helm install my-release --set global.dockerRegistry=null opensearch/opensearch
Error: INSTALLATION FAILED: template: opensearch/templates/_helpers.tpl:110:49: executing "opensearch.dockerRegistry" at <"/">: invalid value; expected string

Host/Environment (please complete the following information):

Additional context

prudhvigodithi commented 11 months ago

[Untriage] Thanks @gsmith-sas, may I know the reason for setting the value as null when installing the chart ?, the null keyword is being inferred as string and adds to backend templates. Instead of setting it to null, you can leave it to "", to ensure it takes the default, only if you have a private registry you can pass in the registry information to dockerRegistry. But I'm also open for design change to make sure the global.dockerRegistry is commented out by default and only if a user wants he can enable the key global.dockerRegistry and pass in the right registry value. @gsmith-sas can you please contribute to this change? Adding @TheAlgo @DandyDeveloper @gaiksaya @peterzhuamazon @bbarani