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

fix: resolve helm lint errors with extraObjects #607

Merged
merged 2 commits into from
Oct 24, 2024

Conversation

cyanidium
Copy link
Contributor

Description

If using extraObjects, the generated manifest fails helm lint with:

[ERROR] templates/extraManifests.yaml: unable to parse YAML: invalid Yaml document separator: apiVersion: apps/v1

This is a known issue [1] just with the go templates used for helm lint and doesn't apply to the actual template that is applied. The fix [2] is to make the whitespace work for both commands.

[1] helm/helm#10149
[2] helm/helm#10149 (comment)

Issues Resolved

Resolves #606

Check List

  • Commits are signed per the DCO using --signoff

For any changes to files within Helm chart directories:

  • Helm chart version bumped
  • 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.

@cyanidium cyanidium changed the title Fix/helm lint fix: resolve helm lint errors with extraObjects Oct 20, 2024
@cyanidium
Copy link
Contributor Author

I just noticed that this also needs to be applied to the dashboards chart. The change undoes some unneeded whitespace removal from an earlier commit.

If using `extraObjects`, the generated manifest fails `helm lint` with:

```shell
[ERROR] templates/extraManifests.yaml: unable to parse YAML: invalid Yaml document separator: apiVersion: apps/v1
```

This is a known issue [1] just with the go templates used for helm lint
and doesn't apply to the actual template that is applied. The fix [2]
is to make the whitespace work for both commands.

[1] helm/helm#10149
[2] helm/helm#10149 (comment)

Signed-off-by: Martin Rowe <[email protected]>
@cyanidium cyanidium force-pushed the fix/helm-lint branch 2 times, most recently from daaf461 to 31edb1f Compare October 20, 2024 05:56
Copy link
Member

@TheAlgo TheAlgo left a comment

Choose a reason for hiding this comment

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

LGTM!

@peterzhuamazon peterzhuamazon merged commit b5d7058 into opensearch-project:main Oct 24, 2024
8 checks passed
@peterzhuamazon
Copy link
Member

Hi @cyanidium could you backport this change to 1.x branch as well?

Thanks.

@cyanidium
Copy link
Contributor Author

Hi @cyanidium could you backport this change to 1.x branch as well?

Thanks.

PR #608 should be the one

@peterzhuamazon
Copy link
Member

Hi @cyanidium could you backport this change to 1.x branch as well?
Thanks.

PR #608 should be the one

Thanks, merged.

@cyanidium cyanidium deleted the fix/helm-lint branch October 24, 2024 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

[BUG][opensearch] helm lint fails if using extraObjects
3 participants