feat(helm)!: Replace hostPath-based aws_config_directory with chart-managed Secret (resolves #2162).#2164
feat(helm)!: Replace hostPath-based aws_config_directory with chart-managed Secret (resolves #2162).#2164junhaoliao wants to merge 3 commits intoy-scope:mainfrom
hostPath-based aws_config_directory with chart-managed Secret (resolves #2162).#2164Conversation
…rt-managed Secret (resolves y-scope#2162).
WalkthroughChart version bumped; AWS config moved from a hostPath directory value to a chart-managed Secret. Templates, helpers, values and deployments were updated to use Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tools/deployment/package-helm/values.yaml`:
- Around line 259-265: Add a new Helm Secret template named
aws-config-secret.yaml that mirrors the pattern used by database-secret.yaml,
queue-secret.yaml, and redis-secret.yaml: read clpConfig.aws_config.credentials
and clpConfig.aws_config.config values and create a Secret (named using the
release name, e.g., {{ include "clp.fullname" . }}-aws-config) with keys for
credentials and config so the clp.awsConfigVolume helper can mount them into
pods; ensure the template is conditional on .Values.clpConfig.aws_config not
being null and uses proper tpl/quote handling for multi-line file content.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ae49a1b6-74b2-4506-9b0e-e4a84879baf6
📒 Files selected for processing (9)
tools/deployment/package-helm/Chart.yamltools/deployment/package-helm/templates/_helpers.tpltools/deployment/package-helm/templates/compression-scheduler-deployment.yamltools/deployment/package-helm/templates/compression-worker-deployment.yamltools/deployment/package-helm/templates/configmap.yamltools/deployment/package-helm/templates/garbage-collector-deployment.yamltools/deployment/package-helm/templates/query-worker-deployment.yamltools/deployment/package-helm/templates/webui-deployment.yamltools/deployment/package-helm/values.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tools/deployment/package-helm/templates/aws-config-secret.yaml`:
- Around line 8-14: When .Values.clpConfig.aws_config is present but neither
.Values.clpConfig.aws_config.credentials nor .Values.clpConfig.aws_config.config
is provided, the two with-blocks in the stringData section (credentials and
config) are skipped producing an empty stringData; add a Helm validation check
that detects this case and fails early with a clear message (e.g., using an if
that tests .Values.clpConfig.aws_config and the absence of both
.Values.clpConfig.aws_config.credentials and
.Values.clpConfig.aws_config.config, then call fail with a descriptive message)
so templates/aws-config-secret.yaml does not render an empty secret.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: adb2ccca-9f56-4fa3-b055-208d60aca59b
📒 Files selected for processing (1)
tools/deployment/package-helm/templates/aws-config-secret.yaml
hoophalab
left a comment
hoophalab
left a comment
There was a problem hiding this comment.
Overall good. Some comments.
| {{- define "clp.awsConfigVolumeMount" -}} | ||
| name: "aws-config" | ||
| mountPath: {{ .Values.clpConfig.aws_config_directory | quote }} | ||
| mountPath: "/opt/clp/.aws" |
There was a problem hiding this comment.
Can we comment that this variable needs to be in sync with CONTAINER_AWS_CONFIG_DIRECTORY in clp_py_utils/clp_config.py?
| {{- if .Values.clpConfig.aws_config_directory }} | ||
| aws_config_directory: {{ .Values.clpConfig.aws_config_directory | quote }} | ||
| {{- if .Values.clpConfig.aws_config }} | ||
| aws_config_directory: "/opt/clp/.aws" |
There was a problem hiding this comment.
Since the package doesn't actually handle the volume mount in Helm, this variable could be set to any string, null, or even omitted entirely, right?
|
Shall we update |
2 participants
Description
The Helm chart's
aws_config_directoryvolume useshostPath, which mounts a directory from the Kubernetes node's filesystem. This doesn't work on managed Kubernetes clusters (e.g., EKS) wherehostPathis either unavailable or restricted by PodSecurityStandards.This PR replaces the
hostPath-basedaws_config_directoryvalue with a newaws_configobject.Users provide their AWS config file contents via Helm's
--set-fileflag, and the chart creates a Kubernetes Secret from those contents. The Secret is then mounted as a volume in all pods that need AWS config access (compression-scheduler, compression-worker, query-worker, garbage-collector, webui).Usage:
Changes:
values.yaml: Replacedaws_config_directory: nullwithaws_config: nulland documented--set-fileusage.templates/aws-config-secret.yaml(new): Creates a Secret fromaws_config.credentialsandaws_config.configfile contents.templates/_helpers.tpl:clp.awsConfigVolumenow emits asecretvolume instead ofhostPath;clp.awsConfigVolumeMounthardcodes/opt/clp/.awsas the mount path.templates/configmap.yaml: Condition and rendered path updated to useaws_config.aws_config_directorytoaws_config.Checklist
breaking change.
Validation performed
helm templaterenders no AWS resources whenaws_configis null (default)Task: Verify that when
aws_configis not set (defaultnull), no AWS-related resources arerendered.
Command:
Output:
No AWS resources are emitted when
aws_configis disabled — all deployments omit the volume and volumeMount, and no Secret is created.helm templaterenders Secret and mounts correctly withaws_configTask: Verify that setting
aws_configvia--set-filecreates a Secret and mounts it in all 5 deployments.Command:
Output:
All 5 deployments (compression-scheduler, compression-worker, query-worker, garbage-collector,
webui) reference the chart-managed Secret as a volume source.
Command (verify configmap sets
aws_config_directory):Output:
The configmap correctly hardcodes the
aws_config_directorypath whenaws_configis provided.Single-node cluster: S3 compress, search, and stream extraction
Task: Deploy with
set-up-test.sh, upgrade with S3 +aws_config, and verify compress, search, and stream extraction all work.Command:
Output:
Command (upgrade with S3 + aws_config):
Output:
Command (verify Secret exists and contains expected keys):
Output:
Command (submit S3 compression job):
Output:
Command (check compression worker logs):
Output:
Explanation: The compression worker successfully read the AWS credentials from the Secret-mounted volume at
/opt/clp/.aws, connected to S3, and uploaded the archive.Command (search via Playwright):
Search for
postgresin the WebUI.Result:
Success - search job 3 found 10 results in 0.301 seconds (13.4 kB/s)Command (stream extraction via Playwright):
Click "Original file" link on a search result.
Result: New tab opens showing the decompressed stream file contents from S3 — confirms the
webui correctly reads AWS config from the Secret-mounted volume.
Multi-node shared cluster: S3 compress and search
Task: Deploy with
set-up-multi-shared-test.sh, upgrade with S3 +aws_config, and verifycompress and search work across worker nodes.
Command:
Output:
Command (upgrade + compress + search):
Output:
Result: S3 compression completed successfully. Search found 10 results in 0.352 seconds.
Multi-node dedicated cluster: S3 compress, search, and stream extraction
Task: Deploy with
set-up-multi-dedicated-test.sh, upgrade with S3 +aws_config(including nodeSelector), and verify compress, search, and stream extraction work on dedicated worker nodes.Command:
Output:
Command (upgrade with nodeSelector):
Output:
Result: S3 compression completed successfully. Search found 10 results in 0.317 seconds. Stream extraction via "Original file" link opened a new tab showing the decompressed file contents from S3.
Summary by CodeRabbit