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

collector chart 0.9.4 cannot be deployed with some helm versions #143

Merged
merged 1 commit into from
Mar 8, 2022

Conversation

bender316
Copy link
Contributor

The helm chart with 0.9.4 set additionalProperties to false in values.schema.json.

Depending on the helm version used the chart fails to be deployed with the following error message:
- (root): Additional property global is not allowed

There is also an issue already in the helm repository:
helm/helm#10392

Current established workaround is to add global to the allowed properties or set additionalProperties to true. I choose the first possibility

@bender316 bender316 requested review from a team March 7, 2022 16:46
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Mar 7, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

@dmitryax
Copy link
Member

dmitryax commented Mar 7, 2022

Thanks @bender316, can you please sign the CLA?

@TylerHelmuth
Copy link
Member

TylerHelmuth commented Mar 7, 2022

Great catch. @dmitryax think there is a test we could add to the workflow that would've caught this?

@dmitryax
Copy link
Member

dmitryax commented Mar 7, 2022

@TylerHelmuth Yes, we have it already, it's failing

@TylerHelmuth
Copy link
Member

TylerHelmuth commented Mar 7, 2022

@TylerHelmuth Yes, we have it already, it's failing

😬 where?

@dmitryax
Copy link
Member

dmitryax commented Mar 7, 2022

@TylerHelmuth Oh I was thinking you are talking about CLA :)

Catching issues like the one fixed in this PR makes sense. Probably we can have a matrix of popular helm versions in the lint-test job.

@bender316 which helm version do you use when it's failing?

@TylerHelmuth
Copy link
Member

TylerHelmuth commented Mar 7, 2022

Catching issues like the one fixed in this PR makes sense. Probably we can have a matrix of popular helm versions in the lint-test job.

I like this idea. Maybe test against the latest helm version and a couple previous Latest Stable Versions.

@dmitryax
Copy link
Member

dmitryax commented Mar 7, 2022

Maybe test against the latest helm version and a couple previous Latest Stable Versions.

Sounds good

@bender316
Copy link
Contributor Author

Thanks @bender316, can you please sign the CLA?

Done.

@dmitryax dmitryax merged commit bd22888 into open-telemetry:main Mar 8, 2022
dmitryax added a commit that referenced this pull request Mar 18, 2022
Tyler has been actively involved in this repo recently. The following PR were submitted or reviewed:
- #129
- #131
- #139
- #147
- #151
- #142
- #143
- #148

cc @open-telemetry/helm-approvers
dmitryax added a commit that referenced this pull request Mar 20, 2022
Tyler has been actively involved in this repo recently. The following PR were submitted or reviewed:
- #129
- #131
- #139
- #147
- #151
- #142
- #143
- #148

cc @open-telemetry/helm-approvers
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.

3 participants