Refactor Secrets Management

go.mod

@@ -24,6 +24,7 @@ require (
 	github.com/google/s2a-go v0.1.9 // indirect
 	github.com/googleapis/enterprise-certificate-proxy v0.3.6 // indirect
 	github.com/googleapis/gax-go/v2 v2.15.0 // indirect
+	github.com/joho/godotenv v1.5.1 // indirect
 	go.opentelemetry.io/auto/sdk v1.1.0 // indirect
 	go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.61.0 // indirect
 	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.61.0 // indirect

go.sum

@@ -33,6 +33,8 @@ github.com/googleapis/enterprise-certificate-proxy v0.3.6 h1:GW/XbdyBFQ8Qe+YAmFU
 github.com/googleapis/enterprise-certificate-proxy v0.3.6/go.mod h1:MkHOF77EYAE7qfSuSS9PU6g4Nt4e11cnsDUowfwewLA=
 github.com/googleapis/gax-go/v2 v2.15.0 h1:SyjDc1mGgZU5LncH8gimWo9lW1DtIfPibOG81vgd/bo=
 github.com/googleapis/gax-go/v2 v2.15.0/go.mod h1:zVVkkxAQHa1RQpg9z2AUCMnKhi0Qld9rcmyfL1OZhoc=
+github.com/joho/godotenv v1.5.1 h1:7eLL/+HRGLY0ldzfGMeQkb7vMd0as4CfYvUVzLqw0N0=
+github.com/joho/godotenv v1.5.1/go.mod h1:f4LDr5Voq0i2e/R5DDNOoa2zzDfwtkZa6DnEwAbqwq4=
 github.com/julwrites/BotPlatform v0.0.0-20220206144002-60e1b8060734 h1:U/z8aO/8zMpOzdR7kK9hnHfXber1fHa7FWlXGeuG3Yc=
 github.com/julwrites/BotPlatform v0.0.0-20220206144002-60e1b8060734/go.mod h1:RAVF1PibRuRYv1Z7VxNapzrikBrjtF48aFPCoCVnLpM=
 github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=

pkg/app/api_client.go

@@ -7,15 +7,11 @@ import (
 	"io"
 	"log"
 	"net/http"
-	"os"
 	"sync"
 
-	"github.com/julwrites/ScriptureBot/pkg/utils"
+	"github.com/julwrites/ScriptureBot/pkg/secrets"
 )
 
-// getSecretFunc is a variable to allow mocking in tests
-var getSecretFunc = utils.GetSecret
-
 var (
 	cachedAPIURL      string
 	cachedAPIKey      string
@@ -43,50 +39,22 @@ func SetAPIConfigOverride(url, key string) {
 	configInitialized = true
 }
 
-func getAPIConfig(projectID string) (string, string) {
+func getAPIConfig() (string, string) {
 	configMutex.Lock()
 	defer configMutex.Unlock()
 
 	if configInitialized {
 		return cachedAPIURL, cachedAPIKey
 	}
 
-	url := os.Getenv("BIBLE_API_URL")
-	key := os.Getenv("BIBLE_API_KEY")
-
-	// If env vars are missing, try to fetch from Secret Manager
-	if url == "" || key == "" {
-		// Try to fetch project ID if not provided.
-		if projectID == "" {
-			projectID = os.Getenv("GCLOUD_PROJECT_ID")
-
-			if projectID == "" {
-				var err error
-				projectID, err = getSecretFunc("", "GCLOUD_PROJECT_ID")
-				if err != nil {
-					log.Printf("Failed to fetch GCLOUD_PROJECT_ID from Secret Manager: %v", err)
-				}
-			}
-		}
+	url, err := secrets.Get("BIBLE_API_URL")
+	if err != nil {
+		log.Printf("Failed to get BIBLE_API_URL: %v", err)
+	}
 
-		if projectID != "" {
-			if url == "" {
-				var err error
-				url, err = getSecretFunc(projectID, "BIBLE_API_URL")
-				if err != nil {
-					log.Printf("Failed to fetch BIBLE_API_URL from Secret Manager: %v", err)
-				}
-			}
-			if key == "" {
-				var err error
-				key, err = getSecretFunc(projectID, "BIBLE_API_KEY")
-				if err != nil {
-					log.Printf("Failed to fetch BIBLE_API_KEY from Secret Manager: %v", err)
-				}
-			}
-		} else {
-			log.Println("GCLOUD_PROJECT_ID is not set and no project ID passed, skipping Secret Manager lookup")
-		}
+	key, err := secrets.Get("BIBLE_API_KEY")
+	if err != nil {
+		log.Printf("Failed to get BIBLE_API_KEY: %v", err)
 	}
 
 	cachedAPIURL = url
@@ -99,7 +67,7 @@ func getAPIConfig(projectID string) (string, string) {
 // SubmitQuery sends the QueryRequest to the Bible API and unmarshals the response into result.
 // result should be a pointer to the expected response struct.
 func SubmitQuery(req QueryRequest, result interface{}, projectID string) error {
-	apiURL, apiKey := getAPIConfig(projectID)
+	apiURL, apiKey := getAPIConfig()
 	if apiURL == "" {
 		return fmt.Errorf("BIBLE_API_URL environment variable is not set")
 	}

pkg/app/api_client_test.go

@@ -1,81 +1,60 @@
 package app
 
 import (
-	"encoding/json"
-	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"testing"
-
-	"github.com/julwrites/ScriptureBot/pkg/utils"
 )
 
 func TestSubmitQuery(t *testing.T) {
-	// Mock server
-	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		// Check headers
-		if r.Header.Get("Content-Type") != "application/json" {
-			t.Errorf("Expected Content-Type application/json, got %s", r.Header.Get("Content-Type"))
-		}
-
-		// Decode request to verify it
-		var req QueryRequest
-		if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
-			http.Error(w, "Bad Request", http.StatusBadRequest)
-			return
-		}
-
-		// Simple response based on input
-		if req.Query.Prompt == "error" {
-			w.WriteHeader(http.StatusInternalServerError)
-			w.Write([]byte(`{"error": {"code": 500, "message": "simulated error"}}`))
-			return
-		}
-
-		if req.Query.Prompt == "badjson" {
-			w.WriteHeader(http.StatusOK)
-			w.Write([]byte(`{invalid json`))
-			return
-		}
-
-		// Success response
-		resp := VerseResponse{Verse: "Success Verse"}
-		json.NewEncoder(w).Encode(resp)
-	}))
+	handler := newMockApiHandler()
+	ts := httptest.NewServer(handler)
 	defer ts.Close()
 
-	// Set env vars
-	defer setEnv("BIBLE_API_URL", ts.URL)()
-
-	// Test Case 1: Success
 	t.Run("Success", func(t *testing.T) {
+		defer setEnv("BIBLE_API_URL", ts.URL)()
+		ResetAPIConfigCache()
+
 		req := QueryRequest{Query: QueryObject{Prompt: "hello"}}
-		var resp VerseResponse
+		var resp OQueryResponse
 		err := SubmitQuery(req, &resp, "")
 		if err != nil {
 			t.Errorf("Unexpected error: %v", err)
 		}
-		if resp.Verse != "Success Verse" {
-			t.Errorf("Expected 'Success Verse', got '%s'", resp.Verse)
+		if resp.Text != "Answer text" {
+			t.Errorf("Expected 'Answer text', got '%s'", resp.Text)
 		}
 	})
 
-	// Test Case 2: API Error
 	t.Run("API Error", func(t *testing.T) {
+		handler.statusCode = http.StatusInternalServerError
+		handler.rawResponse = `{"error": {"code": 500, "message": "simulated error"}}`
+		defer func() { // Reset handler
+			handler.statusCode = http.StatusOK
+			handler.rawResponse = ""
+		}()
+
+		defer setEnv("BIBLE_API_URL", ts.URL)()
+		ResetAPIConfigCache()
+
 		req := QueryRequest{Query: QueryObject{Prompt: "error"}}
 		var resp VerseResponse
 		err := SubmitQuery(req, &resp, "")
 		if err == nil {
 			t.Error("Expected error, got nil")
 		}
-		// Expect error message to contain "simulated error"
-		if err != nil && err.Error() != "api error (500): simulated error" {
+		if err.Error() != "api error (500): simulated error" {
 			t.Errorf("Expected specific API error, got: %v", err)
 		}
 	})
 
-	// Test Case 3: Bad JSON Response
 	t.Run("Bad JSON", func(t *testing.T) {
+		handler.rawResponse = `{invalid json`
+		defer func() { handler.rawResponse = "" }()
+
+		defer setEnv("BIBLE_API_URL", ts.URL)()
+		ResetAPIConfigCache()
+
 		req := QueryRequest{Query: QueryObject{Prompt: "badjson"}}
 		var resp VerseResponse
 		err := SubmitQuery(req, &resp, "")
@@ -84,13 +63,8 @@ func TestSubmitQuery(t *testing.T) {
 		}
 	})
 
-	// Test Case 4: No URL set
 	t.Run("No URL", func(t *testing.T) {
-		// Temporarily unset/clear the env var
-		restore := setEnv("BIBLE_API_URL", "")
-		defer restore()
-		// Also unset PROJECT_ID to avoid Secret Manager lookup
-		defer utils.SetEnv("GCLOUD_PROJECT_ID", "")()
+		defer setEnv("BIBLE_API_URL", "")()
 		ResetAPIConfigCache()
 
 		req := QueryRequest{}
@@ -101,65 +75,3 @@ func TestSubmitQuery(t *testing.T) {
 		}
 	})
 }
-
-func TestGetAPIConfig_SecretManagerFallback(t *testing.T) {
-	// Ensure Env Vars are empty
-	defer utils.SetEnv("BIBLE_API_URL", "")()
-	defer utils.SetEnv("BIBLE_API_KEY", "")()
-	defer utils.SetEnv("GCLOUD_PROJECT_ID", "test-project")()
-	ResetAPIConfigCache()
-
-	// Mock the secret function
-	oldGetSecret := getSecretFunc
-	defer func() { getSecretFunc = oldGetSecret }()
-
-	getSecretFunc = func(project, name string) (string, error) {
-		if project != "test-project" {
-			return "", fmt.Errorf("unexpected project: %s", project)
-		}
-		if name == "BIBLE_API_URL" {
-			return "http://secret-url.com", nil
-		}
-		if name == "BIBLE_API_KEY" {
-			return "secret-key", nil
-		}
-		return "", fmt.Errorf("unexpected secret: %s", name)
-	}
-
-	url, key := getAPIConfig("")
-
-	if url != "http://secret-url.com" {
-		t.Errorf("Expected URL 'http://secret-url.com', got '%s'", url)
-	}
-	if key != "secret-key" {
-		t.Errorf("Expected Key 'secret-key', got '%s'", key)
-	}
-}
-
-func TestGetAPIConfig_PassedProjectID(t *testing.T) {
-	// Ensure Env Vars are empty, including GCLOUD_PROJECT_ID
-	defer utils.SetEnv("BIBLE_API_URL", "")()
-	defer utils.SetEnv("BIBLE_API_KEY", "")()
-	defer utils.SetEnv("GCLOUD_PROJECT_ID", "")()
-	ResetAPIConfigCache()
-
-	// Mock the secret function
-	oldGetSecret := getSecretFunc
-	defer func() { getSecretFunc = oldGetSecret }()
-
-	getSecretFunc = func(project, name string) (string, error) {
-		if project != "passed-project" {
-			return "", fmt.Errorf("unexpected project: %s", project)
-		}
-		if name == "BIBLE_API_URL" {
-			return "http://secret-url-passed.com", nil
-		}
-		return "", fmt.Errorf("unexpected secret: %s", name)
-	}
-
-	url, _ := getAPIConfig("passed-project")
-
-	if url != "http://secret-url-passed.com" {
-		t.Errorf("Expected URL 'http://secret-url-passed.com', got '%s'", url)
-	}
-}

pkg/app/ask_test.go

@@ -1,7 +1,6 @@
 package app
 
 import (
-	"encoding/json"
 	"net/http"
 	"net/http/httptest"
 	"strings"
@@ -12,29 +11,14 @@ import (
 )
 
 func TestGetBibleAsk(t *testing.T) {
-	// Mock server
-	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		var req QueryRequest
-		json.NewDecoder(r.Body).Decode(&req)
-
-		if req.Query.Prompt == "error" {
-			w.WriteHeader(http.StatusInternalServerError)
-			return
-		}
-
-		resp := OQueryResponse{
-			Text: "Answer text",
-			References: []SearchResult{
-				{Verse: "Ref 1:1", URL: "http://ref1"},
-			},
-		}
-		json.NewEncoder(w).Encode(resp)
-	}))
+	handler := newMockApiHandler()
+	ts := httptest.NewServer(handler)
 	defer ts.Close()
 
-	defer setEnv("BIBLE_API_URL", ts.URL)()
-
 	t.Run("Success", func(t *testing.T) {
+		defer setEnv("BIBLE_API_URL", ts.URL)()
+		ResetAPIConfigCache()
+
 		var env def.SessionData
 		env.Msg.Message = "Question"
 		conf := utils.UserConfig{Version: "NIV"}
@@ -51,6 +35,12 @@ func TestGetBibleAsk(t *testing.T) {
 	})
 
 	t.Run("Error", func(t *testing.T) {
+		handler.statusCode = http.StatusInternalServerError
+		defer func() { handler.statusCode = http.StatusOK }()
+
+		defer setEnv("BIBLE_API_URL", ts.URL)()
+		ResetAPIConfigCache()
+
 		var env def.SessionData
 		env.Msg.Message = "error"
 		conf := utils.UserConfig{Version: "NIV"}