Skip to content

Commit e8e0bfa

Browse files
committed
refactor(broker): narrow ProviderHealthLookup to avoid loading secrets
Replace GetProfile (loads full Profile with client_secret, params, etc.) with GetHealthStatus (SELECT only health_status) for the background ConnectionHealthWorker's provider cross-reference. Changes: - Add Store.GetHealthStatus(uuid) (string, error) — single-column query - Narrow ProviderHealthLookup interface to GetHealthStatus - Update isProviderDown to use string status directly - Remove provider package import from connection_health.go - Add sqlmock tests for GetHealthStatus (success, not found) - Update all mock expectations in connection_health_test.go
1 parent bdd2798 commit e8e0bfa

5 files changed

Lines changed: 235 additions & 15 deletions

File tree

nexus-broker/cmd/nexus-broker/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ func main() {
144144
go healthWorker.Start(cleanupCtx)
145145

146146
// Start connection health worker (polls every 1m)
147-
// The store implements ProviderHealthLookup via GetProfile(uuid.UUID)
147+
// The store implements ProviderHealthLookup via GetHealthStatus(uuid.UUID)
148148
connHealthWorker := service.NewConnectionHealthWorker(connRepo, connSvc, store, 1*time.Minute)
149149
go connHealthWorker.Start(cleanupCtx)
150150

nexus-broker/internal/service/connection_health.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,13 @@ import (
1212
"github.com/google/uuid"
1313
"github.com/Prescott-Data/nexus-framework/nexus-broker/internal/domain"
1414
"github.com/Prescott-Data/nexus-framework/nexus-broker/internal/repository"
15-
"github.com/Prescott-Data/nexus-framework/nexus-broker/pkg/provider"
1615
)
1716

1817
// ProviderHealthLookup provides read-only access to provider health status.
19-
// This avoids importing the full Store and keeps the dependency narrow.
18+
// Uses a narrow query that only fetches health_status, avoiding loading
19+
// sensitive fields (client_secret, params, etc.) into worker memory.
2020
type ProviderHealthLookup interface {
21-
GetProfile(id uuid.UUID) (*provider.Profile, error)
21+
GetHealthStatus(id uuid.UUID) (string, error)
2222
}
2323

2424
// ConnectionHealthWorker polls for active connections and verifies their health
@@ -129,12 +129,12 @@ func (w *ConnectionHealthWorker) isProviderDown(providerID uuid.UUID) bool {
129129
return false // No lookup available, assume provider is fine
130130
}
131131

132-
profile, err := w.providerHealth.GetProfile(providerID)
132+
status, err := w.providerHealth.GetHealthStatus(providerID)
133133
if err != nil {
134134
return false // Can't look up, assume provider is fine
135135
}
136136

137-
return profile.HealthStatus == "unhealthy" || profile.HealthStatus == "degraded"
137+
return status == "unhealthy" || status == "degraded"
138138
}
139139

140140
func (w *ConnectionHealthWorker) checkConnection(ctx context.Context, c *domain.ConnectionWithProvider) string {

nexus-broker/internal/service/connection_health_test.go

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515

1616
"github.com/Prescott-Data/nexus-framework/nexus-broker/internal/domain"
1717
"github.com/Prescott-Data/nexus-framework/nexus-broker/internal/service"
18-
"github.com/Prescott-Data/nexus-framework/nexus-broker/pkg/provider"
1918
)
2019

2120
// Add missing mock methods to MockConnectionRepository
@@ -100,12 +99,9 @@ type MockProviderHealthLookup struct {
10099
mock.Mock
101100
}
102101

