feat(hub): use kubernetes secrets to store credentials#107
feat(hub): use kubernetes secrets to store credentials#107
Conversation
…rate, user-override in values, optional existingSecret) Breaking: (Hub) Default passwords have been removed from values.yaml! When upgrading an existing installation, make sure to provide your passwords in your values override file
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCentralizes credentials for Flame Hub and Authup by introducing a credentials Secret template and a helper to resolve its name, updating values/README to prefer secrets, and changing deployments to source connection strings and passwords from the resolved Kubernetes Secret instead of embedded plaintext. Changes
Sequence Diagram(s)sequenceDiagram
participant Helm as Helm (chart renderer)
participant KubeAPI as Kube API
participant Secret as Credentials Secret
participant Flame as Flame Hub Pods
participant Authup as Authup Pod
Helm->>KubeAPI: Apply rendered manifests
Helm->>KubeAPI: Create `credentials` Secret (if not using `.Values.auth.existingSecret`)
KubeAPI->>Secret: Store merged/generated keys (redis/vault/rabbitmq/minio/mysql/...)
Helm->>Flame: Deploy Flame Hub pods referencing secretKeyRef -> include "flameHub.effectiveSecretName"
Helm->>Authup: Deploy Authup pods (conditional secretKeyRef when `existingSecret` set)
Flame->>KubeAPI: Pods read secret keys via secretKeyRef
Authup->>KubeAPI: Pod reads secret keys when `existingSecret` is used
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 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: 7
🤖 Fix all issues with AI agents
In `@charts/flame-hub/README.md`:
- Around line 46-61: The fenced code block containing the credential key list
(lines with symbols like mysql-root-password, mysql-replication-password,
mysql-password, rabbitmq-password, redis-password, vault-dev-root-token,
minio-root-password, harbor-admin-password, grafana-admin-password,
redis-connection-string, rabbitmq-connection-string, vault-connection-string,
minio-connection-string, harbor-connection-string) needs a language specifier to
satisfy MD040; update the opening fence in README.md for that block to include a
language such as "text" (or "yaml") so it becomes ```text (or ```yaml) and leave
the content unchanged.
- Around line 14-17: Update the README to explicitly document the
secret-preservation behavior on helm upgrades: state that the chart uses the
Kubernetes `lookup` function to detect an existing secret named `flame-hub-auth`
and will retain previously generated passwords on `helm upgrade`, only
generating new values for credentials that are missing; mention that supplying
your own k8s secret or setting individual credentials still overrides this
behavior and clarify this applies to upgrades as well as initial installs.
- Around line 22-24: The README uses Helm template markers ({{ $secretName }}
and {{ .Release.Namespace }}) which won't be rendered; update the kubectl
example to use explicit placeholders (e.g. SECRET_NAME and NAMESPACE) and add a
one-line note telling users how to substitute real values or render the template
with Helm (e.g. use helm template/helm get manifests or pass the release
namespace) so consumers replace SECRET_NAME and NAMESPACE with actual values
instead of copying literal {{ $secretName }}/.Release.Namespace.
In `@charts/flame-hub/templates/credentials-secret.yaml`:
- Around line 26-53: The template reads keys from $existingSecret.data and pipes
them to b64dec (e.g., index $existingSecret.data "grafana-admin-password" |
b64dec), which will error if the key is missing; update every key lookup in the
block that sets $mysqlRootPassword, $mysqlReplicationPassword, $mysqlPassword,
$rabbitmqPassword, $redisPassword, $vaultToken, $minioRootPassword,
$harborAdminPassword, and $grafanaAdminPassword to first check hasKey
$existingSecret.data "<key>" (or a nil-safe conditional) before calling index
and b64dec so missing keys are skipped and do not panic during template
rendering. Ensure the same nil-safe pattern is applied consistently for all keys
in this conditional block.
In `@charts/flame-hub/values.yaml`:
- Around line 91-92: The values.yaml currently hardcodes a weak Vault dev root
token (devRootToken: "start123"); remove this default by setting devRootToken to
an empty string (require users to supply it), update the chart README to state
that devRootToken must be overridden for any non-local deployment, and add a
deployment-time warning (in the chart NOTES.txt or a pre-install hook) that
checks if devRootToken is still "start123" and emits a clear error or warning
instructing the user to replace it before proceeding.
- Line 72: Fix the typo in the comment string by adding a space after
"IMPORTANT:" so the line reads "IMPORTANT: If you set this, you must also update
the subchart configurations below to point to your secret!" — update the comment
text where that exact string appears in values.yaml.
In `@charts/third-party/authup/values.yaml`:
- Around line 7-14: The values file still defines hardcoded defaults for
vaultToken, redisPassword, and mysqlRootPassword; remove those defaults by
setting vaultToken, redisPassword, and mysqlRootPassword to empty strings so the
credentials-secret.yaml auto-generation runs when existingSecret is not
provided, or if you intentionally want standalone defaults, add a clear comment
next to vaultToken/redisPassword/mysqlRootPassword explaining they are explicit
standalone defaults (and why) so reviewers/users aren’t misled.
🧹 Nitpick comments (4)
charts/flame-hub/templates/credentials-secret.yaml (2)
1-11: Consider addinghelm.sh/resource-policy: keepannotation to prevent accidental secret deletion.If someone runs
helm uninstalland later reinstalls, the auto-generated passwords will be lost, breaking any persistent data stores (MySQL, Redis, etc.) that still use the old credentials. Adding this annotation preserves the secret across uninstall/reinstall cycles.Proposed annotation
metadata: name: {{ $secretName }} labels: app: {{ .Release.Name }}-flame-hub + annotations: + "helm.sh/resource-policy": keep type: Opaque
57-65: Random passwords regenerate on everyhelm template(dry-run) invocation.This is a known limitation of the
lookup+randAlphaNumpattern —lookupreturns empty duringhelm template, so every dry-run produces different passwords. This is fine for actualhelm install/upgrade(wherelookupworks), but worth documenting for users who rely onhelm templateoutput for GitOps workflows.charts/flame-hub/values.yaml (1)
64-73: Hardcoded subchartexistingSecretvalues will silently diverge fromauth.secretName.Changing
auth.secretNamewithout also updating every subchart'sexistingSecret(rabbitmq, redis, minio, mysql, grafana, harbor, authup) will cause pods to reference a non-existent secret. The comment warns about this, but it's fragile.Consider whether you could use an anchor/alias or a shared global value that subcharts read, so there's a single source of truth. For example:
global: flameHub: auth: secretName: "flame-hub-auth"Subcharts that support
globalvalue passthrough could then referenceglobal.flameHub.auth.secretNameinstead of each needing a hardcoded string. This may not be possible for all upstream charts, but it would reduce the blast radius of a rename.charts/flame-hub/README.md (1)
44-68: Secret key documentation is incomplete — missing connection-string formats.The README lists the required secret keys but doesn't document the expected format/value for the connection-string keys (
redis-connection-string,rabbitmq-connection-string,vault-connection-string,minio-connection-string,harbor-connection-string). Users creating a manualexistingSecretwon't know the expected URI format. Thekubectl create secretexample only shows password keys.Consider adding a brief note with the expected connection string formats, e.g.:
redis-connection-string: "redis://default:<password>@<release>-redis-master:6379"
| ## Credentials | ||
| > 🔐 **By default** this chart will create a new secret with random passwords. | ||
|
|
||
| create `values_local.yaml` then run `helm install hub . -f values_local.yaml` | ||
| This chart manages credentials using a Kubernetes Secret. By default, it will check for an existing secret named `flame-hub-auth`. If it does not exist, it will create a new secret with random passwords. You can override this behavior by providing your own k8s secret or by explicitly setting individual credentials in the values. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat charts/flame-hub/templates/credentials-secret.yamlRepository: PrivateAIM/helm
Length of output: 4557
Document the secret preservation behavior during upgrades.
The template correctly uses the lookup function to preserve existing secret values on upgrade. However, the README documentation does not explicitly mention this behavior. While it states the chart will "check for an existing secret," it should clarify that on helm upgrade, previously generated passwords are retained and only missing credentials are newly generated. This will reduce operational confusion around upgrade safety.
🤖 Prompt for AI Agents
In `@charts/flame-hub/README.md` around lines 14 - 17, Update the README to
explicitly document the secret-preservation behavior on helm upgrades: state
that the chart uses the Kubernetes `lookup` function to detect an existing
secret named `flame-hub-auth` and will retain previously generated passwords on
`helm upgrade`, only generating new values for credentials that are missing;
mention that supplying your own k8s secret or setting individual credentials
still overrides this behavior and clarify this applies to upgrades as well as
initial installs.
| ```bash | ||
| kubectl get secret {{ $secretName }} -n {{ .Release.Namespace }} -o jsonpath='{.data.harbor-admin-password}' | base64 -d && echo | ||
| ``` |
There was a problem hiding this comment.
Bug: Helm template variables in README won't be rendered.
The kubectl example uses {{ $secretName }} and {{ .Release.Namespace }}, but README.md is not processed by the Helm template engine. Users copying this command will get literal {{ ... }} strings.
Proposed fix
-kubectl get secret {{ $secretName }} -n {{ .Release.Namespace }} -o jsonpath='{.data.harbor-admin-password}' | base64 -d && echo
+kubectl get secret flame-hub-auth -n <namespace> -o jsonpath='{.data.harbor-admin-password}' | base64 -d && echo🤖 Prompt for AI Agents
In `@charts/flame-hub/README.md` around lines 22 - 24, The README uses Helm
template markers ({{ $secretName }} and {{ .Release.Namespace }}) which won't be
rendered; update the kubectl example to use explicit placeholders (e.g.
SECRET_NAME and NAMESPACE) and add a one-line note telling users how to
substitute real values or render the template with Helm (e.g. use helm
template/helm get manifests or pass the release namespace) so consumers replace
SECRET_NAME and NAMESPACE with actual values instead of copying literal {{
$secretName }}/.Release.Namespace.
| ``` | ||
| mysql-root-password: | ||
| mysql-replication-password: | ||
| mysql-password: | ||
| rabbitmq-password: | ||
| redis-password: | ||
| vault-dev-root-token: | ||
| minio-root-password: | ||
| harbor-admin-password: | ||
| grafana-admin-password: | ||
| redis-connection-string: | ||
| rabbitmq-connection-string: | ||
| vault-connection-string: | ||
| minio-connection-string: | ||
| harbor-connection-string: | ||
| ``` |
There was a problem hiding this comment.
Add a language specifier to the fenced code block.
The static analysis tool flagged this block (MD040). Since this is a list of key names (not executable code), text or yaml would be appropriate:
-```
+```text
mysql-root-password:🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 46-46: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@charts/flame-hub/README.md` around lines 46 - 61, The fenced code block
containing the credential key list (lines with symbols like mysql-root-password,
mysql-replication-password, mysql-password, rabbitmq-password, redis-password,
vault-dev-root-token, minio-root-password, harbor-admin-password,
grafana-admin-password, redis-connection-string, rabbitmq-connection-string,
vault-connection-string, minio-connection-string, harbor-connection-string)
needs a language specifier to satisfy MD040; update the opening fence in
README.md for that block to include a language such as "text" (or "yaml") so it
becomes ```text (or ```yaml) and leave the content unchanged.
| secretName: "flame-hub-auth" | ||
|
|
||
| # Advanced: Use your own secret instead of auto-generated one, takes precedence over secretName | ||
| # IMPORTANT:If you set this, you must also update the subchart configurations below to point to your secret! |
There was a problem hiding this comment.
Typo: missing space after "IMPORTANT:".
- # IMPORTANT:If you set this, you must also update the subchart configurations below to point to your secret!
+ # IMPORTANT: If you set this, you must also update the subchart configurations below to point to your secret!📝 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.
| # IMPORTANT:If you set this, you must also update the subchart configurations below to point to your secret! | |
| # IMPORTANT: If you set this, you must also update the subchart configurations below to point to your secret! |
🤖 Prompt for AI Agents
In `@charts/flame-hub/values.yaml` at line 72, Fix the typo in the comment string
by adding a space after "IMPORTANT:" so the line reads "IMPORTANT: If you set
this, you must also update the subchart configurations below to point to your
secret!" — update the comment text where that exact string appears in
values.yaml.
| # needs to be set. No option for passing a secret here. | ||
| devRootToken: "start123" |
There was a problem hiding this comment.
Vault dev root token is hardcoded with a weak default.
devRootToken: "start123" is a weak credential committed in plain text. Even though the comment acknowledges the Vault chart doesn't support secret references here, this token grants full Vault access.
Consider:
- Documenting clearly (in the README) that this must be overridden for any non-local deployment.
- Adding a check in a
NOTES.txtor pre-install hook that warns if the token is still"start123". - Alternatively, leave the value empty and require the user to set it, similar to the other credentials.
🤖 Prompt for AI Agents
In `@charts/flame-hub/values.yaml` around lines 91 - 92, The values.yaml currently
hardcodes a weak Vault dev root token (devRootToken: "start123"); remove this
default by setting devRootToken to an empty string (require users to supply it),
update the chart README to state that devRootToken must be overridden for any
non-local deployment, and add a deployment-time warning (in the chart NOTES.txt
or a pre-install hook) that checks if devRootToken is still "start123" and emits
a clear error or warning instructing the user to replace it before proceeding.
| auth: | ||
| # Use an existing secret with keys: vault-connection-string, redis-connection-string, mysql-root-password | ||
| existingSecret: "" | ||
|
|
||
| # or provide credentials in values (used when existingSecret is not set) | ||
| vaultToken: "start123" | ||
| redisPassword: "start123" | ||
| mysqlRootPassword: "start123" |
There was a problem hiding this comment.
Hardcoded default credentials remain in authup values.
The PR description states "default passwords have been removed from values.yaml," but vaultToken, redisPassword, and mysqlRootPassword still default to "start123". If the intent is to force users to supply credentials (as done in the main flame-hub/values.yaml), these should also be empty strings so that the auto-generation logic in credentials-secret.yaml kicks in. If these are intentionally kept for standalone authup usage, consider adding a comment clarifying that.
🤖 Prompt for AI Agents
In `@charts/third-party/authup/values.yaml` around lines 7 - 14, The values file
still defines hardcoded defaults for vaultToken, redisPassword, and
mysqlRootPassword; remove those defaults by setting vaultToken, redisPassword,
and mysqlRootPassword to empty strings so the credentials-secret.yaml
auto-generation runs when existingSecret is not provided, or if you
intentionally want standalone defaults, add a clear comment next to
vaultToken/redisPassword/mysqlRootPassword explaining they are explicit
standalone defaults (and why) so reviewers/users aren’t misled.
There was a problem hiding this comment.
I don't really care about authup
There was a problem hiding this comment.
@maxju, understood! I'll leave this as-is for now. If you'd like, I can open a follow-up issue to track updating the authup credentials for consistency with the main flame-hub chart. Just let me know.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@charts/flame-hub/README.md`:
- Around line 42-61: Update the README section to split the secret keys into two
clearly labeled groups: "Required password keys" (list mysql-root-password,
mysql-replication-password, mysql-password, rabbitmq-password, redis-password,
vault-dev-root-token, minio-root-password, harbor-admin-password,
grafana-admin-password) and "Derived connection string keys" (list
redis-connection-string, rabbitmq-connection-string, vault-connection-string,
minio-connection-string, harbor-connection-string) and state the behavior for
the chart options: when using secretName the chart templates will auto-generate
the connection strings from the provided password keys, but when using
existingSecret the user must include the connection string keys manually in that
secret; reference the configuration names existingSecret and secretName so
readers know which path requires manual connection strings.
In `@charts/flame-hub/templates/credentials-secret.yaml`:
- Around line 79-80: The Redis and RabbitMQ connection strings (keys
redis-connection-string and rabbitmq-connection-string) omit explicit ports;
update the printf templates to append :6379 for Redis and :5672 for RabbitMQ so
clients don't rely on defaults—modify the Redis line that builds
"redis://default:%s@%s-redis-master" to
"redis://default:%s@%s-redis-master:6379" (keeping $redisPassword and
.Release.Name) and modify the RabbitMQ line that builds
"amqp://%s:%s@%s-rabbitmq" to "amqp://%s:%s@%s-rabbitmq:5672" (keeping
.Values.rabbitmq.auth.username and $rabbitmqPassword and .Release.Name).
🧹 Nitpick comments (3)
charts/flame-hub/templates/credentials-secret.yaml (2)
5-11: Consider adding ahelm.sh/resource-policy: keepannotation to prevent accidental secret deletion.If a user runs
helm uninstall, this secret (containing all generated passwords) will be deleted along with the release. Since the passwords are used by stateful services (MySQL, Redis, RabbitMQ, etc.) whose data persists in PVCs, the credentials would be lost while the data remains — making recovery difficult.Suggested fix
metadata: name: {{ $secretName }} labels: app: {{ .Release.Name }}-flame-hub + annotations: + "helm.sh/resource-policy": keep type: Opaque
1-3: Use theflameHub.effectiveSecretNamehelper for consistency with deployment templates.Line 2 directly references
.Values.auth.secretName, but all deployment templates use theflameHub.effectiveSecretNamehelper to resolve the secret name. Although this template is guarded byif not .Values.auth.existingSecret(making the direct reference functionally equivalent to the helper in practice), using the helper would ensure consistency across the chart and reduce duplication of the secret name resolution logic:Suggested change
-{{- $secretName := .Values.auth.secretName }} +{{- $secretName := include "flameHub.effectiveSecretName" . }}charts/flame-hub/README.md (1)
70-97: Minimal configuration code block is missing a language specifier.Line 74 uses a bare
```fence. Addyamlfor consistent formatting and to satisfy linting:-``` +```yaml global:
| ### Creating the secret | ||
|
|
||
| The chart requires a secret with the following keys: | ||
|
|
||
| ``` | ||
| mysql-root-password: | ||
| mysql-replication-password: | ||
| mysql-password: | ||
| rabbitmq-password: | ||
| redis-password: | ||
| vault-dev-root-token: | ||
| minio-root-password: | ||
| harbor-admin-password: | ||
| grafana-admin-password: | ||
| redis-connection-string: | ||
| rabbitmq-connection-string: | ||
| vault-connection-string: | ||
| minio-connection-string: | ||
| harbor-connection-string: | ||
| ``` |
There was a problem hiding this comment.
Clarify which keys are required for existingSecret vs auto-generated.
The section says "The chart requires a secret with the following keys" and lists both individual passwords and connection strings. However, connection strings (Lines 56-61) are auto-generated from passwords in the template. For users providing an existingSecret, it's unclear whether they must manually construct all connection strings or if only the password keys are needed.
Consider splitting into two groups: required password keys (that users must provide) and derived connection strings (auto-generated when using secretName, must be manually provided when using existingSecret).
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 46-46: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@charts/flame-hub/README.md` around lines 42 - 61, Update the README section
to split the secret keys into two clearly labeled groups: "Required password
keys" (list mysql-root-password, mysql-replication-password, mysql-password,
rabbitmq-password, redis-password, vault-dev-root-token, minio-root-password,
harbor-admin-password, grafana-admin-password) and "Derived connection string
keys" (list redis-connection-string, rabbitmq-connection-string,
vault-connection-string, minio-connection-string, harbor-connection-string) and
state the behavior for the chart options: when using secretName the chart
templates will auto-generate the connection strings from the provided password
keys, but when using existingSecret the user must include the connection string
keys manually in that secret; reference the configuration names existingSecret
and secretName so readers know which path requires manual connection strings.
tada5hi
changed the title
tada5hi
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/flame-hub/templates/credentials-secret.yaml`:
- Around line 85-91: Connection strings in the credentials-secret template
interpolate user-provided passwords directly (e.g., redis-connection-string,
rabbitmq-connection-string, vault-connection-string, minio-connection-string,
harbor-connection-string), which will break if passwords contain URI-special
characters; fix by adding a Helm helper (e.g., a percentEncode or urlencode
template in _helpers.tpl) and use that helper to percent-encode password
variables (replace direct uses of $redisPassword, $rabbitmqPassword,
$vaultToken, $minioRootPassword, $harborAdminPassword with the helper) so
passwords are safely encoded when building the printf URIs, or alternatively
update the chart values docs to require alphanumeric-only passwords if you
prefer the simpler docs-only approach.
In `@charts/third-party/authup/templates/deployment.yaml`:
- Around line 79-86: The REDIS environment variable in the deployment.yaml
template (the REDIS env var under the container env list) is missing the port
suffix :6379 in the inline connection string; update the REDIS value expression
to include :6379 after the password host segment (so it matches the
existingSecret form), e.g. adjust the REDIS env value construction that
currently uses "redis://default:{{ .Values.auth.redisPassword }}@{{
.Release.Name }}-redis-master" to include ":6379" on the host portion; leave the
other env entries (VAULT, DB_PASSWORD, USER_ADMIN_PASSWORD) unchanged.
---
Duplicate comments:
In `@charts/flame-hub/README.md`:
- Around line 14-17: The README's Credentials section omits that Helm upgrades
preserve previously generated passwords because the chart uses the Helm lookup
function to detect an existing Secret; update the Credentials paragraph to state
that if a Secret named `flame-hub-auth` already exists (discovered via the
`lookup` function), the chart will reuse those credentials on `helm upgrade`
rather than regenerating them, and mention that you can still override by
supplying your own k8s secret or individual values.
- Around line 52-68: The fenced code block showing the list of secret key names
uses a bare ``` delimiter; update that opening fence to include a language
specifier (use ```text) so the block is rendered correctly as plain text. Locate
the block containing the keys (e.g. the lines listing mysql-root-password,
rabbitmq-password, redis-password, etc.) and replace the opening triple
backticks with ```text, leaving the closing fence unchanged.
- Around line 48-68: The README lists secret keys without clarifying which are
user-provided vs. auto-derived; update the "Creating the secret" section to
split the keys into two labeled groups (e.g., "Required keys when supplying
existingSecret" and "Auto-generated keys when using secretName/managed secret")
and explicitly state that connection-string keys (redis-connection-string,
rabbitmq-connection-string, vault-connection-string, minio-connection-string,
harbor-connection-string) are generated by the chart when using secretName but
must be provided manually when using existingSecret; reference the chart's
secret consumption (secretName and existingSecret) so readers know when to
supply mysql-root-password, mysql-password, rabbitmq-password, redis-password,
minio-root-password, vault-dev-root-token, harbor-admin-password,
grafana-admin-password, authup-default-admin-password versus when connection
strings are auto-derived.
In `@charts/flame-hub/values.yaml`:
- Around line 93-94: The Helm values file still hardcodes a weak Vault dev root
token via the devRootToken key; remove the default hardcoded value and instead
require users to provide a secure token (e.g., leave devRootToken empty or set
to null) and add a deployment-time warning in the chart NOTES.txt that checks
for an unchanged/empty devRootToken and instructs the user to rotate it
immediately; reference the devRootToken key in values.yaml and update NOTES.txt
to print a clear warning if the value is unchanged or missing.
- Line 72: Fix the typo in the YAML comment: change the string "IMPORTANT:If you
set this, you must also update the subchart configurations below to point to
your secret!" to include a space after the colon so it reads "IMPORTANT: If you
set this, you must also update the subchart configurations below to point to
your secret!" (locate the comment containing "IMPORTANT:If" in values.yaml and
update it accordingly).
In `@charts/third-party/authup/values.yaml`:
- Around line 7-15: No code change required: keep the auth block as-is with
standalone defaults and clear comments; ensure the keys under auth
(existingSecret, vaultToken, redisPassword, mysqlRootPassword,
defaultAdminPassword) remain present and unchanged so both the existingSecret
and inline-credentials modes work as intended.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
charts/flame-hub/templates/credentials-secret.yaml (1)
60-70:randAlphaNumwill produce new passwords on everyhelm template/helm diffinvocation.Since
lookupreturns an empty map during dry-run/diff, all passwords will appear to change. This is a known Helm limitation with this pattern, but worth a brief note in the README so operators aren't alarmed by spurious diffs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/flame-hub/templates/credentials-secret.yaml` around lines 60 - 70, Add a short note to the chart README explaining that the credentials-secret template uses randAlphaNum to generate values for variables like $mysqlRootPassword, $mysqlPassword, $rabbitmqPassword, $redisPassword, $vaultToken, etc., and that because lookup returns an empty map during helm template/helm diff dry-runs this will make passwords appear to change on every invocation (spurious diffs); mention this is a Helm limitation and suggest operators run a full install/upgrade or inspect the existing Kubernetes Secret to confirm actual values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/third-party/authup/templates/deployment.yaml`:
- Around line 57-87: The inline branch constructs URIs by interpolating raw
values (.Values.auth.vaultToken and .Values.auth.redisPassword) which can
contain URI-special characters; update the else branch to URL-encode those
values (use the Helm urlquery function on .Values.auth.vaultToken and
.Values.auth.redisPassword) and keep the resulting strings quoted so the VAULT
and REDIS env values are valid URIs even when passwords/tokens contain @, :, /,
etc.; leave DB_PASSWORD and USER_ADMIN_PASSWORD handling (which are quoted)
unchanged.
---
Nitpick comments:
In `@charts/flame-hub/templates/credentials-secret.yaml`:
- Around line 60-70: Add a short note to the chart README explaining that the
credentials-secret template uses randAlphaNum to generate values for variables
like $mysqlRootPassword, $mysqlPassword, $rabbitmqPassword, $redisPassword,
$vaultToken, etc., and that because lookup returns an empty map during helm
template/helm diff dry-runs this will make passwords appear to change on every
invocation (spurious diffs); mention this is a Helm limitation and suggest
operators run a full install/upgrade or inspect the existing Kubernetes Secret
to confirm actual values.
| {{- if .Values.auth.existingSecret }} | ||
| - name: VAULT | ||
| value: "start123@http://{{ .Release.Name }}-vault:8200/v1/" | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: {{ .Values.auth.existingSecret }} | ||
| key: vault-connection-string | ||
| - name: REDIS | ||
| value: "redis://default:{{ .Values.global.redis.password }}@{{ .Release.Name }}-redis-master" | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: {{ .Values.auth.existingSecret }} | ||
| key: redis-connection-string | ||
| - name: DB_PASSWORD | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: {{ .Values.auth.existingSecret }} | ||
| key: mysql-root-password | ||
| - name: USER_ADMIN_PASSWORD | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: {{ .Values.auth.existingSecret }} | ||
| key: authup-default-admin-password | ||
| {{- else }} | ||
| - name: VAULT | ||
| value: "{{ .Values.auth.vaultToken }}@http://{{ .Release.Name }}-vault:8200/v1/" | ||
| - name: REDIS | ||
| value: "redis://default:{{ .Values.auth.redisPassword }}@{{ .Release.Name }}-redis-master:6379" | ||
| - name: DB_PASSWORD | ||
| value: {{ .Values.auth.mysqlRootPassword | quote }} | ||
| - name: USER_ADMIN_PASSWORD | ||
| value: {{ .Values.auth.defaultAdminPassword | quote }} | ||
| {{- end }} |
There was a problem hiding this comment.
Inline connection strings lack URL-encoding, unlike the secret-based path.
The existingSecret branch reads pre-built connection strings from the secret (which use urlquery in credentials-secret.yaml), but the else branch on lines 80 and 82 interpolates passwords directly into URIs without encoding. If a user supplies a password with URI-special characters (@, :, /, etc.) via values, these connection strings will be malformed.
Proposed fix
- name: VAULT
- value: "{{ .Values.auth.vaultToken }}@http://{{ .Release.Name }}-vault:8200/v1/"
+ value: "{{ .Values.auth.vaultToken | urlquery }}@http://{{ .Release.Name }}-vault:8200/v1/"
- name: REDIS
- value: "redis://default:{{ .Values.auth.redisPassword }}@{{ .Release.Name }}-redis-master:6379"
+ value: "redis://default:{{ .Values.auth.redisPassword | urlquery }}@{{ .Release.Name }}-redis-master:6379"📝 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.
| {{- if .Values.auth.existingSecret }} | |
| - name: VAULT | |
| value: "start123@http://{{ .Release.Name }}-vault:8200/v1/" | |
| valueFrom: | |
| secretKeyRef: | |
| name: {{ .Values.auth.existingSecret }} | |
| key: vault-connection-string | |
| - name: REDIS | |
| value: "redis://default:{{ .Values.global.redis.password }}@{{ .Release.Name }}-redis-master" | |
| valueFrom: | |
| secretKeyRef: | |
| name: {{ .Values.auth.existingSecret }} | |
| key: redis-connection-string | |
| - name: DB_PASSWORD | |
| valueFrom: | |
| secretKeyRef: | |
| name: {{ .Values.auth.existingSecret }} | |
| key: mysql-root-password | |
| - name: USER_ADMIN_PASSWORD | |
| valueFrom: | |
| secretKeyRef: | |
| name: {{ .Values.auth.existingSecret }} | |
| key: authup-default-admin-password | |
| {{- else }} | |
| - name: VAULT | |
| value: "{{ .Values.auth.vaultToken }}@http://{{ .Release.Name }}-vault:8200/v1/" | |
| - name: REDIS | |
| value: "redis://default:{{ .Values.auth.redisPassword }}@{{ .Release.Name }}-redis-master:6379" | |
| - name: DB_PASSWORD | |
| value: {{ .Values.auth.mysqlRootPassword | quote }} | |
| - name: USER_ADMIN_PASSWORD | |
| value: {{ .Values.auth.defaultAdminPassword | quote }} | |
| {{- end }} | |
| {{- if .Values.auth.existingSecret }} | |
| - name: VAULT | |
| valueFrom: | |
| secretKeyRef: | |
| name: {{ .Values.auth.existingSecret }} | |
| key: vault-connection-string | |
| - name: REDIS | |
| valueFrom: | |
| secretKeyRef: | |
| name: {{ .Values.auth.existingSecret }} | |
| key: redis-connection-string | |
| - name: DB_PASSWORD | |
| valueFrom: | |
| secretKeyRef: | |
| name: {{ .Values.auth.existingSecret }} | |
| key: mysql-root-password | |
| - name: USER_ADMIN_PASSWORD | |
| valueFrom: | |
| secretKeyRef: | |
| name: {{ .Values.auth.existingSecret }} | |
| key: authup-default-admin-password | |
| {{- else }} | |
| - name: VAULT | |
| value: "{{ .Values.auth.vaultToken | urlquery }}@http://{{ .Release.Name }}-vault:8200/v1/" | |
| - name: REDIS | |
| value: "redis://default:{{ .Values.auth.redisPassword | urlquery }}@{{ .Release.Name }}-redis-master:6379" | |
| - name: DB_PASSWORD | |
| value: {{ .Values.auth.mysqlRootPassword | quote }} | |
| - name: USER_ADMIN_PASSWORD | |
| value: {{ .Values.auth.defaultAdminPassword | quote }} | |
| {{- end }} |
🧰 Tools
🪛 YAMLlint (1.38.0)
[warning] 58-58: too many spaces after hyphen
(hyphens)
[warning] 63-63: too many spaces after hyphen
(hyphens)
[warning] 68-68: too many spaces after hyphen
(hyphens)
[warning] 73-73: too many spaces after hyphen
(hyphens)
[warning] 79-79: too many spaces after hyphen
(hyphens)
[warning] 81-81: too many spaces after hyphen
(hyphens)
[warning] 83-83: too many spaces after hyphen
(hyphens)
[warning] 85-85: too many spaces after hyphen
(hyphens)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@charts/third-party/authup/templates/deployment.yaml` around lines 57 - 87,
The inline branch constructs URIs by interpolating raw values
(.Values.auth.vaultToken and .Values.auth.redisPassword) which can contain
URI-special characters; update the else branch to URL-encode those values (use
the Helm urlquery function on .Values.auth.vaultToken and
.Values.auth.redisPassword) and keep the resulting strings quoted so the VAULT
and REDIS env values are valid URIs even when passwords/tokens contain @, :, /,
etc.; leave DB_PASSWORD and USER_ADMIN_PASSWORD handling (which are quoted)
unchanged.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
charts/flame-hub/values.yaml (1)
64-73: Enumerate the specific subchart fields in both warning comments.Neither warning (line 67–68 for
secretName, line 72 forexistingSecret) tells the operator which fields must be updated. There are currently 7 subchartexistingSecretentries hardcoded to"flame-hub-auth":
Location Field authup.auth.existingSecretline 80 rabbitmq.auth.existingPasswordSecretline 100 redis.auth.existingSecretline 118 minio.auth.existingSecretline 134 mysql.auth.existingSecretline 154 grafana.admin.existingSecretline 182 harbor.existingSecretline 278 Without this list, operators who change
secretNameor setexistingSecretwill likely miss at least one subchart, leading to silent startup failures.💡 Suggested comment update
- # IMPORTANT: If you change this value, you must also update the subchart configurations to match. - # This is because subcharts have their own existingSecret fields that cannot be templated, unfortunately. + # IMPORTANT: If you change this value, you must also update the following subchart fields to match: + # authup.auth.existingSecret, rabbitmq.auth.existingPasswordSecret, redis.auth.existingSecret, + # minio.auth.existingSecret, mysql.auth.existingSecret, grafana.admin.existingSecret, harbor.existingSecret + # This is because subchart existingSecret fields cannot be templated. secretName: "flame-hub-auth" # Advanced: Use your own secret instead of auto-generated one, takes precedence over secretName - # IMPORTANT:If you set this, you must also update the subchart configurations below to point to your secret! + # IMPORTANT: If you set this, you must also update the subchart fields listed above to point to your secret!🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/flame-hub/values.yaml` around lines 64 - 73, Update the two warning comments next to auth.secretName and auth.existingSecret to explicitly list all subchart secret fields that must be updated when changing these values: authup.auth.existingSecret, rabbitmq.auth.existingPasswordSecret, redis.auth.existingSecret, minio.auth.existingSecret, mysql.auth.existingSecret, grafana.admin.existingSecret, and harbor.existingSecret; modify the comments to state that these specific fields are hardcoded to "flame-hub-auth" and must be changed to match any new secretName or existingSecret value so operators know exactly which subchart fields to update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@charts/flame-hub/values.yaml`:
- Around line 93-94: The devRootToken value is a weak hardcoded Vault root token
(devRootToken) in the Helm values and must be removed; replace this with a
secure mechanism by removing the literal "start123", supporting injection from a
Kubernetes Secret or external secret manager and/or adding a new values key
(e.g., devRootTokenSecretName or devRootTokenRef) that the chart consumes;
update any templates referencing .Values.devRootToken to read from the secret
reference (or fail if not provided) so the root token is never stored in
values.yaml or source control.
- Line 72: The comment contains a typography error where "IMPORTANT:If" is
missing a space; update the comment string that currently reads "IMPORTANT:If
you set this, you must also update the subchart configurations below to point to
your secret!" to "IMPORTANT: If you set this, you must also update the subchart
configurations below to point to your secret!" (locate the line containing
"IMPORTANT:If" to apply the fix).
---
Nitpick comments:
In `@charts/flame-hub/values.yaml`:
- Around line 64-73: Update the two warning comments next to auth.secretName and
auth.existingSecret to explicitly list all subchart secret fields that must be
updated when changing these values: authup.auth.existingSecret,
rabbitmq.auth.existingPasswordSecret, redis.auth.existingSecret,
minio.auth.existingSecret, mysql.auth.existingSecret,
grafana.admin.existingSecret, and harbor.existingSecret; modify the comments to
state that these specific fields are hardcoded to "flame-hub-auth" and must be
changed to match any new secretName or existingSecret value so operators know
exactly which subchart fields to update.
|
When upgrading a running installation and want to continue using default credentials, add this to your override values file: harbor:
adminPassword: "start123"
grafana:
admin:
password: "start123"
rabbitmq:
auth:
password: "start123"
redis:
auth:
password: "start123"
mysql:
auth:
rootPassword: "start123"
replicationPassword: "start123"
minio:
auth:
rootPassword: "start123"
vault:
server:
dev:
enabled: true
devRootToken: "start123" |
|
@tada5hi ready for merge from my side |
2 participants
…rate, user-override in values, optional existingSecret)
Breaking: (Hub) Default passwords have been removed from values.yaml! When upgrading an existing installation, make sure to provide your passwords in your values override file
PR for coderabbit
#106
Summary by CodeRabbit
New Features
Bug Fixes
Documentation