added missing secrets on statefulset template #272

phijojo · 2022-05-31T23:48:26Z

Signed-off-by: Phijo Jospeh [email protected]

Description

The Statefulset template is missing 2 secrets when adding individual secrets.

Issues Resolved

#191

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.

…Jospeh <[email protected]> Signed-off-by: Phijo Joseph <[email protected]>

prudhvigodithi · 2022-06-01T10:22:38Z

From Comment #191 (comment)
@TheAlgo @DandyDeveloper @peterzhuamazon @bbarani are we good with adding nodesDnSecret and whitelistSecret option within chart, thoughts ?

bbarani · 2022-06-01T21:06:53Z

charts/opensearch/Chart.yaml

@@ -30,3 +30,5 @@ maintainers:
  - name: peterzhuamazon
  - name: prudhvigodithi
  - name: TheAlgo
+  - name: phijojo


We wont be able to add you as a maintainer without going through the nomination process. Please refer to at maintainers.md guide to understand the process to become a maintainer.

Good to know, thanks for sharing the document. I have removed my name from the maintainer's list.

Signed-off-by: Phijo Joseph <[email protected]>

DandyDeveloper

One of the problems with the whitelist is in 1.X its whitelist but in 2.X its now become allowlist.

We need to cater to both by adding a condition on the version of Opensearch being deployed.

DandyDeveloper · 2022-06-03T01:23:48Z

charts/opensearch/templates/statefulset.yaml

+      {{- if .Values.securityConfig.whitelistSecret }}
+      - name: whitelist
+        secret:
+          secretName: {{ .Values.securityConfig.whitelistSecret }}
+      {{- end }}


Shouldn't this be allowlist? https://opensearch.org/docs/latest/security-plugin/configuration/yaml/#allowlistyml

Yes, it should be.
I have used 2.0.0 image, in which the securityadmin.sh doesn't check for allowlist.yml instead it's still using whitelist.yml.So I guess securityadmin.sh need a change before we use allowlist.yml.

DandyDeveloper · 2022-06-03T01:24:05Z

charts/opensearch/templates/statefulset.yaml

+        {{- if .Values.securityConfig.whitelistSecret }}
+        - mountPath: {{ .Values.securityConfig.path }}/whitelist.yml
+          name: whitelist
+          subPath: whitelist.yml
+        {{- end }}


Shouldn't this be allowlist? https://opensearch.org/docs/latest/security-plugin/configuration/yaml/#allowlistyml

Yes, it should be.
I have used 2.0.0 image, in which the securityadmin.sh doesn't check for allowlist.yml instead it's still using whitelist.yml.So I guess securityadmin.sh need a change before we use allowlist.yml.

@DandyDeveloper @prudhvigodithi @TheAlgo

I think it's a bug in securityadmin.sh. I have added a ticket for that opensearch-project/security#1879

I'll change whitelist.yml to allowlist.yml on this PR and will create another PR for 1.x.x to handle the whitelist.yml

prudhvigodithi · 2022-06-03T10:34:07Z

Hey @DandyDeveloper and @phijojo we have main and 1.x branches that support 2.x and 1.x versions, PR can be raised to main branch that supports allowlist and use whitelist in 1.x branch.
https://github.com/opensearch-project/helm-charts#version-and-branching
@peterzhuamazon

Signed-off-by: Phijo Joseph <[email protected]>

TheAlgo

Changes looks good, the only thing I am not sure on the whitelist/allowlist thingy. I am fine with whatever @prudhvigodithi has suggested

TheAlgo · 2022-06-03T17:12:24Z

charts/opensearch/CHANGELOG.md

@@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ---
 ## [Unreleased]
 ### Added
+- Updated StatefulSet with node_dn.yml and whitelist.yml


Do you mind seeing the PR(s) before on updating the CHANGELOG?

Sure, Do you want me to remove this from the current PR and submit a different one?

Hey @phijojo please consider this as an example
https://github.com/opensearch-project/helm-charts/pull/250/files#diff-96471865d01149e108175156796718f7584b7d5fae730d0cb5dbd98426bcd801

prudhvigodithi · 2022-06-03T18:50:44Z

@phijojo, please test by just keeping allowlist for main branch and create a PR with whitelist for 1.x branch
https://github.com/opensearch-project/security/blob/main/securityconfig/allowlist.yml
https://github.com/opensearch-project/security/blob/main/securityconfig/whitelist.yml
Thank you
@peterzhuamazon @DandyDeveloper @TheAlgo @bbarani

prudhvigodithi · 2022-06-11T14:09:36Z

