feat: prevent unconfigured API initialization and add Docker test suite

## CHANGES - Add BEDROCK_AWS_REGION requirement for Bedrock initialization - Implement IsConfigured check for Ollama API URL - Create comprehensive Docker testing environment with 6 scenarios - Add interactive test runner with shell access - Include environment files for different API configurations - Update spell checker dictionary with new test terms - Document testing workflow and expected results
2026-01-09 14:28:01 -05:00 · 2025-07-25 16:50:44 -07:00
parent 3c200e2883
commit 52246dda28
6 changed files with 399 additions and 5 deletions
--- a/.vscode/settings.json
+++ b/.vscode/settings.json
@@ -1,6 +1,7 @@
 {
 	"cSpell.words": [
 		"addextension",
+		"adduser",
 		"AIML",
 		"anthropics",
 		"badfile",
@@ -110,11 +111,13 @@
 		"talkpanel",
 		"Telos",
 		"testpattern",
+		"testuser",
 		"Thacker",
 		"tidwall",
 		"topp",
 		"ttrc",
 		"unalias",
+		"unconfigured",
 		"unmarshalling",
 		"updatepatterns",
 		"videoid",
--- a/internal/core/plugin_registry.go
+++ b/internal/core/plugin_registry.go
@@ -37,12 +37,16 @@ import (
 	"github.com/danielmiessler/fabric/internal/util"
 )

-// hasAWSCredentials checks if any AWS credentials are present either in the
-// environment variables or in the default/shared credentials file. It doesn't
-// attempt to verify the validity of the credentials, but simply ensures that a
-// potential authentication source exists so we can safely initialize the
-// Bedrock client without causing the AWS SDK to search for credentials.
+// hasAWSCredentials checks if Bedrock is properly configured by ensuring both
+// AWS credentials and BEDROCK_AWS_REGION are present. This prevents the Bedrock
+// client from being initialized when AWS credentials exist for other purposes.
 func hasAWSCredentials() bool {
+	// First check if BEDROCK_AWS_REGION is set - this is required for Bedrock
+	if os.Getenv("BEDROCK_AWS_REGION") == "" {
+		return false
+	}
+
+	// Then check if AWS credentials are available
 	if os.Getenv("AWS_PROFILE") != "" ||
 		os.Getenv("AWS_ROLE_SESSION_NAME") != "" ||
 		(os.Getenv("AWS_ACCESS_KEY_ID") != "" && os.Getenv("AWS_SECRET_ACCESS_KEY") != "") {
--- a/internal/plugins/ai/ollama/ollama.go
+++ b/internal/plugins/ai/ollama/ollama.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
+	"os"
 	"strings"
 	"time"

@@ -61,6 +62,11 @@ func (t *transport_sec) RoundTrip(req *http.Request) (*http.Response, error) {
 	return t.underlyingTransport.RoundTrip(req)
 }

+// IsConfigured returns true only if OLLAMA_API_URL environment variable is explicitly set
+func (o *Client) IsConfigured() bool {
+	return os.Getenv("OLLAMA_API_URL") != ""
+}
+
 func (o *Client) configure() (err error) {
 	if o.apiUrl, err = url.Parse(o.ApiUrl.Value); err != nil {
 		fmt.Printf("cannot parse URL: %s: %v\n", o.ApiUrl.Value, err)
--- a/scripts/docker-test/README.md
+++ b/scripts/docker-test/README.md
@@ -0,0 +1,116 @@
+# Docker Test Environment for API Configuration Fix
+
+This directory contains a Docker-based testing setup for fixing the issue where Fabric calls Ollama and Bedrock APIs even when not configured. This addresses the problem where unconfigured services show error messages during model listing.
+
+## Quick Start
+
+```bash
+# Run all tests
+./scripts/docker-test/test-runner.sh
+
+# Interactive mode - pick which test to run
+./scripts/docker-test/test-runner.sh -i
+
+# Run specific test case
+./scripts/docker-test/test-runner.sh gemini-only
+
+# Shell into test environment
+./scripts/docker-test/test-runner.sh -s gemini-only
+
+# Build image only (for development)
+./scripts/docker-test/test-runner.sh -b
+
+# Show help
+./scripts/docker-test/test-runner.sh -h
+```
+
+## Test Cases
+
+1. **no-config**: No APIs configured
+2. **gemini-only**: Only Gemini configured (reproduces original issue #1195)
+3. **openai-only**: Only OpenAI configured
+4. **ollama-only**: Only Ollama configured
+5. **bedrock-only**: Only Bedrock configured
+6. **mixed**: Multiple APIs configured (Gemini + OpenAI + Ollama)
+
+## Environment Files
+
+Each test case has a corresponding environment file in `scripts/docker-test/env/`:
+
+- `env.no-config` - Empty configuration
+- `env.gemini-only` - Only Gemini API key
+- `env.openai-only` - Only OpenAI API key
+- `env.ollama-only` - Only Ollama URL
+- `env.bedrock-only` - Only Bedrock configuration
+- `env.mixed` - Multiple API configurations
+
+These files are volume-mounted into the Docker container and persist changes made with `fabric -S`.
+
+## Interactive Mode & Shell Access
+
+The interactive mode (`-i`) provides several options:
+
+```
+Available test cases:
+
+1) No APIs configured (no-config)
+2) Only Gemini configured (gemini-only)
+3) Only OpenAI configured (openai-only)
+4) Only Ollama configured (ollama-only)
+5) Only Bedrock configured (bedrock-only)
+6) Mixed configuration (mixed)
+7) Run all tests
+0) Exit
+
+Add '!' after number to shell into test environment (e.g., '1!' to shell into no-config)
+```
+
+### Shell Mode
+
+- Use `1!`, `2!`, etc. to shell into any test environment
+- Run `fabric -S` to configure APIs interactively
+- Run `fabric --listmodels` or `fabric -L` to test model listing
+- Changes persist in the environment files
+- Type `exit` to return to test runner
+
+## Expected Results
+
+**Before Fix:**
+
+- `no-config` and `gemini-only` tests show Ollama connection errors
+- Tests show Bedrock authentication errors when BEDROCK_AWS_REGION not set
+- Error: `Ollama Get "http://localhost:11434/api/tags": dial tcp...`
+- Error: `Bedrock failed to list foundation models...`
+
+**After Fix:**
+
+- Clean output with no error messages for unconfigured services
+- Only configured services appear in model listings
+- Ollama only initialized when `OLLAMA_API_URL` is set
+- Bedrock only initialized when `BEDROCK_AWS_REGION` is set
+
+## Implementation Details
+
+- **Volume-mounted configs**: Environment files are mounted to `/home/testuser/.config/fabric/.env`
+- **Persistent state**: Configuration changes survive between test runs
+- **Single Docker image**: Built once from `scripts/docker-test/base/Dockerfile`, reused for all tests
+- **Isolated environments**: Each test uses its own environment file
+- **Cross-platform**: Works on macOS, Linux, and Windows with Docker
+
+## Development Workflow
+
+1. Make code changes to fix API initialization logic
+2. Run `./scripts/docker-test/test-runner.sh no-config` to test the main issue
+3. Use `./scripts/docker-test/test-runner.sh -i` for interactive testing
+4. Shell into environments (`1!`, `2!`, etc.) to debug specific configurations
+5. Run all tests before submitting PR: `./scripts/docker-test/test-runner.sh`
+
+## Architecture
+
+The fix involves:
+
+1. **Ollama**: Override `IsConfigured()` method to check for `OLLAMA_API_URL` env var
+2. **Bedrock**: Modify `hasAWSCredentials()` to require `BEDROCK_AWS_REGION`
+3. **Plugin Registry**: Only initialize providers when properly configured
+
+This prevents unnecessary API calls and eliminates confusing error messages for users.
--- a/scripts/docker-test/base/Dockerfile
+++ b/scripts/docker-test/base/Dockerfile
@@ -0,0 +1,30 @@
+FROM golang:1.24-alpine AS builder
+
+WORKDIR /app
+COPY go.mod go.sum ./
+RUN go mod download
+
+COPY ./cmd/fabric ./cmd/fabric
+COPY ./internal ./internal
+RUN go build -o fabric ./cmd/fabric
+
+FROM alpine:latest
+RUN apk --no-cache add ca-certificates
+
+# Create a test user
+RUN adduser -D -s /bin/sh testuser
+
+# Switch to test user
+USER testuser
+WORKDIR /home/testuser
+
+# Set environment variables for the test user
+ENV HOME=/home/testuser
+ENV USER=testuser
+
+COPY --from=builder /app/fabric .
+
+# Create fabric config directory and empty .env file
+RUN mkdir -p .config/fabric && touch .config/fabric/.env
+
+ENTRYPOINT ["./fabric"]
--- a/scripts/docker-test/test-runner.sh
+++ b/scripts/docker-test/test-runner.sh
@@ -0,0 +1,235 @@
+#!/usr/bin/env bash
+
+set -e
+
+# Get the directory where this script is located
+top_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+base_name="$(basename "$top_dir")"
+cd "$top_dir"/../.. || exit 1
+
+# Check if bash version supports associative arrays
+if [[ ${BASH_VERSION%%.*} -lt 4 ]]; then
+    echo "This script requires bash 4.0 or later for associative arrays."
+    echo "Current version: $BASH_VERSION"
+    exit 1
+fi
+
+IMAGE_NAME="fabric-test-setup"
+ENV_DIR="scripts/${base_name}/env"
+
+# Test case descriptions
+declare -A test_descriptions=(
+    ["no-config"]="No APIs configured"
+    ["gemini-only"]="Only Gemini configured (reproduces original issue)"
+    ["openai-only"]="Only OpenAI configured"
+    ["ollama-only"]="Only Ollama configured"
+    ["bedrock-only"]="Only Bedrock configured"
+    ["mixed"]="Mixed configuration (Gemini + OpenAI + Ollama)"
+)
+
+# Test case order for consistent display
+test_order=("no-config" "gemini-only" "openai-only" "ollama-only" "bedrock-only" "mixed")
+
+build_image() {
+    echo "=== Building Docker image ==="
+    docker build -f "${top_dir}/base/Dockerfile" -t "$IMAGE_NAME" .
+    echo
+}
+
+check_env_file() {
+    local test_name="$1"
+    local env_file="$ENV_DIR/env.$test_name"
+
+    if [[ ! -f "$env_file" ]]; then
+        echo "Error: Environment file not found: $env_file"
+        exit 1
+    fi
+}
+
+run_test() {
+    local test_name="$1"
+    local description="${test_descriptions[$test_name]}"
+    local env_file="$ENV_DIR/env.$test_name"
+
+    check_env_file "$test_name"
+
+    echo "===================="
+    echo "Test: $description"
+    echo "Config: $test_name"
+    echo "Env file: $env_file"
+    echo "===================="
+
+    echo "Running test..."
+    if docker run --rm \
+        -e HOME=/home/testuser \
+        -e USER=testuser \
+        -v "$(pwd)/$env_file:/home/testuser/.config/fabric/.env:ro" \
+        "$IMAGE_NAME" --listmodels 2>&1; then
+        echo "✅ Test completed"
+    else
+        echo "❌ Test failed"
+    fi
+    echo
+}
+
+shell_into_env() {
+    local test_name="$1"
+    local description="${test_descriptions[$test_name]}"
+    local env_file="$ENV_DIR/env.$test_name"
+
+    check_env_file "$test_name"
+
+    echo "===================="
+    echo "Shelling into: $description"
+    echo "Config: $test_name"
+    echo "Env file: $env_file"
+    echo "===================="
+    echo "You can now run 'fabric -S' to configure, or 'fabric --listmodels' or 'fabric -L' to test."
+    echo "Changes to .env will persist in $env_file"
+    echo "Type 'exit' to return to the test runner."
+    echo
+
+    docker run -it --rm \
+        -e HOME=/home/testuser \
+        -e USER=testuser \
+        -v "$(pwd)/$env_file:/home/testuser/.config/fabric/.env" \
+        --entrypoint=/bin/sh \
+        "$IMAGE_NAME"
+}
+
+interactive_mode() {
+    echo "=== Interactive Mode ==="
+    echo "Available test cases:"
+    echo
+    local i=1
+    local cases=()
+    for test_name in "${test_order[@]}"; do
+        echo "$i) ${test_descriptions[$test_name]} ($test_name)"
+        cases[i]="$test_name"
+        ((i++))
+    done
+    echo "$i) Run all tests"
+    echo "0) Exit"
+    echo
+    echo "Add '!' after number to shell into test environment (e.g., '1!' to shell into no-config)"
+    echo
+
+    while true; do
+        read -r -p "Select test case (0-$i) [or 1!, etc. to shell into test environment]: " choice
+
+        # Check for shell mode (! suffix)
+        local shell_mode=false
+        if [[ "$choice" == *"!" ]]; then
+            shell_mode=true
+            choice="${choice%!}"  # Remove the ! suffix
+        fi
+
+        if [[ "$choice" == "0" ]]; then
+            if [[ "$shell_mode" == true ]]; then
+                echo "Cannot shell into exit option."
+                continue
+            fi
+            echo "Exiting..."
+            exit 0
+        elif [[ "$choice" == "$i" ]]; then
+            if [[ "$shell_mode" == true ]]; then
+                echo "Cannot shell into 'run all tests' option."
+                continue
+            fi
+            echo "Running all tests..."
+            run_all_tests
+            break
+        elif [[ "$choice" -ge 1 && "$choice" -lt "$i" ]]; then
+            local selected_test="${cases[$choice]}"
+            if [[ "$shell_mode" == true ]]; then
+                echo "Shelling into: ${test_descriptions[$selected_test]}"
+                shell_into_env "$selected_test"
+            else
+                echo "Running: ${test_descriptions[$selected_test]}"
+                run_test "$selected_test"
+            fi
+
+            read -r -p "Continue testing? (y/n): " again
+            if [[ "$again" != "y" && "$again" != "Y" ]]; then
+                break
+            fi
+            echo
+        else
+            echo "Invalid choice. Please select 0-$i (optionally with '!' for shell mode)."
+        fi
+    done
+}
+
+run_all_tests() {
+    echo "=== Testing PR #1645: Conditional API initialization ==="
+    echo
+
+    for test_name in "${test_order[@]}"; do
+        run_test "$test_name"
+    done
+
+    echo "=== Test run complete ==="
+    echo "Review the output above to check:"
+    echo "1. No Ollama connection errors when OLLAMA_URL not set"
+    echo "2. No Bedrock authentication errors when BEDROCK_AWS_REGION not set"
+    echo "3. Only configured services appear in model listings"
+}
+
+show_help() {
+    echo "Usage: $0 [OPTIONS] [TEST_CASE]"
+    echo
+    echo "Test PR #1645 conditional API initialization"
+    echo
+    echo "Options:"
+    echo "  -h, --help       Show this help message"
+    echo "  -i, --interactive Run in interactive mode"
+    echo "  -b, --build-only  Build image only, don't run tests"
+    echo "  -s, --shell TEST  Shell into test environment"
+    echo
+    echo "Test cases:"
+    for test_name in "${test_order[@]}"; do
+        echo "  $test_name: ${test_descriptions[$test_name]}"
+    done
+    echo
+    echo "Examples:"
+    echo "  $0                    # Run all tests"
+    echo "  $0 -i                 # Interactive mode"
+    echo "  $0 gemini-only        # Run specific test"
+    echo "  $0 -s gemini-only     # Shell into gemini-only environment"
+    echo "  $0 -b                 # Build image only"
+    echo
+    echo "Environment files are located in $ENV_DIR/ and can be edited directly."
+}
+
+# Parse command line arguments
+if [[ $# -eq 0 ]]; then
+    build_image
+    run_all_tests
+elif [[ "$1" == "-h" || "$1" == "--help" ]]; then
+    show_help
+elif [[ "$1" == "-i" || "$1" == "--interactive" ]]; then
+    build_image
+    interactive_mode
+elif [[ "$1" == "-b" || "$1" == "--build-only" ]]; then
+    build_image
+elif [[ "$1" == "-s" || "$1" == "--shell" ]]; then
+    if [[ -z "$2" ]]; then
+        echo "Error: -s/--shell requires a test case name"
+        echo "Use -h for help."
+        exit 1
+    fi
+    if [[ -z "${test_descriptions[$2]}" ]]; then
+        echo "Error: Unknown test case: $2"
+        echo "Use -h for help."
+        exit 1
+    fi
+    build_image
+    shell_into_env "$2"
+elif [[ -n "${test_descriptions[$1]}" ]]; then
+    build_image
+    run_test "$1"
+else
+    echo "Unknown test case or option: $1"
+    echo "Use -h for help."
+    exit 1
+fi