search

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

Added support for automountServiceAccountToken #425

Closed rdvansloten closed 1 year ago

rdvansloten commented 1 year ago

Description

This change adds support for automountServiceAccountToken, which in turn disables the mounting of /var/run/secrets/kubernetes.io/serviceaccount inside the OpenSearch pods when started. This setting now defaults to true, even when no serviceaccount is set, causing a collision with strict Kubernetes Gatekeeper policies that do not allow auto-mounting of service accounts.

See: https://store.policy.core.windows.net/kubernetes/block-automount-token/v2/template.yaml

Issues Resolved

https://github.com/opensearch-project/helm-charts/issues/427

Check List

For any changes to files within Helm chart directories:

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.

rdvansloten commented 1 year ago

Hi @TheAlgo,

Could you please help me out with the chart version? I versioned based off of main and the tags that were available, but the pipeline test keeps complaining my version is not good.

Issue: https://github.com/opensearch-project/helm-charts/issues/427

rdvansloten commented 1 year ago

Changes looks good to me. Can you add the new option in README? Also for chart version I think we can bump it to 2.12.2. PR is bumping the chart to 2.12.1. Thanks

Done. Also applied linting to the table I put it in, it was very fragmented, hence the line count.

rdvansloten commented 1 year ago

Any update on this @TheAlgo ? I have amended the changes.

rdvansloten commented 1 year ago

@TheAlgo, other reviewers, @bbarani @DandyDeveloper @peterzhuamazon @prudhvigodithi @gaiksaya

Anyone have time to review? This PR has been open for a long time 🙂

TheAlgo commented 1 year ago

Really sorry I was too occupied. Just ran the lint rules. I am reviewing it now.

TheAlgo commented 1 year ago

@prudhvigodithi @peterzhuamazon Please do have a look when free. Thanks

prudhvigodithi commented 1 year ago

Thanks @rdvansloten for your patience LGTM! Can you please check the Chart.yaml and CHANGELOG.md file? It should be as 2.12.1 but its 2.12.2. Can you please change this and we can proceed to merge this PR. Also do you think its useful if we backport to 1.x branch ? Thank you

prudhvigodithi commented 1 year ago

Hey @rdvansloten just merged this PR https://github.com/opensearch-project/helm-charts/pull/417 with 2.12.1, hence this PR can go with 2.12.2. Can you please take a look at the conflicts? Thank you

rdvansloten commented 1 year ago

Hey @rdvansloten just merged this PR #417 with 2.12.1, hence this PR can go with 2.12.2. Can you please take a look at the conflicts? Thank you

Done!

prudhvigodithi commented 1 year ago

Thanks @rdvansloten I see this still removes 2.12.0? have you done upstream fetch as well?

rdvansloten commented 1 year ago

Thanks @rdvansloten I see this still removes 2.12.0? have you done upstream fetch as well?

I did and the changelog is fixed. The DCO is all messed up and I am not experienced enough to fix this issue. Please provide guidance. The unconventional way of providing versioning to PRs at the last minute has really thrown me off several times during this process. Consider changing this.

prudhvigodithi commented 1 year ago

Sure @rdvansloten the versioning process should be updated, agree with you. To fix DCO can you try the following? (Just save your changes either in a new branch or a folder in ur local) git rebase HEAD~11 --signoff git push --force-with-lease origin fix-service-account

rdvansloten commented 1 year ago

Sure @rdvansloten the versioning process should be updated, agree with you. To fix DCO can you try the following? (Just save your changes either in a new branch or a folder in ur local) git rebase HEAD~11 --signoff git push --force-with-lease origin fix-service-account

I think this is fixed now. Waiting for the lint checks.

prudhvigodithi commented 1 year ago

Merged @rdvansloten can you please help backport to 1.x branch?