inveniosoftware / helm-invenio

Helm charts for deploying an Invenio instance
https://helm-invenio.readthedocs.io
8 stars 21 forks source link

Change type of `invenio.extra_env_from_secret` #119

Closed lindhe closed 8 months ago

lindhe commented 8 months ago

Description

This change fixes a typo for the default value (and importantly: type) of the invenio.extra_env_from_secret value.

Before this change, I got the following warning when setting invenio.extra_env_from_secret:

coalesce.go:286: warning: cannot overwrite table with non table for invenio.invenio.extra_env_from_secret (map[])

This was because the default value of invenio.extra_env_from_secret was {} (the empty object) while it is clearly used as a list:

https://github.com/inveniosoftware/helm-invenio/blob/eb6544fdeb7e762972db1dae26f848c2f2e4c029/charts/invenio/templates/web-deployment.yaml#L83-L89

https://github.com/inveniosoftware/helm-invenio/blob/eb6544fdeb7e762972db1dae26f848c2f2e4c029/charts/invenio/templates/worker-deployment.yaml#L87-L93

So when setting invenio.extra_env_from_secret to a list instead of an object, Helm gets nervous. This change fixes that.

I've also added an example comment to show what the structure of each list item should be.

In the long term, I believe a more overarching change as discussed in #117 is better. My change here is just a quick bug fix.

Checklist

Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:

Third-party code

If you've added third-party code (copy/pasted or new dependencies), please reach out to an architect.

Reminder

By using GitHub, you have already agreed to the GitHub’s Terms of Service including that:

  1. You license your contribution under the same terms as the current repository’s license.
  2. You agree that you have the right to license your contribution under the current repository’s license.
lindhe commented 8 months ago

It seems like this did not fix the warning. Let me try some more before merge.

lindhe commented 8 months ago

False alarm! It did fix the bug, I just tested on the wrong branch. 🙃