feat: Add Support for Using Existing Secrets in greptimedb-cluster Helm Chart for Secure Credential Management #150
Conversation
WalkthroughThe recent changes enhance the configuration of object storage in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Template
participant SecretManager
User->>Template: Deploy configuration
Template->>Template: Check existingSecretName
alt existingSecretName provided
Template->>SecretManager: Use existing secret
else no existingSecretName
Template->>SecretManager: Create new secret
end
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
charts/greptimedb-cluster/templates/secret.yaml (1)
16-16: Unclosed conditional blocks detected in Helm templates.Please ensure that all conditional blocks are properly closed to avoid rendering issues in the Helm templates. The following files contain unclosed conditional blocks:
charts/greptimedb-standalone/templates/serviceaccount.yamlcharts/greptimedb-standalone/templates/statefulset.yamlcharts/greptimedb-cluster/templates/serviceaccount-meta.yamlcharts/greptimedb-cluster/templates/serviceaccount-datanode.yamlcharts/greptimedb-cluster/templates/serviceaccount-frontend.yamlcharts/greptimedb-standalone/templates/secret.yamlcharts/greptimedb-cluster/templates/cluster.yamlcharts/greptimedb-cluster/templates/serviceaccount-flownode.yamlcharts/greptimedb-operator/templates/serviceaccount.yamlcharts/greptimedb-cluster/templates/secret.yamlcharts/greptimedb-operator/templates/rolebinding.yamlcharts/greptimedb-operator/templates/deployment.yamlcharts/greptimedb-operator/templates/role.yamlcharts/greptimedb-operator/templates/clusterrolebinding.yamlcharts/greptimedb-standalone/templates/podmonitor.yamlcharts/greptimedb-operator/templates/clusterrole.yamlcharts/greptimedb-standalone/templates/configmap.yamlPlease review and close all conditional blocks appropriately.
Analysis chain
Ensure the closing of all conditional blocks.
Ensure that all conditional blocks are properly closed to avoid any rendering issues in the Helm template.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all conditional blocks are properly closed. # Test: Search for unclosed conditional blocks. Expect: No unclosed blocks. rg --type yaml '{{- if' -A 20 | rg -v '{{- end }}'Length of output: 220
Script:
#!/bin/bash # Description: Verify that all conditional blocks are properly closed. # Test: Search for unclosed conditional blocks. Expect: No unclosed blocks. rg --type yaml '\{\{- if' -A 20 | rg -v '\{\{- end \}'Length of output: 52849
charts/greptimedb-cluster/templates/cluster.yaml (1)
294-294: Ensure the closing of all conditional blocks.The verification script indicates that there are multiple instances of
{{- if ...without corresponding{{- end }}within a 100-line context. This suggests potential unclosed conditional blocks in the Helm templates.
charts/greptimedb-standalone/templates/serviceaccount.yamlcharts/greptimedb-standalone/templates/statefulset.yamlcharts/greptimedb-standalone/templates/secret.yamlcharts/greptimedb-standalone/templates/configmap.yamlcharts/greptimedb-standalone/templates/podmonitor.yamlcharts/greptimedb-operator/templates/serviceaccount.yamlcharts/greptimedb-operator/templates/deployment.yamlcharts/greptimedb-operator/templates/role.yamlcharts/greptimedb-operator/templates/clusterrolebinding.yamlcharts/greptimedb-operator/templates/rolebinding.yamlcharts/greptimedb-operator/templates/clusterrole.yamlcharts/greptimedb-cluster/templates/secret.yamlcharts/greptimedb-cluster/templates/serviceaccount-meta.yamlcharts/greptimedb-cluster/templates/serviceaccount-frontend.yamlcharts/greptimedb-cluster/templates/cluster.yamlcharts/greptimedb-cluster/templates/serviceaccount-flownode.yamlcharts/greptimedb-cluster/templates/serviceaccount-datanode.yamlPlease review these files to ensure that all conditional blocks are properly closed to avoid any rendering issues in the Helm templates.
Analysis chain
Ensure the closing of all conditional blocks.
Ensure that all conditional blocks are properly closed to avoid any rendering issues in the Helm template.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all conditional blocks are properly closed. # Test: Search for unclosed conditional blocks. Expect: No unclosed blocks. rg --type yaml '{{- if' -A 20 | rg -v '{{- end }}'Length of output: 220
Script:
#!/bin/bash # Description: Verify that all conditional blocks are properly closed. # Test: Search for unclosed conditional blocks. Expect: No unclosed blocks. rg --type yaml '\{\{- if' -A 50 | rg -v '\{\{- end \}\}'Length of output: 57902
Script:
#!/bin/bash # Description: Verify that all conditional blocks are properly closed. # Test: Search for unclosed conditional blocks. Expect: No unclosed blocks. rg --type yaml '\{\{- if' -A 100 | rg -v '\{\{- end \}\}'Length of output: 61307
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- charts/greptimedb-cluster/templates/cluster.yaml (1 hunks)
- charts/greptimedb-cluster/templates/secret.yaml (2 hunks)
- charts/greptimedb-cluster/values.yaml (1 hunks)
Additional comments not posted (3)
charts/greptimedb-cluster/templates/secret.yaml (1)
3-4: Ensure proper handling ofexistingSecretName.The conditional logic correctly checks for the presence of
existingSecretNameand skips the creation of a new secret if it is defined. This enhances flexibility and security by allowing the use of existing secrets.charts/greptimedb-cluster/values.yaml (1)
406-406: Addition ofexistingSecretNameconfiguration option.The new configuration option
existingSecretNameis correctly added as a commented-out line within theobjectStoragesection. This ensures that it is optional and does not affect existing functionality.charts/greptimedb-cluster/templates/cluster.yaml (1)
288-290: Ensure proper handling ofexistingSecretName.The conditional logic correctly checks for the presence of
existingSecretNameand uses it as thesecretNameif it is defined. This enhances flexibility and security by allowing the use of existing secrets.
Stephan3555
changed the title
|
@Stephan3555 Thank for your contribution. This configuration needs to be added to OSS storage as well. Step
|
|
Thanks for the review, will take care of it |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- charts/greptimedb-cluster/templates/cluster.yaml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- charts/greptimedb-cluster/templates/cluster.yaml
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- charts/greptimedb-cluster/Chart.yaml (1 hunks)
Files skipped from review due to trivial changes (1)
- charts/greptimedb-cluster/Chart.yaml
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- charts/greptimedb-cluster/README.md (1 hunks)
- charts/greptimedb-cluster/values.yaml (1 hunks)
Files skipped from review due to trivial changes (1)
- charts/greptimedb-cluster/README.md
Files skipped from review as they are similar to previous changes (1)
- charts/greptimedb-cluster/values.yaml
|
I made some changes to the code. |
|
@Stephan3555 There's an oversight in this PR that I need to explain. I found @daviderli614 helping you modify some code to fix CI errors, which is not recommended for open-source cooperation. Subsequently, I inadvertently approved these changes without fully considering the implications. We sincerely apologize for this mistake. We highly value every contributor's work and believe collaboration and discussion lead to the best outcomes. I have already approved most of the modifications of you and @daviderli614. You can double-check the PR and merge it. I hope this will not cause you any inconvenience, and thank you for your contribution. |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- charts/greptimedb-cluster/templates/secret.yaml (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- charts/greptimedb-cluster/templates/secret.yaml
|
I simplified the logic in the secrets.yaml base on your suggestions @zyy17 . May i have another review plz? |
LGTM. When the CI is finished, we can merge the PR. |
This PR introduces a new feature that allows the use of a third-party tool for securely storing credentials outside of the Helm chart.
Currently, the greptimedb-cluster Helm chart does not support this capability, as the 'objectStorage.credentials.secretName' setting only changes the name of the automatically created secret.
With my changes, a new setting, 'objectStorage.credentials.existingSecretName', is added to the values YAML file. By specifying a value for this new setting, the default secret will not be created. Instead, the reference will point to the existing secret specified, allowing for more flexible and secure credential management.
Summary by CodeRabbit
New Features
Documentation