Support string type for extraObjects

opensearch-project / helm-charts

:wheel_of_dharma: A community repository for Helm Charts of OpenSearch Project.

https://opensearch.org/docs/latest/opensearch/install/helm/

Apache License 2.0

170 stars 228 forks source link

Support string type for extraObjects #429

Closed kevinleturc closed 1 year ago

kevinleturc commented 1 year ago

Description

Support string type for extraObjects.

Issues Resolved

[Enhancement][opensearch/opensearch-dashboards] Make possible to give string to extraObjects - https://github.com/opensearch-project/helm-charts/issues/428

Check List

[x] Commits are signed per the DCO using --signoff

For any changes to files within Helm chart directories:

[x] Helm chart version bumped
[x] Helm chart CHANGELOG.md updated to reflect change

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check here.

kevinleturc commented 1 year ago

Hello, I'm unable to see why the check is failing, I ran it locally and it succeeded, can you help me with that?

kevinleturc commented 1 year ago

Hello @TheAlgo, could you help me on the GitHub action issue?

prudhvigodithi commented 1 year ago

Hey @kevinleturc thanks for your patience and contribution, this PR is useful, can you please fix the conflicts and sync up with upstream? we can proceed to merge this PR. thank you

kevinleturc commented 1 year ago

Hey @kevinleturc thanks for your patience and contribution, this PR is useful, can you please fix the conflicts and sync up with upstream? we can proceed to merge this PR. thank you

Hey @prudhvigodithi, no problem, I just rebased the work, thanks for reviewing it.

prudhvigodithi commented 1 year ago

LGTM!, thanks for you contribution. One qq @kevinleturc, looking at the code does not seem like a breaking change, helm upgrade on an existing chart should not break the existing setup right? @peterzhuamazon @TheAlgo can you please take a look? Thank you

prudhvigodithi commented 1 year ago

This change is useful for 1.x release as well, the change should be backported to 1.x branch. Thanks

kevinleturc commented 1 year ago

@prudhvigodithi, indeed upgrading existing chart shouldn't break anything as extraObjects will behave the same for value type (the current), and with this PR the string type will also be handled.

I just did an upgrade test with extraObjects and everything works fine.

prudhvigodithi commented 1 year ago

Thanks @kevinleturc merged, can you please backport to 1.x?

kevinleturc commented 1 year ago

Thanks @kevinleturc merged, can you please backport to 1.x?

Sure

kevinleturc commented 1 year ago

Here is the backport https://github.com/opensearch-project/helm-charts/pull/441.

Thanks