Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
gsmith-sas opened this issue Dec 18, 2023 · 1 comment
Labels
bug Something isn't working

Comments

@gsmith-sas
Copy link

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):

  • Helm Version: 3.12.3
  • Kubernetes Version: 1.27

Additional context

  • See the topic Deleting a default key in the Helm documentation for more information about this special value.
  • Rather than including the global.dockerRegistry key and setting it to an empty string ("") as the default value, the values.yaml file chart should have this key commented out (indicating that it is NOT enabled by default). I believe this would be the more standard Helm way of handling this.
  • The code in _helpers.tpl will need to be revised to handle null values rather than (or, for backward compatibility, in addition to) empty strings.
  • Using Helm standards allows this Helm chart to be used with other Helm charts and/or work with more generic tooling built around those standards.
  • I notice other keys in the same Helm chart that either default to empty strings or (based on comments in the file) support the use of an empty string value. These may be other places where similar changes may be needed.
@gsmith-sas gsmith-sas added bug Something isn't working untriaged Issues that have not yet been triaged labels Dec 18, 2023
@gsmith-sas gsmith-sas changed the title [BUG][OpenSearch] global.dockerRegistry does not support "null" value [BUG][OpenSearch] global.dockerRegistry does not support the value "null" Dec 18, 2023
@prudhvigodithi
Copy link
Collaborator

[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

@prudhvigodithi prudhvigodithi removed the untriaged Issues that have not yet been triaged label Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Backlog
Development

No branches or pull requests

2 participants