Skip to content

Autodetect kind cluster configuration#365

Merged
celdrake merged 1 commit into
flightctl:mainfrom
celdrake:auto-detect-kind-configuration
Oct 29, 2025
Merged

Autodetect kind cluster configuration#365
celdrake merged 1 commit into
flightctl:mainfrom
celdrake:auto-detect-kind-configuration

Conversation

@celdrake

@celdrake celdrake commented Oct 9, 2025

Copy link
Copy Markdown
Collaborator

Currently, the UI needs to have the ORGANIZATIONS_ENABLED environment variable that matches the backend deployment configuration for organanizations, otherwise it fails.

Now by default, the npm run dev:kind will autodetect the backend deployment settings to configure the UI accordingly.

Also made some fixes to README.md based on Coderabbit's report from markdownlint

Supported by Cursor AI.

Summary by CodeRabbit

  • New Features

    • Dev setup now validates prerequisites, autodetects cluster services and feature flags (Organizations, CLI artifacts, AlertManager), improves external-IP detection, and exits if external IP can't be determined; environment values are exported and displayed.
  • Documentation

    • Added CONFIGURATION.md listing environment variables, defaults, and examples.
    • README clarified setup/run instructions, Kind guidance, and links to the new configuration guide.
  • Chores

    • Dev script invocation updated to source the centralized setup script from package scripts.

@coderabbitai

coderabbitai Bot commented Oct 9, 2025

Copy link
Copy Markdown

Walkthrough

Adds CONFIGURATION.md, edits README.md, changes apps/standalone/package.json dev script to source ./scripts/setup_env.sh, and rewrites apps/standalone/scripts/setup_env.sh to use /bin/sh, validate prerequisites, detect external IP, probe the kind cluster for services/ConfigMap, and autodetect/export related environment variables.

Changes

