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

Change type of invenio.extra_env_from_secret #119

Merged

Conversation

lindhe
Copy link
Contributor

@lindhe lindhe commented Mar 25, 2024

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:

{{- range .Values.invenio.extra_env_from_secret }}
- name: {{ .name }}
valueFrom:
secretKeyRef:
name: {{ .valueFrom.secretKeyRef.name }}
key: {{ .valueFrom.secretKeyRef.key }}
{{- end }}

{{- range .Values.invenio.extra_env_from_secret }}
- name: {{ .name }}
valueFrom:
secretKeyRef:
name: {{ .valueFrom.secretKeyRef.name }}
key: {{ .valueFrom.secretKeyRef.key }}
{{- end }}

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
Copy link
Contributor Author

lindhe commented Mar 25, 2024

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

@Samk13 Samk13 requested review from slint and removed request for slint March 25, 2024 22:29
@lindhe
Copy link
Contributor Author

lindhe commented Mar 26, 2024

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

@ntarocco ntarocco merged commit 7792f94 into inveniosoftware:master Mar 26, 2024
@lindhe lindhe deleted the lindhe/fix-extra-env-secret-type branch March 26, 2024 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Released | Done 🚀
Development

Successfully merging this pull request may close these issues.

2 participants