Conversation
WalkthroughThis update introduces a new AES secret key configuration for the silo service in the plane-enterprise Helm chart. The change adds the key to the chart's values, documentation, configuration questions, and Kubernetes secret manifests, and increments the chart version from 1.2.0 to 1.2.1. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm Chart
participant Kubernetes
participant Silo Service
User->>Helm Chart: Install/Upgrade plane-enterprise chart
Helm Chart->>Kubernetes: Deploy secrets with AES_SECRET_KEY from values.yaml
Kubernetes->>Silo Service: Injects AES_SECRET_KEY as environment variable
Silo Service-->>User: Runs with configured AES secret key
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
charts/plane-enterprise/README.md (1)
270-270: Document Security Considerations for AES KeyAdd a note that the default AES key is for demonstration only and should be overridden in production with a secure, user-supplied value. For example:
Security Note: Always provide your own AES key via
--set env.silo_envs.aes_secret_key=<your-secret>; do not rely on the default.charts/plane-enterprise/questions.yml (1)
252-255: Enhance security for AES secret key configuration
- Use
type: passwordinstead ofstringso the key is masked in UI prompts.- Avoid committing a fixed default key; instead set
default: ""and generate a strong random value at install time (e.g., via a Helm helper or external secrets).- variable: env.silo_envs.aes_secret_key label: "Silo AES Secret Key" - type: string - default: "dsOdt7YrvxsTIFJ37pOaEVvLxN8KGBCr" + type: password + default: ""Let me know if you’d like assistance drafting a
helpers.tplfunction to generate arandAlphaNum 32key when none is provided.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
charts/plane-enterprise/Chart.yaml(1 hunks)charts/plane-enterprise/README.md(1 hunks)charts/plane-enterprise/questions.yml(1 hunks)charts/plane-enterprise/templates/config-secrets/app-env.yaml(1 hunks)charts/plane-enterprise/templates/config-secrets/silo.yaml(1 hunks)charts/plane-enterprise/values.yaml(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.21.2)
charts/plane-enterprise/values.yaml
207-207: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (2)
charts/plane-enterprise/Chart.yaml (1)
8-8: Semantic Version Bump VerifiedThe Chart version has been correctly incremented from
1.2.0to1.2.1to reflect the addition of the AES secret key feature. Ensure that corresponding updates (e.g.,Chart.lock, release notes) are applied in the release process.charts/plane-enterprise/templates/config-secrets/app-env.yaml (1)
10-10: Verify the Values Path for App SecretThe
AES_SECRET_KEYentry in the app secret is sourcing from.Values.env.silo_envs.aes_secret_key. Confirm whether this key should live undersilo_envsor be its own top-levelenv.aes_secret_key. Align the chart values and documentation to reflect the correct path.
| mq_prefetch_count: 1 | ||
| request_interval: 400 | ||
| hmac_secret_key: '' | ||
| aes_secret_key: 'dsOdt7YrvxsTIFJ37pOaEVvLxN8KGBCr' |
There was a problem hiding this comment.
Remove Hardcoded Default Secret
Embedding a static AES key in values.yaml poses a security risk. It's recommended to default this value to empty (forcing users to provide their own) or to generate a random key at deployment time rather than checking in a fixed secret.
Suggest:
- aes_secret_key: 'dsOdt7YrvxsTIFJ37pOaEVvLxN8KGBCr'
+ aes_secret_key: '' # Required: provide a secure AES keyOr leverage Helm functions (e.g., randAlphaNum) in the templates to auto-generate one if none is supplied.
📝 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.
| aes_secret_key: 'dsOdt7YrvxsTIFJ37pOaEVvLxN8KGBCr' | |
| aes_secret_key: '' # Required: provide a secure AES key |
🧰 Tools
🪛 Gitleaks (8.21.2)
207-207: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
| AES_SECRET_KEY: {{ .Values.env.silo_envs.aes_secret_key | default "dsOdt7YrvxsTIFJ37pOaEVvLxN8KGBCr" | quote }} | ||
|
|
There was a problem hiding this comment.
Avoid Static Fallback for AES Secret
Similar to SILO_HMAC_SECRET_KEY, the AES key should not default to a hardcoded constant. Use Helm’s randAlphaNum or require the user to supply it to ensure secrecy:
- AES_SECRET_KEY: {{ .Values.env.silo_envs.aes_secret_key | default "dsOdt7YrvxsTIFJ37pOaEVvLxN8KGBCr" | quote }}
+ AES_SECRET_KEY: {{ .Values.env.silo_envs.aes_secret_key | default (randAlphaNum 32) | quote }}
2 participants
Summary by CodeRabbit
New Features
Chores