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

Print annotation and label value as YAML to support array string values #589

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mark-hofmeijer
Copy link
Contributor

@mark-hofmeijer mark-hofmeijer commented Oct 14, 2024

What was changed

This PR adds support for array strings as annotations and labels.

Why?

Annotations and labels with array string as value (e.g. ad.datadoghq.com/temporal-web.logs: '[{"source":"temporal","service":"temporal-web"}]') results in an error, both on installation and upgrade.

Error message:
Error: UPGRADE FAILED: cannot patch "temporal-admintools" with kind Deployment: "" is invalid: patch: Invalid value: "{...}": json: cannot unmarshal array into Go struct field ObjectMeta.spec.template.metadata.annotations of type string

Checklist

  1. How was this tested:
    Automated.

Or manual via:
helm install -f values.yaml --set web.podAnnotations.arrayannotation='[{"test":"true"}]' debug . --dry-run --debug

  1. Any docs updates needed?
    No.

@mark-hofmeijer mark-hofmeijer requested a review from a team as a code owner October 14, 2024 14:13
@mark-hofmeijer
Copy link
Contributor Author

@robholland Can you take a look at this one too? Although the latests PRs add support for "normal" string/integer/boolean values, it does not work for strings with arrays in it. This PR fixes this (and is eventually what I need :-)). Thanks!

@stevengubler
Copy link

+1. We ran into this issue as well on the latest releases. Version 0.48.0 still respects structured data as annotations, so we've pinned the helm release to it as a temporary fix.

@mark-hofmeijer
Copy link
Contributor Author

@robholland Do you have time to give your review on this PR? Many thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants