Skip to content

feat(helm): Add S3 storage support for archives, streams, and log inputs. #1825

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
junhaoliao merged 14 commits into from
Jan 7, 2026
Merged

feat(helm): Add S3 storage support for archives, streams, and log inputs. #1825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
9a88d1e
feat(helm): Add S3 storage support for archives, streams, and logs in…
junhaoliao Dec 19, 2025
d37168c
chore(helm): Bump chart version to 0.1.2-dev.15
junhaoliao Dec 19, 2025
1479250
Merge branch 'main' into helm-s3
junhaoliao Jan 5, 2026
ffb7199
style(helm): Format conditional statements for better readability in …
junhaoliao Jan 5, 2026
0f8c6b4
Conditionally enable shared data archive and streams only when the ty…
junhaoliao Jan 5, 2026
2ce2999
fix(storage): Update hostPath for staged archives and streams
junhaoliao Jan 5, 2026
ea9decb
fix(helm): Correct S3 path prefix formatting for webui config in conf…
junhaoliao Jan 5, 2026
24ca2dd
Merge branch 'refs/heads/main' into helm-s3
junhaoliao Jan 5, 2026
715163a
fix(helm): Add missing newlines at the end of shared data YAML files
junhaoliao Jan 5, 2026
2e35de0
fix(helm): Reduce storage capacity for staged archives and streams fr…
junhaoliao Jan 6, 2026
c41c896
Merge branch 'main' into helm-s3
junhaoliao Jan 6, 2026
2310f22
Merge branch 'main' into helm-s3
junhaoliao Jan 6, 2026
9ff049f
Merge branch 'main' into helm-s3
junhaoliao Jan 7, 2026
c9dcb52
bump chart version
junhaoliao Jan 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tools/deployment/package-helm/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
apiVersion: "v2"
name: "clp"
version: "0.1.2-dev.15"
version: "0.1.2-dev.16"
description: "A Helm chart for CLP's (Compressed Log Processor) package deployment"
type: "application"
appVersion: "0.7.1-dev"
Expand Down
25 changes: 25 additions & 0 deletions tools/deployment/package-helm/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,31 @@ hostPath:
type: "Directory"
{{- end }}

{{/*
Creates a volumeMount for the AWS config directory.

@param {object} . Root template context
@return {string} YAML-formatted volumeMount definition
*/}}
{{- define "clp.awsConfigVolumeMount" -}}
name: "aws-config"
mountPath: {{ .Values.clpConfig.aws_config_directory | quote }}
readOnly: true
{{- end }}
Comment on lines +250 to +260
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Document the precondition that aws_config_directory must be non-null.

The helper assumes aws_config_directory is set. While all current callers guard with {{- if .Values.clpConfig.aws_config_directory }}, documenting this precondition in the helper's docstring would prevent future misuse.

📝 Suggested documentation enhancement 
 {{/*
 Creates a volumeMount for the AWS config directory.
 
+@pre {string} .Values.clpConfig.aws_config_directory must be non-null
 @param {object} . Root template context
 @return {string} YAML-formatted volumeMount definition
 */}}
