feat(init): shared first-run wizard for compose + k8s-local - #1361
Merged
cyberantonz merged 1 commit intoJun 19, 2026
Conversation
|
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:
📝 WalkthroughWalkthroughExtracts the first-run configuration wizard from ChangesFirst-run wizard extraction and k8s-local support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~26 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 5
🧹 Nitpick comments (1)
dev-compose.sh (1)
157-157: ⚡ Quick winInvoke the wizard via
bashfor portability.Direct execution here depends on executable mode being preserved; invoking with
bashavoids first-run failures on environments that lose script mode bits.Suggested fix
- "$ROOT_DIR/compose/insight-init.sh" --target=compose || return $? + bash "$ROOT_DIR/compose/insight-init.sh" --target=compose || return $?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dev-compose.sh` at line 157, The direct execution of the insight-init.sh script depends on executable mode being preserved, which can cause failures in environments that lose script mode bits. Modify the invocation of insight-init.sh to use bash command explicitly instead of relying on the shebang execution. Change the command to invoke bash with the script path as an argument, ensuring portable execution across different environments while maintaining the same command-line arguments and error handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@compose/insight-init.sh`:
- Around line 573-579: The variables $airbyte_email and $airbyte_org are being
written unquoted to the .env.local file in the heredoc block, which will cause
issues when sourced if the values contain spaces or special characters. Quote
both AIRBYTE_SETUP_EMAIL and AIRBYTE_SETUP_ORG variable assignments in the
heredoc to properly escape and preserve the user input values during sourcing.
- Around line 670-675: The current logic only updates `.global.tenantDefaultId`
in the values.yaml file when TENANT_DEFAULT_ID differs from TENANT_DEFAULT_UUID.
This causes stale tenant ID values to persist in values.yaml during re-runs,
potentially pointing the app to the wrong tenant context. Remove the conditional
check around the yq update command so that the values.yaml file is always
synchronized with the current TENANT_DEFAULT_ID value, regardless of whether it
matches TENANT_DEFAULT_UUID or not.
- Around line 588-642: The heredoc that creates secrets-store.yaml directly
injects password variables into double-quoted YAML scalars without escaping
them. Passwords containing backslash (\) or double quote (") characters will
break the YAML syntax. Before the heredoc block begins, escape the special YAML
characters in the variables MARIADB_ROOT_PASSWORD, MARIADB_PASSWORD, and
CLICKHOUSE_PASSWORD by replacing backslashes with double backslashes and double
quotes with escaped double quotes, then use these escaped versions within the
heredoc where the stringData values are substituted.
- Around line 269-272: The MARIADB_ROOT_PASSWORD is hard-coded to "root-local"
regardless of whether an in-stack or external MariaDB is being used. When
external MariaDB is selected, the actual root credentials from the external
database should be collected and used instead of the placeholder value, since
write_k8s_local() will write this password to insight-db-creds which cluster
bootstrap jobs use for authentication. Update the code to conditionally set
MARIADB_ROOT_PASSWORD by checking the database mode: when external MariaDB is
selected, prompt for or retrieve the actual root password from the external
database configuration, otherwise use the local placeholder value for in-stack
deployments.
In `@CONTRIBUTING.md`:
- Around line 581-585: The documentation for the k8s-local wizard in the
CONTRIBUTING.md file lists the generated artifacts as
`environments/local/inventory.yaml` and `secrets-store.yaml`, but omits
`environments/local/.env.local` which is also generated when Airbyte is enabled.
Update the artifact list in the wizard description section to include
`.env.local` with a note that it is generated when Airbyte is enabled, similar
to how the other generated files are documented.
---
Nitpick comments:
In `@dev-compose.sh`:
- Line 157: The direct execution of the insight-init.sh script depends on
executable mode being preserved, which can cause failures in environments that
lose script mode bits. Modify the invocation of insight-init.sh to use bash
command explicitly instead of relying on the shebang execution. Change the
command to invoke bash with the script path as an argument, ensuring portable
execution across different environments while maintaining the same command-line
arguments and error handling.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e42671ba-14eb-4d55-be75-e0dc66ca9d66
📥 Commits
Reviewing files that changed from the base of the PR and between c32821d and 4e10e21921b152aa6e4d9ffbfe092e4a9aafa781.
📒 Files selected for processing (44)
CONTRIBUTING.mdcharts/insight/values.yamlcompose/insight-init.shdeploy/gitops/.gitignoredeploy/gitops/.gitleaks.tomldeploy/gitops/.insight-versiondeploy/gitops/Brewfiledeploy/gitops/Makefiledeploy/gitops/README.mddeploy/gitops/bootstrap/argo-rbac.yaml.templatedeploy/gitops/bootstrap/local/cert-manager-values.yamldeploy/gitops/bootstrap/local/ingress-nginx-values.yamldeploy/gitops/bootstrap/local/sealed-secrets-values.yamldeploy/gitops/bootstrap/local/selfsigned-issuer.yamldeploy/gitops/environments/local/inventory.yaml.templatedeploy/gitops/environments/local/sealed-secrets/insight-infra/clickhouse-creds-sealedsecret.yaml.templatedeploy/gitops/environments/local/sealed-secrets/insight-infra/mariadb-creds-sealedsecret.yaml.templatedeploy/gitops/environments/local/sealed-secrets/insight-infra/redis-creds-sealedsecret.yaml.templatedeploy/gitops/environments/local/sealed-secrets/insight/insight-db-creds-sealedsecret.yaml.templatedeploy/gitops/environments/local/sealed-secrets/insight/insight-oidc-sealedsecret.yaml.templatedeploy/gitops/environments/local/values.yamldeploy/gitops/scripts/airbyte-setup.shdeploy/gitops/scripts/compose-app-secrets.shdeploy/gitops/scripts/doctor.shdeploy/gitops/scripts/push-deploy-log.shdeploy/gitops/scripts/render-diff.shdeploy/gitops/scripts/secret-fetch.shdeploy/gitops/secrets-store.yaml.templatedeploy/gitops/system/README.mddeploy/gitops/system/airbyte/values.yamldeploy/gitops/system/alloy/values.yamldeploy/gitops/system/argo-workflows/values.yamldeploy/gitops/system/clickhouse/SECRETS.mddeploy/gitops/system/clickhouse/values.yamldeploy/gitops/system/grafana/values.yamldeploy/gitops/system/loki/values.yamldeploy/gitops/system/mariadb/SECRETS.mddeploy/gitops/system/mariadb/values.yamldeploy/gitops/system/redis/SECRETS.mddeploy/gitops/system/redis/values.yamldeploy/gitops/system/redpanda-console/values.yamldeploy/gitops/system/redpanda/values.yamldev-compose.shdocs/deploy/environments/local/pub-cert.pem.sample
💤 Files with no reviewable changes (1)
- docs/deploy/environments/local/pub-cert.pem.sample
cyberantonz
force-pushed
the
shared-init-wizard
branch
8 times, most recently
from
June 17, 2026 17:07
ddf3085 to
ffe56ac
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
deploy/gitops/README.md (1)
279-280:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix likely broken ADR link path.
Line 279 points to
../components/deployment/..., which appears inconsistent with the repo’s docs path used earlier in this file (../../docs/components/deployment/...), and is likely a stale link after the restructure.Proposed fix
-[`../components/deployment/specs/ADR/0001-chart-publishing-on-merge.md`](../components/deployment/specs/ADR/0001-chart-publishing-on-merge.md) +[`../../docs/components/deployment/specs/ADR/0001-chart-publishing-on-merge.md`](../../docs/components/deployment/specs/ADR/0001-chart-publishing-on-merge.md)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/gitops/README.md` around lines 279 - 280, The markdown link reference in the README.md file uses an inconsistent relative path `../components/deployment/specs/ADR/0001-chart-publishing-on-merge.md` that does not match the documentation path pattern established elsewhere in the file. Update this link path to use `../../docs/components/deployment/specs/ADR/0001-chart-publishing-on-merge.md` to ensure consistency with other documentation references and fix the broken link after the repository restructure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@compose/insight-init.sh`:
- Around line 542-545: The unquoted $available variable on lines 543 and 545
causes word-splitting issues when context names contain whitespace. Quote the
$available variable in both locations: change printf ' - %s\n' $available >&2
to printf ' - %s\n' "$available" >&2 on line 543, and change printf '%s\n'
$available to printf '%s\n' "$available" in the grep pipe on line 545. This
preserves the newline-separated list format while preventing shell
word-splitting on spaces within context names.
- Around line 120-130: The update_env_var function uses pipe (|) as the
delimiter in the sed command on line 125, but when the value contains a pipe
character, it gets escaped as \| which doesn't work properly in sed's
replacement string since backslash doesn't escape the delimiter there. Change
the sed delimiter from | to a character unlikely to appear in environment
variable values, such as # or /, and update all three occurrences of the
delimiter in the substitution pattern to match the new delimiter consistently.
- Around line 53-54: The issue is that when `--target` is the last argument
without a value, the `shift 2` command fails immediately because there is no
second argument to shift, causing an unhelpful error before the proper
validation message on line 62 can be displayed. In the second case handling
(`--target` without `=`), modify the shift operation to safely handle the case
where there is no second argument. Either check if a second argument exists
before shifting two positions, or use conditional shifting that only shifts the
available arguments. This ensures the script reaches the error validation that
checks if TARGET is empty rather than failing on the shift command itself.
In `@deploy/gitops/README.md`:
- Line 22: The fenced code block starting on line 22 is missing a language
identifier after the opening triple backticks, which violates markdownlint rule
MD040 and limits renderer support. Add the language identifier "text" to the
opening fence by changing the plain ``` to ```text to properly specify that the
code block contains plain text content.
---
Outside diff comments:
In `@deploy/gitops/README.md`:
- Around line 279-280: The markdown link reference in the README.md file uses an
inconsistent relative path
`../components/deployment/specs/ADR/0001-chart-publishing-on-merge.md` that does
not match the documentation path pattern established elsewhere in the file.
Update this link path to use
`../../docs/components/deployment/specs/ADR/0001-chart-publishing-on-merge.md`
to ensure consistency with other documentation references and fix the broken
link after the repository restructure.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: cc65e73f-7e3e-448a-9d6d-3674c9d3e51e
📥 Commits
Reviewing files that changed from the base of the PR and between 4e10e21921b152aa6e4d9ffbfe092e4a9aafa781 and ddf3085a0ddd1edd145a5401e48e8eeba04ed1cc.
📒 Files selected for processing (45)
CONTRIBUTING.mdcharts/insight/values.yamlcompose/insight-init.shdeploy/gitops/.gitignoredeploy/gitops/.gitleaks.tomldeploy/gitops/.insight-versiondeploy/gitops/Brewfiledeploy/gitops/Makefiledeploy/gitops/README.mddeploy/gitops/bootstrap/argo-rbac.yaml.templatedeploy/gitops/bootstrap/local/cert-manager-values.yamldeploy/gitops/bootstrap/local/ingress-nginx-values.yamldeploy/gitops/bootstrap/local/sealed-secrets-values.yamldeploy/gitops/bootstrap/local/selfsigned-issuer.yamldeploy/gitops/environments/local/inventory.yaml.templatedeploy/gitops/environments/local/sealed-secrets/insight-infra/clickhouse-creds-sealedsecret.yaml.templatedeploy/gitops/environments/local/sealed-secrets/insight-infra/mariadb-creds-sealedsecret.yaml.templatedeploy/gitops/environments/local/sealed-secrets/insight-infra/redis-creds-sealedsecret.yaml.templatedeploy/gitops/environments/local/sealed-secrets/insight/insight-db-creds-sealedsecret.yaml.templatedeploy/gitops/environments/local/sealed-secrets/insight/insight-oidc-sealedsecret.yaml.templatedeploy/gitops/environments/local/values.yamldeploy/gitops/scripts/airbyte-setup.shdeploy/gitops/scripts/compose-app-secrets.shdeploy/gitops/scripts/doctor.shdeploy/gitops/scripts/push-deploy-log.shdeploy/gitops/scripts/render-diff.shdeploy/gitops/scripts/secret-fetch.shdeploy/gitops/secrets-store.yaml.templatedeploy/gitops/system/README.mddeploy/gitops/system/airbyte/values.yamldeploy/gitops/system/alloy/values.yamldeploy/gitops/system/argo-workflows/values.yamldeploy/gitops/system/clickhouse/SECRETS.mddeploy/gitops/system/clickhouse/values.yamldeploy/gitops/system/grafana/values.yamldeploy/gitops/system/loki/values.yamldeploy/gitops/system/mariadb/SECRETS.mddeploy/gitops/system/mariadb/values.yamldeploy/gitops/system/redis/SECRETS.mddeploy/gitops/system/redis/values.yamldeploy/gitops/system/redpanda-console/values.yamldeploy/gitops/system/redpanda/values.yamldev-compose.shdocs/deploy/.gitignoredocs/deploy/environments/local/pub-cert.pem.sample
💤 Files with no reviewable changes (2)
- docs/deploy/.gitignore
- docs/deploy/environments/local/pub-cert.pem.sample
✅ Files skipped from review due to trivial changes (8)
- deploy/gitops/environments/local/sealed-secrets/insight-infra/redis-creds-sealedsecret.yaml.template
- deploy/gitops/scripts/secret-fetch.sh
- deploy/gitops/.gitignore
- deploy/gitops/environments/local/sealed-secrets/insight/insight-oidc-sealedsecret.yaml.template
- charts/insight/values.yaml
- deploy/gitops/environments/local/sealed-secrets/insight/insight-db-creds-sealedsecret.yaml.template
- deploy/gitops/environments/local/sealed-secrets/insight-infra/clickhouse-creds-sealedsecret.yaml.template
- CONTRIBUTING.md
🚧 Files skipped from review as they are similar to previous changes (1)
- dev-compose.sh
cyberantonz
force-pushed
the
shared-init-wizard
branch
2 times, most recently
from
June 18, 2026 10:01
a8a713d to
f920a3f
Compare
mitasovr
approved these changes
Jun 18, 2026
cyberantonz
force-pushed
the
shared-init-wizard
branch
from
June 18, 2026 13:50
f920a3f to
972d5f0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
deploy/gitops/README.md (1)
22-22:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language to the fenced directory-tree block.
Line [22] should specify a fence language to satisfy markdownlint MD040.
Suggested fix
-``` +```text deploy/gitops/ ...</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@deploy/gitops/README.mdat line 22, The fenced code block on line 22 in
deploy/gitops/README.md is missing a language specification, which violates
markdownlint rule MD040. Add the language identifier "text" to the opening fence
by changing the bare triple backticks () totext so the code block is
properly formatted and complies with the linting rule.</details> <!-- cr-comment:v1:ebcf1e1701f348aead31f5ee --> _Source: Linters/SAST tools_ </blockquote></details> <details> <summary>compose/insight-init.sh (6)</summary><blockquote> `650-656`: _⚠️ Potential issue_ | _🟠 Major_ | _⚡ Quick win_ **Shell-escape `.env.local` values before writing.** Lines [654]-[655] write raw user input. When sourced later, spaces/special chars can break parsing, and command substitutions may execute. <details> <summary>Suggested fix</summary> ```diff - cat > "$env_local_out" <<EOF -# Generated by compose/insight-init.sh on first \`make deploy ENV=local\`. -# Sourced by the local-chain target before invoking \`make system-airbyte\`. -# Gitignored; safe to delete and regenerate. -AIRBYTE_SETUP_EMAIL=$airbyte_email -AIRBYTE_SETUP_ORG=$airbyte_org -EOF + { + echo "# Generated by compose/insight-init.sh on first \`make deploy ENV=local\`." + echo "# Sourced by the local-chain target before invoking \`make system-airbyte\`." + echo "# Gitignored; safe to delete and regenerate." + printf 'AIRBYTE_SETUP_EMAIL=%q\n' "$airbyte_email" + printf 'AIRBYTE_SETUP_ORG=%q\n' "$airbyte_org" + } > "$env_local_out" ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compose/insight-init.sh` around lines 650 - 656, The variables airbyte_email and airbyte_org are written directly to the env_local_out file without proper shell escaping in the heredoc. When this file is sourced later, unescaped values containing spaces, special characters, or command substitutions could break parsing or execute unintended commands. Wrap the variable assignments AIRBYTE_SETUP_EMAIL and AIRBYTE_SETUP_ORG with proper quoting (typically single quotes or escaped double quotes) to safely handle any special characters or spaces in the input values while preventing command substitution. ``` </details> <!-- cr-comment:v1:17228cac6843c54ce207be34 --> --- `543-545`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Quote kube-context list handling to avoid splitting context names.** Lines [543] and [545] expand `$available` unquoted. Context names with spaces are split and validated incorrectly. <details> <summary>Suggested fix</summary> ```diff - printf ' - %s\n' $available >&2 + printf '%s\n' "$available" | while IFS= read -r ctx; do + printf ' - %s\n' "$ctx" >&2 + done @@ - if ! printf '%s\n' $available | grep -qFx -- "$kube_ctx"; then + if ! printf '%s\n' "$available" | grep -qFx -- "$kube_ctx"; then ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compose/insight-init.sh` around lines 543 - 545, The variables containing Kubernetes context names are expanded unquoted in the printf statements at lines 543 and 545, which causes context names with spaces to be incorrectly split during validation. Quote the $available variable in both the printf command on line 543 and in the grep pipe on line 545 by wrapping it with double quotes to preserve context names with spaces as single values. ``` </details> <!-- cr-comment:v1:0699cdb1a30d9fc661f7eb3c --> _Source: Linters/SAST tools_ --- `747-753`: _⚠️ Potential issue_ | _🟠 Major_ | _⚡ Quick win_ **Always sync `values.yaml` tenant ID on wizard runs.** Lines [747]-[753] only update `.global.tenantDefaultId` when non-sentinel. Re-running after switching back to local DB can leave stale tenant configuration. <details> <summary>Suggested fix</summary> ```diff - if [[ "$TENANT_DEFAULT_ID" != "$TENANT_DEFAULT_UUID" ]]; then - local values_file="$gitops_dir/environments/local/values.yaml" - if [[ -f "$values_file" ]]; then - yq -i ".global.tenantDefaultId = \"$TENANT_DEFAULT_ID\"" "$values_file" - echo "Updated .global.tenantDefaultId in $values_file." >&2 - fi - fi + local values_file="$gitops_dir/environments/local/values.yaml" + if [[ -f "$values_file" ]]; then + yq -i ".global.tenantDefaultId = \"$TENANT_DEFAULT_ID\"" "$values_file" + echo "Updated .global.tenantDefaultId in $values_file." >&2 + fi ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compose/insight-init.sh` around lines 747 - 753, The conditional check in the values.yaml update block for `.global.tenantDefaultId` only performs the update when TENANT_DEFAULT_ID is different from TENANT_DEFAULT_UUID, which can leave stale tenant configuration when re-running the wizard. Remove the outer conditional check that compares the two tenant ID values and ensure the yq command always executes to update .global.tenantDefaultId in the values_file whenever the file exists, guaranteeing the tenant ID stays synchronized across all wizard runs. ``` </details> <!-- cr-comment:v1:0efc6d27effffa6dff0770f0 --> --- `54-54`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Handle missing `--target` value without early shell failure.** Line [54] uses `shift 2`; with `set -e`, `insight-init.sh --target` exits before the explicit `--target is required` validation. <details> <summary>Suggested fix</summary> ```diff - --target) TARGET="${2:-}"; shift 2 ;; + --target) + TARGET="${2:-}" + shift + [[ $# -gt 0 ]] && shift + ;; ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compose/insight-init.sh` at line 54, The issue is that when `--target` is provided without a value at the end of the command line arguments, the `shift 2` operation on the `--target` case in the argument parsing section will attempt to shift more arguments than exist, causing an immediate shell failure with `set -e` before reaching the explicit validation check. Fix this by adding a check to ensure sufficient arguments exist before performing the shift operation, or by using a conditional shift pattern that gracefully handles the case where --target is the last argument without a value provided. ``` </details> <!-- cr-comment:v1:0b3cb5dbc706426360d48abd --> --- `665-717`: _⚠️ Potential issue_ | _🟠 Major_ | _⚡ Quick win_ **Escape secret strings before interpolating into YAML.** Lines [684], [685], [695], [715]-[717] inject raw credentials into quoted YAML scalars. `"` and `\` in passwords can produce invalid manifests. <details> <summary>Suggested fix</summary> ```diff + yaml_escape() { + printf '%s' "$1" | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g' + } + local mdb_root_esc mdb_pass_esc ch_pass_esc + mdb_root_esc=$(yaml_escape "$MARIADB_ROOT_PASSWORD") + mdb_pass_esc=$(yaml_escape "$MARIADB_PASSWORD") + ch_pass_esc=$(yaml_escape "$CLICKHOUSE_PASSWORD") @@ - mariadb-root-password: "$MARIADB_ROOT_PASSWORD" - mariadb-password: "$MARIADB_PASSWORD" + mariadb-root-password: "$mdb_root_esc" + mariadb-password: "$mdb_pass_esc" @@ - admin-password: "$CLICKHOUSE_PASSWORD" + admin-password: "$ch_pass_esc" @@ - mariadb-root-password: "$MARIADB_ROOT_PASSWORD" - mariadb-password: "$MARIADB_PASSWORD" - clickhouse-password: "$CLICKHOUSE_PASSWORD" + mariadb-root-password: "$mdb_root_esc" + mariadb-password: "$mdb_pass_esc" + clickhouse-password: "$ch_pass_esc" ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compose/insight-init.sh` around lines 665 - 717, The password variables (MARIADB_ROOT_PASSWORD, MARIADB_PASSWORD, CLICKHOUSE_PASSWORD) are being interpolated directly into YAML quoted strings without escaping special characters. If any password contains a backslash or double quote, it will break the YAML manifest format. Before interpolating these variables into the heredoc string, apply proper YAML escaping by replacing backslashes with double backslashes and double quotes with backslash-escaped quotes. Consider creating a helper function to escape these variables or use sed substitution to ensure all secret variables are properly escaped before being inserted into the stringData sections of the Kubernetes Secret manifests. ``` </details> <!-- cr-comment:v1:f984d7ab944ee9f506fc73ca --> --- `122-125`: _⚠️ Potential issue_ | _🟠 Major_ | _⚡ Quick win_ **`update_env_var` breaks when values contain `|`.** Line [125] uses `|` as the sed delimiter. A value containing `|` can corrupt replacement output and produce invalid `.env.compose`. <details> <summary>Suggested fix</summary> ```diff - escaped=$(printf '%s' "$value" | sed -e 's/[\\&|]/\\&/g') + escaped=$(printf '%s' "$value" | sed -e 's/[\\&#]/\\&/g') @@ - sed -E "s|^[[:space:]]*${key}=.*|${key}=${escaped}|" "$file" > "$tmp" + sed -E "s#^[[:space:]]*${key}=.*#${key}=${escaped}#" "$file" > "$tmp" ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compose/insight-init.sh` around lines 122 - 125, The sed command on line 125 uses `|` as the delimiter, but when environment variable values contain pipe characters, this breaks the sed replacement even after they are escaped. Change the sed delimiter from `|` to a different character such as `#` that is unlikely to appear in environment variable values. Apply this change to the sed command in the replacement operation where it currently reads sed -E "s|^[[:space:]]*${key}=.*|${key}=${escaped}|" to use the new delimiter consistently throughout the pattern matching and replacement text. ``` </details> <!-- cr-comment:v1:de24eb32ff3781cacddae43b --> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Inline comments:
In@deploy/gitops/Makefile:
- Around line 821-823: The
xargs -rflag used in the sealed-secrets repo
cleanup is not portable to BSD/macOS systems, causing the stale repository
removal to fail silently. Replace thexargs -rapproach with a portable shell
conditional that checks if the awk output is non-empty before attempting to
remove the repository. Store the repo name from the awk command in a variable
and use an if statement with a test for non-empty string to conditionally
execute helm repo remove, ensuring the cleanup works consistently across all
Unix-like systems including macOS.
Duplicate comments:
In@compose/insight-init.sh:
- Around line 650-656: The variables airbyte_email and airbyte_org are written
directly to the env_local_out file without proper shell escaping in the heredoc.
When this file is sourced later, unescaped values containing spaces, special
characters, or command substitutions could break parsing or execute unintended
commands. Wrap the variable assignments AIRBYTE_SETUP_EMAIL and
AIRBYTE_SETUP_ORG with proper quoting (typically single quotes or escaped double
quotes) to safely handle any special characters or spaces in the input values
while preventing command substitution.- Around line 543-545: The variables containing Kubernetes context names are
expanded unquoted in the printf statements at lines 543 and 545, which causes
context names with spaces to be incorrectly split during validation. Quote the
$available variable in both the printf command on line 543 and in the grep pipe
on line 545 by wrapping it with double quotes to preserve context names with
spaces as single values.- Around line 747-753: The conditional check in the values.yaml update block for
.global.tenantDefaultIdonly performs the update when TENANT_DEFAULT_ID is
different from TENANT_DEFAULT_UUID, which can leave stale tenant configuration
when re-running the wizard. Remove the outer conditional check that compares the
two tenant ID values and ensure the yq command always executes to update
.global.tenantDefaultId in the values_file whenever the file exists,
guaranteeing the tenant ID stays synchronized across all wizard runs.- Line 54: The issue is that when
--targetis provided without a value at the
end of the command line arguments, theshift 2operation on the--target
case in the argument parsing section will attempt to shift more arguments than
exist, causing an immediate shell failure withset -ebefore reaching the
explicit validation check. Fix this by adding a check to ensure sufficient
arguments exist before performing the shift operation, or by using a conditional
shift pattern that gracefully handles the case where --target is the last
argument without a value provided.- Around line 665-717: The password variables (MARIADB_ROOT_PASSWORD,
MARIADB_PASSWORD, CLICKHOUSE_PASSWORD) are being interpolated directly into YAML
quoted strings without escaping special characters. If any password contains a
backslash or double quote, it will break the YAML manifest format. Before
interpolating these variables into the heredoc string, apply proper YAML
escaping by replacing backslashes with double backslashes and double quotes with
backslash-escaped quotes. Consider creating a helper function to escape these
variables or use sed substitution to ensure all secret variables are properly
escaped before being inserted into the stringData sections of the Kubernetes
Secret manifests.- Around line 122-125: The sed command on line 125 uses
|as the delimiter,
but when environment variable values contain pipe characters, this breaks the
sed replacement even after they are escaped. Change the sed delimiter from|
to a different character such as#that is unlikely to appear in environment
variable values. Apply this change to the sed command in the replacement
operation where it currently reads sed -E
"s|^[[:space:]]${key}=.|${key}=${escaped}|" to use the new delimiter
consistently throughout the pattern matching and replacement text.In
@deploy/gitops/README.md:
- Line 22: The fenced code block on line 22 in deploy/gitops/README.md is
missing a language specification, which violates markdownlint rule MD040. Add
the language identifier "text" to the opening fence by changing the bare triple
backticks () totext so the code block is properly formatted and complies
with the linting rule.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `a441e60a-1640-4351-a29f-8e0b116e2bff` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between a8a713d6ded033fd921d54052fa16078aa316f22 and 972d5f0e0ca66ee78c2fbed95b581e61c55cf5d1. </details> <details> <summary>📒 Files selected for processing (45)</summary> * `CONTRIBUTING.md` * `charts/insight/values.yaml` * `compose/insight-init.sh` * `deploy/gitops/.gitignore` * `deploy/gitops/.gitleaks.toml` * `deploy/gitops/.insight-version` * `deploy/gitops/Brewfile` * `deploy/gitops/Makefile` * `deploy/gitops/README.md` * `deploy/gitops/bootstrap/argo-rbac.yaml.template` * `deploy/gitops/bootstrap/local/cert-manager-values.yaml` * `deploy/gitops/bootstrap/local/ingress-nginx-values.yaml` * `deploy/gitops/bootstrap/local/sealed-secrets-values.yaml` * `deploy/gitops/bootstrap/local/selfsigned-issuer.yaml` * `deploy/gitops/environments/local/inventory.yaml.template` * `deploy/gitops/environments/local/sealed-secrets/insight-infra/clickhouse-creds-sealedsecret.yaml.template` * `deploy/gitops/environments/local/sealed-secrets/insight-infra/mariadb-creds-sealedsecret.yaml.template` * `deploy/gitops/environments/local/sealed-secrets/insight-infra/redis-creds-sealedsecret.yaml.template` * `deploy/gitops/environments/local/sealed-secrets/insight/insight-db-creds-sealedsecret.yaml.template` * `deploy/gitops/environments/local/sealed-secrets/insight/insight-oidc-sealedsecret.yaml.template` * `deploy/gitops/environments/local/values.yaml` * `deploy/gitops/scripts/airbyte-setup.sh` * `deploy/gitops/scripts/compose-app-secrets.sh` * `deploy/gitops/scripts/doctor.sh` * `deploy/gitops/scripts/push-deploy-log.sh` * `deploy/gitops/scripts/render-diff.sh` * `deploy/gitops/scripts/secret-fetch.sh` * `deploy/gitops/secrets-store.yaml.template` * `deploy/gitops/system/README.md` * `deploy/gitops/system/airbyte/values.yaml` * `deploy/gitops/system/alloy/values.yaml` * `deploy/gitops/system/argo-workflows/values.yaml` * `deploy/gitops/system/clickhouse/SECRETS.md` * `deploy/gitops/system/clickhouse/values.yaml` * `deploy/gitops/system/grafana/values.yaml` * `deploy/gitops/system/loki/values.yaml` * `deploy/gitops/system/mariadb/SECRETS.md` * `deploy/gitops/system/mariadb/values.yaml` * `deploy/gitops/system/redis/SECRETS.md` * `deploy/gitops/system/redis/values.yaml` * `deploy/gitops/system/redpanda-console/values.yaml` * `deploy/gitops/system/redpanda/values.yaml` * `dev-compose.sh` * `docs/deploy/.gitignore` * `docs/deploy/environments/local/pub-cert.pem.sample` </details> <details> <summary>💤 Files with no reviewable changes (2)</summary> * docs/deploy/.gitignore * docs/deploy/environments/local/pub-cert.pem.sample </details> <details> <summary>✅ Files skipped from review due to trivial changes (8)</summary> * deploy/gitops/environments/local/sealed-secrets/insight/insight-db-creds-sealedsecret.yaml.template * charts/insight/values.yaml * deploy/gitops/environments/local/sealed-secrets/insight-infra/clickhouse-creds-sealedsecret.yaml.template * deploy/gitops/environments/local/sealed-secrets/insight-infra/redis-creds-sealedsecret.yaml.template * deploy/gitops/.gitignore * deploy/gitops/environments/local/sealed-secrets/insight/insight-oidc-sealedsecret.yaml.template * deploy/gitops/scripts/secret-fetch.sh * CONTRIBUTING.md </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (3)</summary> * deploy/gitops/scripts/doctor.sh * deploy/gitops/environments/local/values.yaml * dev-compose.sh </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
cyberantonz
force-pushed
the
shared-init-wizard
branch
11 times, most recently
from
June 18, 2026 16:34
f1a5887 to
dd4e678
Compare
Extract the docker-compose first-run prompts into compose/insight-init.sh, make the same wizard drive the gitops bring-up, and move the gitops tree out of docs/. - New compose/insight-init.sh with --target=compose|k8s-local. The compose target lifts the existing bootstrap_env_wizard logic verbatim; the k8s-local target adds cross-OS tooling preflight, kube-context probe + cluster-type auto-detect, L2-service toggles, and writes inventory.yaml + secrets-store.yaml + .env.local. - dev-compose.sh up's first-run branch becomes a thin exec of the shared wizard; the dead helper functions are removed. The --start-airbyte / --start-argo rejection now points at the new `cd deploy/gitops && make deploy ENV=local` path. - docs/deploy/ → deploy/gitops/ (real deployment, not a sample). Sample/tmpl suffixes normalised to .template across the tree; pub-cert.pem.sample deleted (fetched from cluster via fetch-cert). - `make deploy ENV=local` chains wizard → bootstrap → fetch-cert → seal → system → deploy-app, with .env.local sourced for airbyte setup creds. Other envs keep the narrow deploy-app behavior. - CONTRIBUTING.md "Two dev paths" + "Beyond compose" point at the new k8s flow; dev-up.sh references dropped (path is being retired). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
cyberantonz
force-pushed
the
shared-init-wizard
branch
from
June 18, 2026 16:35
dd4e678 to
91aec2b
Compare
ktursunov
approved these changes
Jun 19, 2026
This was referenced Jun 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extract the docker-compose first-run prompts into compose/insight-init.sh, make the same wizard drive the gitops bring-up, and move the gitops tree out of docs/.
New compose/insight-init.sh with --target=compose|k8s-local. The compose target lifts the existing bootstrap_env_wizard logic verbatim; the k8s-local target adds cross-OS tooling preflight, kube-context probe + cluster-type auto-detect, L2-service toggles, and writes inventory.yaml + secrets-store.yaml + .env.local.
dev-compose.sh up's first-run branch becomes a thin exec of the shared wizard; the dead helper functions are removed. The --start-airbyte / --start-argo rejection now points at the new
cd deploy/gitops && make deploy ENV=localpath.docs/deploy/ → deploy/gitops/ (real deployment, not a sample). Sample/tmpl suffixes normalised to .template across the tree; pub-cert.pem.sample deleted (fetched from cluster via fetch-cert).
make deploy ENV=localchains wizard → bootstrap → fetch-cert → seal → system → deploy-app, with .env.local sourced for airbyte setup creds. Other envs keep the narrow deploy-app behavior.CONTRIBUTING.md "Two dev paths" + "Beyond compose" point at the new k8s flow; dev-up.sh references dropped (path is being retired).
Summary by CodeRabbit
New Features
make deploy ENV=localflow that auto-bootstraps when local inventory is missing.Bug Fixes
--waitregression.Documentation
Chores