103-
func (m *MockProviderHealthLookup) GetProfile(id uuid.UUID) (*provider.Profile, error) {
102+
func (m *MockProviderHealthLookup) GetHealthStatus(id uuid.UUID) (string, error) {
104103
args := m.Called(id)
105-
if args.Get(0) != nil {
106-
return args.Get(0).(*provider.Profile), args.Error(1)
107-
}
108-
return nil, args.Error(1)
104+
return args.String(0), args.Error(1)
109105
}
110106

111107
func TestConnectionHealthWorker_OAuth2_Healthy(t *testing.T) {
@@ -167,7 +163,7 @@ func TestConnectionHealthWorker_OAuth2_Expired(t *testing.T) {
167163
mockSvc.On("Refresh", mock.Anything, connID).Return(&service.RefreshResponse{StatusCode: 400}, errors.New("invalid_grant")).Once()
168164

169165
// Provider is healthy, so the connection should be expired (not shielded)
170-
mockHealth.On("GetProfile", providerID).Return(&provider.Profile{HealthStatus: "healthy"}, nil).Once()
166+
mockHealth.On("GetHealthStatus", providerID).Return("healthy", nil).Once()
171167

172168
// Should update connection status to expired
173169
mockRepo.On("UpdateStatus", mock.Anything, connID, "expired").Return(nil).Once()
@@ -213,7 +209,7 @@ func TestConnectionHealthWorker_OAuth2_ProviderDown_ShieldsExpiration(t *testing
213209
mockSvc.On("Refresh", mock.Anything, connID).Return(&service.RefreshResponse{StatusCode: 401}, errors.New("token revoked")).Once()
214210

215211
// Provider is unhealthy → should shield the connection from being expired
216-
mockHealth.On("GetProfile", providerID).Return(&provider.Profile{HealthStatus: "unhealthy"}, nil).Once()
212+
mockHealth.On("GetHealthStatus", providerID).Return("unhealthy", nil).Once()
217213

218214
// Should NOT call UpdateStatus (no expiration)
219215
// Should update health to "unhealthy" instead of "expired"
@@ -265,7 +261,7 @@ func TestConnectionHealthWorker_APIKey_Expired(t *testing.T) {
265261
mockSvc.On("GetToken", mock.Anything, connID).Return(creds, "api_key_strategy", nil).Once()
266262

267263
// Provider is healthy, so expiration should proceed
268-
mockHealth.On("GetProfile", providerID).Return(&provider.Profile{HealthStatus: "healthy"}, nil).Once()
264+
mockHealth.On("GetHealthStatus", providerID).Return("healthy", nil).Once()
269265

270266
// Should update connection status to expired
271267
mockRepo.On("UpdateStatus", mock.Anything, connID, "expired").Return(nil).Once()

nexus-broker/pkg/provider/store.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,21 @@ func (s *Store) UpdateHealthStatus(id uuid.UUID, status string, message *string)
424424
return nil
425425
}
426426

427+
// GetHealthStatus returns only the health_status for a provider.
428+
// This is a narrow query intended for background workers that need to
429+
// cross-reference provider health without loading sensitive fields.
430+
func (s *Store) GetHealthStatus(id uuid.UUID) (string, error) {
431+
var status string
432+
err := s.db.QueryRow(
433+
`SELECT COALESCE(health_status, 'unknown') FROM provider_profiles WHERE id = $1 AND deleted_at IS NULL`,
434+
id,
435+
).Scan(&status)
436+
if err != nil {
437+
return "", fmt.Errorf("failed to get provider health status: %w", err)
438+
}
439+
return status, nil
440+
}
441+
427442
// GetMetadata retrieves integration metadata for all providers, grouped by auth_type
428443
func (s *Store) GetMetadata() (map[string]map[string]interface{}, error) {
429444

nexus-broker/pkg/provider/store_test.go

Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"database/sql"
55
"encoding/json"
66
"testing"
7+
"time"
78

89
"github.com/google/uuid"
910
"github.com/jmoiron/sqlx"
@@ -208,3 +209,211 @@ func TestGetProfile_NullValues(t *testing.T) {
208209
assert.Equal(t, "null-provider", profile.Name)
209210
}
210211
}
212+
213+
func TestGetAllProfiles_Success(t *testing.T) {
214+
db, mock, err := sqlmock.New()
215+
assert.NoError(t, err)
216+
defer db.Close()
217+
218+
sqlxDB := sqlx.NewDb(db, "sqlmock")
219+
store := NewStore(sqlxDB)
220+
221+
id1 := uuid.New()
222+
id2 := uuid.New()
223+
now := time.Now()
224+
msg := "timeout reaching token_endpoint"
225+
226+
// Must match the exact 19-column order in GetAllProfiles SELECT:
227+
// id, name, client_id, client_secret, auth_url, token_url, issuer,
228+
// enable_discovery, scopes, auth_type, auth_header,
229+
// api_base_url, user_info_endpoint, params, description, category,
230+
// last_health_check_at, health_status, health_message
231+
rows := sqlmock.NewRows([]string{
232+
"id", "name", "client_id", "client_secret", "auth_url", "token_url", "issuer",
233+
"enable_discovery", "scopes", "auth_type", "auth_header",
234+
"api_base_url", "user_info_endpoint", "params", "description", "category",
235+
"last_health_check_at", "health_status", "health_message",
236+
}).AddRow(
237+
id1.String(), "google", ptr("cid"), ptr("csec"), ptr("https://auth"), ptr("https://token"), nil,
238+
true, []byte("{email,profile}"), "oauth2", "",
239+
"https://api.google.com", "/userinfo", nil, "Google OAuth", "Identity",
240+
now, "healthy", nil,
241+
).AddRow(
242+
id2.String(), "stripe", nil, nil, nil, nil, nil,
243+
false, []byte("{}"), "api_key", "Authorization",
244+
"https://api.stripe.com", "/v1/account", nil, "Stripe API", "Payments",
245+
now, "unhealthy", &msg,
246+
)
247+
248+
mock.ExpectQuery(`SELECT .* FROM provider_profiles`).WillReturnRows(rows)
249+
250+
profiles, err := store.GetAllProfiles()
251+
assert.NoError(t, err)
252+
assert.Len(t, profiles, 2)
253+
254+
// Verify first profile health fields
255+
assert.Equal(t, id1, profiles[0].ID)
256+
assert.Equal(t, "google", profiles[0].Name)
257+
assert.Equal(t, "healthy", profiles[0].HealthStatus)
258+
assert.NotNil(t, profiles[0].LastHealthCheckAt)
259+
assert.Nil(t, profiles[0].HealthMessage)
260+
261+
// Verify second profile health fields
262+
assert.Equal(t, id2, profiles[1].ID)
263+
assert.Equal(t, "stripe", profiles[1].Name)
264+
assert.Equal(t, "unhealthy", profiles[1].HealthStatus)
265+
assert.NotNil(t, profiles[1].HealthMessage)
266+
assert.Equal(t, "timeout reaching token_endpoint", *profiles[1].HealthMessage)
267+
268+
assert.NoError(t, mock.ExpectationsWereMet())
269+
}
270+
271+
func TestGetAllProfiles_Empty(t *testing.T) {
272+
db, mock, err := sqlmock.New()
273+
assert.NoError(t, err)
274+
defer db.Close()
275+
276+
sqlxDB := sqlx.NewDb(db, "sqlmock")
277+
store := NewStore(sqlxDB)
278+
279+
rows := sqlmock.NewRows([]string{
280+
"id", "name", "client_id", "client_secret", "auth_url", "token_url", "issuer",
281+
"enable_discovery", "scopes", "auth_type", "auth_header",
282+
"api_base_url", "user_info_endpoint", "params", "description", "category",
283+
"last_health_check_at", "health_status", "health_message",
284+
})
285+
286+
mock.ExpectQuery(`SELECT .* FROM provider_profiles`).WillReturnRows(rows)
287+
288+
profiles, err := store.GetAllProfiles()
289+
assert.NoError(t, err)
290+
assert.Nil(t, profiles) // append to nil slice returns nil
291+
292+
assert.NoError(t, mock.ExpectationsWereMet())
293+
}
294+
295+
func TestGetAllProfiles_QueryError(t *testing.T) {
296+
db, mock, err := sqlmock.New()
297+
assert.NoError(t, err)
298+
defer db.Close()
299+
300+
sqlxDB := sqlx.NewDb(db, "sqlmock")
301+
store := NewStore(sqlxDB)
302+
303+
mock.ExpectQuery(`SELECT .* FROM provider_profiles`).
304+
WillReturnError(sql.ErrConnDone)
305+
306+
profiles, err := store.GetAllProfiles()
307+
assert.Error(t, err)
308+
assert.Nil(t, profiles)
309+
assert.Contains(t, err.Error(), "failed to query all profiles")
310+
311+
assert.NoError(t, mock.ExpectationsWereMet())
312+
}
313+
314+
func TestUpdateHealthStatus_Success(t *testing.T) {
315+
db, mock, err := sqlmock.New()
316+
assert.NoError(t, err)
317+
defer db.Close()
318+
319+
sqlxDB := sqlx.NewDb(db, "sqlmock")
320+
store := NewStore(sqlxDB)
321+
322+
providerID := uuid.New()
323+
msg := "token_url 503"
324+
325+
// Verify the UPDATE is called with (status, message, id) in correct order
326+
mock.ExpectExec(`UPDATE provider_profiles SET health_status = \$1, health_message = \$2, last_health_check_at = NOW\(\) WHERE id = \$3`).
327+
WithArgs("unhealthy", &msg, providerID).
328+
WillReturnResult(sqlmock.NewResult(0, 1))
329+
330+
err = store.UpdateHealthStatus(providerID, "unhealthy", &msg)
331+
assert.NoError(t, err)
332+
333+
assert.NoError(t, mock.ExpectationsWereMet())
334+
}
335+
336+
func TestUpdateHealthStatus_NilMessage(t *testing.T) {
337+
db, mock, err := sqlmock.New()
338+
assert.NoError(t, err)
339+
defer db.Close()
340+
341+
sqlxDB := sqlx.NewDb(db, "sqlmock")
342+
store := NewStore(sqlxDB)
343+
344+
providerID := uuid.New()
345+
346+
mock.ExpectExec(`UPDATE provider_profiles SET health_status = \$1, health_message = \$2, last_health_check_at = NOW\(\) WHERE id = \$3`).
347+
WithArgs("healthy", nil, providerID).
348+
WillReturnResult(sqlmock.NewResult(0, 1))
349+
350+
err = store.UpdateHealthStatus(providerID, "healthy", nil)
351+
assert.NoError(t, err)
352+
353+
assert.NoError(t, mock.ExpectationsWereMet())
354+
}
355+
356+
func TestUpdateHealthStatus_DBError(t *testing.T) {
357+
db, mock, err := sqlmock.New()
358+
assert.NoError(t, err)
359+
defer db.Close()
360+
361+
sqlxDB := sqlx.NewDb(db, "sqlmock")
362+
store := NewStore(sqlxDB)
363+
364+
providerID := uuid.New()
365+
366+
mock.ExpectExec(`UPDATE provider_profiles SET health_status`).
367+
WithArgs("unhealthy", nil, providerID).
368+
WillReturnError(sql.ErrConnDone)
369+
370+
err = store.UpdateHealthStatus(providerID, "unhealthy", nil)
371+
assert.Error(t, err)
372+
assert.Contains(t, err.Error(), "failed to update provider health status")
373+
374+
assert.NoError(t, mock.ExpectationsWereMet())
375+
}
376+
377+
func TestGetHealthStatus_Success(t *testing.T) {
378+
db, mock, err := sqlmock.New()
379+
assert.NoError(t, err)
380+
defer db.Close()
381+
382+
sqlxDB := sqlx.NewDb(db, "sqlmock")
383+
store := NewStore(sqlxDB)
384+
385+
providerID := uuid.New()
386+
rows := sqlmock.NewRows([]string{"health_status"}).AddRow("unhealthy")
387+
388+
mock.ExpectQuery(`SELECT COALESCE\(health_status, 'unknown'\) FROM provider_profiles WHERE id = \$1`).
389+
WithArgs(providerID).
390+
WillReturnRows(rows)
391+
392+
status, err := store.GetHealthStatus(providerID)
393+
assert.NoError(t, err)
394+
assert.Equal(t, "unhealthy", status)
395+
396+
assert.NoError(t, mock.ExpectationsWereMet())
397+
}
398+
399+
func TestGetHealthStatus_NotFound(t *testing.T) {
400+
db, mock, err := sqlmock.New()
401+
assert.NoError(t, err)
402+
defer db.Close()
403+
404+
sqlxDB := sqlx.NewDb(db, "sqlmock")
405+
store := NewStore(sqlxDB)
406+
407+
providerID := uuid.New()
408+
409+
mock.ExpectQuery(`SELECT COALESCE\(health_status, 'unknown'\) FROM provider_profiles WHERE id = \$1`).
410+
WithArgs(providerID).
411+
WillReturnError(sql.ErrNoRows)
412+
413+
status, err := store.GetHealthStatus(providerID)
414+
assert.Error(t, err)
415+
assert.Equal(t, "", status)
416+
assert.Contains(t, err.Error(), "failed to get provider health status")
417+
418+
assert.NoError(t, mock.ExpectationsWereMet())
419+
}

0 commit comments

Comments
 (0)