🤖 Prompt for AI Agents 
In @tools/deployment/package-helm/templates/_helpers.tpl around lines 250-260,
The helper "clp.awsConfigVolumeMount" assumes
.Values.clpConfig.aws_config_directory is set and will render an invalid mount
if it is null; update the helper's docstring to state the precondition that
.Values.clpConfig.aws_config_directory must be non-null (e.g., "Requires
.Values.clpConfig.aws_config_directory to be set/non-empty; callers must guard
with an if check"), and optionally mention that callers should wrap the helper
with {{- if .Values.clpConfig.aws_config_directory }} ... {{- end }} to avoid
misuse.


{{/*
Creates a volume for the AWS config directory.

@param {object} . Root template context
@return {string} YAML-formatted volume definition
*/}}
{{- define "clp.awsConfigVolume" -}}
name: "aws-config"
hostPath:
path: {{ .Values.clpConfig.aws_config_directory | quote }}
type: "Directory"
{{- end }}
Comment on lines +268 to +273
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for setup scripts or documentation mentioning AWS config directory
rg -i "aws.?config.?dir" -g "*.md" -g "*.sh" -g "*.yaml" -C3

Repository: y-scope/clp

Length of output: 16202

🏁 Script executed:

#!/bin/bash
# Search for deployment guides and README files with setup instructions
find . -type f -name "README*" -o -name "INSTALL*" -o -name "DEPLOY*" | head -20

# Also search for helm chart documentation
find tools/deployment/package-helm -type f \( -name "*.md" -o -name "README*" \)

Repository: y-scope/clp

Length of output: 494

🏁 Script executed:

#!/bin/bash
# Search for Kubernetes/Helm deployment documentation
find docs -type f -name "*.md" | xargs rg -l "kubernetes\|helm\|deploy" -i

# Check if there's documentation in the helm chart directory
ls -la tools/deployment/package-helm/

# Look for prerequisites or setup documentation
rg "prerequisite\|setup\|initialize\|create.*director" docs -i -A2 | head -50

Repository: y-scope/clp

Length of output: 464

🏁 Script executed:

#!/bin/bash
# Check values.yaml for aws_config_directory documentation
sed -n '1,100p' tools/deployment/package-helm/values.yaml | grep -A5 -B2 "aws_config"

# Search for deployment guides and prerequisites documentation
find docs -type f -name "*.md" | head -20

Repository: y-scope/clp

Length of output: 976

🏁 Script executed:

#!/bin/bash
# Find deployment/installation guides
find docs/src/user-docs -type f -name "*.md" | sort

# Also check for any prerequisite documentation
rg "hostPath\|directory.*exist\|prerequisite" docs -i -B2 -A2

Repository: y-scope/clp

Length of output: 1602

🏁 Script executed:

#!/bin/bash
# Check the object storage config documentation for aws_config_directory setup
cat docs/src/user-docs/guides-using-object-storage/clp-config.md

# Also check if there's any helm/kubernetes specific deployment guide
find docs -type f -name "*.md" | xargs grep -l "helm\|kubernetes" -i

Repository: y-scope/clp

Length of output: 5960

🏁 Script executed:

#!/bin/bash
# Check if there's any helm-specific deployment documentation
find docs -type f -name "*.md" -path "*/user-docs/*" | xargs grep -l "kubernetes\|helm\|deploy" -i

# Check the quick-start guides which might have setup instructions
cat docs/src/user-docs/quick-start/index.md

Repository: y-scope/clp

Length of output: 4023

🏁 Script executed:

#!/bin/bash
# Check multi-host deployment guide for setup instructions
cat docs/src/user-docs/guides-multi-host.md | head -100

# Check if there's any helm/kubernetes specific deployment documentation
find docs -name "*.md" -type f | xargs grep -l "hostPath" -i

Repository: y-scope/clp

Length of output: 4012

🏁 Script executed:

#!/bin/bash
# Check for Helm-specific deployment documentation
find docs -name "*.md" -type f | xargs grep -l "helm\|values.yaml" -i

# Check Chart.yaml for documentation references
cat tools/deployment/package-helm/Chart.yaml

# Search for any kubernetes prerequisites or setup documentation
rg "kubernetes|k8s|helm" docs/src/user-docs -i -l

Repository: y-scope/clp

Length of output: 662

🏁 Script executed:

#!/bin/bash
# Check for helm hooks or init scripts that might create the aws_config_directory
rg "initContainers\|initContainer" tools/deployment/package-helm/templates -A5

# Search for any helm hooks
rg "hook\|init" tools/deployment/package-helm/templates -i

# Check the full values.yaml for aws_config_directory context
grep -A10 -B5 "aws_config_directory" tools/deployment/package-helm/values.yaml

Repository: y-scope/clp

Length of output: 496

🏁 Script executed:

#!/bin/bash
# Check if there's any documentation about Kubernetes deployment or prerequisites
find docs -name "*.md" -type f | xargs grep -l "prerequisites\|deployment\|install" -i | head -10

# Check the main README for kubernetes/helm instructions
cat README.md | head -100

Repository: y-scope/clp

Length of output: 6051

Document the hostPath directory prerequisite for AWS config in deployment guides.

When aws_config_directory is set to a non-null value, Kubernetes requires the directory to exist on all target nodes before pod scheduling. The Helm chart uses hostPath volumes with type: "Directory", which will fail to mount if the path doesn't exist. Add a note to the deployment documentation and/or values.yaml comments stating that users must ensure this directory exists on all worker nodes (or is created as part of pre-deployment setup).

🤖 Prompt for AI Agents 
In @tools/deployment/package-helm/templates/_helpers.tpl around lines 268-273,
The Helm template define "clp.awsConfigVolume" uses a hostPath with type
"Directory" sourced from .Values.clpConfig.aws_config_directory; update the
deployment docs and add a comment in values.yaml next to
clpConfig.aws_config_directory to explicitly state that when this value is set,
the specified directory must already exist on every target node (or be created
during pre-deployment) because Kubernetes will fail to mount a hostPath
Directory that does not exist. Ensure the note mentions potential alternatives
(e.g., using a different volume type) and the exact values key
(.Values.clpConfig.aws_config_directory) so users can find and configure it
easily.


{{/*
Creates an initContainer that waits for a Kubernetes resource to be ready.

Expand Down
10 changes: 7 additions & 3 deletions tools/deployment/package-helm/templates/api-server-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,13 @@ spec:
mountPath: "/etc/clp-config.yaml"
subPath: "clp-config.yaml"
readOnly: true
{{- if eq .Values.clpConfig.stream_output.storage.type "fs" }}
- name: {{ include "clp.volumeName" (dict
"component_category" "shared-data"
"name" "streams"
) | quote }}
mountPath: "/var/data/streams"
{{- end }}
command: [
"/opt/clp/bin/api_server",
"--host", "0.0.0.0",
Expand All @@ -92,12 +94,14 @@ spec:
"component_category" "api-server"
"name" "logs"
) | nindent 10 }}
- name: "config"
configMap:
name: {{ include "clp.fullname" . }}-config
{{- if eq .Values.clpConfig.stream_output.storage.type "fs" }}
- {{- include "clp.pvcVolume" (dict
"root" .
"component_category" "shared-data"
"name" "streams"
) | nindent 10 }}
- name: "config"
configMap:
name: {{ include "clp.fullname" . }}-config
{{- end }}
{{- end }}
14 changes: 12 additions & 2 deletions tools/deployment/package-helm/templates/compression-scheduler-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ spec:
- name: "PYTHONPATH"
value: "/opt/clp/lib/python3/site-packages"
volumeMounts:
- {{- include "clp.logsInputVolumeMount" . | nindent 14 }}
- name: {{ include "clp.volumeName" (dict
"component_category" "compression-scheduler"
"name" "logs"
Expand All @@ -81,13 +80,18 @@ spec:
mountPath: "/etc/clp-config.yaml"
subPath: "clp-config.yaml"
readOnly: true
{{- if .Values.clpConfig.aws_config_directory }}
- {{- include "clp.awsConfigVolumeMount" . | nindent 14 }}
{{- end }}
{{- if eq .Values.clpConfig.logs_input.type "fs" }}
- {{- include "clp.logsInputVolumeMount" . | nindent 14 }}
{{- end }}
command: [
"python3", "-u",
"-m", "job_orchestration.scheduler.compress.compression_scheduler",
"--config", "/etc/clp-config.yaml"
]
volumes:
- {{- include "clp.logsInputVolume" . | nindent 10 }}
- {{- include "clp.pvcVolume" (dict
"root" .
"component_category" "compression-scheduler"
Expand All @@ -101,3 +105,9 @@ spec:
- name: "config"
configMap:
name: {{ include "clp.fullname" . }}-config
{{- if .Values.clpConfig.aws_config_directory }}
- {{- include "clp.awsConfigVolume" . | nindent 10 }}
{{- end }}
{{- if eq .Values.clpConfig.logs_input.type "fs" }}
- {{- include "clp.logsInputVolume" . | nindent 10 }}
{{- end }}
36 changes: 31 additions & 5 deletions tools/deployment/package-helm/templates/compression-worker-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ spec:
- name: "PYTHONPATH"
value: "/opt/clp/lib/python3/site-packages"
volumeMounts:
- {{- include "clp.logsInputVolumeMount" . | nindent 14 }}
- name: {{ include "clp.volumeName" (dict
"component_category" "compression-worker"
"name" "logs"
Expand All @@ -60,11 +59,25 @@ spec:
mountPath: "/etc/clp-config.yaml"
subPath: "clp-config.yaml"
readOnly: true
{{- if eq .Values.clpConfig.archive_output.storage.type "fs" }}
- name: {{ include "clp.volumeName" (dict
"component_category" "shared-data"
"name" "archives"
) | quote }}
mountPath: "/var/data/archives"
{{- else if eq .Values.clpConfig.archive_output.storage.type "s3" }}
- name: {{ include "clp.volumeName" (dict
"component_category" "compression-worker"
"name" "staged-archives"
) | quote }}
mountPath: "/var/data/staged-archives"
{{- end }}
{{- if .Values.clpConfig.aws_config_directory }}
- {{- include "clp.awsConfigVolumeMount" . | nindent 14 }}
{{- end }}
{{- if eq .Values.clpConfig.logs_input.type "fs" }}
- {{- include "clp.logsInputVolumeMount" . | nindent 14 }}
{{- end }}
command: [
"python3", "-u",
"/opt/clp/lib/python3/site-packages/bin/celery",
Expand All @@ -77,7 +90,6 @@ spec:
"-n", "compression-worker"
]
volumes:
- {{- include "clp.logsInputVolume" . | nindent 10 }}
- {{- include "clp.pvcVolume" (dict
"root" .
"component_category" "compression-worker"
Expand All @@ -88,11 +100,25 @@ spec:
"component_category" "compression-worker"
"name" "tmp"
) | nindent 10 }}
- name: "config"
configMap:
name: {{ include "clp.fullname" . }}-config
{{- if eq .Values.clpConfig.archive_output.storage.type "fs" }}
- {{- include "clp.pvcVolume" (dict
"root" .
"component_category" "shared-data"
"name" "archives"
) | nindent 10 }}
- name: "config"
configMap:
name: {{ include "clp.fullname" . }}-config
{{- else if eq .Values.clpConfig.archive_output.storage.type "s3" }}
- {{- include "clp.pvcVolume" (dict
"root" .
"component_category" "compression-worker"
"name" "staged-archives"
) | nindent 10 }}
{{- end }}
{{- if .Values.clpConfig.aws_config_directory }}
- {{- include "clp.awsConfigVolume" . | nindent 10 }}
{{- end }}
{{- if eq .Values.clpConfig.logs_input.type "fs" }}
- {{- include "clp.logsInputVolume" . | nindent 10 }}
{{- end }}
11 changes: 11 additions & 0 deletions tools/deployment/package-helm/templates/compression-worker-staged-archives-pv.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{{- if eq .Values.clpConfig.archive_output.storage.type "s3" }}
{{- include "clp.createLocalPv" (dict
"root" .
"component_category" "compression-worker"
"name" "staged-archives"
"nodeRole" "worker"
"capacity" "20Gi"
"accessModes" (list "ReadWriteOnce")
"hostPath" .Values.clpConfig.archive_output.storage.staging_directory
) }}
{{- end }}
9 changes: 9 additions & 0 deletions tools/deployment/package-helm/templates/compression-worker-staged-archives-pvc.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{{- if eq .Values.clpConfig.archive_output.storage.type "s3" }}
{{- include "clp.createPvc" (dict
"root" .
"component_category" "compression-worker"
"name" "staged-archives"
"capacity" "20Gi"
"accessModes" (list "ReadWriteOnce")
) }}
{{- end }}
118 changes: 117 additions & 1 deletion tools/deployment/package-helm/templates/configmap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,42 @@ data:
{{- end }}
archive_output:
compression_level: {{ .Values.clpConfig.archive_output.compression_level | int }}
{{- with .Values.clpConfig.archive_output.storage }}
storage:
{{- if eq .type "fs" }}
directory: "/var/data/archives"
type: "fs"
{{- else if eq .type "s3" }}
type: "s3"
staging_directory: "/var/data/staged-archives"
{{- with .s3_config }}
s3_config:
{{- if .endpoint_url }}
endpoint_url: {{ .endpoint_url | quote }}
{{- end }}
{{- if .region_code }}
region_code: {{ .region_code | quote }}
{{- end }}
bucket: {{ .bucket | quote }}
key_prefix: {{ .key_prefix | quote }}
{{- with .aws_authentication }}
aws_authentication:
type: {{ .type | quote }}
{{- if .profile }}
profile: {{ .profile | quote }}
{{- end }}
{{- if .credentials }}
credentials:
access_key_id: {{ .credentials.access_key_id | quote }}
secret_access_key: {{ .credentials.secret_access_key | quote }}
{{- if .credentials.session_token }}
session_token: {{ .credentials.session_token | quote }}
{{- end }}
{{- end }}
{{- end }}{{/* with .aws_authentication */}}
{{- end }}{{/* with .s3_config */}}
{{- end }}{{/* if eq .type "fs" */}}
{{- end }}{{/* with .Values.clpConfig.archive_output.storage */}}
{{- if .Values.clpConfig.archive_output.retention_period }}
retention_period: {{ .Values.clpConfig.archive_output.retention_period | int }}
{{- else }}
Expand Down Expand Up @@ -58,9 +91,30 @@ data:
ssl_cert: null
type: {{ .Values.clpConfig.database.type | quote }}
logs_directory: "/var/log"
{{- with .Values.clpConfig.logs_input }}
logs_input:
{{- if eq .type "fs" }}
directory: "/mnt/logs"
type: "fs"
{{- else if eq .type "s3" }}
type: "s3"
{{- with .aws_authentication }}
aws_authentication:
type: {{ .type | quote }}
{{- if .profile }}
profile: {{ .profile | quote }}
{{- end }}
{{- if .credentials }}
credentials:
access_key_id: {{ .credentials.access_key_id | quote }}
secret_access_key: {{ .credentials.secret_access_key | quote }}
{{- if .credentials.session_token }}
session_token: {{ .credentials.session_token | quote }}
{{- end }}
{{- end }}
Comment on lines +107 to +114
Copy link
Contributor

@gibber9809 gibber9809 Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that credentials and other privileged data should be put in a "Secret" instead of a "ConfigMap" for Kubernetes. I guess this is pretty much the same way we currently treat credentials access, so it's probably fine for now, but it might be worth changing in the future. If we don't have an issue tracking this, it might be worth creating one.

Copy link
Member Author

@junhaoliao junhaoliao Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, the plan is to isolate credentials from other clp-config fields so that their lifecycle can be managed more securely. Once this separation is in place, moving those values into Kubernetes Secrets would be more meaningful and appropriate

@coderabbitai in case we haven't created an issue for such refactoring, create one to track

Copy link
Contributor

@coderabbitai coderabbitai bot Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for existing issues related to credentials, secrets, or configmap refactoring
gh issue list --repo y-scope/clp --state all --search "credentials secret configmap helm" --json number,title,state,url --limit 20

Length of output: 2

🏁 Script executed:

#!/bin/bash
# Create a new issue for tracking the credentials/secrets refactoring
gh issue create \
  --repo y-scope/clp \
  --title "Refactor Helm chart to use Kubernetes Secrets for credentials instead of ConfigMaps" \
  --body "## Background

Currently, AWS credentials and other sensitive authentication data are stored in Kubernetes ConfigMaps within the Helm chart deployment. For better security practices, these should be stored in Kubernetes Secrets instead.

## Proposed approach

1. Isolate credentials from other clp-config fields so their lifecycle can be managed separately
2. Once isolated, move credential values into Kubernetes Secrets for more secure handling

## References

- PR #1825: https://github.com/y-scope/clp/pull/1825
- Discussion: https://github.com/y-scope/clp/pull/1825#discussion_r2665630820
- Requested by: @junhaoliao" \
  --assignee junhaoliao

Length of output: 42

@junhaoliao, I've created issue #1844 to track the refactoring of credentials from ConfigMaps to Kubernetes Secrets. The issue includes the proposed approach and references this PR and discussion thread.

🐰📝

🧠 Learnings used 
Learnt from: hoophalab
Repo: y-scope/clp PR: 1767
File: components/clp-py-utils/clp_py_utils/clp_config.py:580-585
Timestamp: 2025-12-10T23:32:24.363Z
Learning: In PR #1767, custom S3 endpoint support was added to CLP. The S3Config.region_code field is now optional (NonEmptyStr | None) because custom S3-compatible endpoints (MinIO, LocalStack, etc.) use path-style URLs and don't require AWS region codes. Only AWS S3 endpoints require region_code. Presto integration still requires region_code because it only works with AWS S3.

{{- end }}{{/* with .aws_authentication */}}
{{- end }}{{/* if eq .type "fs" */}}
{{- end }}{{/* with .Values.clpConfig.logs_input */}}
package:
query_engine: {{ .Values.clpConfig.package.query_engine | quote }}
storage_engine: {{ .Values.clpConfig.package.storage_engine | quote }}
Expand Down Expand Up @@ -96,12 +150,50 @@ data:
retention_period: null
{{- end }}
stream_collection_name: {{ .Values.clpConfig.results_cache.stream_collection_name | quote }}
{{- with .Values.clpConfig.stream_output }}
stream_output:
{{- with .storage }}
storage:
{{- if eq .type "fs" }}
directory: "/var/data/streams"
type: "fs"
target_uncompressed_size: {{ .Values.clpConfig.stream_output.target_uncompressed_size | int }}
{{- else if eq .type "s3" }}
type: "s3"
staging_directory: "/var/data/staged-streams"
{{- with .s3_config }}
s3_config:
{{- if .endpoint_url }}
endpoint_url: {{ .endpoint_url | quote }}
{{- end }}
{{- if .region_code }}
region_code: {{ .region_code | quote }}
{{- end }}
bucket: {{ .bucket | quote }}
key_prefix: {{ .key_prefix | quote }}
{{- with .aws_authentication }}
aws_authentication:
type: {{ .type | quote }}
{{- if .profile }}
profile: {{ .profile | quote }}
{{- end }}
{{- if .credentials }}
credentials:
access_key_id: {{ .credentials.access_key_id | quote }}
secret_access_key: {{ .credentials.secret_access_key | quote }}
{{- if .credentials.session_token }}
session_token: {{ .credentials.session_token | quote }}
{{- end }}
{{- end }}
{{- end }}{{/* with .aws_authentication */}}
{{- end }}{{/* with .s3_config */}}
{{- end }}{{/* if eq .type "fs" */}}
{{- end }}{{/* with .storage */}}
target_uncompressed_size: {{ .target_uncompressed_size | int }}
{{- end }}{{/* with .Values.clpConfig.stream_output */}}
tmp_directory: "/var/tmp"
{{- if .Values.clpConfig.aws_config_directory }}
aws_config_directory: {{ .Values.clpConfig.aws_config_directory | quote }}
{{- end }}
webui:
host: "localhost"
port: 4000
Expand Down Expand Up @@ -150,7 +242,11 @@ data:
"ClpStorageEngine": {{ .Values.clpConfig.package.storage_engine | quote }},
"ClpQueryEngine": {{ .Values.clpConfig.package.query_engine | quote }},
"LogsInputType": {{ .Values.clpConfig.logs_input.type | quote }},
{{- if eq .Values.clpConfig.logs_input.type "fs" }}
"LogsInputRootDir": "/mnt/logs",
{{- else }}
"LogsInputRootDir": null,
Copy link
Contributor

@gibber9809 gibber9809 Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does LogsInputRootDir and StreamFilesDir, StreamFilesS3Region , etc.. Need to be explicitly null when not in use?

Copy link
Member Author

@junhaoliao junhaoliao Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In JavaScript/TypeScript, null and undefined are distinct values with different types. Consumers of settings.json (the web UI code) expect each value to be either null or a string. While we could also tolerate undefined when a field is missing, this would require additional handling and stricter typings

{{- end }}
"MongoDbSearchResultsMetadataCollectionName":
{{ .Values.clpConfig.webui.results_metadata_collection_name | quote }},
"SqlDbClpArchivesTableName": "clp_archives",
Expand All @@ -176,13 +272,33 @@ data:
{{ .Values.clpConfig.results_cache.stream_collection_name | quote }},
"ClientDir": "/opt/clp/var/www/webui/client",
"LogViewerDir": "/opt/clp/var/www/webui/yscope-log-viewer",
{{- if eq .Values.clpConfig.logs_input.type "fs" }}
"LogsInputRootDir": "/mnt/logs",
{{- else }}
"LogsInputRootDir": null,
{{- end }}
{{- if eq .Values.clpConfig.stream_output.storage.type "fs" }}
"StreamFilesDir": "/var/data/streams",
{{- else }}
"StreamFilesDir": null,
{{- end }}
"StreamTargetUncompressedSize":
{{ .Values.clpConfig.stream_output.target_uncompressed_size | int }},
{{- if eq .Values.clpConfig.stream_output.storage.type "s3" }}
{{- with .Values.clpConfig.stream_output.storage.s3_config }}
"StreamFilesS3Region": {{ .region_code | default "null" | quote }},
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Consider using unquoted null for proper JSON null value.

{{ .region_code | default "null" | quote }} will produce "null" (string) instead of null (JSON null). If the intent is to have a proper JSON null when region_code is not set, this needs adjustment.

🔎 Proposed fix for proper null handling 
-      "StreamFilesS3Region": {{ .region_code | default "null" | quote }},
+      {{- if .region_code }}
+      "StreamFilesS3Region": {{ .region_code | quote }},
+      {{- else }}
+      "StreamFilesS3Region": null,
+      {{- end }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"StreamFilesS3Region": {{ .region_code | default "null" | quote }},
{{- if .region_code }}
"StreamFilesS3Region": {{ .region_code | quote }},
{{- else }}
"StreamFilesS3Region": null,
{{- end }}
🤖 Prompt for AI Agents 
In @tools/deployment/package-helm/templates/configmap.yaml around line 288, The
template currently renders "StreamFilesS3Region" as a JSON string "null" because
{{ .region_code | default "null" | quote }} wraps the default in quotes; change
the template to emit a real JSON null when .region_code is unset by using a
conditional: replace the expression with {{ if .region_code }}{{ .region_code |
quote }}{{ else }}null{{ end }} so "StreamFilesS3Region" gets either a quoted
region string or an unquoted null.

"StreamFilesS3PathPrefix": {{ printf "%s/%s" .bucket .key_prefix | quote }},
{{- if eq .aws_authentication.type "profile" }}
"StreamFilesS3Profile": {{ .aws_authentication.profile | quote }},
{{- else }}
"StreamFilesS3Profile": null,
{{- end }}
{{- end }}{{/* with .Values.clpConfig.stream_output.storage.s3_config */}}
Comment on lines +287 to +296
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Potential nil pointer dereference when accessing aws_authentication.type.

Line 290 accesses .aws_authentication.type without first checking if aws_authentication exists. If aws_authentication is not defined in the values, this will cause a template rendering error.

🔎 Proposed fix to add nil check 
      {{- with .Values.clpConfig.stream_output.storage.s3_config }}
      "StreamFilesS3Region": {{ .region_code | default "null" | quote }},
      "StreamFilesS3PathPrefix": {{ printf "%s/%s" .bucket .key_prefix | quote }},
-      {{- if eq .aws_authentication.type "profile" }}
+      {{- if and .aws_authentication (eq .aws_authentication.type "profile") }}
      "StreamFilesS3Profile": {{ .aws_authentication.profile | quote }},
      {{- else }}
      "StreamFilesS3Profile": null,
      {{- end }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{{- if eq .Values.clpConfig.stream_output.storage.type "s3" }}
{{- with .Values.clpConfig.stream_output.storage.s3_config }}
"StreamFilesS3Region": {{ .region_code | default "null" | quote }},
"StreamFilesS3PathPrefix": {{ printf "%s/%s" .bucket .key_prefix | quote }},
{{- if eq .aws_authentication.type "profile" }}
"StreamFilesS3Profile": {{ .aws_authentication.profile | quote }},
{{- else }}
"StreamFilesS3Profile": null,
{{- end }}
{{- end }}{{/* with .Values.clpConfig.stream_output.storage.s3_config */}}
{{- with .Values.clpConfig.stream_output.storage.s3_config }}
"StreamFilesS3Region": {{ .region_code | default "null" | quote }},
"StreamFilesS3PathPrefix": {{ printf "%s/%s" .bucket .key_prefix | quote }},
{{- if and .aws_authentication (eq .aws_authentication.type "profile") }}
"StreamFilesS3Profile": {{ .aws_authentication.profile | quote }},
{{- else }}
"StreamFilesS3Profile": null,
{{- end }}
{{- end }}{{/* with .Values.clpConfig.stream_output.storage.s3_config */}}
🤖 Prompt for AI Agents 
In @tools/deployment/package-helm/templates/configmap.yaml around lines 286-295,
The template accesses .aws_authentication.type without ensuring
.aws_authentication exists; change the conditional to guard for nil, e.g. use an
AND: replace {{- if eq .aws_authentication.type "profile" }} with {{- if and
.aws_authentication (eq .aws_authentication.type "profile") }} so
StreamFilesS3Profile is only read when aws_authentication is present, keeping
the else branch that emits null.

{{- else }}
"StreamFilesS3Region": null,
"StreamFilesS3PathPrefix": null,
"StreamFilesS3Profile": null,
{{- end }}
"ArchiveOutputCompressionLevel": {{ .Values.clpConfig.archive_output.compression_level | int }},
"ArchiveOutputTargetArchiveSize":
{{ .Values.clpConfig.archive_output.target_archive_size | int }},
Expand Down
Loading
Loading