Skip to content

Commit 96e7e82

Browse files
julwritesjulwrites
authored
Merge pull request #124 from julwrites/staging
Refactoring secret manaement
2 parents 7a83940 + d378a36 commit 96e7e82

15 files changed

Lines changed: 427 additions & 333 deletions

‎go.mod‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ require (
2424
github.com/google/s2a-go v0.1.9 // indirect
2525
github.com/googleapis/enterprise-certificate-proxy v0.3.6 // indirect
2626
github.com/googleapis/gax-go/v2 v2.15.0 // indirect
27+
github.com/joho/godotenv v1.5.1 // indirect
2728
go.opentelemetry.io/auto/sdk v1.1.0 // indirect
2829
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.61.0 // indirect
2930
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.61.0 // indirect

‎go.sum‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ github.com/googleapis/enterprise-certificate-proxy v0.3.6 h1:GW/XbdyBFQ8Qe+YAmFU
3333
github.com/googleapis/enterprise-certificate-proxy v0.3.6/go.mod h1:MkHOF77EYAE7qfSuSS9PU6g4Nt4e11cnsDUowfwewLA=
3434
github.com/googleapis/gax-go/v2 v2.15.0 h1:SyjDc1mGgZU5LncH8gimWo9lW1DtIfPibOG81vgd/bo=
3535
github.com/googleapis/gax-go/v2 v2.15.0/go.mod h1:zVVkkxAQHa1RQpg9z2AUCMnKhi0Qld9rcmyfL1OZhoc=
36+
github.com/joho/godotenv v1.5.1 h1:7eLL/+HRGLY0ldzfGMeQkb7vMd0as4CfYvUVzLqw0N0=
37+
github.com/joho/godotenv v1.5.1/go.mod h1:f4LDr5Voq0i2e/R5DDNOoa2zzDfwtkZa6DnEwAbqwq4=
3638
github.com/julwrites/BotPlatform v0.0.0-20220206144002-60e1b8060734 h1:U/z8aO/8zMpOzdR7kK9hnHfXber1fHa7FWlXGeuG3Yc=
3739
github.com/julwrites/BotPlatform v0.0.0-20220206144002-60e1b8060734/go.mod h1:RAVF1PibRuRYv1Z7VxNapzrikBrjtF48aFPCoCVnLpM=
3840
github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=

‎pkg/app/api_client.go‎

Lines changed: 10 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,11 @@ import (
77
"io"
88
"log"
99
"net/http"
10-
"os"
1110
"sync"
1211

13-
"github.com/julwrites/ScriptureBot/pkg/utils"
12+
"github.com/julwrites/ScriptureBot/pkg/secrets"
1413
)
1514

16-
// getSecretFunc is a variable to allow mocking in tests
17-
var getSecretFunc = utils.GetSecret
18-
1915
var (
2016
cachedAPIURL string
2117
cachedAPIKey string
@@ -43,50 +39,22 @@ func SetAPIConfigOverride(url, key string) {
4339
configInitialized = true
4440
}
4541

46-
func getAPIConfig(projectID string) (string, string) {
42+
func getAPIConfig() (string, string) {
4743
configMutex.Lock()
4844
defer configMutex.Unlock()
4945

5046
if configInitialized {
5147
return cachedAPIURL, cachedAPIKey
5248
}
5349

54-
url := os.Getenv("BIBLE_API_URL")
55-
key := os.Getenv("BIBLE_API_KEY")
56-
57-
// If env vars are missing, try to fetch from Secret Manager
58-
if url == "" || key == "" {
59-
// Try to fetch project ID if not provided.
60-
if projectID == "" {
61-
projectID = os.Getenv("GCLOUD_PROJECT_ID")
62-
63-
if projectID == "" {
64-
var err error
65-
projectID, err = getSecretFunc("", "GCLOUD_PROJECT_ID")
66-
if err != nil {
67-
log.Printf("Failed to fetch GCLOUD_PROJECT_ID from Secret Manager: %v", err)
68-
}
69-
}
70-
}
50+
url, err := secrets.Get("BIBLE_API_URL")
51+
if err != nil {
52+
log.Printf("Failed to get BIBLE_API_URL: %v", err)
53+
}
7154

72-
if projectID != "" {
73-
if url == "" {
74-
var err error
75-
url, err = getSecretFunc(projectID, "BIBLE_API_URL")
76-
if err != nil {
77-
log.Printf("Failed to fetch BIBLE_API_URL from Secret Manager: %v", err)
78-
}
79-
}
80-
if key == "" {
81-
var err error
82-
key, err = getSecretFunc(projectID, "BIBLE_API_KEY")
83-
if err != nil {
84-
log.Printf("Failed to fetch BIBLE_API_KEY from Secret Manager: %v", err)
85-
}
86-
}
87-
} else {
88-
log.Println("GCLOUD_PROJECT_ID is not set and no project ID passed, skipping Secret Manager lookup")
89-
}
55+
key, err := secrets.Get("BIBLE_API_KEY")
56+
if err != nil {
57+
log.Printf("Failed to get BIBLE_API_KEY: %v", err)
9058
}
9159

9260
cachedAPIURL = url
@@ -99,7 +67,7 @@ func getAPIConfig(projectID string) (string, string) {
9967
// SubmitQuery sends the QueryRequest to the Bible API and unmarshals the response into result.
10068
// result should be a pointer to the expected response struct.
10169
func SubmitQuery(req QueryRequest, result interface{}, projectID string) error {
102-
apiURL, apiKey := getAPIConfig(projectID)
70+
apiURL, apiKey := getAPIConfig()
10371
if apiURL == "" {
10472
return fmt.Errorf("BIBLE_API_URL environment variable is not set")
10573
}

‎pkg/app/api_client_test.go‎

Lines changed: 26 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -1,81 +1,60 @@
11
package app
22

33
import (
4-
"encoding/json"
5-
"fmt"
64
"net/http"
75
"net/http/httptest"
86
"testing"
9-
10-
"github.com/julwrites/ScriptureBot/pkg/utils"
117
)
128

139
func TestSubmitQuery(t *testing.T) {
14-
// Mock server
15-
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
16-
// Check headers
17-
if r.Header.Get("Content-Type") != "application/json" {
18-
t.Errorf("Expected Content-Type application/json, got %s", r.Header.Get("Content-Type"))
19-
}
20-
21-
// Decode request to verify it
22-
var req QueryRequest
23-
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
24-
http.Error(w, "Bad Request", http.StatusBadRequest)
25-
return
26-
}
27-
28-
// Simple response based on input
29-
if req.Query.Prompt == "error" {
30-
w.WriteHeader(http.StatusInternalServerError)
31-
w.Write([]byte(`{"error": {"code": 500, "message": "simulated error"}}`))
32-
return
33-
}
34-
35-
if req.Query.Prompt == "badjson" {
36-
w.WriteHeader(http.StatusOK)
37-
w.Write([]byte(`{invalid json`))
38-
return
39-
}
40-
41-
// Success response
42-
resp := VerseResponse{Verse: "Success Verse"}
43-
json.NewEncoder(w).Encode(resp)
44-
}))
10+
handler := newMockApiHandler()
11+
ts := httptest.NewServer(handler)
4512
defer ts.Close()
4613

47-
// Set env vars
48-
defer setEnv("BIBLE_API_URL", ts.URL)()
49-
50-
// Test Case 1: Success
5114
t.Run("Success", func(t *testing.T) {
15+
defer setEnv("BIBLE_API_URL", ts.URL)()
16+
ResetAPIConfigCache()
17+
5218
req := QueryRequest{Query: QueryObject{Prompt: "hello"}}
53-
var resp VerseResponse
19+
var resp OQueryResponse
5420
err := SubmitQuery(req, &resp, "")
5521
if err != nil {
5622
t.Errorf("Unexpected error: %v", err)
5723
}
58-
if resp.Verse != "Success Verse" {
59-
t.Errorf("Expected 'Success Verse', got '%s'", resp.Verse)
24+
if resp.Text != "Answer text" {
25+
t.Errorf("Expected 'Answer text', got '%s'", resp.Text)
6026
}
6127
})
6228

63-
// Test Case 2: API Error
6429
t.Run("API Error", func(t *testing.T) {
30+
handler.statusCode = http.StatusInternalServerError
31+
handler.rawResponse = `{"error": {"code": 500, "message": "simulated error"}}`
32+
defer func() { // Reset handler
33+
handler.statusCode = http.StatusOK
34+
handler.rawResponse = ""
35+
}()
36+
37+
defer setEnv("BIBLE_API_URL", ts.URL)()
38+
ResetAPIConfigCache()
39+
6540
req := QueryRequest{Query: QueryObject{Prompt: "error"}}
6641
var resp VerseResponse
6742
err := SubmitQuery(req, &resp, "")
6843
if err == nil {
6944
t.Error("Expected error, got nil")
7045
}
71-
// Expect error message to contain "simulated error"
72-
if err != nil && err.Error() != "api error (500): simulated error" {
46+
if err.Error() != "api error (500): simulated error" {
7347
t.Errorf("Expected specific API error, got: %v", err)
7448
}
7549
})
7650

77-
// Test Case 3: Bad JSON Response
7851
t.Run("Bad JSON", func(t *testing.T) {
52+
handler.rawResponse = `{invalid json`
53+
defer func() { handler.rawResponse = "" }()
54+
55+
defer setEnv("BIBLE_API_URL", ts.URL)()
56+
ResetAPIConfigCache()
57+
7958
req := QueryRequest{Query: QueryObject{Prompt: "badjson"}}
8059
var resp VerseResponse
8160
err := SubmitQuery(req, &resp, "")
@@ -84,13 +63,8 @@ func TestSubmitQuery(t *testing.T) {
8463
}
8564
})
8665

87-
// Test Case 4: No URL set
8866
t.Run("No URL", func(t *testing.T) {
89-
// Temporarily unset/clear the env var
90-
restore := setEnv("BIBLE_API_URL", "")
91-
defer restore()
92-
// Also unset PROJECT_ID to avoid Secret Manager lookup
93-
defer utils.SetEnv("GCLOUD_PROJECT_ID", "")()
67+
defer setEnv("BIBLE_API_URL", "")()
9468
ResetAPIConfigCache()
9569

9670
req := QueryRequest{}
@@ -101,65 +75,3 @@ func TestSubmitQuery(t *testing.T) {
10175
}
10276
})
10377
}
104-
105-
func TestGetAPIConfig_SecretManagerFallback(t *testing.T) {
106-
// Ensure Env Vars are empty
107-
defer utils.SetEnv("BIBLE_API_URL", "")()
108-
defer utils.SetEnv("BIBLE_API_KEY", "")()
109-
defer utils.SetEnv("GCLOUD_PROJECT_ID", "test-project")()
110-
ResetAPIConfigCache()
111-
112-
// Mock the secret function
113-
oldGetSecret := getSecretFunc
114-
defer func() { getSecretFunc = oldGetSecret }()
115-
116-
getSecretFunc = func(project, name string) (string, error) {
117-
if project != "test-project" {
118-
return "", fmt.Errorf("unexpected project: %s", project)
119-
}
120-
if name == "BIBLE_API_URL" {
121-
return "http://secret-url.com", nil
122-
}
123-
if name == "BIBLE_API_KEY" {
124-
return "secret-key", nil
125-
}
126-
return "", fmt.Errorf("unexpected secret: %s", name)
127-
}
128-
129-
url, key := getAPIConfig("")
130-
131-
if url != "http://secret-url.com" {
132-
t.Errorf("Expected URL 'http://secret-url.com', got '%s'", url)
133-
}
134-
if key != "secret-key" {
135-
t.Errorf("Expected Key 'secret-key', got '%s'", key)
136-
}
137-
}
138-
139-
func TestGetAPIConfig_PassedProjectID(t *testing.T) {
140-
// Ensure Env Vars are empty, including GCLOUD_PROJECT_ID
141-
defer utils.SetEnv("BIBLE_API_URL", "")()
142-
defer utils.SetEnv("BIBLE_API_KEY", "")()
143-
defer utils.SetEnv("GCLOUD_PROJECT_ID", "")()
144-
ResetAPIConfigCache()
145-
146-
// Mock the secret function
147-
oldGetSecret := getSecretFunc
148-
defer func() { getSecretFunc = oldGetSecret }()
149-
150-
getSecretFunc = func(project, name string) (string, error) {
151-
if project != "passed-project" {
152-
return "", fmt.Errorf("unexpected project: %s", project)
153-
}
154-
if name == "BIBLE_API_URL" {
155-
return "http://secret-url-passed.com", nil
156-
}
157-
return "", fmt.Errorf("unexpected secret: %s", name)
158-
}
159-
160-
url, _ := getAPIConfig("passed-project")
161-
162-
if url != "http://secret-url-passed.com" {
163-
t.Errorf("Expected URL 'http://secret-url-passed.com', got '%s'", url)
164-
}
165-
}

‎pkg/app/ask_test.go‎

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package app
22

33
import (
4-
"encoding/json"
54
"net/http"
65
"net/http/httptest"
76
"strings"
@@ -12,29 +11,14 @@ import (
1211
)
1312

1413
func TestGetBibleAsk(t *testing.T) {
15-
// Mock server
16-
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
17-
var req QueryRequest
18-
json.NewDecoder(r.Body).Decode(&req)
19-
20-
if req.Query.Prompt == "error" {
21-
w.WriteHeader(http.StatusInternalServerError)
22-
return
23-
}
24-
25-
resp := OQueryResponse{
26-
Text: "Answer text",
27-
References: []SearchResult{
28-
{Verse: "Ref 1:1", URL: "http://ref1"},
29-
},
30-
}
31-
json.NewEncoder(w).Encode(resp)
32-
}))
14+
handler := newMockApiHandler()
15+
ts := httptest.NewServer(handler)
3316
defer ts.Close()
3417

35-
defer setEnv("BIBLE_API_URL", ts.URL)()
36-
3718
t.Run("Success", func(t *testing.T) {
19+
defer setEnv("BIBLE_API_URL", ts.URL)()
20+
ResetAPIConfigCache()
21+
3822
var env def.SessionData
3923
env.Msg.Message = "Question"
4024
conf := utils.UserConfig{Version: "NIV"}
@@ -51,6 +35,12 @@ func TestGetBibleAsk(t *testing.T) {
5135
})
5236

5337
t.Run("Error", func(t *testing.T) {
38+
handler.statusCode = http.StatusInternalServerError
39+
defer func() { handler.statusCode = http.StatusOK }()
40+
41+
defer setEnv("BIBLE_API_URL", ts.URL)()
42+
ResetAPIConfigCache()
43+
5444
var env def.SessionData
5545
env.Msg.Message = "error"
5646
conf := utils.UserConfig{Version: "NIV"}

0 commit comments

Comments
 (0)