Hey since whitelist works for both 2.x and 1.x we can push this to add whitelist and ignore allowlist, until opensearch-project/security#1879 this gets addressed, I have tested with whitelist secret and the chart works fine. Thoughts ? @peterzhuamazon @DandyDeveloper @TheAlgo @bbarani @phijojo @smlx

peterzhuamazon · 2022-08-02T16:09:07Z

Hi @phijojo any progress to keep this PR going?
Thanks.

bbarani · 2023-02-09T20:28:20Z

Hi @phijojo @prudhvigodithi Any updates on this PR?

prudhvigodithi · 2023-03-02T00:36:15Z

Hey @phijojo I have converted this PR to draft, please let us know if this PR required and a valid scenario.
Thank you

prudhvigodithi · 2023-03-22T18:30:19Z

Hey @phijojo closing this PR, please re-open if required.
Thank you for your contribution.

phijojo requested a review from a team as a code owner May 31, 2022 23:48

phijojo mentioned this pull request May 31, 2022

[BUG][opensearch] securityConfigSecret not volumeMount(ed) #191

Closed

phijojo force-pushed the add-missing-secrets branch from 6ef1fe9 to b9c70eb Compare June 1, 2022 00:01

- added missing secrets on statefulset template Signed-off-by: Phijo …

2460922

…Jospeh <[email protected]> Signed-off-by: Phijo Joseph <[email protected]>

phijojo force-pushed the add-missing-secrets branch from b9c70eb to 2460922 Compare June 1, 2022 09:21

bbarani reviewed Jun 1, 2022

View reviewed changes

removed phijojo from maintainer list

29b6dd2

Signed-off-by: Phijo Joseph <[email protected]>

peterzhuamazon requested review from prudhvigodithi, TheAlgo and DandyDeveloper June 2, 2022 21:11

DandyDeveloper requested changes Jun 3, 2022

View reviewed changes

removed Whitespace from chart.yml

1cee68a

Signed-off-by: Phijo Joseph <[email protected]>

TheAlgo reviewed Jun 3, 2022

View reviewed changes

TheAlgo requested changes Jun 3, 2022

View reviewed changes

phijojo mentioned this pull request Jun 8, 2022

[BUG] on opensearch 2.0.0, securityadmin.sh is still looking for whitelist.yml instead of allowlist.yml opensearch-project/security#1879

Closed

peterzhuamazon linked an issue Jul 11, 2022 that may be closed by this pull request

[BUG][opensearch] securityConfigSecret not volumeMount(ed) #191

Closed

prudhvigodithi marked this pull request as draft March 2, 2023 00:35

prudhvigodithi closed this Mar 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added missing secrets on statefulset template #272

added missing secrets on statefulset template #272

phijojo commented May 31, 2022 •

edited

Loading

prudhvigodithi commented Jun 1, 2022 •

edited

Loading

bbarani Jun 1, 2022 •

edited

Loading

phijojo Jun 2, 2022

DandyDeveloper left a comment

DandyDeveloper Jun 3, 2022

phijojo Jun 3, 2022

DandyDeveloper Jun 3, 2022

phijojo Jun 3, 2022

phijojo Jun 8, 2022

prudhvigodithi commented Jun 3, 2022 •

edited

Loading

TheAlgo left a comment •

edited

Loading

TheAlgo Jun 3, 2022

phijojo Jun 3, 2022

prudhvigodithi Jun 3, 2022

prudhvigodithi commented Jun 3, 2022

prudhvigodithi commented Jun 11, 2022 •

edited

Loading

peterzhuamazon commented Aug 2, 2022

bbarani commented Feb 9, 2023

prudhvigodithi commented Mar 2, 2023

prudhvigodithi commented Mar 22, 2023

added missing secrets on statefulset template #272

added missing secrets on statefulset template #272

Conversation

phijojo commented May 31, 2022 • edited Loading

Description

Issues Resolved

Check List

prudhvigodithi commented Jun 1, 2022 • edited Loading

bbarani Jun 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DandyDeveloper left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prudhvigodithi commented Jun 3, 2022 • edited Loading

TheAlgo left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prudhvigodithi commented Jun 3, 2022

prudhvigodithi commented Jun 11, 2022 • edited Loading

peterzhuamazon commented Aug 2, 2022

bbarani commented Feb 9, 2023

prudhvigodithi commented Mar 2, 2023

prudhvigodithi commented Mar 22, 2023

phijojo commented May 31, 2022 •

edited

Loading

prudhvigodithi commented Jun 1, 2022 •

edited

Loading

bbarani Jun 1, 2022 •

edited

Loading

prudhvigodithi commented Jun 3, 2022 •

edited

Loading

TheAlgo left a comment •

edited

Loading

prudhvigodithi commented Jun 11, 2022 •

edited

Loading