Cohort / File(s) Summary of Changes
Configuration doc & README
CONFIGURATION.md, README.md
Added CONFIGURATION.md documenting environment variables, defaults, allowed values, and examples; updated README.md text/formatting and added references to CONFIGURATION.md and Kind auto-detection.
Standalone env setup script
apps/standalone/scripts/setup_env.sh
Rewritten script (now #!/bin/sh): added prerequisite validation (kubectl presence, kind context, cluster reachability), improved external IP detection, added detect_service_setting() and detect_organizations_setting() to probe cluster Services/ConfigMap, autodetection of ENABLE_ORGANIZATIONS, ENABLE_CLI_ARTIFACTS, ENABLE_ALERTMANAGER, export/unset logic including derived ORGANIZATIONS_ENABLED, and updated console output.
NPM scripts
apps/standalone/package.json
Updated dev:kind script to source the setup script before running dev: changed from inline env assignments + source ./scripts/setup_env.sh to . ./scripts/setup_env.sh && npm run dev.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant NPM as package.json (dev:kind)
  participant SH as setup_env.sh
  participant K8s as Kind Cluster (kubectl)
  participant Env as Process Environment

  Dev->>NPM: npm run dev:kind
  NPM->>SH: . ./scripts/setup_env.sh
  SH->>K8s: validate_prerequisites (kubectl, kind context, cluster reachable)
  alt prerequisites OK
    SH->>SH: get_external_ip()
    alt IP resolved
      SH->>K8s: detect_organizations_setting() (read ConfigMap)
      SH->>K8s: detect_service_setting(flightctl-cli-artifacts)
      SH->>K8s: detect_service_setting(flightctl-alertmanager-proxy)
      SH->>Env: export/unset ENABLE_* and set ORGANIZATIONS_ENABLED
      SH->>Dev: print environment summary
      NPM->>NPM: npm run dev
    else no IP
      SH-->>Dev: exit (missing external IP)
    end
  else prerequisites fail
    SH-->>Dev: exit (prereq check failed)
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45–60 minutes

  • Pay attention to apps/standalone/scripts/setup_env.sh: shell portability (/bin/sh), command substitutions, kubectl exit handling, and OS-specific IP detection branches.
  • Verify environment export/unset behavior and derived ORGANIZATIONS_ENABLED logic.
  • Confirm package.json script change semantics in typical shells (sourcing vs. environment-inline).

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "Autodetect kind cluster configuration" accurately reflects the primary change in the changeset. The core functionality introduced is the autodetection logic in setup_env.sh that detects cluster settings (ENABLE_ORGANIZATIONS, ENABLE_CLI_ARTIFACTS, ENABLE_ALERTMANAGER) from the kind cluster at runtime, which eliminates the need for manual environment variable configuration. While the PR includes supporting changes such as the new CONFIGURATION.md file and README.md updates, the title appropriately captures the main objective. The title is concise, specific, and clearly conveys the primary change without vague language or noise, making it easy for teammates reviewing the commit history to understand the purpose of this PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
README.md (1)

11-12: Align heading levels with markdownlint

### Prerequisites jumps directly from # to ###, triggering markdownlint (MD001). Promote it to ## (or insert a ## section above) to keep heading increments by one.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a676260 and 4940691.

📒 Files selected for processing (4)
  • CONFIGURATION.md (1 hunks)
  • README.md (2 hunks)
  • apps/standalone/package.json (1 hunks)
  • apps/standalone/scripts/setup_env.sh (2 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md

5-5: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: integration-tests
  • GitHub Check: Build ocp plugin
  • GitHub Check: Build
  • GitHub Check: Lint

Comment thread apps/standalone/package.json Outdated
@celdrake celdrake force-pushed the auto-detect-kind-configuration branch from 4940691 to ebe2c05 Compare October 9, 2025 09:54

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
CONFIGURATION.md (1)

5-42: Fix markdown heading hierarchy and code fence languages.

markdownlint flags the jump from # to ###, and the shell examples lack language identifiers. Please bump these section headings to ## and annotate the fenced blocks with bash (or shell) so docs pass lint.

README.md (1)

21-48: Add shell language tags to fenced command blocks.

markdownlint (MD040) requires a language specifier; the new command examples should use bash (or shell) to satisfy lint and improve highlighting.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4940691 and ebe2c05.

📒 Files selected for processing (4)
  • CONFIGURATION.md (1 hunks)
  • README.md (2 hunks)
  • apps/standalone/package.json (1 hunks)
  • apps/standalone/scripts/setup_env.sh (2 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md

5-5: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)


13-13: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


24-24: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


34-34: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

CONFIGURATION.md

5-5: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: integration-tests
  • GitHub Check: Build
  • GitHub Check: Build ocp plugin
  • GitHub Check: Lint

Comment thread apps/standalone/scripts/setup_env.sh
@celdrake celdrake force-pushed the auto-detect-kind-configuration branch 2 times, most recently from f09b571 to a630fa6 Compare October 9, 2025 10:03

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
CONFIGURATION.md (1)

5-12: Fix heading level progression

Heading levels jump from # to ###, tripping markdownlint (MD001) and hurting screen-reader navigation. Promote these sections to ## to keep the hierarchy intact.

-### Feature toggles
+## Feature toggles
@@
-### Backend configuration
+## Backend configuration
@@
-### Configuration examples
+## Configuration examples
README.md (1)

24-36: Add language hints to fenced blocks

markdownlint (MD040) still complains because these code fences lack a language. Tag them as bash to quiet the linter and improve highlighting.

-```
+```bash
 npm run dev:kind
@@
-```
+```bash
 FLIGHTCTL_SERVER=<api_server_url> npm run dev
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f09b571 and a630fa6.

📒 Files selected for processing (4)
  • CONFIGURATION.md (1 hunks)
  • README.md (2 hunks)
  • apps/standalone/package.json (1 hunks)
  • apps/standalone/scripts/setup_env.sh (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/standalone/package.json
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md

5-5: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)


13-13: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


24-24: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


34-34: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

CONFIGURATION.md

5-5: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)


13-13: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


24-24: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


34-34: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: integration-tests
  • GitHub Check: Build ocp plugin
  • GitHub Check: Build
  • GitHub Check: Lint

@celdrake celdrake force-pushed the auto-detect-kind-configuration branch 3 times, most recently from 1003497 to 78a93fc Compare October 10, 2025 12:59

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/standalone/scripts/setup_env.sh (1)

25-31: Fix non-POSIX/fragile IP detection (grep -P, which) to avoid failures on many systems

grep -oP requires GNU grep with PCRE and often isn’t available; which is non-POSIX. This can cause EXTERNAL_IP to be empty and abort the flow. Use command -v and awk instead; also quote the INTERFACE in the macOS path.

Apply:

 get_external_ip() {
-    if which ip 2>/dev/null 1>/dev/null; then
-        ip route get 1.1.1.1 | grep -oP 'src \K\S+'
+    if command -v ip >/dev/null 2>&1; then
+        # Extract the IP after the 'src' token using awk (portable)
+        ip route get 1.1.1.1 2>/dev/null | awk '
+          /src/ {
+            for (i=1;i<=NF;i++) if ($i=="src") { print $(i+1); exit }
+          }'
     else
         # MacOS does not have ip, so we use route and ifconfig instead
-        INTERFACE=$(route get 1.1.1.1 | grep interface | awk '{print $2}')
-        ifconfig | grep $INTERFACE -A 10 | grep "inet " | grep -Fv 127.0.0.1 | awk '{print $2}' | head -n 1
+        INTERFACE=$(route get 1.1.1.1 2>/dev/null | awk '/interface:/{print $2; exit}')
+        [ -n "$INTERFACE" ] && ifconfig "$INTERFACE" 2>/dev/null | awk '$1=="inet" && $2!="127.0.0.1" {print $2; exit}'
     fi
 }
🧹 Nitpick comments (4)
README.md (1)

21-26: Clarify the Kind context assumption in docs

setup_env.sh uses kubectl with a hardcoded context name kind-kind. Consider adding a short note here that the dev flow assumes a Kind cluster named kind (context kind-kind), or document how to override it if customized.

Reference: apps/standalone/scripts/setup_env.sh Lines 12-20 and 39, 51 use --context kind-kind.

apps/standalone/scripts/setup_env.sh (3)

3-4: Allow overriding context/namespace instead of hardcoding kind-kind and improve dev ergonomics

Hardcoding --context kind-kind and flightctl-external reduces portability. Allow overrides via env vars with sensible defaults, and use them consistently.

Apply:

- flightctl_namespace="flightctl-external"
+ # Allow overrides; defaults match current dev assumptions
+ KUBECONTEXT="${KUBECONTEXT:-kind-kind}"
+ flightctl_namespace="${FLIGHTCTL_NAMESPACE:-flightctl-external}"
@@
-    if ! kubectl config get-contexts kind-kind >/dev/null 2>&1; then
+    if ! kubectl config get-contexts "$KUBECONTEXT" >/dev/null 2>&1; then
         echo "❌ Error: kind-kind context not found. Please deploy flightctl first." >&2
         exit 1
     fi
@@
-    if ! kubectl get namespaces --context kind-kind >/dev/null 2>&1; then
+    if ! kubectl get namespaces --context "$KUBECONTEXT" >/dev/null 2>&1; then
         echo "❌ Error: Cannot connect to kind cluster. Please check your setup." >&2
         exit 1
     fi
@@
-    if kubectl get service "$service_name" -n "$flightctl_namespace" --context kind-kind >/dev/null 2>&1; then
+    if kubectl get service "$service_name" -n "$flightctl_namespace" --context "$KUBECONTEXT" >/dev/null 2>&1; then
@@
-    SETTING_VALUE=$(kubectl get configmap flightctl-api-config -n "$flightctl_namespace" --context kind-kind -o jsonpath='{.data.config\.yaml}' 2>/dev/null | grep -A1 "organizations:" | grep "enabled:" | awk '{print $2}' | tr -d ' ')
+    SETTING_VALUE=$(kubectl get configmap flightctl-api-config -n "$flightctl_namespace" --context "$KUBECONTEXT" -o jsonpath='{.data.config\.yaml}' 2>/dev/null | grep -A1 "organizations:" | grep "enabled:" | awk '{print $2}' | tr -d ' ')

Additionally, consider documenting KUBECONTEXT and FLIGHTCTL_NAMESPACE in CONFIGURATION.md. Based on learnings

Also applies to: 12-20, 39-40, 51-52


7-15: Avoid exiting the parent shell when sourced; use return-or-exit pattern

The script is sourced via . ./scripts/setup_env.sh. Using exit aborts the npm shell entirely and can surprise interactive usage. Prefer returning when sourced and exiting otherwise.

Apply:

-        echo "❌ Error: kubectl not found. " >&2
-        exit 1
+        echo "❌ Error: kubectl not found. " >&2
+        return 1 2>/dev/null || exit 1
@@
-        echo "❌ Error: kind-kind context not found. Please deploy flightctl first." >&2
-        exit 1
+        echo "❌ Error: kind-kind context not found. Please deploy flightctl first." >&2
+        return 1 2>/dev/null || exit 1
@@
-        echo "❌ Error: Cannot connect to kind cluster. Please check your setup." >&2
-        exit 1
+        echo "❌ Error: Cannot connect to kind cluster. Please check your setup." >&2
+        return 1 2>/dev/null || exit 1

Also applies to: 17-20


51-63: YAML parsing is brittle; make orgs detection more robust without extra deps

The grep -A1 chain may fail with formatting changes. Use awk to track the organizations: block and read enabled: reliably. Keeps POSIX and avoids external deps.

Apply:

-    SETTING_VALUE=$(kubectl get configmap flightctl-api-config -n "$flightctl_namespace" --context "$KUBECONTEXT" -o jsonpath='{.data.config\.yaml}' 2>/dev/null | grep -A1 "organizations:" | grep "enabled:" | awk '{print $2}' | tr -d ' ')
+    SETTING_VALUE=$(
+      kubectl get configmap flightctl-api-config -n "$flightctl_namespace" --context "$KUBECONTEXT" \
+        -o jsonpath='{.data.config\.yaml}' 2>/dev/null \
+      | awk '
+          $0 ~ /^[[:space:]]*organizations:[[:space:]]*$/ { in_org=1; next }
+          in_org && $0 ~ /^[^[:space:]]/ { in_org=0 }           # next top-level key
+          in_org && $1 ~ /enabled:/ {
+            # handle forms: "enabled: true" or "enabled:true"
+            for (i=1;i<=NF;i++) if ($i ~ /enabled:/) {
+              val=$i; sub(/enabled:/,"",val); if (val=="") val=$(i+1);
+              gsub(/[[:space:]]/,"",val); print val; exit
+            }
+          }
+        '
+      | tr -d '"' \
+    )

Note: defaults below still treat empty as false, which is fine for dev.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a630fa6 and 78a93fc.

📒 Files selected for processing (4)
  • CONFIGURATION.md (1 hunks)
  • README.md (2 hunks)
  • apps/standalone/package.json (1 hunks)
  • apps/standalone/scripts/setup_env.sh (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • CONFIGURATION.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: integration-tests
  • GitHub Check: Lint
🔇 Additional comments (3)
README.md (1)

5-8: Docs improvements LGTM

The restructure and configuration references read well and align with the new autodetect behavior.

Also applies to: 11-17, 28-29, 32-36, 38-42, 45-47, 48-48, 57-59, 65-65

apps/standalone/scripts/setup_env.sh (1)

118-125: Nice: stdout/stderr separation and clear final output

Good practice echoing user-facing logs to stderr while returning machine-readable values via stdout. Final summary is helpful.

apps/standalone/package.json (1)

14-15: dev:kind sourcing fix LGTM

Using the POSIX . builtin ensures compatibility with npm’s /bin/sh. Autodetect via setup_env.sh is clean.

@rawagner rawagner left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

very nice. Thank you!

@celdrake celdrake force-pushed the auto-detect-kind-configuration branch from 78a93fc to 36f1089 Compare October 29, 2025 13:20

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
apps/standalone/scripts/setup_env.sh (1)

24-32: Minor: Use idiomatic redirect order in the shell condition.

Line 25 uses 2>/dev/null 1>/dev/null, which is functionally correct but non-idiomatic. Prefer >/dev/null 2>&1 for clarity and consistency with shell scripting conventions.

-    if which ip 2>/dev/null 1>/dev/null; then
+    if which ip >/dev/null 2>&1; then
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 78a93fc and 36f1089.

📒 Files selected for processing (4)
  • CONFIGURATION.md (1 hunks)
  • README.md (2 hunks)
  • apps/standalone/package.json (1 hunks)
  • apps/standalone/scripts/setup_env.sh (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • README.md
  • apps/standalone/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: integration-tests
  • GitHub Check: Build
  • GitHub Check: Build ocp plugin
  • GitHub Check: Lint
🔇 Additional comments (5)
CONFIGURATION.md (2)

1-3: Clear header and introduction. The document purpose is immediately evident.


32-49: Practical and progressive configuration examples.

The three examples effectively demonstrate the key workflow: basic autodetection, selective overrides, and remote backend configuration. The progression from simple to complex clearly shows the primary use case (autodetection via npm run dev:kind), which aligns well with the PR objectives.

apps/standalone/scripts/setup_env.sh (3)

1-3: Correct POSIX shebang and good namespace centralization.

The change to #!/bin/sh makes the script POSIX-compliant, fixing the /bin/sh (dash) compatibility issue with bash-only constructs. The flightctl_namespace variable reduces duplication.


6-21: Comprehensive prerequisite validation.

The checks ensure kubectl exists, the kind-kind context is present, and the cluster is reachable before attempting autodetection. Good defensive programming.


85-116: Well-structured autodetection with sensible fallbacks.

The pattern of checking for preset environment variables before calling autodetection functions is clean and allows manual override when needed. The explicit unset of disabled service URLs (lines 104, 115) ensures no stale values persist. The summary output clearly shows which features are enabled/disabled.

Comment on lines +35 to +47
detect_service_setting() {
setting_name="$1"
service_name="$2"

if kubectl get service "$service_name" -n "$flightctl_namespace" --context kind-kind >/dev/null 2>&1; then
echo "Autodetected: $setting_name enabled ✅" >&2
echo "true"
else
echo "Autodetected: $setting_name disabled ❌" >&2
echo "false"
fi
return 0
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Better distinguish between "service not found" and kubectl errors.

Line 39 silently suppresses all error output from kubectl get service, which means unexpected failures (permission denied, network issues, invalid context) are indistinguishable from a legitimate "service not found" result. This could mask configuration problems and cause the UI to degrade silently.

Consider capturing the exit code to differentiate between actual "not found" (exit 1 with "NotFound" message) and other errors that should be surfaced to the user.

-detect_service_setting() {
-    setting_name="$1"
-    service_name="$2"
-    
-    if kubectl get service "$service_name" -n "$flightctl_namespace" --context kind-kind >/dev/null 2>&1; then
+detect_service_setting() {
+    setting_name="$1"
+    service_name="$2"
+    
+    if kubectl get service "$service_name" -n "$flightctl_namespace" --context kind-kind >/dev/null 2>&1; then
         echo "Autodetected: $setting_name enabled ✅" >&2
         echo "true"
     else
         echo "Autodetected: $setting_name disabled ❌" >&2
         echo "false"
     fi

Alternatively, if the current behavior is acceptable for development, add a comment explaining why all errors are treated as "not found".

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +50 to +65
detect_organizations_setting() {
SETTING_VALUE=$(kubectl get configmap flightctl-api-config -n "$flightctl_namespace" --context kind-kind -o jsonpath='{.data.config\.yaml}' 2>/dev/null | grep -A1 "organizations:" | grep "enabled:" | awk '{print $2}' | tr -d ' ')

if [ -n "$SETTING_VALUE" ]; then
if [ "$SETTING_VALUE" = "true" ]; then
echo "Autodetected: Organizations enabled ✅" >&2
else
echo "Autodetected: Organizations disabled ❌" >&2
fi
echo "$SETTING_VALUE"
else
echo "Autodetected: Organizations disabled ❌ (no setting found in ConfigMap)" >&2
echo "false"
fi
return 0
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Strengthen the ConfigMap YAML parsing with defensive coding.

Line 51 uses a fragile grep chain that assumes a specific YAML format. If the ConfigMap structure varies (different spacing, comments, format changes), the parsing silently fails and defaults to "false", potentially disabling organizations features even when enabled in the backend.

The pattern also assumes enabled: is always on the line immediately after organizations:, which is brittle YAML parsing.

Consider using a more robust approach. If yq is available in the environment, use it for reliable YAML querying:

-detect_organizations_setting() {
-    SETTING_VALUE=$(kubectl get configmap flightctl-api-config -n "$flightctl_namespace" --context kind-kind -o jsonpath='{.data.config\.yaml}' 2>/dev/null | grep -A1 "organizations:" | grep "enabled:" | awk '{print $2}' | tr -d ' ')
+detect_organizations_setting() {
+    SETTING_VALUE=$(kubectl get configmap flightctl-api-config -n "$flightctl_namespace" --context kind-kind -o jsonpath='{.data.config\.yaml}' 2>/dev/null | grep -oP 'organizations:\s+enabled:\s+\K(true|false)' | head -n1)
     
     if [ -n "$SETTING_VALUE" ]; then
         if [ "$SETTING_VALUE" = "true" ]; then
             echo "Autodetected: Organizations enabled ✅" >&2
         else
             echo "Autodetected: Organizations disabled ❌" >&2
         fi
         echo "$SETTING_VALUE"
     else
         echo "Autodetected: Organizations disabled ❌ (no setting found in ConfigMap)" >&2
         echo "false"
     fi
     return 0
 }

Alternatively, use yq if available for authoritative YAML parsing, or add logging to help debug parsing failures.

Committable suggestion skipped: line range outside the PR's diff.

Comment thread CONFIGURATION.md
Comment on lines +5 to +30
## Feature toggles

| Variable | Description | Default | Values |
| ---------------------- | ----------------------------------------- | ------- | --------------- |
| `ENABLE_ORGANIZATIONS` | Enable/disable organizations support | `false` | `true`, `false` |
| `ENABLE_CLI_ARTIFACTS` | Enable/disable CLI download functionality | `true` | `true`, `false` |
| `ENABLE_ALERTMANAGER` | Enable/disable alerts functionality | `true` | `true`, `false` |

## Backend configuration

| Variable | Description | Default | Values |
| --------------------------------------- | ------------------------------------ | ------------------------ | -------------------------------------- |
| `BASE_UI_URL` | Base URL for UI application | `http://localhost:9000` | `https://ui.flightctl.example.com` |
| `FLIGHTCTL_SERVER` | Flight Control API server URL | `https://localhost:3443` | `https://api.flightctl.example.com` |
| `FLIGHTCTL_SERVER_INSECURE_SKIP_VERIFY` | Skip backend server TLS verification | `false` | `true`, `false` |
| `FLIGHTCTL_CLI_ARTIFACTS_SERVER` | CLI artifacts server URL | `http://localhost:8090` | `https://cli.flightctl.example.com` |
| `FLIGHTCTL_ALERTMANAGER_PROXY` | AlertManager proxy server URL | `https://localhost:8443` | `https://alerts.flightctl.example.com` |
| `INTERNAL_AUTH_URL` | Internal authentication URL | _(empty)_ | `https://auth.internal.example.com` |
| `AUTH_INSECURE_SKIP_VERIFY` | Skip auth server TLS verification | `false` | `true`, `false` |
| `AUTH_CLIENT_ID` | OAuth client ID for authentication | `flightctl` | Custom client ID |
| `TLS_CERT` | Path to TLS certificate | _(empty)_ | `/path/to/server.crt` |
| `TLS_KEY` | Path to TLS private key | _(empty)_ | `/path/to/server.key` |
| `API_PORT` | UI proxy server port | `3001` | `8080`, `3000`, etc. |
| `K8S_RBAC_NS` | Kubernetes RBAC namespace | _(empty)_ | `flightctl` |
| `IS_OCP_PLUGIN` | Run as OpenShift Console plugin | `false` | `true`, `false` |
| `IS_RHEM` | Red Hat Enterprise Mode | _(empty)_ | `true`, `false` |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Well-structured and comprehensive variable documentation.

The tables clearly document all configuration options with descriptions, defaults, and acceptable values. Variable naming is consistent and conventions are followed throughout.

Verify that all documented variables are actually supported and autodetected by the apps/standalone/scripts/setup_env.sh script, and that all environment variables used in that script are documented here.


🏁 Script executed:

#!/bin/bash
# Description: Verify all documented variables match those exported in setup_env.sh

# Extract documented variables from CONFIGURATION.md
echo "=== Variables in CONFIGURATION.md ==="
grep -E '^\| `[A-Z_]+`' CONFIGURATION.md | sed -E 's/^\| `([A-Z_]+)`.*/\1/' | sort | uniq

# Extract exported variables from setup_env.sh
echo ""
echo "=== Variables exported in setup_env.sh ==="
grep -E 'export [A-Z_]+=' apps/standalone/scripts/setup_env.sh | sed -E 's/.*export ([A-Z_]+)=.*/\1/' | sort | uniq

# Show differences
echo ""
echo "=== Documented but not in script (check for stale/unused vars) ==="
comm -23 <(grep -E '^\| `[A-Z_]+`' CONFIGURATION.md | sed -E 's/^\| `([A-Z_]+)`.*/\1/' | sort | uniq) <(grep -E 'export [A-Z_]+=' apps/standalone/scripts/setup_env.sh | sed -E 's/.*export ([A-Z_]+)=.*/\1/' | sort | uniq)

echo ""
echo "=== In script but not documented (check for missing documentation) ==="
comm -13 <(grep -E '^\| `[A-Z_]+`' CONFIGURATION.md | sed -E 's/^\| `([A-Z_]+)`.*/\1/' | sort | uniq) <(grep -E 'export [A-Z_]+=' apps/standalone/scripts/setup_env.sh | sed -E 's/.*export ([A-Z_]+)=.*/\1/' | sort | uniq)

Length of output: 742



🏁 Script executed:

#!/bin/bash
# Extract documented variables from CONFIGURATION.md
echo "=== Documented variables (from CONFIGURATION.md) ==="
doc_vars=$(grep -E '^\| `[A-Z_]+`' CONFIGURATION.md | sed -E 's/^\| `([A-Z_]+)`.*/\1/')
echo "$doc_vars" | sort

# Extract all variable references in setup_env.sh (exports, assignments, references)
echo ""
echo "=== Variables in setup_env.sh (all patterns) ==="
script_vars=$(grep -oE '[A-Z_]+(?==|})' apps/standalone/scripts/setup_env.sh | sort | uniq)
echo "$script_vars" | sort

# Check for specific variable name matches
echo ""
echo "=== Checking specific variables ==="
echo "ENABLE_ORGANIZATIONS in docs: $(echo "$doc_vars" | grep -c 'ENABLE_ORGANIZATIONS')"
echo "ORGANIZATIONS_ENABLED in script: $(grep -c 'ORGANIZATIONS_ENABLED' apps/standalone/scripts/setup_env.sh)"
echo "ENABLE_ORGANIZATIONS in script: $(grep -c 'ENABLE_ORGANIZATIONS' apps/standalone/scripts/setup_env.sh)"

# Show excerpt of setup_env.sh to understand structure
echo ""
echo "=== First 50 lines of setup_env.sh ==="
head -50 apps/standalone/scripts/setup_env.sh

Length of output: 2360



🏁 Script executed:

#!/bin/bash
# Get complete setup_env.sh
echo "=== Complete setup_env.sh ==="
cat apps/standalone/scripts/setup_env.sh

Length of output: 4501


Missing documentation for exported variable and scope mismatch between setup script and configuration documentation.

The verification revealed critical discrepancies:

  1. Undocumented variable: The setup script exports ORGANIZATIONS_ENABLED (line exporting it based on ENABLE_ORGANIZATIONS), but this variable is not documented in CONFIGURATION.md. Add it to the Feature toggles table.

  2. Incomplete coverage: Nine documented variables are not set by setup_env.sh: BASE_UI_URL, INTERNAL_AUTH_URL, AUTH_INSECURE_SKIP_VERIFY, AUTH_CLIENT_ID, TLS_CERT, TLS_KEY, API_PORT, K8S_RBAC_NS, IS_OCP_PLUGIN, IS_RHEM. Either these require separate configuration outside this script, or the CONFIGURATION.md scope needs clarification (e.g., add a note explaining which variables are auto-detected vs. manually configured).

🤖 Prompt for AI Agents
In CONFIGURATION.md around lines 5 to 30, the Feature toggles table is missing
the exported variable ORGANIZATIONS_ENABLED and the Backend configuration
section implies variables are set by the setup script when nine of them are not;
add a new row to the Feature toggles table documenting `ORGANIZATIONS_ENABLED`
(description, default and values) and add a brief note in the Backend
configuration header clarifying scope: explicitly list which variables are not
exported by setup_env.sh (BASE_UI_URL, INTERNAL_AUTH_URL,
AUTH_INSECURE_SKIP_VERIFY, AUTH_CLIENT_ID, TLS_CERT, TLS_KEY, API_PORT,
K8S_RBAC_NS, IS_OCP_PLUGIN, IS_RHEM) and state they must be configured
externally or provide defaults/exports in setup_env.sh if you intend them to be
auto-configured.

@celdrake celdrake merged commit f7b0dc7 into flightctl:main Oct 29, 2025
6 checks passed
@celdrake celdrake deleted the auto-detect-kind-configuration branch October 29, 2025 13:31
Sign up for free to 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.

2 participants