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

[bitnami/redis] fix: update JSON schema to allow string values for values passed to tpl #30526

Merged
merged 7 commits into from
Jan 14, 2025

Conversation

piccit
Copy link
Contributor

@piccit piccit commented Nov 19, 2024

Description of the change

Updates all properties in values.schema.json that are directly passed to tpl to also allow for string fields.

Benefits

Fixes schema to allow passing strings to tpl for all values that are template-able

Possible drawbacks

None

Applicable issues

Additional information

None

Checklist

  • Chart version bumped in Chart.yaml according to semver. This is not necessary when the changes only affect README.md files.
  • Variables are documented in the values.yaml and added to the README.md using readme-generator-for-helm
  • Title of the pull request follows this pattern [bitnami/<name_of_the_chart>] Descriptive title
  • All commits signed off and in agreement of Developer Certificate of Origin (DCO)

Signed-off-by: Thomas Piccioli <[email protected]>
bitnami-bot and others added 3 commits November 19, 2024 14:11
Signed-off-by: Bitnami Containers <[email protected]>
Signed-off-by: Thomas Piccioli <[email protected]>
@carrodher carrodher added verify Execute verification workflow for these changes in-progress labels Nov 19, 2024
@github-actions github-actions bot removed the triage Triage is needed label Nov 19, 2024
@github-actions github-actions bot removed the request for review from carrodher November 19, 2024 18:18
@github-actions github-actions bot requested a review from jotamartos November 19, 2024 18:18
Copy link
Contributor

@jotamartos jotamartos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for taking the time to contribute but I can approve this change, sorry. There are many parameters from there that do not support a simple string to configure them (hostAliases, customLivenessProbe, customRedinessProbe, extraEnvVars, ...). As you can see, those are objects/arrays that you need to define in the values.yaml file when deploying the solution.

@piccit
Copy link
Contributor Author

piccit commented Dec 4, 2024

There are many parameters from there that do not support a simple string to configure them (hostAliases, customLivenessProbe, customRedinessProbe, extraEnvVars, ...). As you can see, those are objects/arrays that you need to define in the values.yaml file when deploying the solution.

@jotamartos No, that's not true - per the issue #29451 and related #27319 and PR #27441 anything that is passed to the template function(s) can accept a string. Having the schema enforce not-string breaks the ability to utilize the template (see example in #29451.

@piccit
Copy link
Contributor Author

piccit commented Dec 21, 2024

Bump @jotamartos, please re-open and re-review per the linked issues and pr

@jotamartos
Copy link
Contributor

Sorry for the late reply here. I've checked the issues you shared and you are completely right. It's important to take into account what @migruiz4 mentioned in #27441 but all in all this PR is completely legit. Please bump the branch and update the version so I merge the changes.

@jotamartos jotamartos reopened this Jan 14, 2025
@github-actions github-actions bot added triage Triage is needed and removed solved labels Jan 14, 2025
@github-actions github-actions bot requested a review from javsalgar January 14, 2025 14:43
@jotamartos
Copy link
Contributor

Update: I could resolve the conflicts using the UI and the changelog will be automatically generated soon. If tests pass, I'll merge the changes

Signed-off-by: Bitnami Containers <[email protected]>
@jotamartos jotamartos enabled auto-merge (squash) January 14, 2025 15:00
@jotamartos jotamartos merged commit 2c78a06 into bitnami:main Jan 14, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
redis solved triage Triage is needed verify Execute verification workflow for these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bitnami/redis] json schema too strict for any template-able value
5 participants