feat: add Azure Blob storage #207
Conversation
WalkthroughThe changes in this pull request involve updates to the Changes
Possibly related PRs
Suggested reviewers
Poem
Warning Rate limit exceeded@daviderli614 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 3 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
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 using PR comments)
Other keywords and placeholders
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: 5
🧹 Outside diff range and nitpick comments (5)
charts/greptimedb-standalone/templates/secret.yaml (2)
Line range hint
1-33: Consider enhancing secret management capabilitiesWhile the current implementation is solid, consider the following enhancements:
- Add support for external secret management systems (e.g., HashiCorp Vault, AWS Secrets Manager)
- Document best practices for credential rotation
- Add support for pod identity/IAM roles where applicable
This would improve security and operational aspects in production environments.
Special handling of serviceAccountKey is required but needs consistent implementation
The values.yaml files in both charts explicitly document that
serviceAccountKeyshould be a "JSON-formatted base64 value" for GCP cloud provider. However, there's an inconsistency in implementation:
- In standalone chart: The value is base64 encoded again (
b64enc)- In cluster chart: The value is used as-is
This suggests a potential issue where the standalone chart is double-encoding the already base64-encoded value.
🔗 Analysis chain
Line range hint
8-14: Verify the special handling of serviceAccountKeyWhile the template correctly uses
stringDatafor most credentials, theserviceAccountKeyis handled differently with explicit base64 encoding. Please verify if this special handling is required for Azure compatibility.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any documentation or code comments explaining the serviceAccountKey encoding requirement # Search for documentation about serviceAccountKey echo "Checking for serviceAccountKey documentation..." rg -i "serviceaccountkey|service.?account.?key" -A 5 "charts/greptimedb-standalone/" # Check if other charts handle it similarly echo "Checking other charts for similar patterns..." rg -i "serviceaccountkey|service.?account.?key" -A 5 "charts/greptimedb-cluster/"Length of output: 2963
charts/greptimedb-standalone/templates/_helpers.tpl (2)
99-112: Consider adding Azure-specific configuration examplesWhile the configuration structure is correct, it would be helpful to add examples in the README.md for Azure Blob storage configuration.
Would you like me to help generate documentation examples for Azure Blob storage configuration?
Line range hint
65-137: Well-structured storage provider integrationThe integration of Azure Blob storage follows a clean and consistent pattern with existing providers. The template maintains good separation of concerns and allows for easy addition of future storage providers.
Consider creating a separate helper template for provider-specific configurations if more storage providers are planned, to keep the main template more maintainable.
charts/greptimedb-cluster/values.yaml (1)
774-779: Consider adding validation for Azure Blob Storage configurationThe Azure Blob Storage configuration block is properly structured, but consider adding:
- Input validation for the container name
- Documentation for the endpoint format
- Default value for the root directory
azblob: {} - # container: "" - # endpoint: "" - # root: "mycluster" + # # The Azure Blob Storage container name + # container: "" + # # The Azure Blob Storage endpoint (e.g., https://<account>.blob.core.windows.net) + # endpoint: "" + # # The root directory for data storage (default: mycluster) + # root: "mycluster"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
charts/greptimedb-cluster/Chart.yaml(2 hunks)charts/greptimedb-cluster/README.md(3 hunks)charts/greptimedb-cluster/templates/cluster.yaml(1 hunks)charts/greptimedb-cluster/templates/secret.yaml(1 hunks)charts/greptimedb-cluster/values.yaml(3 hunks)charts/greptimedb-standalone/Chart.yaml(2 hunks)charts/greptimedb-standalone/README.md(2 hunks)charts/greptimedb-standalone/templates/_helpers.tpl(3 hunks)charts/greptimedb-standalone/templates/secret.yaml(1 hunks)charts/greptimedb-standalone/values.yaml(2 hunks)
✅ Files skipped from review due to trivial changes (3)
- charts/greptimedb-cluster/Chart.yaml
- charts/greptimedb-standalone/Chart.yaml
- charts/greptimedb-standalone/README.md
🔇 Additional comments (11)
charts/greptimedb-cluster/templates/secret.yaml (2)
15-20: LGTM: AWS-style credentials handling looks correct
The conditional checks for accessKeyId and secretAccessKey ensure these fields are only included when provided, maintaining backward compatibility.
21-26: LGTM: Azure Blob storage credentials implementation
The new Azure Blob storage credential fields (accountName and accountKey) are properly implemented with conditional checks.
Let's verify the credential field names match Azure's expectations:
✅ Verification successful
Let me gather more information about Azure Blob storage configuration to verify if these field names align with Azure's SDK and common practices.
Azure Blob storage credential fields are correctly implemented
The implementation is verified to be correct for the following reasons:
- The field names
accountNameandaccountKeyin camelCase format are consistent with Kubernetes conventions for secret keys - The corresponding secret keys are properly hyphenated as
account-nameandaccount-keyin the secret template, following Kubernetes secret naming conventions - The implementation is consistent across both standalone and cluster charts
- The field names align with Azure Blob Storage's standard authentication mechanism using storage account credentials
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Azure credential field names in values.yaml and documentation
# Expected: Field names should match Azure SDK expectations (account_name, account_key)
# Check values.yaml for Azure credential field documentation
rg -A 5 "accountName|accountKey" charts/greptimedb-cluster/values.yaml
# Check README for Azure configuration examples
rg -A 5 "Azure Blob|azblob" charts/greptimedb-cluster/README.md
Length of output: 1151
Script:
#!/bin/bash
# Search for Azure blob configuration patterns in the codebase
rg -A 10 "azblob|azure.*blob" --type rust --type go --type yaml
# Look for any existing Azure storage configurations
rg -A 5 "account[-_]name|account[-_]key" --type yaml --type toml --type rust
Length of output: 9061
charts/greptimedb-standalone/templates/secret.yaml (1)
Line range hint 1-4: LGTM! Well-structured conditional checks
The template properly validates the existence of object storage configuration and credentials before creating the secret, following Helm best practices.
charts/greptimedb-standalone/templates/_helpers.tpl (2)
65-71: LGTM: Variable declarations are properly extended for Azure Blob support
The changes correctly add Azure Blob storage support to the condition check and introduce the necessary container variable.
136-137: Verify Azure Blob endpoint format
The endpoint configuration for Azure Blob is correctly added. However, let's ensure the endpoint format follows Azure's conventions.
charts/greptimedb-standalone/values.yaml (2)
54-57: LGTM! Azure credentials configuration looks complete.
The Azure credentials configuration follows the established pattern and includes the necessary fields (accountName, accountKey) for Azure Blob storage authentication.
Let's verify the credentials configuration is consistent with Azure Blob SDK requirements:
✅ Verification successful
Let me gather more information about the Azure Blob storage configuration to ensure completeness.
Azure Blob storage credentials configuration is properly implemented
The verification confirms that:
- The Azure credentials (accountName and accountKey) are properly handled in the templates/secret.yaml as environment variables
- The Azure Blob configuration is well integrated in the _helpers.tpl with all necessary components:
- Provider type as "Azblob"
- Container configuration
- Endpoint settings
- Root path and cache settings
- The implementation follows the same pattern as other storage providers (S3, OSS, GCS)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Azure Blob storage credential configuration matches SDK requirements
# and is consistent with documentation
# Check if README contains matching credential documentation
rg -A 5 "accountName|accountKey" "charts/greptimedb-standalone/README.md"
# Check if templates properly handle these credentials
rg -A 5 "accountName|accountKey" "charts/greptimedb-standalone/templates/"
Length of output: 1039
Script:
#!/bin/bash
# Check for Azure Blob storage configuration in templates and related files
rg -A 5 "azure|blob" "charts/greptimedb-standalone/templates/"
# Check if there are any Azure-related environment variables or configuration
rg -A 5 "AZURE|BLOB" "charts/greptimedb-standalone/templates/"
# Check for any Azure documentation or examples
rg -A 5 "azure|blob" "charts/greptimedb-standalone/README.md"
Length of output: 3517
109-120: Consider adding optional Azure-specific configuration parameters.
While the basic configuration looks good, Azure Blob storage supports additional options that might be valuable:
use_emulator: For development with Azuritemax_retries: For retry configurationtimeout: For operation timeouts
Consider adding these optional parameters:
azblob: {}
# container: ""
# endpoint: ""
# root: "greptimedb-standalone"
+ # use_emulator: false
+ # max_retries: 3
+ # timeout: "60s"
# # Cache path configuration
# cache_path: ""
# The cache capacity
# cache_capacity = ""Let's verify the configuration is consistent with other object storage providers:
charts/greptimedb-cluster/templates/cluster.yaml (1)
500-510: Implementation of Azure Blob storage looks good!
The Azure Blob storage configuration follows the established patterns used for other cloud storage providers (S3, OSS, GCS) and properly handles credentials through Kubernetes secrets.
Let's verify the implementation matches other providers:
✅ Verification successful
Azure Blob storage implementation matches other providers' patterns
The implementation is correctly structured and consistent with other storage providers (S3, OSS) in the following aspects:
- Uses the same conditional logic pattern
{{- else if .Values.objectStorage.azblob }} - Implements identical secret handling with both default and existing secret options
- Follows the same YAML structure and indentation
- Properly configured in values.yaml with similar options (container, endpoint, root)
- Maintains mutual exclusivity with other storage options in the template
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Compare Azure Blob implementation with other storage providers
# Expected: Similar structure and credential handling across all providers
# Check the structure consistency across storage providers
rg -A 5 "objectStorage:.*\n.*\w+:.*\n.*secretName:" charts/
Length of output: 253
Script:
#!/bin/bash
# Let's fix the regex and use multiline mode to compare storage implementations
rg -U --multiline "objectStorage:.*(\n|\r\n).*\w+:.*(\n|\r\n).*secretName:" charts/
# Let's also check each storage provider's configuration block separately
echo "=== Checking individual storage configurations ==="
rg -U --multiline "s3:(\n|\r\n).*secretName:" charts/
echo "\n=== Azure Blob config ==="
rg -U --multiline "azblob:(\n|\r\n).*secretName:" charts/
echo "\n=== GCS config ==="
rg -U --multiline "gcs:(\n|\r\n).*secretName:" charts/
echo "\n=== OSS config ==="
rg -U --multiline "oss:(\n|\r\n).*secretName:" charts/
Length of output: 758
Script:
#!/bin/bash
# Let's examine the entire objectStorage configuration block in the template
echo "=== Checking cluster.yaml template ==="
rg -A 20 "objectStorage:" charts/greptimedb-cluster/templates/cluster.yaml
# Also check the values.yaml for storage configuration structure
echo "=== Checking values.yaml storage configuration ==="
rg -A 20 "objectStorage:" charts/greptimedb-cluster/values.yaml
Length of output: 2052
Script:
#!/bin/bash
# Let's get more context around the azblob configuration
echo "=== Checking azblob configuration in cluster.yaml ==="
rg -B 5 -A 15 "azblob:" charts/greptimedb-cluster/templates/cluster.yaml
# Also check if azblob is properly defined in values.yaml
echo "=== Checking azblob configuration in values.yaml ==="
rg -B 5 -A 15 "azblob:" charts/greptimedb-cluster/values.yaml
Length of output: 1756
charts/greptimedb-cluster/README.md (1)
227-227: Verify the stability of the initializer version change
The initializer version has been updated from an alpha release (v0.1.3-alpha.8) to a stable release (v0.1.3). Please ensure this change has been thoroughly tested and is ready for production use.
✅ Verification successful
Initializer version change appears to be a routine stable release update
The change from v0.1.3-alpha.8 to v0.1.3 aligns with standard version progression, and the initializer's role is well-defined in the cluster configuration. The CRD and templates show that the initializer is a core component with basic image configuration requirements, without any specific version constraints or special handling needed for the version change.
Key observations:
- The initializer is properly integrated into the Helm chart structure
- The version change follows semantic versioning principles
- No special handling or breaking changes are indicated in the configuration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any recent issues or discussions about the initializer stability
gh issue list --repo GreptimeTeam/greptimedb --label initializer --state all
gh pr list --repo GreptimeTeam/greptimedb --label initializer --state all
Length of output: 154
Script:
#!/bin/bash
# Let's check the git history for changes related to the initializer version
git log -n 10 --oneline -- charts/greptimedb-cluster/values.yaml
# Also check the current values.yaml to verify the initializer version
cat charts/greptimedb-cluster/values.yaml | grep -A 5 "initializer:"
# Check if there are any changelog or release notes
fd "CHANGELOG|RELEASE" --type f
Length of output: 282
Script:
#!/bin/bash
# Let's check the git history specifically for the README changes to understand the version transition
git log -p charts/greptimedb-cluster/README.md | grep -A 5 -B 5 "initializer.tag"
# Let's also check if there are any related test files or documentation
fd "test|spec" --type f | grep -i "initializer"
# Check for any related configuration files
fd . --type f --extension yaml --extension yml | xargs grep -l "initializer"
Length of output: 352
Script:
#!/bin/bash
# Let's examine the CRD and cluster template to understand the initializer's role and requirements
cat charts/greptimedb-cluster/templates/cluster.yaml | grep -A 10 -B 10 "initializer"
# Check the operator CRD for initializer-related specifications
cat charts/greptimedb-operator/templates/crds/crd-greptimedbcluster.yaml | grep -A 10 -B 10 "initializer"
# Look for any version constraints or requirements in the Chart.yaml
cat charts/greptimedb-cluster/Chart.yaml
Length of output: 3419
charts/greptimedb-cluster/values.yaml (2)
17-17: LGTM: Stable version upgrade
The initializer tag has been updated from an alpha version to a stable release (v0.1.3), which is a good practice for production deployments.
737-740: Add validation for Azure credentials
The Azure storage credentials configuration looks good, but we should verify that the credentials are properly validated when used.
Summary by CodeRabbit
Release Notes
New Features
greptimedb-clusterandgreptimedb-standalonecharts, enhancing cloud storage options.Version Updates
greptimedb-cluster(0.2.35 to 0.2.36) andgreptimedb-standalone(0.1.34 to 0.1.35).Documentation