Skip to content

improvement(helm): added additional envvars to helm charts - #1695

Merged
waleedlatif1 merged 2 commits into
stagingfrom
fix/deployment-docs
Oct 21, 2025
Merged

improvement(helm): added additional envvars to helm charts#1695
waleedlatif1 merged 2 commits into
stagingfrom
fix/deployment-docs

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

added additional envvars to helm charts with documentation

Type of Change

  • Documentation

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercelBot commented Oct 19, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentPreviewCommentsUpdated (UTC)
docsReadyReadyPreviewCommentOct 21, 2025 6:57pm

@greptile-appsgreptile-appsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Greptile Overview

Summary

This PR adds three new environment variables to the Helm charts: INTERNAL_API_SECRET, CRON_SECRET, and API_ENCRYPTION_KEY. The changes are consistently applied across all example configuration files.

Key Changes:

  • Added INTERNAL_API_SECRET for internal service-to-service authentication (required, validated in apps/sim/lib/env.ts:27)
  • Added CRON_SECRET for authenticating scheduled job requests (used in helm/sim/templates/cronjobs.yaml:64)
  • Added API_ENCRYPTION_KEY for encrypting API keys at rest in the database (optional, validated in apps/sim/lib/env.ts:26)
  • Updated README.md with comprehensive security documentation
  • Consistently updated all 7 example values files (aws, azure, gcp, production, development, external-db, whitelabeled)

Issues Found:

  • Documentation inconsistency: CRON_SECRET is marked as REQUIRED in the Helm chart documentation but defined as optional in the environment schema
  • Confusing instructions for API_ENCRYPTION_KEY generation (mentions "64-character hex string" but schema requires min 32)

Confidence Score: 3/5

  • This PR is mostly safe to merge but has documentation inconsistencies that should be addressed
  • Score of 3 reflects that while the implementation is correct and consistent across all files, there are documentation inconsistencies between the Helm chart documentation and the actual environment schema that could confuse users and lead to runtime issues. The CRON_SECRET mismatch could result in users not setting it when they should (if it's truly required) or setting it unnecessarily (if it's truly optional).
  • Pay attention to helm/sim/README.md to resolve the CRON_SECRET requirement inconsistency and API_ENCRYPTION_KEY generation instruction clarity

Important Files Changed

File Analysis

FilenameScoreOverview
helm/sim/README.md3/5Adds documentation for three new environment variables (INTERNAL_API_SECRET, CRON_SECRET, API_ENCRYPTION_KEY). Contains inconsistency where CRON_SECRET is marked as required but is optional in env schema, and confusing instructions for API_ENCRYPTION_KEY generation.
helm/sim/values.yaml4/5Adds three new environment variables with inline comments. Properly marks INTERNAL_API_SECRET and CRON_SECRET as REQUIRED, API_ENCRYPTION_KEY as OPTIONAL. Comments are clear and helpful.
helm/sim/examples/values-production.yaml5/5Adds the three new environment variables to production example with placeholder values and clear comments. Properly structured for production use case.

Sequence Diagram

sequenceDiagram
participant Dev as Developer
participant Helm as Helm Chart
participant K8s as Kubernetes
participant App as Application Pod
participant Cron as CronJob Pod
participant EnvSchema as Environment Schema
Note over Dev,EnvSchema: Environment Variable Configuration Flow
Dev->>Helm: Deploy with values.yaml
Note over Helm: INTERNAL_API_SECRET=""<br/>CRON_SECRET=""<br/>API_ENCRYPTION_KEY=""
Helm->>K8s: Create deployment manifests
K8s->>App: Start application pod
App->>EnvSchema: Validate env vars (apps/sim/lib/env.ts)
alt INTERNAL_API_SECRET validation
EnvSchema->>EnvSchema: Check .string().min(32) [REQUIRED]
EnvSchema-->>App: ✓ Pass or ✗ Fail
end
alt CRON_SECRET validation
EnvSchema->>EnvSchema: Check .string().optional()
Note over EnvSchema: Inconsistency: Helm docs say REQUIRED<br/>but env.ts defines as optional
EnvSchema-->>App: ✓ Always passes (optional)
end
alt API_ENCRYPTION_KEY validation
EnvSchema->>EnvSchema: Check .string().min(32).optional()
EnvSchema-->>App: ✓ Pass (optional)
end
App->>App: Use INTERNAL_API_SECRET for service auth
App->>App: Use API_ENCRYPTION_KEY for API key encryption (if set)
K8s->>Cron: Start scheduled cron job
Cron->>Cron: Read CRON_SECRET from env
Cron->>App: HTTP Request with Authorization: Bearer ${CRON_SECRET}
App->>App: Validate CRON_SECRET
App-->>Cron: Response
Loading

9 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment threadhelm/sim/README.md Outdated
Comment threadhelm/sim/README.md Outdated
@waleedlatif1
waleedlatif1 merged commit c178a90 into stagingOct 21, 2025
4 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/deployment-docs branch October 21, 2025 19:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@waleedlatif1