Merge pull request #55 from bnema/review-fixes
Fix review feedback: concurrency, security, startup, and deployment improvements
24 files · +691 -175
M internal/adapters/in/http/admin/handler.go +1 -1
| @@ -943,7 +943,7 @@ func (h *Handler) handleContainerLogs(w http.ResponseWriter, r *http.Request, lo | |||
| 943 | 943 | logLines, err := h.logSvc.GetContainerLogs(ctx, logDomain, lines) | |
| 944 | 944 | if err != nil { | |
| 945 | 945 | log.Warn().Err(err).Str("domain", logDomain).Msg("failed to get container logs") | |
| 946 | - | h.sendError(w, http.StatusInternalServerError, fmt.Sprintf("failed to get logs: %v", err)) | |
| 946 | + | h.sendError(w, http.StatusInternalServerError, "failed to get container logs") | |
| 947 | 947 | return | |
| 948 | 948 | } | |
| 949 | 949 | ||
| @@ -943,7 +943,7 @@ func (h *Handler) handleContainerLogs(w http.ResponseWriter, r *http.Request, lo | |||||
| 943 | logLines, err := h.logSvc.GetContainerLogs(ctx, logDomain, lines) | 943 | logLines, err := h.logSvc.GetContainerLogs(ctx, logDomain, lines) | ||
| 944 | if err != nil { | 944 | if err != nil { | ||
| 945 | log.Warn().Err(err).Str("domain", logDomain).Msg("failed to get container logs") | 945 | log.Warn().Err(err).Str("domain", logDomain).Msg("failed to get container logs") | ||
| 946 | - | h.sendError(w, http.StatusInternalServerError, fmt.Sprintf("failed to get logs: %v", err)) | 946 | + | h.sendError(w, http.StatusInternalServerError, "failed to get container logs") |
| 947 | return | 947 | return | ||
| 948 | } | 948 | } | ||
| 949 | 949 | ||||
M internal/adapters/in/http/auth/handler.go +4 -1
| @@ -2,6 +2,7 @@ | |||
| 2 | 2 | package auth | |
| 3 | 3 | ||
| 4 | 4 | import ( | |
| 5 | + | "crypto/subtle" | |
| 5 | 6 | "encoding/json" | |
| 6 | 7 | "net/http" | |
| 7 | 8 | "strings" | |
| @@ -342,5 +343,7 @@ func (h *Handler) isInternalAuth(username, password string) bool { | |||
| 342 | 343 | if h.internalAuth.Username == "" || h.internalAuth.Password == "" { | |
| 343 | 344 | return false | |
| 344 | 345 | } | |
| 345 | - | return username == h.internalAuth.Username && password == h.internalAuth.Password | |
| 346 | + | usernameMatch := subtle.ConstantTimeCompare([]byte(username), []byte(h.internalAuth.Username)) == 1 | |
| 347 | + | passwordMatch := subtle.ConstantTimeCompare([]byte(password), []byte(h.internalAuth.Password)) == 1 | |
| 348 | + | return usernameMatch && passwordMatch | |
| 346 | 349 | } | |
| @@ -2,6 +2,7 @@ | |||||
| 2 | package auth | 2 | package auth | ||
| 3 | 3 | ||||
| 4 | import ( | 4 | import ( | ||
| 5 | + | "crypto/subtle" | |||
| 5 | "encoding/json" | 6 | "encoding/json" | ||
| 6 | "net/http" | 7 | "net/http" | ||
| 7 | "strings" | 8 | "strings" | ||
| @@ -342,5 +343,7 @@ func (h *Handler) isInternalAuth(username, password string) bool { | |||||
| 342 | if h.internalAuth.Username == "" || h.internalAuth.Password == "" { | 343 | if h.internalAuth.Username == "" || h.internalAuth.Password == "" { | ||
| 343 | return false | 344 | return false | ||
| 344 | } | 345 | } | ||
| 345 | - | return username == h.internalAuth.Username && password == h.internalAuth.Password | 346 | + | usernameMatch := subtle.ConstantTimeCompare([]byte(username), []byte(h.internalAuth.Username)) == 1 |
| 347 | + | passwordMatch := subtle.ConstantTimeCompare([]byte(password), []byte(h.internalAuth.Password)) == 1 | |||
| 348 | + | return usernameMatch && passwordMatch | |||
| 346 | } | 349 | } | ||
M internal/adapters/in/http/auth/handler_test.go +83 -0
| @@ -413,3 +413,86 @@ func TestIsLocalhostRequest(t *testing.T) { | |||
| 413 | 413 | }) | |
| 414 | 414 | } | |
| 415 | 415 | } | |
| 416 | + | ||
| 417 | + | func TestHandler_isInternalAuth(t *testing.T) { | |
| 418 | + | tests := []struct { | |
| 419 | + | name string | |
| 420 | + | username string | |
| 421 | + | password string | |
| 422 | + | internalAuth InternalAuth | |
| 423 | + | wantResult bool | |
| 424 | + | }{ | |
| 425 | + | { | |
| 426 | + | name: "correct credentials", | |
| 427 | + | username: "gordon-internal", | |
| 428 | + | password: "internal-secret", | |
| 429 | + | internalAuth: InternalAuth{ | |
| 430 | + | Username: "gordon-internal", | |
| 431 | + | Password: "internal-secret", | |
| 432 | + | }, | |
| 433 | + | wantResult: true, | |
| 434 | + | }, | |
| 435 | + | { | |
| 436 | + | name: "incorrect username", | |
| 437 | + | username: "wrong-username", | |
| 438 | + | password: "internal-secret", | |
| 439 | + | internalAuth: InternalAuth{ | |
| 440 | + | Username: "gordon-internal", | |
| 441 | + | Password: "internal-secret", | |
| 442 | + | }, | |
| 443 | + | wantResult: false, | |
| 444 | + | }, | |
| 445 | + | { | |
| 446 | + | name: "incorrect password", | |
| 447 | + | username: "gordon-internal", | |
| 448 | + | password: "wrong-password", | |
| 449 | + | internalAuth: InternalAuth{ | |
| 450 | + | Username: "gordon-internal", | |
| 451 | + | Password: "internal-secret", | |
| 452 | + | }, | |
| 453 | + | wantResult: false, | |
| 454 | + | }, | |
| 455 | + | { | |
| 456 | + | name: "empty internal auth credentials", | |
| 457 | + | username: "gordon-internal", | |
| 458 | + | password: "internal-secret", | |
| 459 | + | internalAuth: InternalAuth{ | |
| 460 | + | Username: "", | |
| 461 | + | Password: "", | |
| 462 | + | }, | |
| 463 | + | wantResult: false, | |
| 464 | + | }, | |
| 465 | + | { | |
| 466 | + | name: "partially empty internal auth - empty password", | |
| 467 | + | username: "gordon-internal", | |
| 468 | + | password: "internal-secret", | |
| 469 | + | internalAuth: InternalAuth{ | |
| 470 | + | Username: "gordon-internal", | |
| 471 | + | Password: "", | |
| 472 | + | }, | |
| 473 | + | wantResult: false, | |
| 474 | + | }, | |
| 475 | + | { | |
| 476 | + | name: "partially empty internal auth - empty username", | |
| 477 | + | username: "gordon-internal", | |
| 478 | + | password: "internal-secret", | |
| 479 | + | internalAuth: InternalAuth{ | |
| 480 | + | Username: "", | |
| 481 | + | Password: "internal-secret", | |
| 482 | + | }, | |
| 483 | + | wantResult: false, | |
| 484 | + | }, | |
| 485 | + | } | |
| 486 | + | ||
| 487 | + | for _, tt := range tests { | |
| 488 | + | t.Run(tt.name, func(t *testing.T) { | |
| 489 | + | h := &Handler{ | |
| 490 | + | internalAuth: tt.internalAuth, | |
| 491 | + | } | |
| 492 | + | ||
| 493 | + | result := h.isInternalAuth(tt.username, tt.password) | |
| 494 | + | ||
| 495 | + | assert.Equal(t, tt.wantResult, result) | |
| 496 | + | }) | |
| 497 | + | } | |
| 498 | + | } | |
| @@ -413,3 +413,86 @@ func TestIsLocalhostRequest(t *testing.T) { | |||||
| 413 | }) | 413 | }) | ||
| 414 | } | 414 | } | ||
| 415 | } | 415 | } | ||
| 416 | + | ||||
| 417 | + | func TestHandler_isInternalAuth(t *testing.T) { | |||
| 418 | + | tests := []struct { | |||
| 419 | + | name string | |||
| 420 | + | username string | |||
| 421 | + | password string | |||
| 422 | + | internalAuth InternalAuth | |||
| 423 | + | wantResult bool | |||
| 424 | + | }{ | |||
| 425 | + | { | |||
| 426 | + | name: "correct credentials", | |||
| 427 | + | username: "gordon-internal", | |||
| 428 | + | password: "internal-secret", | |||
| 429 | + | internalAuth: InternalAuth{ | |||
| 430 | + | Username: "gordon-internal", | |||
| 431 | + | Password: "internal-secret", | |||
| 432 | + | }, | |||
| 433 | + | wantResult: true, | |||
| 434 | + | }, | |||
| 435 | + | { | |||
| 436 | + | name: "incorrect username", | |||
| 437 | + | username: "wrong-username", | |||
| 438 | + | password: "internal-secret", | |||
| 439 | + | internalAuth: InternalAuth{ | |||
| 440 | + | Username: "gordon-internal", | |||
| 441 | + | Password: "internal-secret", | |||
| 442 | + | }, | |||
| 443 | + | wantResult: false, | |||
| 444 | + | }, | |||
| 445 | + | { | |||
| 446 | + | name: "incorrect password", | |||
| 447 | + | username: "gordon-internal", | |||
| 448 | + | password: "wrong-password", | |||
| 449 | + | internalAuth: InternalAuth{ | |||
| 450 | + | Username: "gordon-internal", | |||
| 451 | + | Password: "internal-secret", | |||
| 452 | + | }, | |||
| 453 | + | wantResult: false, | |||
| 454 | + | }, | |||
| 455 | + | { | |||
| 456 | + | name: "empty internal auth credentials", | |||
| 457 | + | username: "gordon-internal", | |||
| 458 | + | password: "internal-secret", | |||
| 459 | + | internalAuth: InternalAuth{ | |||
| 460 | + | Username: "", | |||
| 461 | + | Password: "", | |||
| 462 | + | }, | |||
| 463 | + | wantResult: false, | |||
| 464 | + | }, | |||
| 465 | + | { | |||
| 466 | + | name: "partially empty internal auth - empty password", | |||
| 467 | + | username: "gordon-internal", | |||
| 468 | + | password: "internal-secret", | |||
| 469 | + | internalAuth: InternalAuth{ | |||
| 470 | + | Username: "gordon-internal", | |||
| 471 | + | Password: "", | |||
| 472 | + | }, | |||
| 473 | + | wantResult: false, | |||
| 474 | + | }, | |||
| 475 | + | { | |||
| 476 | + | name: "partially empty internal auth - empty username", | |||
| 477 | + | username: "gordon-internal", | |||
| 478 | + | password: "internal-secret", | |||
| 479 | + | internalAuth: InternalAuth{ | |||
| 480 | + | Username: "", | |||
| 481 | + | Password: "internal-secret", | |||
| 482 | + | }, | |||
| 483 | + | wantResult: false, | |||
| 484 | + | }, | |||
| 485 | + | } | |||
| 486 | + | ||||
| 487 | + | for _, tt := range tests { | |||
| 488 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 489 | + | h := &Handler{ | |||
| 490 | + | internalAuth: tt.internalAuth, | |||
| 491 | + | } | |||
| 492 | + | ||||
| 493 | + | result := h.isInternalAuth(tt.username, tt.password) | |||
| 494 | + | ||||
| 495 | + | assert.Equal(t, tt.wantResult, result) | |||
| 496 | + | }) | |||
| 497 | + | } | |||
| 498 | + | } | |||
M internal/adapters/in/http/middleware/auth.go +1 -2
| @@ -303,8 +303,7 @@ func isLocalhostRequest(r *http.Request) bool { | |||
| 303 | 303 | // RemoteAddr includes port, e.g., "127.0.0.1:12345" or "[::1]:12345" | |
| 304 | 304 | if strings.HasPrefix(host, "127.") || | |
| 305 | 305 | strings.HasPrefix(host, "[::1]") || | |
| 306 | - | strings.HasPrefix(host, "::1") || | |
| 307 | - | strings.HasPrefix(host, "localhost") { | |
| 306 | + | strings.HasPrefix(host, "::1") { | |
| 308 | 307 | return true | |
| 309 | 308 | } | |
| 310 | 309 | return false | |
| @@ -303,8 +303,7 @@ func isLocalhostRequest(r *http.Request) bool { | |||||
| 303 | // RemoteAddr includes port, e.g., "127.0.0.1:12345" or "[::1]:12345" | 303 | // RemoteAddr includes port, e.g., "127.0.0.1:12345" or "[::1]:12345" | ||
| 304 | if strings.HasPrefix(host, "127.") || | 304 | if strings.HasPrefix(host, "127.") || | ||
| 305 | strings.HasPrefix(host, "[::1]") || | 305 | strings.HasPrefix(host, "[::1]") || | ||
| 306 | - | strings.HasPrefix(host, "::1") || | 306 | + | strings.HasPrefix(host, "::1") { |
| 307 | - | strings.HasPrefix(host, "localhost") { | |||
| 308 | return true | 307 | return true | ||
| 309 | } | 308 | } | ||
| 310 | return false | 309 | return false | ||
M internal/adapters/in/http/registry/handler_test.go +9 -9
| @@ -257,7 +257,7 @@ func TestHandler_StartBlobUpload_Success(t *testing.T) { | |||
| 257 | 257 | ||
| 258 | 258 | handler := NewHandler(registrySvc, testLogger()) | |
| 259 | 259 | ||
| 260 | - | registrySvc.EXPECT().StartUpload(mock.Anything, "myapp").Return("1234567890-myapp", nil) | |
| 260 | + | registrySvc.EXPECT().StartUpload(mock.Anything, "myapp").Return("550e8400-e29b-41d4-a716-446655440000", nil) | |
| 261 | 261 | ||
| 262 | 262 | req := httptest.NewRequest("POST", "/v2/myapp/blobs/uploads/", nil) | |
| 263 | 263 | rec := httptest.NewRecorder() | |
| @@ -265,7 +265,7 @@ func TestHandler_StartBlobUpload_Success(t *testing.T) { | |||
| 265 | 265 | handler.ServeHTTP(rec, req) | |
| 266 | 266 | ||
| 267 | 267 | assert.Equal(t, http.StatusAccepted, rec.Code) | |
| 268 | - | assert.Contains(t, rec.Header().Get("Location"), "/v2/myapp/blobs/uploads/1234567890-myapp") | |
| 268 | + | assert.Contains(t, rec.Header().Get("Location"), "/v2/myapp/blobs/uploads/550e8400-e29b-41d4-a716-446655440000") | |
| 269 | 269 | assert.Equal(t, "0-0", rec.Header().Get("Range")) | |
| 270 | 270 | } | |
| 271 | 271 | ||
| @@ -291,15 +291,15 @@ func TestHandler_BlobUpload_PATCH(t *testing.T) { | |||
| 291 | 291 | handler := NewHandler(registrySvc, testLogger()) | |
| 292 | 292 | ||
| 293 | 293 | chunkData := []byte("chunk content") | |
| 294 | - | registrySvc.EXPECT().AppendBlobChunk(mock.Anything, "myapp", "1234567890-myapp", chunkData).Return(int64(len(chunkData)), nil) | |
| 294 | + | registrySvc.EXPECT().AppendBlobChunk(mock.Anything, "myapp", "550e8400-e29b-41d4-a716-446655440000", chunkData).Return(int64(len(chunkData)), nil) | |
| 295 | 295 | ||
| 296 | - | req := httptest.NewRequest("PATCH", "/v2/myapp/blobs/uploads/1234567890-myapp", bytes.NewReader(chunkData)) | |
| 296 | + | req := httptest.NewRequest("PATCH", "/v2/myapp/blobs/uploads/550e8400-e29b-41d4-a716-446655440000", bytes.NewReader(chunkData)) | |
| 297 | 297 | rec := httptest.NewRecorder() | |
| 298 | 298 | ||
| 299 | 299 | handler.ServeHTTP(rec, req) | |
| 300 | 300 | ||
| 301 | 301 | assert.Equal(t, http.StatusAccepted, rec.Code) | |
| 302 | - | assert.Contains(t, rec.Header().Get("Location"), "/v2/myapp/blobs/uploads/1234567890-myapp") | |
| 302 | + | assert.Contains(t, rec.Header().Get("Location"), "/v2/myapp/blobs/uploads/550e8400-e29b-41d4-a716-446655440000") | |
| 303 | 303 | } | |
| 304 | 304 | ||
| 305 | 305 | func TestHandler_BlobUpload_PUT_Finalize(t *testing.T) { | |
| @@ -308,10 +308,10 @@ func TestHandler_BlobUpload_PUT_Finalize(t *testing.T) { | |||
| 308 | 308 | handler := NewHandler(registrySvc, testLogger()) | |
| 309 | 309 | ||
| 310 | 310 | chunkData := []byte("final chunk") | |
| 311 | - | registrySvc.EXPECT().AppendBlobChunk(mock.Anything, "myapp", "1234567890-myapp", chunkData).Return(int64(len(chunkData)), nil) | |
| 312 | - | registrySvc.EXPECT().FinishUpload(mock.Anything, "1234567890-myapp", "sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4").Return(nil) | |
| 311 | + | registrySvc.EXPECT().AppendBlobChunk(mock.Anything, "myapp", "550e8400-e29b-41d4-a716-446655440000", chunkData).Return(int64(len(chunkData)), nil) | |
| 312 | + | registrySvc.EXPECT().FinishUpload(mock.Anything, "550e8400-e29b-41d4-a716-446655440000", "sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4").Return(nil) | |
| 313 | 313 | ||
| 314 | - | req := httptest.NewRequest("PUT", "/v2/myapp/blobs/uploads/1234567890-myapp?digest=sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4", bytes.NewReader(chunkData)) | |
| 314 | + | req := httptest.NewRequest("PUT", "/v2/myapp/blobs/uploads/550e8400-e29b-41d4-a716-446655440000?digest=sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4", bytes.NewReader(chunkData)) | |
| 315 | 315 | rec := httptest.NewRecorder() | |
| 316 | 316 | ||
| 317 | 317 | handler.ServeHTTP(rec, req) | |
| @@ -330,7 +330,7 @@ func TestHandler_BlobUpload_PUT_DigestMismatch(t *testing.T) { | |||
| 330 | 330 | // Note: Invalid digest format is rejected by validation before reaching storage layer | |
| 331 | 331 | // This test verifies that invalid digests are properly rejected | |
| 332 | 332 | ||
| 333 | - | req := httptest.NewRequest("PUT", "/v2/myapp/blobs/uploads/1234567890-myapp?digest=sha256:wrong", bytes.NewReader(chunkData)) | |
| 333 | + | req := httptest.NewRequest("PUT", "/v2/myapp/blobs/uploads/550e8400-e29b-41d4-a716-446655440000?digest=sha256:wrong", bytes.NewReader(chunkData)) | |
| 334 | 334 | rec := httptest.NewRecorder() | |
| 335 | 335 | ||
| 336 | 336 | handler.ServeHTTP(rec, req) | |
| @@ -257,7 +257,7 @@ func TestHandler_StartBlobUpload_Success(t *testing.T) { | |||||
| 257 | 257 | ||||
| 258 | handler := NewHandler(registrySvc, testLogger()) | 258 | handler := NewHandler(registrySvc, testLogger()) | ||
| 259 | 259 | ||||
| 260 | - | registrySvc.EXPECT().StartUpload(mock.Anything, "myapp").Return("1234567890-myapp", nil) | 260 | + | registrySvc.EXPECT().StartUpload(mock.Anything, "myapp").Return("550e8400-e29b-41d4-a716-446655440000", nil) |
| 261 | 261 | ||||
| 262 | req := httptest.NewRequest("POST", "/v2/myapp/blobs/uploads/", nil) | 262 | req := httptest.NewRequest("POST", "/v2/myapp/blobs/uploads/", nil) | ||
| 263 | rec := httptest.NewRecorder() | 263 | rec := httptest.NewRecorder() | ||
| @@ -265,7 +265,7 @@ func TestHandler_StartBlobUpload_Success(t *testing.T) { | |||||
| 265 | handler.ServeHTTP(rec, req) | 265 | handler.ServeHTTP(rec, req) | ||
| 266 | 266 | ||||
| 267 | assert.Equal(t, http.StatusAccepted, rec.Code) | 267 | assert.Equal(t, http.StatusAccepted, rec.Code) | ||
| 268 | - | assert.Contains(t, rec.Header().Get("Location"), "/v2/myapp/blobs/uploads/1234567890-myapp") | 268 | + | assert.Contains(t, rec.Header().Get("Location"), "/v2/myapp/blobs/uploads/550e8400-e29b-41d4-a716-446655440000") |
| 269 | assert.Equal(t, "0-0", rec.Header().Get("Range")) | 269 | assert.Equal(t, "0-0", rec.Header().Get("Range")) | ||
| 270 | } | 270 | } | ||
| 271 | 271 | ||||
| @@ -291,15 +291,15 @@ func TestHandler_BlobUpload_PATCH(t *testing.T) { | |||||
| 291 | handler := NewHandler(registrySvc, testLogger()) | 291 | handler := NewHandler(registrySvc, testLogger()) | ||
| 292 | 292 | ||||
| 293 | chunkData := []byte("chunk content") | 293 | chunkData := []byte("chunk content") | ||
| 294 | - | registrySvc.EXPECT().AppendBlobChunk(mock.Anything, "myapp", "1234567890-myapp", chunkData).Return(int64(len(chunkData)), nil) | 294 | + | registrySvc.EXPECT().AppendBlobChunk(mock.Anything, "myapp", "550e8400-e29b-41d4-a716-446655440000", chunkData).Return(int64(len(chunkData)), nil) |
| 295 | 295 | ||||
| 296 | - | req := httptest.NewRequest("PATCH", "/v2/myapp/blobs/uploads/1234567890-myapp", bytes.NewReader(chunkData)) | 296 | + | req := httptest.NewRequest("PATCH", "/v2/myapp/blobs/uploads/550e8400-e29b-41d4-a716-446655440000", bytes.NewReader(chunkData)) |
| 297 | rec := httptest.NewRecorder() | 297 | rec := httptest.NewRecorder() | ||
| 298 | 298 | ||||
| 299 | handler.ServeHTTP(rec, req) | 299 | handler.ServeHTTP(rec, req) | ||
| 300 | 300 | ||||
| 301 | assert.Equal(t, http.StatusAccepted, rec.Code) | 301 | assert.Equal(t, http.StatusAccepted, rec.Code) | ||
| 302 | - | assert.Contains(t, rec.Header().Get("Location"), "/v2/myapp/blobs/uploads/1234567890-myapp") | 302 | + | assert.Contains(t, rec.Header().Get("Location"), "/v2/myapp/blobs/uploads/550e8400-e29b-41d4-a716-446655440000") |
| 303 | } | 303 | } | ||
| 304 | 304 | ||||
| 305 | func TestHandler_BlobUpload_PUT_Finalize(t *testing.T) { | 305 | func TestHandler_BlobUpload_PUT_Finalize(t *testing.T) { | ||
| @@ -308,10 +308,10 @@ func TestHandler_BlobUpload_PUT_Finalize(t *testing.T) { | |||||
| 308 | handler := NewHandler(registrySvc, testLogger()) | 308 | handler := NewHandler(registrySvc, testLogger()) | ||
| 309 | 309 | ||||
| 310 | chunkData := []byte("final chunk") | 310 | chunkData := []byte("final chunk") | ||
| 311 | - | registrySvc.EXPECT().AppendBlobChunk(mock.Anything, "myapp", "1234567890-myapp", chunkData).Return(int64(len(chunkData)), nil) | 311 | + | registrySvc.EXPECT().AppendBlobChunk(mock.Anything, "myapp", "550e8400-e29b-41d4-a716-446655440000", chunkData).Return(int64(len(chunkData)), nil) |
| 312 | - | registrySvc.EXPECT().FinishUpload(mock.Anything, "1234567890-myapp", "sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4").Return(nil) | 312 | + | registrySvc.EXPECT().FinishUpload(mock.Anything, "550e8400-e29b-41d4-a716-446655440000", "sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4").Return(nil) |
| 313 | 313 | ||||
| 314 | - | req := httptest.NewRequest("PUT", "/v2/myapp/blobs/uploads/1234567890-myapp?digest=sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4", bytes.NewReader(chunkData)) | 314 | + | req := httptest.NewRequest("PUT", "/v2/myapp/blobs/uploads/550e8400-e29b-41d4-a716-446655440000?digest=sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4", bytes.NewReader(chunkData)) |
| 315 | rec := httptest.NewRecorder() | 315 | rec := httptest.NewRecorder() | ||
| 316 | 316 | ||||
| 317 | handler.ServeHTTP(rec, req) | 317 | handler.ServeHTTP(rec, req) | ||
| @@ -330,7 +330,7 @@ func TestHandler_BlobUpload_PUT_DigestMismatch(t *testing.T) { | |||||
| 330 | // Note: Invalid digest format is rejected by validation before reaching storage layer | 330 | // Note: Invalid digest format is rejected by validation before reaching storage layer | ||
| 331 | // This test verifies that invalid digests are properly rejected | 331 | // This test verifies that invalid digests are properly rejected | ||
| 332 | 332 | ||||
| 333 | - | req := httptest.NewRequest("PUT", "/v2/myapp/blobs/uploads/1234567890-myapp?digest=sha256:wrong", bytes.NewReader(chunkData)) | 333 | + | req := httptest.NewRequest("PUT", "/v2/myapp/blobs/uploads/550e8400-e29b-41d4-a716-446655440000?digest=sha256:wrong", bytes.NewReader(chunkData)) |
| 334 | rec := httptest.NewRecorder() | 334 | rec := httptest.NewRecorder() | ||
| 335 | 335 | ||||
| 336 | handler.ServeHTTP(rec, req) | 336 | handler.ServeHTTP(rec, req) | ||
M internal/adapters/out/eventbus/inmemory.go +33 -5
| @@ -74,7 +74,15 @@ func (bus *InMemory) Publish(eventType domain.EventType, payload any) error { | |||
| 74 | 74 | return nil | |
| 75 | 75 | case <-bus.ctx.Done(): | |
| 76 | 76 | return fmt.Errorf("event bus is stopped") | |
| 77 | - | default: | |
| 77 | + | case <-time.After(5 * time.Second): | |
| 78 | + | bus.log.Error(). | |
| 79 | + | Str(zerowrap.FieldLayer, "adapter"). | |
| 80 | + | Str(zerowrap.FieldAdapter, "eventbus"). | |
| 81 | + | Str("event_id", event.ID). | |
| 82 | + | Str(zerowrap.FieldEvent, string(event.Type)). | |
| 83 | + | Str("image_name", event.ImageName). | |
| 84 | + | Str("tag", event.Tag). | |
| 85 | + | Msg("event channel is full, dropping event after 5s timeout") | |
| 78 | 86 | return fmt.Errorf("event channel is full, dropping event %s", event.ID) | |
| 79 | 87 | } | |
| 80 | 88 | } | |
| @@ -178,10 +186,20 @@ func (bus *InMemory) handleEvent(event domain.Event) { | |||
| 178 | 186 | ||
| 179 | 187 | for _, handler := range handlers { | |
| 180 | 188 | if handler.CanHandle(event.Type) { | |
| 181 | - | go func(h out.EventHandler) { | |
| 182 | - | start := time.Now() | |
| 189 | + | h := handler // shadow loop variable for goroutine capture | |
| 190 | + | start := time.Now() | |
| 191 | + | ||
| 192 | + | ctx, cancel := context.WithTimeout(bus.ctx, 30*time.Second) | |
| 193 | + | ||
| 194 | + | done := make(chan error, 1) | |
| 195 | + | go func() { | |
| 196 | + | done <- h.Handle(ctx, event) | |
| 197 | + | }() | |
| 183 | 198 | ||
| 184 | - | if err := h.Handle(event); err != nil { | |
| 199 | + | select { | |
| 200 | + | case err := <-done: | |
| 201 | + | cancel() | |
| 202 | + | if err != nil { | |
| 185 | 203 | bus.log.Error(). | |
| 186 | 204 | Str(zerowrap.FieldLayer, "adapter"). | |
| 187 | 205 | Str(zerowrap.FieldAdapter, "eventbus"). | |
| @@ -200,7 +218,17 @@ func (bus *InMemory) handleEvent(event domain.Event) { | |||
| 200 | 218 | Dur(zerowrap.FieldDuration, time.Since(start)). | |
| 201 | 219 | Msg("event handled successfully") | |
| 202 | 220 | } | |
| 203 | - | }(handler) | |
| 221 | + | case <-ctx.Done(): | |
| 222 | + | cancel() | |
| 223 | + | bus.log.Warn(). | |
| 224 | + | Str(zerowrap.FieldLayer, "adapter"). | |
| 225 | + | Str(zerowrap.FieldAdapter, "eventbus"). | |
| 226 | + | Str("event_id", event.ID). | |
| 227 | + | Str(zerowrap.FieldEvent, string(event.Type)). | |
| 228 | + | Str(zerowrap.FieldHandler, fmt.Sprintf("%T", h)). | |
| 229 | + | Dur(zerowrap.FieldDuration, time.Since(start)). | |
| 230 | + | Msg("handler timeout after 30s") | |
| 231 | + | } | |
| 204 | 232 | } | |
| 205 | 233 | } | |
| 206 | 234 | } | |
| @@ -74,7 +74,15 @@ func (bus *InMemory) Publish(eventType domain.EventType, payload any) error { | |||||
| 74 | return nil | 74 | return nil | ||
| 75 | case <-bus.ctx.Done(): | 75 | case <-bus.ctx.Done(): | ||
| 76 | return fmt.Errorf("event bus is stopped") | 76 | return fmt.Errorf("event bus is stopped") | ||
| 77 | - | default: | 77 | + | case <-time.After(5 * time.Second): |
| 78 | + | bus.log.Error(). | |||
| 79 | + | Str(zerowrap.FieldLayer, "adapter"). | |||
| 80 | + | Str(zerowrap.FieldAdapter, "eventbus"). | |||
| 81 | + | Str("event_id", event.ID). | |||
| 82 | + | Str(zerowrap.FieldEvent, string(event.Type)). | |||
| 83 | + | Str("image_name", event.ImageName). | |||
| 84 | + | Str("tag", event.Tag). | |||
| 85 | + | Msg("event channel is full, dropping event after 5s timeout") | |||
| 78 | return fmt.Errorf("event channel is full, dropping event %s", event.ID) | 86 | return fmt.Errorf("event channel is full, dropping event %s", event.ID) | ||
| 79 | } | 87 | } | ||
| 80 | } | 88 | } | ||
| @@ -178,10 +186,20 @@ func (bus *InMemory) handleEvent(event domain.Event) { | |||||
| 178 | 186 | ||||
| 179 | for _, handler := range handlers { | 187 | for _, handler := range handlers { | ||
| 180 | if handler.CanHandle(event.Type) { | 188 | if handler.CanHandle(event.Type) { | ||
| 181 | - | go func(h out.EventHandler) { | 189 | + | h := handler // shadow loop variable for goroutine capture |
| 182 | - | start := time.Now() | 190 | + | start := time.Now() |
| 191 | + | ||||
| 192 | + | ctx, cancel := context.WithTimeout(bus.ctx, 30*time.Second) | |||
| 193 | + | ||||
| 194 | + | done := make(chan error, 1) | |||
| 195 | + | go func() { | |||
| 196 | + | done <- h.Handle(ctx, event) | |||
| 197 | + | }() | |||
| 183 | 198 | ||||
| 184 | - | if err := h.Handle(event); err != nil { | 199 | + | select { |
| 200 | + | case err := <-done: | |||
| 201 | + | cancel() | |||
| 202 | + | if err != nil { | |||
| 185 | bus.log.Error(). | 203 | bus.log.Error(). | ||
| 186 | Str(zerowrap.FieldLayer, "adapter"). | 204 | Str(zerowrap.FieldLayer, "adapter"). | ||
| 187 | Str(zerowrap.FieldAdapter, "eventbus"). | 205 | Str(zerowrap.FieldAdapter, "eventbus"). | ||
| @@ -200,7 +218,17 @@ func (bus *InMemory) handleEvent(event domain.Event) { | |||||
| 200 | Dur(zerowrap.FieldDuration, time.Since(start)). | 218 | Dur(zerowrap.FieldDuration, time.Since(start)). | ||
| 201 | Msg("event handled successfully") | 219 | Msg("event handled successfully") | ||
| 202 | } | 220 | } | ||
| 203 | - | }(handler) | 221 | + | case <-ctx.Done(): |
| 222 | + | cancel() | |||
| 223 | + | bus.log.Warn(). | |||
| 224 | + | Str(zerowrap.FieldLayer, "adapter"). | |||
| 225 | + | Str(zerowrap.FieldAdapter, "eventbus"). | |||
| 226 | + | Str("event_id", event.ID). | |||
| 227 | + | Str(zerowrap.FieldEvent, string(event.Type)). | |||
| 228 | + | Str(zerowrap.FieldHandler, fmt.Sprintf("%T", h)). | |||
| 229 | + | Dur(zerowrap.FieldDuration, time.Since(start)). | |||
| 230 | + | Msg("handler timeout after 30s") | |||
| 231 | + | } | |||
| 204 | } | 232 | } | ||
| 205 | } | 233 | } | ||
| 206 | } | 234 | } | ||
M internal/adapters/out/filesystem/blob.go +5 -6
| @@ -11,9 +11,9 @@ import ( | |||
| 11 | 11 | "os" | |
| 12 | 12 | "path/filepath" | |
| 13 | 13 | "strings" | |
| 14 | - | "time" | |
| 15 | 14 | ||
| 16 | 15 | "github.com/bnema/zerowrap" | |
| 16 | + | "github.com/google/uuid" | |
| 17 | 17 | ||
| 18 | 18 | "github.com/bnema/gordon/pkg/validation" | |
| 19 | 19 | ) | |
| @@ -167,10 +167,9 @@ func (s *BlobStorage) BlobExists(digest string) bool { | |||
| 167 | 167 | ||
| 168 | 168 | // StartBlobUpload starts a new blob upload and returns the upload UUID. | |
| 169 | 169 | func (s *BlobStorage) StartBlobUpload(name string) (string, error) { | |
| 170 | - | // Generate UUID-like upload ID using timestamp | |
| 171 | - | uuid := fmt.Sprintf("%d-%s", time.Now().UnixNano(), strings.ReplaceAll(name, "/", "_")) | |
| 170 | + | uploadID := uuid.New().String() | |
| 172 | 171 | ||
| 173 | - | uploadPath, err := s.getUploadPath(uuid) | |
| 172 | + | uploadPath, err := s.getUploadPath(uploadID) | |
| 174 | 173 | if err != nil { | |
| 175 | 174 | return "", fmt.Errorf("invalid upload path: %w", err) | |
| 176 | 175 | } | |
| @@ -190,11 +189,11 @@ func (s *BlobStorage) StartBlobUpload(name string) (string, error) { | |||
| 190 | 189 | s.log.Info(). | |
| 191 | 190 | Str(zerowrap.FieldLayer, "adapter"). | |
| 192 | 191 | Str(zerowrap.FieldAdapter, "filesystem"). | |
| 193 | - | Str("uuid", uuid). | |
| 192 | + | Str("uuid", uploadID). | |
| 194 | 193 | Str("name", name). | |
| 195 | 194 | Msg("blob upload started") | |
| 196 | 195 | ||
| 197 | - | return uuid, nil | |
| 196 | + | return uploadID, nil | |
| 198 | 197 | } | |
| 199 | 198 | ||
| 200 | 199 | // AppendBlobChunk appends data to an in-progress upload. | |
| @@ -11,9 +11,9 @@ import ( | |||||
| 11 | "os" | 11 | "os" | ||
| 12 | "path/filepath" | 12 | "path/filepath" | ||
| 13 | "strings" | 13 | "strings" | ||
| 14 | - | "time" | |||
| 15 | 14 | ||||
| 16 | "github.com/bnema/zerowrap" | 15 | "github.com/bnema/zerowrap" | ||
| 16 | + | "github.com/google/uuid" | |||
| 17 | 17 | ||||
| 18 | "github.com/bnema/gordon/pkg/validation" | 18 | "github.com/bnema/gordon/pkg/validation" | ||
| 19 | ) | 19 | ) | ||
| @@ -167,10 +167,9 @@ func (s *BlobStorage) BlobExists(digest string) bool { | |||||
| 167 | 167 | ||||
| 168 | // StartBlobUpload starts a new blob upload and returns the upload UUID. | 168 | // StartBlobUpload starts a new blob upload and returns the upload UUID. | ||
| 169 | func (s *BlobStorage) StartBlobUpload(name string) (string, error) { | 169 | func (s *BlobStorage) StartBlobUpload(name string) (string, error) { | ||
| 170 | - | // Generate UUID-like upload ID using timestamp | 170 | + | uploadID := uuid.New().String() |
| 171 | - | uuid := fmt.Sprintf("%d-%s", time.Now().UnixNano(), strings.ReplaceAll(name, "/", "_")) | |||
| 172 | 171 | ||||
| 173 | - | uploadPath, err := s.getUploadPath(uuid) | 172 | + | uploadPath, err := s.getUploadPath(uploadID) |
| 174 | if err != nil { | 173 | if err != nil { | ||
| 175 | return "", fmt.Errorf("invalid upload path: %w", err) | 174 | return "", fmt.Errorf("invalid upload path: %w", err) | ||
| 176 | } | 175 | } | ||
| @@ -190,11 +189,11 @@ func (s *BlobStorage) StartBlobUpload(name string) (string, error) { | |||||
| 190 | s.log.Info(). | 189 | s.log.Info(). | ||
| 191 | Str(zerowrap.FieldLayer, "adapter"). | 190 | Str(zerowrap.FieldLayer, "adapter"). | ||
| 192 | Str(zerowrap.FieldAdapter, "filesystem"). | 191 | Str(zerowrap.FieldAdapter, "filesystem"). | ||
| 193 | - | Str("uuid", uuid). | 192 | + | Str("uuid", uploadID). |
| 194 | Str("name", name). | 193 | Str("name", name). | ||
| 195 | Msg("blob upload started") | 194 | Msg("blob upload started") | ||
| 196 | 195 | ||||
| 197 | - | return uuid, nil | 196 | + | return uploadID, nil |
| 198 | } | 197 | } | ||
| 199 | 198 | ||||
| 200 | // AppendBlobChunk appends data to an in-progress upload. | 199 | // AppendBlobChunk appends data to an in-progress upload. | ||
M internal/adapters/out/filesystem/blob_test.go +4 -4
| @@ -222,7 +222,7 @@ func TestBlobStorage_StartBlobUpload(t *testing.T) { | |||
| 222 | 222 | ||
| 223 | 223 | require.NoError(t, err) | |
| 224 | 224 | assert.NotEmpty(t, uuid) | |
| 225 | - | assert.Contains(t, uuid, "myapp") | |
| 225 | + | assert.Len(t, uuid, 36) // standard UUID format | |
| 226 | 226 | ||
| 227 | 227 | // Verify upload file was created | |
| 228 | 228 | uploadPath := filepath.Join(tmpDir, "uploads", uuid) | |
| @@ -260,7 +260,7 @@ func TestBlobStorage_AppendBlobChunk_NotFound(t *testing.T) { | |||
| 260 | 260 | storage, err := NewBlobStorage(tmpDir, log) | |
| 261 | 261 | require.NoError(t, err) | |
| 262 | 262 | ||
| 263 | - | _, err = storage.AppendBlobChunk("myapp", "1234567890-invalid", []byte("data")) | |
| 263 | + | _, err = storage.AppendBlobChunk("myapp", "00000000-0000-4000-a000-000000000000", []byte("data")) | |
| 264 | 264 | ||
| 265 | 265 | assert.Error(t, err) | |
| 266 | 266 | assert.Contains(t, err.Error(), "upload not found") | |
| @@ -292,7 +292,7 @@ func TestBlobStorage_GetBlobUpload_NotFound(t *testing.T) { | |||
| 292 | 292 | storage, err := NewBlobStorage(tmpDir, log) | |
| 293 | 293 | require.NoError(t, err) | |
| 294 | 294 | ||
| 295 | - | writer, err := storage.GetBlobUpload("1234567890-invalid") | |
| 295 | + | writer, err := storage.GetBlobUpload("00000000-0000-4000-a000-000000000000") | |
| 296 | 296 | ||
| 297 | 297 | assert.Error(t, err) | |
| 298 | 298 | assert.Nil(t, writer) | |
| @@ -355,7 +355,7 @@ func TestBlobStorage_CancelBlobUpload_NotFound(t *testing.T) { | |||
| 355 | 355 | storage, err := NewBlobStorage(tmpDir, log) | |
| 356 | 356 | require.NoError(t, err) | |
| 357 | 357 | ||
| 358 | - | err = storage.CancelBlobUpload("1234567890-invalid") | |
| 358 | + | err = storage.CancelBlobUpload("00000000-0000-4000-a000-000000000000") | |
| 359 | 359 | ||
| 360 | 360 | assert.Error(t, err) | |
| 361 | 361 | assert.Contains(t, err.Error(), "upload not found") | |
| @@ -222,7 +222,7 @@ func TestBlobStorage_StartBlobUpload(t *testing.T) { | |||||
| 222 | 222 | ||||
| 223 | require.NoError(t, err) | 223 | require.NoError(t, err) | ||
| 224 | assert.NotEmpty(t, uuid) | 224 | assert.NotEmpty(t, uuid) | ||
| 225 | - | assert.Contains(t, uuid, "myapp") | 225 | + | assert.Len(t, uuid, 36) // standard UUID format |
| 226 | 226 | ||||
| 227 | // Verify upload file was created | 227 | // Verify upload file was created | ||
| 228 | uploadPath := filepath.Join(tmpDir, "uploads", uuid) | 228 | uploadPath := filepath.Join(tmpDir, "uploads", uuid) | ||
| @@ -260,7 +260,7 @@ func TestBlobStorage_AppendBlobChunk_NotFound(t *testing.T) { | |||||
| 260 | storage, err := NewBlobStorage(tmpDir, log) | 260 | storage, err := NewBlobStorage(tmpDir, log) | ||
| 261 | require.NoError(t, err) | 261 | require.NoError(t, err) | ||
| 262 | 262 | ||||
| 263 | - | _, err = storage.AppendBlobChunk("myapp", "1234567890-invalid", []byte("data")) | 263 | + | _, err = storage.AppendBlobChunk("myapp", "00000000-0000-4000-a000-000000000000", []byte("data")) |
| 264 | 264 | ||||
| 265 | assert.Error(t, err) | 265 | assert.Error(t, err) | ||
| 266 | assert.Contains(t, err.Error(), "upload not found") | 266 | assert.Contains(t, err.Error(), "upload not found") | ||
| @@ -292,7 +292,7 @@ func TestBlobStorage_GetBlobUpload_NotFound(t *testing.T) { | |||||
| 292 | storage, err := NewBlobStorage(tmpDir, log) | 292 | storage, err := NewBlobStorage(tmpDir, log) | ||
| 293 | require.NoError(t, err) | 293 | require.NoError(t, err) | ||
| 294 | 294 | ||||
| 295 | - | writer, err := storage.GetBlobUpload("1234567890-invalid") | 295 | + | writer, err := storage.GetBlobUpload("00000000-0000-4000-a000-000000000000") |
| 296 | 296 | ||||
| 297 | assert.Error(t, err) | 297 | assert.Error(t, err) | ||
| 298 | assert.Nil(t, writer) | 298 | assert.Nil(t, writer) | ||
| @@ -355,7 +355,7 @@ func TestBlobStorage_CancelBlobUpload_NotFound(t *testing.T) { | |||||
| 355 | storage, err := NewBlobStorage(tmpDir, log) | 355 | storage, err := NewBlobStorage(tmpDir, log) | ||
| 356 | require.NoError(t, err) | 356 | require.NoError(t, err) | ||
| 357 | 357 | ||||
| 358 | - | err = storage.CancelBlobUpload("1234567890-invalid") | 358 | + | err = storage.CancelBlobUpload("00000000-0000-4000-a000-000000000000") |
| 359 | 359 | ||||
| 360 | assert.Error(t, err) | 360 | assert.Error(t, err) | ||
| 361 | assert.Contains(t, err.Error(), "upload not found") | 361 | assert.Contains(t, err.Error(), "upload not found") | ||
M internal/adapters/out/tokenstore/pass.go +38 -2
| @@ -19,6 +19,11 @@ import ( | |||
| 19 | 19 | // ansiRegex matches ANSI escape sequences for stripping from pass output. | |
| 20 | 20 | var ansiRegex = regexp.MustCompile(`\x1b\[[0-9;]*m`) | |
| 21 | 21 | ||
| 22 | + | // subjectRegex validates token subjects to prevent path injection. | |
| 23 | + | // Allows alphanumeric characters, forward slashes, underscores, dots, hyphens, and @. | |
| 24 | + | // This prevents path traversal (../) and command injection attacks. | |
| 25 | + | var subjectRegex = regexp.MustCompile(`^[a-zA-Z0-9/_.@-]+$`) | |
| 26 | + | ||
| 22 | 27 | const ( | |
| 23 | 28 | // passTokenPath is the base path for tokens in pass. | |
| 24 | 29 | passTokenPath = "gordon/registry/tokens" //nolint:gosec // Not a credential, this is a pass store path | |
| @@ -32,6 +37,24 @@ type cachedToken struct { | |||
| 32 | 37 | token *domain.Token | |
| 33 | 38 | } | |
| 34 | 39 | ||
| 40 | + | // validateSubject validates a token subject to prevent path injection and command injection. | |
| 41 | + | // Returns an error if the subject is invalid. | |
| 42 | + | func validateSubject(subject string) error { | |
| 43 | + | if subject == "" { | |
| 44 | + | return fmt.Errorf("subject cannot be empty") | |
| 45 | + | } | |
| 46 | + | ||
| 47 | + | if !subjectRegex.MatchString(subject) { | |
| 48 | + | return fmt.Errorf("invalid subject: must contain only alphanumeric characters, /, _, ., @, -") | |
| 49 | + | } | |
| 50 | + | ||
| 51 | + | if strings.Contains(subject, "..") { | |
| 52 | + | return fmt.Errorf("invalid subject: cannot contain '..' to prevent path traversal") | |
| 53 | + | } | |
| 54 | + | ||
| 55 | + | return nil | |
| 56 | + | } | |
| 57 | + | ||
| 35 | 58 | // PassStore implements TokenStore using the pass password manager. | |
| 36 | 59 | type PassStore struct { | |
| 37 | 60 | timeout time.Duration | |
| @@ -66,6 +89,10 @@ type tokenMetadata struct { | |||
| 66 | 89 | ||
| 67 | 90 | // SaveToken stores a token JWT and metadata in pass. | |
| 68 | 91 | func (s *PassStore) SaveToken(ctx context.Context, token *domain.Token, jwt string) error { | |
| 92 | + | if err := validateSubject(token.Subject); err != nil { | |
| 93 | + | return err | |
| 94 | + | } | |
| 95 | + | ||
| 69 | 96 | ctx, cancel := context.WithTimeout(ctx, s.timeout) | |
| 70 | 97 | defer cancel() | |
| 71 | 98 | ||
| @@ -111,6 +138,10 @@ func (s *PassStore) SaveToken(ctx context.Context, token *domain.Token, jwt stri | |||
| 111 | 138 | ||
| 112 | 139 | // GetToken retrieves token JWT by subject from pass. | |
| 113 | 140 | func (s *PassStore) GetToken(ctx context.Context, subject string) (string, *domain.Token, error) { | |
| 141 | + | if err := validateSubject(subject); err != nil { | |
| 142 | + | return "", nil, err | |
| 143 | + | } | |
| 144 | + | ||
| 114 | 145 | // Check cache first | |
| 115 | 146 | s.cacheMu.RLock() | |
| 116 | 147 | if cached, ok := s.tokenCache[subject]; ok { | |
| @@ -212,6 +243,9 @@ func (s *PassStore) ListTokens(ctx context.Context) ([]domain.Token, error) { | |||
| 212 | 243 | ||
| 213 | 244 | // Revoke adds token ID to revocation list in pass. | |
| 214 | 245 | func (s *PassStore) Revoke(ctx context.Context, tokenID string) error { | |
| 246 | + | s.cacheMu.Lock() | |
| 247 | + | defer s.cacheMu.Unlock() | |
| 248 | + | ||
| 215 | 249 | ctx, cancel := context.WithTimeout(ctx, s.timeout) | |
| 216 | 250 | defer cancel() | |
| 217 | 251 | ||
| @@ -240,13 +274,11 @@ func (s *PassStore) Revoke(ctx context.Context, tokenID string) error { | |||
| 240 | 274 | } | |
| 241 | 275 | ||
| 242 | 276 | // Update cache | |
| 243 | - | s.cacheMu.Lock() | |
| 244 | 277 | s.revokedList = revokedList | |
| 245 | 278 | s.revokedSet = make(map[string]struct{}, len(revokedList)) | |
| 246 | 279 | for _, id := range revokedList { | |
| 247 | 280 | s.revokedSet[id] = struct{}{} | |
| 248 | 281 | } | |
| 249 | - | s.cacheMu.Unlock() | |
| 250 | 282 | ||
| 251 | 283 | s.log.Info(). | |
| 252 | 284 | Str(zerowrap.FieldLayer, "adapter"). | |
| @@ -291,6 +323,10 @@ func (s *PassStore) IsRevoked(ctx context.Context, tokenID string) (bool, error) | |||
| 291 | 323 | ||
| 292 | 324 | // DeleteToken removes token from pass. | |
| 293 | 325 | func (s *PassStore) DeleteToken(ctx context.Context, subject string) error { | |
| 326 | + | if err := validateSubject(subject); err != nil { | |
| 327 | + | return err | |
| 328 | + | } | |
| 329 | + | ||
| 294 | 330 | ctx, cancel := context.WithTimeout(ctx, s.timeout) | |
| 295 | 331 | defer cancel() | |
| 296 | 332 | ||
| @@ -19,6 +19,11 @@ import ( | |||||
| 19 | // ansiRegex matches ANSI escape sequences for stripping from pass output. | 19 | // ansiRegex matches ANSI escape sequences for stripping from pass output. | ||
| 20 | var ansiRegex = regexp.MustCompile(`\x1b\[[0-9;]*m`) | 20 | var ansiRegex = regexp.MustCompile(`\x1b\[[0-9;]*m`) | ||
| 21 | 21 | ||||
| 22 | + | // subjectRegex validates token subjects to prevent path injection. | |||
| 23 | + | // Allows alphanumeric characters, forward slashes, underscores, dots, hyphens, and @. | |||
| 24 | + | // This prevents path traversal (../) and command injection attacks. | |||
| 25 | + | var subjectRegex = regexp.MustCompile(`^[a-zA-Z0-9/_.@-]+$`) | |||
| 26 | + | ||||
| 22 | const ( | 27 | const ( | ||
| 23 | // passTokenPath is the base path for tokens in pass. | 28 | // passTokenPath is the base path for tokens in pass. | ||
| 24 | passTokenPath = "gordon/registry/tokens" //nolint:gosec // Not a credential, this is a pass store path | 29 | passTokenPath = "gordon/registry/tokens" //nolint:gosec // Not a credential, this is a pass store path | ||
| @@ -32,6 +37,24 @@ type cachedToken struct { | |||||
| 32 | token *domain.Token | 37 | token *domain.Token | ||
| 33 | } | 38 | } | ||
| 34 | 39 | ||||
| 40 | + | // validateSubject validates a token subject to prevent path injection and command injection. | |||
| 41 | + | // Returns an error if the subject is invalid. | |||
| 42 | + | func validateSubject(subject string) error { | |||
| 43 | + | if subject == "" { | |||
| 44 | + | return fmt.Errorf("subject cannot be empty") | |||
| 45 | + | } | |||
| 46 | + | ||||
| 47 | + | if !subjectRegex.MatchString(subject) { | |||
| 48 | + | return fmt.Errorf("invalid subject: must contain only alphanumeric characters, /, _, ., @, -") | |||
| 49 | + | } | |||
| 50 | + | ||||
| 51 | + | if strings.Contains(subject, "..") { | |||
| 52 | + | return fmt.Errorf("invalid subject: cannot contain '..' to prevent path traversal") | |||
| 53 | + | } | |||
| 54 | + | ||||
| 55 | + | return nil | |||
| 56 | + | } | |||
| 57 | + | ||||
| 35 | // PassStore implements TokenStore using the pass password manager. | 58 | // PassStore implements TokenStore using the pass password manager. | ||
| 36 | type PassStore struct { | 59 | type PassStore struct { | ||
| 37 | timeout time.Duration | 60 | timeout time.Duration | ||
| @@ -66,6 +89,10 @@ type tokenMetadata struct { | |||||
| 66 | 89 | ||||
| 67 | // SaveToken stores a token JWT and metadata in pass. | 90 | // SaveToken stores a token JWT and metadata in pass. | ||
| 68 | func (s *PassStore) SaveToken(ctx context.Context, token *domain.Token, jwt string) error { | 91 | func (s *PassStore) SaveToken(ctx context.Context, token *domain.Token, jwt string) error { | ||
| 92 | + | if err := validateSubject(token.Subject); err != nil { | |||
| 93 | + | return err | |||
| 94 | + | } | |||
| 95 | + | ||||
| 69 | ctx, cancel := context.WithTimeout(ctx, s.timeout) | 96 | ctx, cancel := context.WithTimeout(ctx, s.timeout) | ||
| 70 | defer cancel() | 97 | defer cancel() | ||
| 71 | 98 | ||||
| @@ -111,6 +138,10 @@ func (s *PassStore) SaveToken(ctx context.Context, token *domain.Token, jwt stri | |||||
| 111 | 138 | ||||
| 112 | // GetToken retrieves token JWT by subject from pass. | 139 | // GetToken retrieves token JWT by subject from pass. | ||
| 113 | func (s *PassStore) GetToken(ctx context.Context, subject string) (string, *domain.Token, error) { | 140 | func (s *PassStore) GetToken(ctx context.Context, subject string) (string, *domain.Token, error) { | ||
| 141 | + | if err := validateSubject(subject); err != nil { | |||
| 142 | + | return "", nil, err | |||
| 143 | + | } | |||
| 144 | + | ||||
| 114 | // Check cache first | 145 | // Check cache first | ||
| 115 | s.cacheMu.RLock() | 146 | s.cacheMu.RLock() | ||
| 116 | if cached, ok := s.tokenCache[subject]; ok { | 147 | if cached, ok := s.tokenCache[subject]; ok { | ||
| @@ -212,6 +243,9 @@ func (s *PassStore) ListTokens(ctx context.Context) ([]domain.Token, error) { | |||||
| 212 | 243 | ||||
| 213 | // Revoke adds token ID to revocation list in pass. | 244 | // Revoke adds token ID to revocation list in pass. | ||
| 214 | func (s *PassStore) Revoke(ctx context.Context, tokenID string) error { | 245 | func (s *PassStore) Revoke(ctx context.Context, tokenID string) error { | ||
| 246 | + | s.cacheMu.Lock() | |||
| 247 | + | defer s.cacheMu.Unlock() | |||
| 248 | + | ||||
| 215 | ctx, cancel := context.WithTimeout(ctx, s.timeout) | 249 | ctx, cancel := context.WithTimeout(ctx, s.timeout) | ||
| 216 | defer cancel() | 250 | defer cancel() | ||
| 217 | 251 | ||||
| @@ -240,13 +274,11 @@ func (s *PassStore) Revoke(ctx context.Context, tokenID string) error { | |||||
| 240 | } | 274 | } | ||
| 241 | 275 | ||||
| 242 | // Update cache | 276 | // Update cache | ||
| 243 | - | s.cacheMu.Lock() | |||
| 244 | s.revokedList = revokedList | 277 | s.revokedList = revokedList | ||
| 245 | s.revokedSet = make(map[string]struct{}, len(revokedList)) | 278 | s.revokedSet = make(map[string]struct{}, len(revokedList)) | ||
| 246 | for _, id := range revokedList { | 279 | for _, id := range revokedList { | ||
| 247 | s.revokedSet[id] = struct{}{} | 280 | s.revokedSet[id] = struct{}{} | ||
| 248 | } | 281 | } | ||
| 249 | - | s.cacheMu.Unlock() | |||
| 250 | 282 | ||||
| 251 | s.log.Info(). | 283 | s.log.Info(). | ||
| 252 | Str(zerowrap.FieldLayer, "adapter"). | 284 | Str(zerowrap.FieldLayer, "adapter"). | ||
| @@ -291,6 +323,10 @@ func (s *PassStore) IsRevoked(ctx context.Context, tokenID string) (bool, error) | |||||
| 291 | 323 | ||||
| 292 | // DeleteToken removes token from pass. | 324 | // DeleteToken removes token from pass. | ||
| 293 | func (s *PassStore) DeleteToken(ctx context.Context, subject string) error { | 325 | func (s *PassStore) DeleteToken(ctx context.Context, subject string) error { | ||
| 326 | + | if err := validateSubject(subject); err != nil { | |||
| 327 | + | return err | |||
| 328 | + | } | |||
| 329 | + | ||||
| 294 | ctx, cancel := context.WithTimeout(ctx, s.timeout) | 330 | ctx, cancel := context.WithTimeout(ctx, s.timeout) | ||
| 295 | defer cancel() | 331 | defer cancel() | ||
| 296 | 332 | ||||
M internal/adapters/out/tokenstore/pass_test.go +94 -0
| @@ -258,3 +258,97 @@ func TestPassStore_RevokeUpdatesCache(t *testing.T) { | |||
| 258 | 258 | require.NoError(t, err) | |
| 259 | 259 | assert.True(t, revoked) | |
| 260 | 260 | } | |
| 261 | + | ||
| 262 | + | func TestPassStore_SaveToken_RejectsInvalidSubject(t *testing.T) { | |
| 263 | + | store := newTestPassStore() | |
| 264 | + | ||
| 265 | + | tests := []struct { | |
| 266 | + | name string | |
| 267 | + | subject string | |
| 268 | + | }{ | |
| 269 | + | {"path traversal with ../", "../../../etc/passwd"}, | |
| 270 | + | {"path traversal with ..\\", "..\\..\\windows\\system32"}, | |
| 271 | + | {"shell metacharacter semicolon", "user;rm -rf /"}, | |
| 272 | + | {"shell metacharacter pipe", "user|cat /etc/passwd"}, | |
| 273 | + | {"shell metacharacter ampersand", "user&&malicious"}, | |
| 274 | + | {"shell metacharacter backtick", "user`evil`"}, | |
| 275 | + | {"shell metacharacter dollar sign", "user$(evil)"}, | |
| 276 | + | {"newline character", "user\nmalicious"}, | |
| 277 | + | {"null byte", "user\x00malicious"}, | |
| 278 | + | } | |
| 279 | + | ||
| 280 | + | for _, tt := range tests { | |
| 281 | + | t.Run(tt.name, func(t *testing.T) { | |
| 282 | + | token := &domain.Token{ | |
| 283 | + | ID: "test-id", | |
| 284 | + | Subject: tt.subject, | |
| 285 | + | Scopes: []string{"push"}, | |
| 286 | + | IssuedAt: time.Now(), | |
| 287 | + | ExpiresAt: time.Now().Add(24 * time.Hour), | |
| 288 | + | } | |
| 289 | + | ||
| 290 | + | err := store.SaveToken(context.Background(), token, "jwt-token") | |
| 291 | + | assert.Error(t, err, "should reject invalid subject: %s", tt.subject) | |
| 292 | + | assert.Contains(t, err.Error(), "invalid subject", "error message should mention invalid subject") | |
| 293 | + | }) | |
| 294 | + | } | |
| 295 | + | } | |
| 296 | + | ||
| 297 | + | func TestPassStore_SaveToken_AcceptsValidSubject(t *testing.T) { | |
| 298 | + | tests := []struct { | |
| 299 | + | name string | |
| 300 | + | subject string | |
| 301 | + | }{ | |
| 302 | + | {"simple username", "myuser"}, | |
| 303 | + | {"username with dots", "user.name"}, | |
| 304 | + | {"username with hyphens", "user-name"}, | |
| 305 | + | {"username with underscores", "user_name"}, | |
| 306 | + | {"email-like", "user@example.com"}, | |
| 307 | + | {"path-like but safe", "registry/user"}, | |
| 308 | + | } | |
| 309 | + | ||
| 310 | + | for _, tt := range tests { | |
| 311 | + | t.Run(tt.name, func(t *testing.T) { | |
| 312 | + | // Verify the validation logic accepts valid subjects | |
| 313 | + | err := validateSubject(tt.subject) | |
| 314 | + | assert.NoError(t, err, "should accept valid subject: %s", tt.subject) | |
| 315 | + | }) | |
| 316 | + | } | |
| 317 | + | } | |
| 318 | + | ||
| 319 | + | func TestPassStore_GetToken_RejectsInvalidSubject(t *testing.T) { | |
| 320 | + | store := newTestPassStore() | |
| 321 | + | ||
| 322 | + | invalidSubjects := []string{ | |
| 323 | + | "../../../etc/passwd", | |
| 324 | + | "user;rm -rf /", | |
| 325 | + | "user|cat", | |
| 326 | + | "user\nnewline", | |
| 327 | + | } | |
| 328 | + | ||
| 329 | + | for _, subject := range invalidSubjects { | |
| 330 | + | t.Run(subject, func(t *testing.T) { | |
| 331 | + | _, _, err := store.GetToken(context.Background(), subject) | |
| 332 | + | assert.Error(t, err, "should reject invalid subject") | |
| 333 | + | assert.Contains(t, err.Error(), "invalid subject") | |
| 334 | + | }) | |
| 335 | + | } | |
| 336 | + | } | |
| 337 | + | ||
| 338 | + | func TestPassStore_DeleteToken_RejectsInvalidSubject(t *testing.T) { | |
| 339 | + | store := newTestPassStore() | |
| 340 | + | ||
| 341 | + | invalidSubjects := []string{ | |
| 342 | + | "../../../etc/passwd", | |
| 343 | + | "user;rm -rf /", | |
| 344 | + | "user|cat", | |
| 345 | + | } | |
| 346 | + | ||
| 347 | + | for _, subject := range invalidSubjects { | |
| 348 | + | t.Run(subject, func(t *testing.T) { | |
| 349 | + | err := store.DeleteToken(context.Background(), subject) | |
| 350 | + | assert.Error(t, err, "should reject invalid subject") | |
| 351 | + | assert.Contains(t, err.Error(), "invalid subject") | |
| 352 | + | }) | |
| 353 | + | } | |
| 354 | + | } | |
| @@ -258,3 +258,97 @@ func TestPassStore_RevokeUpdatesCache(t *testing.T) { | |||||
| 258 | require.NoError(t, err) | 258 | require.NoError(t, err) | ||
| 259 | assert.True(t, revoked) | 259 | assert.True(t, revoked) | ||
| 260 | } | 260 | } | ||
| 261 | + | ||||
| 262 | + | func TestPassStore_SaveToken_RejectsInvalidSubject(t *testing.T) { | |||
| 263 | + | store := newTestPassStore() | |||
| 264 | + | ||||
| 265 | + | tests := []struct { | |||
| 266 | + | name string | |||
| 267 | + | subject string | |||
| 268 | + | }{ | |||
| 269 | + | {"path traversal with ../", "../../../etc/passwd"}, | |||
| 270 | + | {"path traversal with ..\\", "..\\..\\windows\\system32"}, | |||
| 271 | + | {"shell metacharacter semicolon", "user;rm -rf /"}, | |||
| 272 | + | {"shell metacharacter pipe", "user|cat /etc/passwd"}, | |||
| 273 | + | {"shell metacharacter ampersand", "user&&malicious"}, | |||
| 274 | + | {"shell metacharacter backtick", "user`evil`"}, | |||
| 275 | + | {"shell metacharacter dollar sign", "user$(evil)"}, | |||
| 276 | + | {"newline character", "user\nmalicious"}, | |||
| 277 | + | {"null byte", "user\x00malicious"}, | |||
| 278 | + | } | |||
| 279 | + | ||||
| 280 | + | for _, tt := range tests { | |||
| 281 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 282 | + | token := &domain.Token{ | |||
| 283 | + | ID: "test-id", | |||
| 284 | + | Subject: tt.subject, | |||
| 285 | + | Scopes: []string{"push"}, | |||
| 286 | + | IssuedAt: time.Now(), | |||
| 287 | + | ExpiresAt: time.Now().Add(24 * time.Hour), | |||
| 288 | + | } | |||
| 289 | + | ||||
| 290 | + | err := store.SaveToken(context.Background(), token, "jwt-token") | |||
| 291 | + | assert.Error(t, err, "should reject invalid subject: %s", tt.subject) | |||
| 292 | + | assert.Contains(t, err.Error(), "invalid subject", "error message should mention invalid subject") | |||
| 293 | + | }) | |||
| 294 | + | } | |||
| 295 | + | } | |||
| 296 | + | ||||
| 297 | + | func TestPassStore_SaveToken_AcceptsValidSubject(t *testing.T) { | |||
| 298 | + | tests := []struct { | |||
| 299 | + | name string | |||
| 300 | + | subject string | |||
| 301 | + | }{ | |||
| 302 | + | {"simple username", "myuser"}, | |||
| 303 | + | {"username with dots", "user.name"}, | |||
| 304 | + | {"username with hyphens", "user-name"}, | |||
| 305 | + | {"username with underscores", "user_name"}, | |||
| 306 | + | {"email-like", "user@example.com"}, | |||
| 307 | + | {"path-like but safe", "registry/user"}, | |||
| 308 | + | } | |||
| 309 | + | ||||
| 310 | + | for _, tt := range tests { | |||
| 311 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 312 | + | // Verify the validation logic accepts valid subjects | |||
| 313 | + | err := validateSubject(tt.subject) | |||
| 314 | + | assert.NoError(t, err, "should accept valid subject: %s", tt.subject) | |||
| 315 | + | }) | |||
| 316 | + | } | |||
| 317 | + | } | |||
| 318 | + | ||||
| 319 | + | func TestPassStore_GetToken_RejectsInvalidSubject(t *testing.T) { | |||
| 320 | + | store := newTestPassStore() | |||
| 321 | + | ||||
| 322 | + | invalidSubjects := []string{ | |||
| 323 | + | "../../../etc/passwd", | |||
| 324 | + | "user;rm -rf /", | |||
| 325 | + | "user|cat", | |||
| 326 | + | "user\nnewline", | |||
| 327 | + | } | |||
| 328 | + | ||||
| 329 | + | for _, subject := range invalidSubjects { | |||
| 330 | + | t.Run(subject, func(t *testing.T) { | |||
| 331 | + | _, _, err := store.GetToken(context.Background(), subject) | |||
| 332 | + | assert.Error(t, err, "should reject invalid subject") | |||
| 333 | + | assert.Contains(t, err.Error(), "invalid subject") | |||
| 334 | + | }) | |||
| 335 | + | } | |||
| 336 | + | } | |||
| 337 | + | ||||
| 338 | + | func TestPassStore_DeleteToken_RejectsInvalidSubject(t *testing.T) { | |||
| 339 | + | store := newTestPassStore() | |||
| 340 | + | ||||
| 341 | + | invalidSubjects := []string{ | |||
| 342 | + | "../../../etc/passwd", | |||
| 343 | + | "user;rm -rf /", | |||
| 344 | + | "user|cat", | |||
| 345 | + | } | |||
| 346 | + | ||||
| 347 | + | for _, subject := range invalidSubjects { | |||
| 348 | + | t.Run(subject, func(t *testing.T) { | |||
| 349 | + | err := store.DeleteToken(context.Background(), subject) | |||
| 350 | + | assert.Error(t, err, "should reject invalid subject") | |||
| 351 | + | assert.Contains(t, err.Error(), "invalid subject") | |||
| 352 | + | }) | |||
| 353 | + | } | |||
| 354 | + | } | |||
M internal/app/run.go +60 -24
| @@ -7,6 +7,7 @@ import ( | |||
| 7 | 7 | "encoding/hex" | |
| 8 | 8 | "encoding/json" | |
| 9 | 9 | "fmt" | |
| 10 | + | "net" | |
| 10 | 11 | "net/http" | |
| 11 | 12 | "os" | |
| 12 | 13 | "os/signal" | |
| @@ -181,14 +182,11 @@ func Run(ctx context.Context, configPath string) error { | |||
| 181 | 182 | } | |
| 182 | 183 | defer svc.eventBus.Stop() | |
| 183 | 184 | ||
| 184 | - | // Sync and auto-start containers | |
| 185 | - | syncAndAutoStart(ctx, svc, log) | |
| 186 | - | ||
| 187 | 185 | // Create HTTP handlers | |
| 188 | 186 | registryHandler, proxyHandler := createHTTPHandlers(svc, cfg, log) | |
| 189 | 187 | ||
| 190 | - | // Start servers and wait for shutdown | |
| 191 | - | return runServers(ctx, cfg, registryHandler, proxyHandler, svc.containerSvc, svc.eventBus, log) | |
| 188 | + | // Start servers, wait for listeners to bind, then sync/auto-start containers. | |
| 189 | + | return runServers(ctx, cfg, registryHandler, proxyHandler, svc.containerSvc, svc.eventBus, svc, log) | |
| 192 | 190 | } | |
| 193 | 191 | ||
| 194 | 192 | // initConfig loads configuration from file. | |
| @@ -914,6 +912,12 @@ func registerEventHandlers(ctx context.Context, svc *services, cfg Config) error | |||
| 914 | 912 | return fmt.Errorf("failed to subscribe container deployed handler: %w", err) | |
| 915 | 913 | } | |
| 916 | 914 | ||
| 915 | + | // Proxy cache invalidation on config reload (clears stale targets for removed routes) | |
| 916 | + | configReloadProxyHandler := proxy.NewConfigReloadProxyHandler(ctx, svc.proxySvc) | |
| 917 | + | if err := svc.eventBus.Subscribe(configReloadProxyHandler); err != nil { | |
| 918 | + | return fmt.Errorf("failed to subscribe config reload proxy handler: %w", err) | |
| 919 | + | } | |
| 920 | + | ||
| 917 | 921 | return nil | |
| 918 | 922 | } | |
| 919 | 923 | ||
| @@ -955,7 +959,7 @@ func syncAndAutoStart(ctx context.Context, svc *services, log zerowrap.Logger) { | |||
| 955 | 959 | ||
| 956 | 960 | if svc.configSvc.IsAutoRouteEnabled() { | |
| 957 | 961 | routes := svc.configSvc.GetRoutes(ctx) | |
| 958 | - | if err := svc.containerSvc.AutoStart(ctx, routes); err != nil { | |
| 962 | + | if err := svc.containerSvc.AutoStart(domain.WithInternalDeploy(ctx), routes); err != nil { | |
| 959 | 963 | log.Warn().Err(err).Msg("failed to auto-start containers") | |
| 960 | 964 | } | |
| 961 | 965 | } | |
| @@ -1063,7 +1067,7 @@ func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger) (http.Ha | |||
| 1063 | 1067 | // - SIGUSR2: Triggers manual deploy for a specific route | |
| 1064 | 1068 | // The deferred signal.Stop calls ensure signal handlers are properly | |
| 1065 | 1069 | // cleaned up before program exit, preventing signal handler leaks. | |
| 1066 | - | func runServers(ctx context.Context, cfg Config, registryHandler, proxyHandler http.Handler, containerSvc *container.Service, eventBus out.EventBus, log zerowrap.Logger) error { | |
| 1070 | + | func runServers(ctx context.Context, cfg Config, registryHandler, proxyHandler http.Handler, containerSvc *container.Service, eventBus out.EventBus, svc *services, log zerowrap.Logger) error { | |
| 1067 | 1071 | ctx, cancel := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM) | |
| 1068 | 1072 | defer cancel() | |
| 1069 | 1073 | ||
| @@ -1081,14 +1085,30 @@ func runServers(ctx context.Context, cfg Config, registryHandler, proxyHandler h | |||
| 1081 | 1085 | ||
| 1082 | 1086 | errChan := make(chan error, 2) | |
| 1083 | 1087 | ||
| 1084 | - | go startServer(fmt.Sprintf(":%d", cfg.Server.RegistryPort), registryHandler, "registry", errChan, log) | |
| 1085 | - | go startServer(fmt.Sprintf(":%d", cfg.Server.Port), proxyHandler, "proxy", errChan, log) | |
| 1088 | + | registryReady := startServer(fmt.Sprintf(":%d", cfg.Server.RegistryPort), registryHandler, "registry", errChan, log) | |
| 1089 | + | proxyReady := startServer(fmt.Sprintf(":%d", cfg.Server.Port), proxyHandler, "proxy", errChan, log) | |
| 1090 | + | ||
| 1091 | + | // Wait for both servers to bind their ports before auto-starting containers. | |
| 1092 | + | // This prevents the race where auto-start pulls from the registry before it's listening. | |
| 1093 | + | select { | |
| 1094 | + | case <-registryReady: | |
| 1095 | + | case err := <-errChan: | |
| 1096 | + | return err | |
| 1097 | + | } | |
| 1098 | + | select { | |
| 1099 | + | case <-proxyReady: | |
| 1100 | + | case err := <-errChan: | |
| 1101 | + | return err | |
| 1102 | + | } | |
| 1086 | 1103 | ||
| 1087 | 1104 | log.Info(). | |
| 1088 | 1105 | Int("proxy_port", cfg.Server.Port). | |
| 1089 | 1106 | Int("registry_port", cfg.Server.RegistryPort). | |
| 1090 | 1107 | Msg("Gordon is running") | |
| 1091 | 1108 | ||
| 1109 | + | // Auto-start after servers are listening (registry port is now bound). | |
| 1110 | + | syncAndAutoStart(ctx, svc, log) | |
| 1111 | + | ||
| 1092 | 1112 | for { | |
| 1093 | 1113 | select { | |
| 1094 | 1114 | case err := <-errChan: | |
| @@ -1129,23 +1149,39 @@ shutdown: | |||
| 1129 | 1149 | return nil | |
| 1130 | 1150 | } | |
| 1131 | 1151 | ||
| 1132 | - | // startServer starts an HTTP server. | |
| 1133 | - | func startServer(addr string, handler http.Handler, name string, errChan chan<- error, log zerowrap.Logger) { | |
| 1134 | - | log.Info().Str("address", addr).Msgf("%s server starting", name) | |
| 1152 | + | // startServer starts an HTTP server, returning a channel that closes once the | |
| 1153 | + | // listening socket is bound. This lets callers wait for the port to be ready | |
| 1154 | + | // before taking actions that depend on it (e.g. auto-start pulling from the | |
| 1155 | + | // local registry). | |
| 1156 | + | func startServer(addr string, handler http.Handler, name string, errChan chan<- error, log zerowrap.Logger) <-chan struct{} { | |
| 1157 | + | ready := make(chan struct{}) | |
| 1158 | + | ||
| 1159 | + | go func() { | |
| 1160 | + | log.Info().Str("address", addr).Msgf("%s server starting", name) | |
| 1161 | + | ||
| 1162 | + | server := &http.Server{ | |
| 1163 | + | Addr: addr, | |
| 1164 | + | Handler: handler, | |
| 1165 | + | ReadHeaderTimeout: 10 * time.Second, | |
| 1166 | + | ReadTimeout: 5 * time.Minute, | |
| 1167 | + | WriteTimeout: 5 * time.Minute, | |
| 1168 | + | IdleTimeout: 120 * time.Second, | |
| 1169 | + | MaxHeaderBytes: 1 << 20, | |
| 1170 | + | } | |
| 1135 | 1171 | ||
| 1136 | - | server := &http.Server{ | |
| 1137 | - | Addr: addr, | |
| 1138 | - | Handler: handler, | |
| 1139 | - | ReadHeaderTimeout: 10 * time.Second, | |
| 1140 | - | ReadTimeout: 5 * time.Minute, // Timeout for reading entire request | |
| 1141 | - | WriteTimeout: 5 * time.Minute, // Timeout for writing response | |
| 1142 | - | IdleTimeout: 120 * time.Second, // Timeout for idle keep-alive connections | |
| 1143 | - | MaxHeaderBytes: 1 << 20, // 1MB max header size | |
| 1144 | - | } | |
| 1172 | + | ln, err := net.Listen("tcp", addr) | |
| 1173 | + | if err != nil { | |
| 1174 | + | errChan <- fmt.Errorf("%s server error: %w", name, err) | |
| 1175 | + | return | |
| 1176 | + | } | |
| 1177 | + | close(ready) // signal: port is bound and accepting connections | |
| 1145 | 1178 | ||
| 1146 | - | if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { | |
| 1147 | - | errChan <- fmt.Errorf("%s server error: %w", name, err) | |
| 1148 | - | } | |
| 1179 | + | if err := server.Serve(ln); err != nil && err != http.ErrServerClosed { | |
| 1180 | + | errChan <- fmt.Errorf("%s server error: %w", name, err) | |
| 1181 | + | } | |
| 1182 | + | }() | |
| 1183 | + | ||
| 1184 | + | return ready | |
| 1149 | 1185 | } | |
| 1150 | 1186 | ||
| 1151 | 1187 | // SendReloadSignal sends SIGUSR1 to the running Gordon process. | |
| @@ -7,6 +7,7 @@ import ( | |||||
| 7 | "encoding/hex" | 7 | "encoding/hex" | ||
| 8 | "encoding/json" | 8 | "encoding/json" | ||
| 9 | "fmt" | 9 | "fmt" | ||
| 10 | + | "net" | |||
| 10 | "net/http" | 11 | "net/http" | ||
| 11 | "os" | 12 | "os" | ||
| 12 | "os/signal" | 13 | "os/signal" | ||
| @@ -181,14 +182,11 @@ func Run(ctx context.Context, configPath string) error { | |||||
| 181 | } | 182 | } | ||
| 182 | defer svc.eventBus.Stop() | 183 | defer svc.eventBus.Stop() | ||
| 183 | 184 | ||||
| 184 | - | // Sync and auto-start containers | |||
| 185 | - | syncAndAutoStart(ctx, svc, log) | |||
| 186 | - | ||||
| 187 | // Create HTTP handlers | 185 | // Create HTTP handlers | ||
| 188 | registryHandler, proxyHandler := createHTTPHandlers(svc, cfg, log) | 186 | registryHandler, proxyHandler := createHTTPHandlers(svc, cfg, log) | ||
| 189 | 187 | ||||
| 190 | - | // Start servers and wait for shutdown | 188 | + | // Start servers, wait for listeners to bind, then sync/auto-start containers. |
| 191 | - | return runServers(ctx, cfg, registryHandler, proxyHandler, svc.containerSvc, svc.eventBus, log) | 189 | + | return runServers(ctx, cfg, registryHandler, proxyHandler, svc.containerSvc, svc.eventBus, svc, log) |
| 192 | } | 190 | } | ||
| 193 | 191 | ||||
| 194 | // initConfig loads configuration from file. | 192 | // initConfig loads configuration from file. | ||
| @@ -914,6 +912,12 @@ func registerEventHandlers(ctx context.Context, svc *services, cfg Config) error | |||||
| 914 | return fmt.Errorf("failed to subscribe container deployed handler: %w", err) | 912 | return fmt.Errorf("failed to subscribe container deployed handler: %w", err) | ||
| 915 | } | 913 | } | ||
| 916 | 914 | ||||
| 915 | + | // Proxy cache invalidation on config reload (clears stale targets for removed routes) | |||
| 916 | + | configReloadProxyHandler := proxy.NewConfigReloadProxyHandler(ctx, svc.proxySvc) | |||
| 917 | + | if err := svc.eventBus.Subscribe(configReloadProxyHandler); err != nil { | |||
| 918 | + | return fmt.Errorf("failed to subscribe config reload proxy handler: %w", err) | |||
| 919 | + | } | |||
| 920 | + | ||||
| 917 | return nil | 921 | return nil | ||
| 918 | } | 922 | } | ||
| 919 | 923 | ||||
| @@ -955,7 +959,7 @@ func syncAndAutoStart(ctx context.Context, svc *services, log zerowrap.Logger) { | |||||
| 955 | 959 | ||||
| 956 | if svc.configSvc.IsAutoRouteEnabled() { | 960 | if svc.configSvc.IsAutoRouteEnabled() { | ||
| 957 | routes := svc.configSvc.GetRoutes(ctx) | 961 | routes := svc.configSvc.GetRoutes(ctx) | ||
| 958 | - | if err := svc.containerSvc.AutoStart(ctx, routes); err != nil { | 962 | + | if err := svc.containerSvc.AutoStart(domain.WithInternalDeploy(ctx), routes); err != nil { |
| 959 | log.Warn().Err(err).Msg("failed to auto-start containers") | 963 | log.Warn().Err(err).Msg("failed to auto-start containers") | ||
| 960 | } | 964 | } | ||
| 961 | } | 965 | } | ||
| @@ -1063,7 +1067,7 @@ func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger) (http.Ha | |||||
| 1063 | // - SIGUSR2: Triggers manual deploy for a specific route | 1067 | // - SIGUSR2: Triggers manual deploy for a specific route | ||
| 1064 | // The deferred signal.Stop calls ensure signal handlers are properly | 1068 | // The deferred signal.Stop calls ensure signal handlers are properly | ||
| 1065 | // cleaned up before program exit, preventing signal handler leaks. | 1069 | // cleaned up before program exit, preventing signal handler leaks. | ||
| 1066 | - | func runServers(ctx context.Context, cfg Config, registryHandler, proxyHandler http.Handler, containerSvc *container.Service, eventBus out.EventBus, log zerowrap.Logger) error { | 1070 | + | func runServers(ctx context.Context, cfg Config, registryHandler, proxyHandler http.Handler, containerSvc *container.Service, eventBus out.EventBus, svc *services, log zerowrap.Logger) error { |
| 1067 | ctx, cancel := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM) | 1071 | ctx, cancel := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM) | ||
| 1068 | defer cancel() | 1072 | defer cancel() | ||
| 1069 | 1073 | ||||
| @@ -1081,14 +1085,30 @@ func runServers(ctx context.Context, cfg Config, registryHandler, proxyHandler h | |||||
| 1081 | 1085 | ||||
| 1082 | errChan := make(chan error, 2) | 1086 | errChan := make(chan error, 2) | ||
| 1083 | 1087 | ||||
| 1084 | - | go startServer(fmt.Sprintf(":%d", cfg.Server.RegistryPort), registryHandler, "registry", errChan, log) | 1088 | + | registryReady := startServer(fmt.Sprintf(":%d", cfg.Server.RegistryPort), registryHandler, "registry", errChan, log) |
| 1085 | - | go startServer(fmt.Sprintf(":%d", cfg.Server.Port), proxyHandler, "proxy", errChan, log) | 1089 | + | proxyReady := startServer(fmt.Sprintf(":%d", cfg.Server.Port), proxyHandler, "proxy", errChan, log) |
| 1090 | + | ||||
| 1091 | + | // Wait for both servers to bind their ports before auto-starting containers. | |||
| 1092 | + | // This prevents the race where auto-start pulls from the registry before it's listening. | |||
| 1093 | + | select { | |||
| 1094 | + | case <-registryReady: | |||
| 1095 | + | case err := <-errChan: | |||
| 1096 | + | return err | |||
| 1097 | + | } | |||
| 1098 | + | select { | |||
| 1099 | + | case <-proxyReady: | |||
| 1100 | + | case err := <-errChan: | |||
| 1101 | + | return err | |||
| 1102 | + | } | |||
| 1086 | 1103 | ||||
| 1087 | log.Info(). | 1104 | log.Info(). | ||
| 1088 | Int("proxy_port", cfg.Server.Port). | 1105 | Int("proxy_port", cfg.Server.Port). | ||
| 1089 | Int("registry_port", cfg.Server.RegistryPort). | 1106 | Int("registry_port", cfg.Server.RegistryPort). | ||
| 1090 | Msg("Gordon is running") | 1107 | Msg("Gordon is running") | ||
| 1091 | 1108 | ||||
| 1109 | + | // Auto-start after servers are listening (registry port is now bound). | |||
| 1110 | + | syncAndAutoStart(ctx, svc, log) | |||
| 1111 | + | ||||
| 1092 | for { | 1112 | for { | ||
| 1093 | select { | 1113 | select { | ||
| 1094 | case err := <-errChan: | 1114 | case err := <-errChan: | ||
| @@ -1129,23 +1149,39 @@ shutdown: | |||||
| 1129 | return nil | 1149 | return nil | ||
| 1130 | } | 1150 | } | ||
| 1131 | 1151 | ||||
| 1132 | - | // startServer starts an HTTP server. | 1152 | + | // startServer starts an HTTP server, returning a channel that closes once the |
| 1133 | - | func startServer(addr string, handler http.Handler, name string, errChan chan<- error, log zerowrap.Logger) { | 1153 | + | // listening socket is bound. This lets callers wait for the port to be ready |
| 1134 | - | log.Info().Str("address", addr).Msgf("%s server starting", name) | 1154 | + | // before taking actions that depend on it (e.g. auto-start pulling from the |
| 1155 | + | // local registry). | |||
| 1156 | + | func startServer(addr string, handler http.Handler, name string, errChan chan<- error, log zerowrap.Logger) <-chan struct{} { | |||
| 1157 | + | ready := make(chan struct{}) | |||
| 1158 | + | ||||
| 1159 | + | go func() { | |||
| 1160 | + | log.Info().Str("address", addr).Msgf("%s server starting", name) | |||
| 1161 | + | ||||
| 1162 | + | server := &http.Server{ | |||
| 1163 | + | Addr: addr, | |||
| 1164 | + | Handler: handler, | |||
| 1165 | + | ReadHeaderTimeout: 10 * time.Second, | |||
| 1166 | + | ReadTimeout: 5 * time.Minute, | |||
| 1167 | + | WriteTimeout: 5 * time.Minute, | |||
| 1168 | + | IdleTimeout: 120 * time.Second, | |||
| 1169 | + | MaxHeaderBytes: 1 << 20, | |||
| 1170 | + | } | |||
| 1135 | 1171 | ||||
| 1136 | - | server := &http.Server{ | 1172 | + | ln, err := net.Listen("tcp", addr) |
| 1137 | - | Addr: addr, | 1173 | + | if err != nil { |
| 1138 | - | Handler: handler, | 1174 | + | errChan <- fmt.Errorf("%s server error: %w", name, err) |
| 1139 | - | ReadHeaderTimeout: 10 * time.Second, | 1175 | + | return |
| 1140 | - | ReadTimeout: 5 * time.Minute, // Timeout for reading entire request | 1176 | + | } |
| 1141 | - | WriteTimeout: 5 * time.Minute, // Timeout for writing response | 1177 | + | close(ready) // signal: port is bound and accepting connections |
| 1142 | - | IdleTimeout: 120 * time.Second, // Timeout for idle keep-alive connections | |||
| 1143 | - | MaxHeaderBytes: 1 << 20, // 1MB max header size | |||
| 1144 | - | } | |||
| 1145 | 1178 | ||||
| 1146 | - | if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { | 1179 | + | if err := server.Serve(ln); err != nil && err != http.ErrServerClosed { |
| 1147 | - | errChan <- fmt.Errorf("%s server error: %w", name, err) | 1180 | + | errChan <- fmt.Errorf("%s server error: %w", name, err) |
| 1148 | - | } | 1181 | + | } |
| 1182 | + | }() | |||
| 1183 | + | ||||
| 1184 | + | return ready | |||
| 1149 | } | 1185 | } | ||
| 1150 | 1186 | ||||
| 1151 | // SendReloadSignal sends SIGUSR1 to the running Gordon process. | 1187 | // SendReloadSignal sends SIGUSR1 to the running Gordon process. | ||
M internal/boundaries/out/eventbus.go +3 -1
| @@ -1,12 +1,14 @@ | |||
| 1 | 1 | package out | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "context" | |
| 5 | + | ||
| 4 | 6 | "github.com/bnema/gordon/internal/domain" | |
| 5 | 7 | ) | |
| 6 | 8 | ||
| 7 | 9 | // EventHandler defines the contract for handling events. | |
| 8 | 10 | type EventHandler interface { | |
| 9 | - | Handle(event domain.Event) error | |
| 11 | + | Handle(ctx context.Context, event domain.Event) error | |
| 10 | 12 | CanHandle(eventType domain.EventType) bool | |
| 11 | 13 | } | |
| 12 | 14 | ||
| @@ -1,12 +1,14 @@ | |||||
| 1 | package out | 1 | package out | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "context" | |||
| 5 | + | ||||
| 4 | "github.com/bnema/gordon/internal/domain" | 6 | "github.com/bnema/gordon/internal/domain" | ||
| 5 | ) | 7 | ) | ||
| 6 | 8 | ||||
| 7 | // EventHandler defines the contract for handling events. | 9 | // EventHandler defines the contract for handling events. | ||
| 8 | type EventHandler interface { | 10 | type EventHandler interface { | ||
| 9 | - | Handle(event domain.Event) error | 11 | + | Handle(ctx context.Context, event domain.Event) error |
| 10 | CanHandle(eventType domain.EventType) bool | 12 | CanHandle(eventType domain.EventType) bool | ||
| 11 | } | 13 | } | ||
| 12 | 14 | ||||
M internal/boundaries/out/mocks/mock_event_handler.go +18 -10
| @@ -5,6 +5,8 @@ | |||
| 5 | 5 | package mocks | |
| 6 | 6 | ||
| 7 | 7 | import ( | |
| 8 | + | "context" | |
| 9 | + | ||
| 8 | 10 | "github.com/bnema/gordon/internal/domain" | |
| 9 | 11 | mock "github.com/stretchr/testify/mock" | |
| 10 | 12 | ) | |
| @@ -88,16 +90,16 @@ func (_c *MockEventHandler_CanHandle_Call) RunAndReturn(run func(eventType domai | |||
| 88 | 90 | } | |
| 89 | 91 | ||
| 90 | 92 | // Handle provides a mock function for the type MockEventHandler | |
| 91 | - | func (_mock *MockEventHandler) Handle(event domain.Event) error { | |
| 92 | - | ret := _mock.Called(event) | |
| 93 | + | func (_mock *MockEventHandler) Handle(ctx context.Context, event domain.Event) error { | |
| 94 | + | ret := _mock.Called(ctx, event) | |
| 93 | 95 | ||
| 94 | 96 | if len(ret) == 0 { | |
| 95 | 97 | panic("no return value specified for Handle") | |
| 96 | 98 | } | |
| 97 | 99 | ||
| 98 | 100 | var r0 error | |
| 99 | - | if returnFunc, ok := ret.Get(0).(func(domain.Event) error); ok { | |
| 100 | - | r0 = returnFunc(event) | |
| 101 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, domain.Event) error); ok { | |
| 102 | + | r0 = returnFunc(ctx, event) | |
| 101 | 103 | } else { | |
| 102 | 104 | r0 = ret.Error(0) | |
| 103 | 105 | } | |
| @@ -110,19 +112,25 @@ type MockEventHandler_Handle_Call struct { | |||
| 110 | 112 | } | |
| 111 | 113 | ||
| 112 | 114 | // Handle is a helper method to define mock.On call | |
| 115 | + | // - ctx context.Context | |
| 113 | 116 | // - event domain.Event | |
| 114 | - | func (_e *MockEventHandler_Expecter) Handle(event interface{}) *MockEventHandler_Handle_Call { | |
| 115 | - | return &MockEventHandler_Handle_Call{Call: _e.mock.On("Handle", event)} | |
| 117 | + | func (_e *MockEventHandler_Expecter) Handle(ctx interface{}, event interface{}) *MockEventHandler_Handle_Call { | |
| 118 | + | return &MockEventHandler_Handle_Call{Call: _e.mock.On("Handle", ctx, event)} | |
| 116 | 119 | } | |
| 117 | 120 | ||
| 118 | - | func (_c *MockEventHandler_Handle_Call) Run(run func(event domain.Event)) *MockEventHandler_Handle_Call { | |
| 121 | + | func (_c *MockEventHandler_Handle_Call) Run(run func(ctx context.Context, event domain.Event)) *MockEventHandler_Handle_Call { | |
| 119 | 122 | _c.Call.Run(func(args mock.Arguments) { | |
| 120 | - | var arg0 domain.Event | |
| 123 | + | var arg0 context.Context | |
| 121 | 124 | if args[0] != nil { | |
| 122 | - | arg0 = args[0].(domain.Event) | |
| 125 | + | arg0 = args[0].(context.Context) | |
| 126 | + | } | |
| 127 | + | var arg1 domain.Event | |
| 128 | + | if args[1] != nil { | |
| 129 | + | arg1 = args[1].(domain.Event) | |
| 123 | 130 | } | |
| 124 | 131 | run( | |
| 125 | 132 | arg0, | |
| 133 | + | arg1, | |
| 126 | 134 | ) | |
| 127 | 135 | }) | |
| 128 | 136 | return _c | |
| @@ -133,7 +141,7 @@ func (_c *MockEventHandler_Handle_Call) Return(err error) *MockEventHandler_Hand | |||
| 133 | 141 | return _c | |
| 134 | 142 | } | |
| 135 | 143 | ||
| 136 | - | func (_c *MockEventHandler_Handle_Call) RunAndReturn(run func(event domain.Event) error) *MockEventHandler_Handle_Call { | |
| 144 | + | func (_c *MockEventHandler_Handle_Call) RunAndReturn(run func(ctx context.Context, event domain.Event) error) *MockEventHandler_Handle_Call { | |
| 137 | 145 | _c.Call.Return(run) | |
| 138 | 146 | return _c | |
| 139 | 147 | } | |
| @@ -5,6 +5,8 @@ | |||||
| 5 | package mocks | 5 | package mocks | ||
| 6 | 6 | ||||
| 7 | import ( | 7 | import ( | ||
| 8 | + | "context" | |||
| 9 | + | ||||
| 8 | "github.com/bnema/gordon/internal/domain" | 10 | "github.com/bnema/gordon/internal/domain" | ||
| 9 | mock "github.com/stretchr/testify/mock" | 11 | mock "github.com/stretchr/testify/mock" | ||
| 10 | ) | 12 | ) | ||
| @@ -88,16 +90,16 @@ func (_c *MockEventHandler_CanHandle_Call) RunAndReturn(run func(eventType domai | |||||
| 88 | } | 90 | } | ||
| 89 | 91 | ||||
| 90 | // Handle provides a mock function for the type MockEventHandler | 92 | // Handle provides a mock function for the type MockEventHandler | ||
| 91 | - | func (_mock *MockEventHandler) Handle(event domain.Event) error { | 93 | + | func (_mock *MockEventHandler) Handle(ctx context.Context, event domain.Event) error { |
| 92 | - | ret := _mock.Called(event) | 94 | + | ret := _mock.Called(ctx, event) |
| 93 | 95 | ||||
| 94 | if len(ret) == 0 { | 96 | if len(ret) == 0 { | ||
| 95 | panic("no return value specified for Handle") | 97 | panic("no return value specified for Handle") | ||
| 96 | } | 98 | } | ||
| 97 | 99 | ||||
| 98 | var r0 error | 100 | var r0 error | ||
| 99 | - | if returnFunc, ok := ret.Get(0).(func(domain.Event) error); ok { | 101 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, domain.Event) error); ok { |
| 100 | - | r0 = returnFunc(event) | 102 | + | r0 = returnFunc(ctx, event) |
| 101 | } else { | 103 | } else { | ||
| 102 | r0 = ret.Error(0) | 104 | r0 = ret.Error(0) | ||
| 103 | } | 105 | } | ||
| @@ -110,19 +112,25 @@ type MockEventHandler_Handle_Call struct { | |||||
| 110 | } | 112 | } | ||
| 111 | 113 | ||||
| 112 | // Handle is a helper method to define mock.On call | 114 | // Handle is a helper method to define mock.On call | ||
| 115 | + | // - ctx context.Context | |||
| 113 | // - event domain.Event | 116 | // - event domain.Event | ||
| 114 | - | func (_e *MockEventHandler_Expecter) Handle(event interface{}) *MockEventHandler_Handle_Call { | 117 | + | func (_e *MockEventHandler_Expecter) Handle(ctx interface{}, event interface{}) *MockEventHandler_Handle_Call { |
| 115 | - | return &MockEventHandler_Handle_Call{Call: _e.mock.On("Handle", event)} | 118 | + | return &MockEventHandler_Handle_Call{Call: _e.mock.On("Handle", ctx, event)} |
| 116 | } | 119 | } | ||
| 117 | 120 | ||||
| 118 | - | func (_c *MockEventHandler_Handle_Call) Run(run func(event domain.Event)) *MockEventHandler_Handle_Call { | 121 | + | func (_c *MockEventHandler_Handle_Call) Run(run func(ctx context.Context, event domain.Event)) *MockEventHandler_Handle_Call { |
| 119 | _c.Call.Run(func(args mock.Arguments) { | 122 | _c.Call.Run(func(args mock.Arguments) { | ||
| 120 | - | var arg0 domain.Event | 123 | + | var arg0 context.Context |
| 121 | if args[0] != nil { | 124 | if args[0] != nil { | ||
| 122 | - | arg0 = args[0].(domain.Event) | 125 | + | arg0 = args[0].(context.Context) |
| 126 | + | } | |||
| 127 | + | var arg1 domain.Event | |||
| 128 | + | if args[1] != nil { | |||
| 129 | + | arg1 = args[1].(domain.Event) | |||
| 123 | } | 130 | } | ||
| 124 | run( | 131 | run( | ||
| 125 | arg0, | 132 | arg0, | ||
| 133 | + | arg1, | |||
| 126 | ) | 134 | ) | ||
| 127 | }) | 135 | }) | ||
| 128 | return _c | 136 | return _c | ||
| @@ -133,7 +141,7 @@ func (_c *MockEventHandler_Handle_Call) Return(err error) *MockEventHandler_Hand | |||||
| 133 | return _c | 141 | return _c | ||
| 134 | } | 142 | } | ||
| 135 | 143 | ||||
| 136 | - | func (_c *MockEventHandler_Handle_Call) RunAndReturn(run func(event domain.Event) error) *MockEventHandler_Handle_Call { | 144 | + | func (_c *MockEventHandler_Handle_Call) RunAndReturn(run func(ctx context.Context, event domain.Event) error) *MockEventHandler_Handle_Call { |
| 137 | _c.Call.Return(run) | 145 | _c.Call.Return(run) | ||
| 138 | return _c | 146 | return _c | ||
| 139 | } | 147 | } | ||
M internal/usecase/container/autoroute.go +2 -2
| @@ -60,8 +60,8 @@ func (h *AutoRouteHandler) WithEnvExtractor(extractor EnvFileExtractor, envDir s | |||
| 60 | 60 | } | |
| 61 | 61 | ||
| 62 | 62 | // Handle handles an image.pushed event and creates routes from labels. | |
| 63 | - | func (h *AutoRouteHandler) Handle(event domain.Event) error { | |
| 64 | - | ctx := zerowrap.CtxWithFields(h.ctx, map[string]any{ | |
| 63 | + | func (h *AutoRouteHandler) Handle(ctx context.Context, event domain.Event) error { | |
| 64 | + | ctx = zerowrap.CtxWithFields(ctx, map[string]any{ | |
| 65 | 65 | zerowrap.FieldLayer: "usecase", | |
| 66 | 66 | zerowrap.FieldHandler: "AutoRouteHandler", | |
| 67 | 67 | zerowrap.FieldEvent: string(event.Type), | |
| @@ -60,8 +60,8 @@ func (h *AutoRouteHandler) WithEnvExtractor(extractor EnvFileExtractor, envDir s | |||||
| 60 | } | 60 | } | ||
| 61 | 61 | ||||
| 62 | // Handle handles an image.pushed event and creates routes from labels. | 62 | // Handle handles an image.pushed event and creates routes from labels. | ||
| 63 | - | func (h *AutoRouteHandler) Handle(event domain.Event) error { | 63 | + | func (h *AutoRouteHandler) Handle(ctx context.Context, event domain.Event) error { |
| 64 | - | ctx := zerowrap.CtxWithFields(h.ctx, map[string]any{ | 64 | + | ctx = zerowrap.CtxWithFields(ctx, map[string]any{ |
| 65 | zerowrap.FieldLayer: "usecase", | 65 | zerowrap.FieldLayer: "usecase", | ||
| 66 | zerowrap.FieldHandler: "AutoRouteHandler", | 66 | zerowrap.FieldHandler: "AutoRouteHandler", | ||
| 67 | zerowrap.FieldEvent: string(event.Type), | 67 | zerowrap.FieldEvent: string(event.Type), | ||
M internal/usecase/container/autoroute_test.go +8 -8
| @@ -368,7 +368,7 @@ func TestAutoRouteHandler_Handle_DisabledAutoRoute(t *testing.T) { | |||
| 368 | 368 | }, | |
| 369 | 369 | } | |
| 370 | 370 | ||
| 371 | - | err := handler.Handle(event) | |
| 371 | + | err := handler.Handle(context.Background(), event) | |
| 372 | 372 | ||
| 373 | 373 | assert.NoError(t, err) | |
| 374 | 374 | } | |
| @@ -389,7 +389,7 @@ func TestAutoRouteHandler_Handle_InvalidPayload(t *testing.T) { | |||
| 389 | 389 | Data: "invalid payload type", | |
| 390 | 390 | } | |
| 391 | 391 | ||
| 392 | - | err := handler.Handle(event) | |
| 392 | + | err := handler.Handle(context.Background(), event) | |
| 393 | 393 | ||
| 394 | 394 | // Handler skips invalid payload, doesn't error | |
| 395 | 395 | assert.NoError(t, err) | |
| @@ -415,7 +415,7 @@ func TestAutoRouteHandler_Handle_EmptyManifest(t *testing.T) { | |||
| 415 | 415 | }, | |
| 416 | 416 | } | |
| 417 | 417 | ||
| 418 | - | err := handler.Handle(event) | |
| 418 | + | err := handler.Handle(context.Background(), event) | |
| 419 | 419 | ||
| 420 | 420 | assert.NoError(t, err) | |
| 421 | 421 | } | |
| @@ -475,7 +475,7 @@ func TestAutoRouteHandler_Handle_CreatesNewRoute(t *testing.T) { | |||
| 475 | 475 | }, | |
| 476 | 476 | } | |
| 477 | 477 | ||
| 478 | - | err := handler.Handle(event) | |
| 478 | + | err := handler.Handle(context.Background(), event) | |
| 479 | 479 | ||
| 480 | 480 | assert.NoError(t, err) | |
| 481 | 481 | } | |
| @@ -537,7 +537,7 @@ func TestAutoRouteHandler_Handle_UpdatesExistingRoute(t *testing.T) { | |||
| 537 | 537 | }, | |
| 538 | 538 | } | |
| 539 | 539 | ||
| 540 | - | err := handler.Handle(event) | |
| 540 | + | err := handler.Handle(context.Background(), event) | |
| 541 | 541 | ||
| 542 | 542 | assert.NoError(t, err) | |
| 543 | 543 | } | |
| @@ -589,7 +589,7 @@ func TestAutoRouteHandler_Handle_NoUpdateIfSameImage(t *testing.T) { | |||
| 589 | 589 | }, | |
| 590 | 590 | } | |
| 591 | 591 | ||
| 592 | - | err := handler.Handle(event) | |
| 592 | + | err := handler.Handle(context.Background(), event) | |
| 593 | 593 | ||
| 594 | 594 | assert.NoError(t, err) | |
| 595 | 595 | } | |
| @@ -636,7 +636,7 @@ func TestAutoRouteHandler_Handle_NoDomainLabel(t *testing.T) { | |||
| 636 | 636 | }, | |
| 637 | 637 | } | |
| 638 | 638 | ||
| 639 | - | err := handler.Handle(event) | |
| 639 | + | err := handler.Handle(context.Background(), event) | |
| 640 | 640 | ||
| 641 | 641 | assert.NoError(t, err) | |
| 642 | 642 | } | |
| @@ -702,7 +702,7 @@ func TestAutoRouteHandler_Handle_MultipleDomains(t *testing.T) { | |||
| 702 | 702 | }, | |
| 703 | 703 | } | |
| 704 | 704 | ||
| 705 | - | err := handler.Handle(event) | |
| 705 | + | err := handler.Handle(context.Background(), event) | |
| 706 | 706 | ||
| 707 | 707 | assert.NoError(t, err) | |
| 708 | 708 | } | |
| @@ -368,7 +368,7 @@ func TestAutoRouteHandler_Handle_DisabledAutoRoute(t *testing.T) { | |||||
| 368 | }, | 368 | }, | ||
| 369 | } | 369 | } | ||
| 370 | 370 | ||||
| 371 | - | err := handler.Handle(event) | 371 | + | err := handler.Handle(context.Background(), event) |
| 372 | 372 | ||||
| 373 | assert.NoError(t, err) | 373 | assert.NoError(t, err) | ||
| 374 | } | 374 | } | ||
| @@ -389,7 +389,7 @@ func TestAutoRouteHandler_Handle_InvalidPayload(t *testing.T) { | |||||
| 389 | Data: "invalid payload type", | 389 | Data: "invalid payload type", | ||
| 390 | } | 390 | } | ||
| 391 | 391 | ||||
| 392 | - | err := handler.Handle(event) | 392 | + | err := handler.Handle(context.Background(), event) |
| 393 | 393 | ||||
| 394 | // Handler skips invalid payload, doesn't error | 394 | // Handler skips invalid payload, doesn't error | ||
| 395 | assert.NoError(t, err) | 395 | assert.NoError(t, err) | ||
| @@ -415,7 +415,7 @@ func TestAutoRouteHandler_Handle_EmptyManifest(t *testing.T) { | |||||
| 415 | }, | 415 | }, | ||
| 416 | } | 416 | } | ||
| 417 | 417 | ||||
| 418 | - | err := handler.Handle(event) | 418 | + | err := handler.Handle(context.Background(), event) |
| 419 | 419 | ||||
| 420 | assert.NoError(t, err) | 420 | assert.NoError(t, err) | ||
| 421 | } | 421 | } | ||
| @@ -475,7 +475,7 @@ func TestAutoRouteHandler_Handle_CreatesNewRoute(t *testing.T) { | |||||
| 475 | }, | 475 | }, | ||
| 476 | } | 476 | } | ||
| 477 | 477 | ||||
| 478 | - | err := handler.Handle(event) | 478 | + | err := handler.Handle(context.Background(), event) |
| 479 | 479 | ||||
| 480 | assert.NoError(t, err) | 480 | assert.NoError(t, err) | ||
| 481 | } | 481 | } | ||
| @@ -537,7 +537,7 @@ func TestAutoRouteHandler_Handle_UpdatesExistingRoute(t *testing.T) { | |||||
| 537 | }, | 537 | }, | ||
| 538 | } | 538 | } | ||
| 539 | 539 | ||||
| 540 | - | err := handler.Handle(event) | 540 | + | err := handler.Handle(context.Background(), event) |
| 541 | 541 | ||||
| 542 | assert.NoError(t, err) | 542 | assert.NoError(t, err) | ||
| 543 | } | 543 | } | ||
| @@ -589,7 +589,7 @@ func TestAutoRouteHandler_Handle_NoUpdateIfSameImage(t *testing.T) { | |||||
| 589 | }, | 589 | }, | ||
| 590 | } | 590 | } | ||
| 591 | 591 | ||||
| 592 | - | err := handler.Handle(event) | 592 | + | err := handler.Handle(context.Background(), event) |
| 593 | 593 | ||||
| 594 | assert.NoError(t, err) | 594 | assert.NoError(t, err) | ||
| 595 | } | 595 | } | ||
| @@ -636,7 +636,7 @@ func TestAutoRouteHandler_Handle_NoDomainLabel(t *testing.T) { | |||||
| 636 | }, | 636 | }, | ||
| 637 | } | 637 | } | ||
| 638 | 638 | ||||
| 639 | - | err := handler.Handle(event) | 639 | + | err := handler.Handle(context.Background(), event) |
| 640 | 640 | ||||
| 641 | assert.NoError(t, err) | 641 | assert.NoError(t, err) | ||
| 642 | } | 642 | } | ||
| @@ -702,7 +702,7 @@ func TestAutoRouteHandler_Handle_MultipleDomains(t *testing.T) { | |||||
| 702 | }, | 702 | }, | ||
| 703 | } | 703 | } | ||
| 704 | 704 | ||||
| 705 | - | err := handler.Handle(event) | 705 | + | err := handler.Handle(context.Background(), event) |
| 706 | 706 | ||||
| 707 | assert.NoError(t, err) | 707 | assert.NoError(t, err) | ||
| 708 | } | 708 | } | ||
M internal/usecase/container/events.go +11 -11
| @@ -28,8 +28,8 @@ func NewImagePushedHandler(ctx context.Context, containerSvc in.ContainerService | |||
| 28 | 28 | } | |
| 29 | 29 | ||
| 30 | 30 | // Handle handles an event. | |
| 31 | - | func (h *ImagePushedHandler) Handle(event domain.Event) error { | |
| 32 | - | ctx := zerowrap.CtxWithFields(h.ctx, map[string]any{ | |
| 31 | + | func (h *ImagePushedHandler) Handle(ctx context.Context, event domain.Event) error { | |
| 32 | + | ctx = zerowrap.CtxWithFields(ctx, map[string]any{ | |
| 33 | 33 | zerowrap.FieldLayer: "usecase", | |
| 34 | 34 | zerowrap.FieldHandler: "ImagePushedHandler", | |
| 35 | 35 | zerowrap.FieldEvent: string(event.Type), | |
| @@ -122,8 +122,8 @@ func NewConfigReloadHandler(ctx context.Context, containerSvc in.ContainerServic | |||
| 122 | 122 | } | |
| 123 | 123 | ||
| 124 | 124 | // Handle handles an event. | |
| 125 | - | func (h *ConfigReloadHandler) Handle(event domain.Event) error { | |
| 126 | - | ctx := zerowrap.CtxWithFields(h.ctx, map[string]any{ | |
| 125 | + | func (h *ConfigReloadHandler) Handle(ctx context.Context, event domain.Event) error { | |
| 126 | + | ctx = zerowrap.CtxWithFields(ctx, map[string]any{ | |
| 127 | 127 | zerowrap.FieldLayer: "usecase", | |
| 128 | 128 | zerowrap.FieldHandler: "ConfigReloadHandler", | |
| 129 | 129 | zerowrap.FieldEvent: string(event.Type), | |
| @@ -159,7 +159,7 @@ func (h *ConfigReloadHandler) Handle(event domain.Event) error { | |||
| 159 | 159 | Str("new_image", route.Image). | |
| 160 | 160 | Msg("image changed for route, redeploying") | |
| 161 | 161 | ||
| 162 | - | if _, err := h.containerSvc.Deploy(ctx, route); err != nil { | |
| 162 | + | if _, err := h.containerSvc.Deploy(domain.WithInternalDeploy(ctx), route); err != nil { | |
| 163 | 163 | log.WrapErrWithFields(err, "failed to redeploy container", map[string]any{"domain": route.Domain}) | |
| 164 | 164 | } | |
| 165 | 165 | } | |
| @@ -171,7 +171,7 @@ func (h *ConfigReloadHandler) Handle(event domain.Event) error { | |||
| 171 | 171 | Str("image", route.Image). | |
| 172 | 172 | Msg("route missing container, deploying") | |
| 173 | 173 | ||
| 174 | - | if _, err := h.containerSvc.Deploy(ctx, route); err != nil { | |
| 174 | + | if _, err := h.containerSvc.Deploy(domain.WithInternalDeploy(ctx), route); err != nil { | |
| 175 | 175 | log.WrapErrWithFields(err, "failed to deploy container for route", map[string]any{"domain": route.Domain}) | |
| 176 | 176 | } | |
| 177 | 177 | } | |
| @@ -219,8 +219,8 @@ func NewManualReloadHandler(ctx context.Context, containerSvc in.ContainerServic | |||
| 219 | 219 | // Handle handles an event. | |
| 220 | 220 | // It starts containers for configured routes that don't have a running container. | |
| 221 | 221 | // Running containers are NEVER restarted to ensure 100% uptime. | |
| 222 | - | func (h *ManualReloadHandler) Handle(event domain.Event) error { | |
| 223 | - | ctx := zerowrap.CtxWithFields(h.ctx, map[string]any{ | |
| 222 | + | func (h *ManualReloadHandler) Handle(ctx context.Context, event domain.Event) error { | |
| 223 | + | ctx = zerowrap.CtxWithFields(ctx, map[string]any{ | |
| 224 | 224 | zerowrap.FieldLayer: "usecase", | |
| 225 | 225 | zerowrap.FieldHandler: "ManualReloadHandler", | |
| 226 | 226 | zerowrap.FieldEvent: string(event.Type), | |
| @@ -253,7 +253,7 @@ func (h *ManualReloadHandler) Handle(event domain.Event) error { | |||
| 253 | 253 | Str("image", route.Image). | |
| 254 | 254 | Msg("starting container for route") | |
| 255 | 255 | ||
| 256 | - | if _, err := h.containerSvc.Deploy(ctx, route); err != nil { | |
| 256 | + | if _, err := h.containerSvc.Deploy(domain.WithInternalDeploy(ctx), route); err != nil { | |
| 257 | 257 | log.WrapErrWithFields(err, "failed to start container", map[string]any{"domain": route.Domain}) | |
| 258 | 258 | errorCount++ | |
| 259 | 259 | continue | |
| @@ -302,8 +302,8 @@ func NewManualDeployHandler(ctx context.Context, containerSvc in.ContainerServic | |||
| 302 | 302 | } | |
| 303 | 303 | ||
| 304 | 304 | // Handle handles a manual deploy event for a specific route. | |
| 305 | - | func (h *ManualDeployHandler) Handle(event domain.Event) error { | |
| 306 | - | ctx := zerowrap.CtxWithFields(h.ctx, map[string]any{ | |
| 305 | + | func (h *ManualDeployHandler) Handle(ctx context.Context, event domain.Event) error { | |
| 306 | + | ctx = zerowrap.CtxWithFields(ctx, map[string]any{ | |
| 307 | 307 | zerowrap.FieldLayer: "usecase", | |
| 308 | 308 | zerowrap.FieldHandler: "ManualDeployHandler", | |
| 309 | 309 | zerowrap.FieldEvent: string(event.Type), | |
| @@ -28,8 +28,8 @@ func NewImagePushedHandler(ctx context.Context, containerSvc in.ContainerService | |||||
| 28 | } | 28 | } | ||
| 29 | 29 | ||||
| 30 | // Handle handles an event. | 30 | // Handle handles an event. | ||
| 31 | - | func (h *ImagePushedHandler) Handle(event domain.Event) error { | 31 | + | func (h *ImagePushedHandler) Handle(ctx context.Context, event domain.Event) error { |
| 32 | - | ctx := zerowrap.CtxWithFields(h.ctx, map[string]any{ | 32 | + | ctx = zerowrap.CtxWithFields(ctx, map[string]any{ |
| 33 | zerowrap.FieldLayer: "usecase", | 33 | zerowrap.FieldLayer: "usecase", | ||
| 34 | zerowrap.FieldHandler: "ImagePushedHandler", | 34 | zerowrap.FieldHandler: "ImagePushedHandler", | ||
| 35 | zerowrap.FieldEvent: string(event.Type), | 35 | zerowrap.FieldEvent: string(event.Type), | ||
| @@ -122,8 +122,8 @@ func NewConfigReloadHandler(ctx context.Context, containerSvc in.ContainerServic | |||||
| 122 | } | 122 | } | ||
| 123 | 123 | ||||
| 124 | // Handle handles an event. | 124 | // Handle handles an event. | ||
| 125 | - | func (h *ConfigReloadHandler) Handle(event domain.Event) error { | 125 | + | func (h *ConfigReloadHandler) Handle(ctx context.Context, event domain.Event) error { |
| 126 | - | ctx := zerowrap.CtxWithFields(h.ctx, map[string]any{ | 126 | + | ctx = zerowrap.CtxWithFields(ctx, map[string]any{ |
| 127 | zerowrap.FieldLayer: "usecase", | 127 | zerowrap.FieldLayer: "usecase", | ||
| 128 | zerowrap.FieldHandler: "ConfigReloadHandler", | 128 | zerowrap.FieldHandler: "ConfigReloadHandler", | ||
| 129 | zerowrap.FieldEvent: string(event.Type), | 129 | zerowrap.FieldEvent: string(event.Type), | ||
| @@ -159,7 +159,7 @@ func (h *ConfigReloadHandler) Handle(event domain.Event) error { | |||||
| 159 | Str("new_image", route.Image). | 159 | Str("new_image", route.Image). | ||
| 160 | Msg("image changed for route, redeploying") | 160 | Msg("image changed for route, redeploying") | ||
| 161 | 161 | ||||
| 162 | - | if _, err := h.containerSvc.Deploy(ctx, route); err != nil { | 162 | + | if _, err := h.containerSvc.Deploy(domain.WithInternalDeploy(ctx), route); err != nil { |
| 163 | log.WrapErrWithFields(err, "failed to redeploy container", map[string]any{"domain": route.Domain}) | 163 | log.WrapErrWithFields(err, "failed to redeploy container", map[string]any{"domain": route.Domain}) | ||
| 164 | } | 164 | } | ||
| 165 | } | 165 | } | ||
| @@ -171,7 +171,7 @@ func (h *ConfigReloadHandler) Handle(event domain.Event) error { | |||||
| 171 | Str("image", route.Image). | 171 | Str("image", route.Image). | ||
| 172 | Msg("route missing container, deploying") | 172 | Msg("route missing container, deploying") | ||
| 173 | 173 | ||||
| 174 | - | if _, err := h.containerSvc.Deploy(ctx, route); err != nil { | 174 | + | if _, err := h.containerSvc.Deploy(domain.WithInternalDeploy(ctx), route); err != nil { |
| 175 | log.WrapErrWithFields(err, "failed to deploy container for route", map[string]any{"domain": route.Domain}) | 175 | log.WrapErrWithFields(err, "failed to deploy container for route", map[string]any{"domain": route.Domain}) | ||
| 176 | } | 176 | } | ||
| 177 | } | 177 | } | ||
| @@ -219,8 +219,8 @@ func NewManualReloadHandler(ctx context.Context, containerSvc in.ContainerServic | |||||
| 219 | // Handle handles an event. | 219 | // Handle handles an event. | ||
| 220 | // It starts containers for configured routes that don't have a running container. | 220 | // It starts containers for configured routes that don't have a running container. | ||
| 221 | // Running containers are NEVER restarted to ensure 100% uptime. | 221 | // Running containers are NEVER restarted to ensure 100% uptime. | ||
| 222 | - | func (h *ManualReloadHandler) Handle(event domain.Event) error { | 222 | + | func (h *ManualReloadHandler) Handle(ctx context.Context, event domain.Event) error { |
| 223 | - | ctx := zerowrap.CtxWithFields(h.ctx, map[string]any{ | 223 | + | ctx = zerowrap.CtxWithFields(ctx, map[string]any{ |
| 224 | zerowrap.FieldLayer: "usecase", | 224 | zerowrap.FieldLayer: "usecase", | ||
| 225 | zerowrap.FieldHandler: "ManualReloadHandler", | 225 | zerowrap.FieldHandler: "ManualReloadHandler", | ||
| 226 | zerowrap.FieldEvent: string(event.Type), | 226 | zerowrap.FieldEvent: string(event.Type), | ||
| @@ -253,7 +253,7 @@ func (h *ManualReloadHandler) Handle(event domain.Event) error { | |||||
| 253 | Str("image", route.Image). | 253 | Str("image", route.Image). | ||
| 254 | Msg("starting container for route") | 254 | Msg("starting container for route") | ||
| 255 | 255 | ||||
| 256 | - | if _, err := h.containerSvc.Deploy(ctx, route); err != nil { | 256 | + | if _, err := h.containerSvc.Deploy(domain.WithInternalDeploy(ctx), route); err != nil { |
| 257 | log.WrapErrWithFields(err, "failed to start container", map[string]any{"domain": route.Domain}) | 257 | log.WrapErrWithFields(err, "failed to start container", map[string]any{"domain": route.Domain}) | ||
| 258 | errorCount++ | 258 | errorCount++ | ||
| 259 | continue | 259 | continue | ||
| @@ -302,8 +302,8 @@ func NewManualDeployHandler(ctx context.Context, containerSvc in.ContainerServic | |||||
| 302 | } | 302 | } | ||
| 303 | 303 | ||||
| 304 | // Handle handles a manual deploy event for a specific route. | 304 | // Handle handles a manual deploy event for a specific route. | ||
| 305 | - | func (h *ManualDeployHandler) Handle(event domain.Event) error { | 305 | + | func (h *ManualDeployHandler) Handle(ctx context.Context, event domain.Event) error { |
| 306 | - | ctx := zerowrap.CtxWithFields(h.ctx, map[string]any{ | 306 | + | ctx = zerowrap.CtxWithFields(ctx, map[string]any{ |
| 307 | zerowrap.FieldLayer: "usecase", | 307 | zerowrap.FieldLayer: "usecase", | ||
| 308 | zerowrap.FieldHandler: "ManualDeployHandler", | 308 | zerowrap.FieldHandler: "ManualDeployHandler", | ||
| 309 | zerowrap.FieldEvent: string(event.Type), | 309 | zerowrap.FieldEvent: string(event.Type), | ||
M internal/usecase/container/events_test.go +19 -19
| @@ -63,7 +63,7 @@ func TestImagePushedHandler_Handle_DeploysMatchingRoutes(t *testing.T) { | |||
| 63 | 63 | Tag: "latest", | |
| 64 | 64 | } | |
| 65 | 65 | ||
| 66 | - | err := handler.Handle(event) | |
| 66 | + | err := handler.Handle(context.Background(), event) | |
| 67 | 67 | ||
| 68 | 68 | assert.NoError(t, err) | |
| 69 | 69 | } | |
| @@ -89,7 +89,7 @@ func TestImagePushedHandler_Handle_NoMatchingRoutes(t *testing.T) { | |||
| 89 | 89 | Tag: "latest", | |
| 90 | 90 | } | |
| 91 | 91 | ||
| 92 | - | err := handler.Handle(event) | |
| 92 | + | err := handler.Handle(context.Background(), event) | |
| 93 | 93 | ||
| 94 | 94 | assert.NoError(t, err) | |
| 95 | 95 | } | |
| @@ -107,7 +107,7 @@ func TestImagePushedHandler_Handle_EmptyImageName(t *testing.T) { | |||
| 107 | 107 | Tag: "latest", | |
| 108 | 108 | } | |
| 109 | 109 | ||
| 110 | - | err := handler.Handle(event) | |
| 110 | + | err := handler.Handle(context.Background(), event) | |
| 111 | 111 | ||
| 112 | 112 | assert.ErrorIs(t, err, domain.ErrInvalidImageFormat) | |
| 113 | 113 | } | |
| @@ -137,7 +137,7 @@ func TestImagePushedHandler_Handle_DefaultTag(t *testing.T) { | |||
| 137 | 137 | Tag: "", // Empty tag should default to "latest" | |
| 138 | 138 | } | |
| 139 | 139 | ||
| 140 | - | err := handler.Handle(event) | |
| 140 | + | err := handler.Handle(context.Background(), event) | |
| 141 | 141 | ||
| 142 | 142 | assert.NoError(t, err) | |
| 143 | 143 | } | |
| @@ -164,7 +164,7 @@ func TestImagePushedHandler_Handle_DeployError(t *testing.T) { | |||
| 164 | 164 | } | |
| 165 | 165 | ||
| 166 | 166 | // Handler logs error but doesn't fail | |
| 167 | - | err := handler.Handle(event) | |
| 167 | + | err := handler.Handle(context.Background(), event) | |
| 168 | 168 | ||
| 169 | 169 | assert.NoError(t, err) | |
| 170 | 170 | } | |
| @@ -192,7 +192,7 @@ func TestImagePushedHandler_Handle_StripsRegistryDomain(t *testing.T) { | |||
| 192 | 192 | Tag: "latest", | |
| 193 | 193 | } | |
| 194 | 194 | ||
| 195 | - | err := handler.Handle(event) | |
| 195 | + | err := handler.Handle(context.Background(), event) | |
| 196 | 196 | ||
| 197 | 197 | assert.NoError(t, err) | |
| 198 | 198 | } | |
| @@ -237,7 +237,7 @@ func TestConfigReloadHandler_Handle_DeploysNewRoutes(t *testing.T) { | |||
| 237 | 237 | Type: domain.EventConfigReload, | |
| 238 | 238 | } | |
| 239 | 239 | ||
| 240 | - | err := handler.Handle(event) | |
| 240 | + | err := handler.Handle(context.Background(), event) | |
| 241 | 241 | ||
| 242 | 242 | assert.NoError(t, err) | |
| 243 | 243 | } | |
| @@ -273,7 +273,7 @@ func TestConfigReloadHandler_Handle_StopsRemovedRoutes(t *testing.T) { | |||
| 273 | 273 | Type: domain.EventConfigReload, | |
| 274 | 274 | } | |
| 275 | 275 | ||
| 276 | - | err := handler.Handle(event) | |
| 276 | + | err := handler.Handle(context.Background(), event) | |
| 277 | 277 | ||
| 278 | 278 | assert.NoError(t, err) | |
| 279 | 279 | } | |
| @@ -313,7 +313,7 @@ func TestConfigReloadHandler_Handle_RedeploysChangedImage(t *testing.T) { | |||
| 313 | 313 | Type: domain.EventConfigReload, | |
| 314 | 314 | } | |
| 315 | 315 | ||
| 316 | - | err := handler.Handle(event) | |
| 316 | + | err := handler.Handle(context.Background(), event) | |
| 317 | 317 | ||
| 318 | 318 | assert.NoError(t, err) | |
| 319 | 319 | } | |
| @@ -349,7 +349,7 @@ func TestConfigReloadHandler_Handle_NoChanges(t *testing.T) { | |||
| 349 | 349 | Type: domain.EventConfigReload, | |
| 350 | 350 | } | |
| 351 | 351 | ||
| 352 | - | err := handler.Handle(event) | |
| 352 | + | err := handler.Handle(context.Background(), event) | |
| 353 | 353 | ||
| 354 | 354 | assert.NoError(t, err) | |
| 355 | 355 | } | |
| @@ -397,7 +397,7 @@ func TestManualReloadHandler_Handle_StartsOnlyMissingContainers(t *testing.T) { | |||
| 397 | 397 | Type: domain.EventManualReload, | |
| 398 | 398 | } | |
| 399 | 399 | ||
| 400 | - | err := handler.Handle(event) | |
| 400 | + | err := handler.Handle(context.Background(), event) | |
| 401 | 401 | ||
| 402 | 402 | assert.NoError(t, err) | |
| 403 | 403 | } | |
| @@ -428,7 +428,7 @@ func TestManualReloadHandler_Handle_StartsMissingContainer(t *testing.T) { | |||
| 428 | 428 | Type: domain.EventManualReload, | |
| 429 | 429 | } | |
| 430 | 430 | ||
| 431 | - | err := handler.Handle(event) | |
| 431 | + | err := handler.Handle(context.Background(), event) | |
| 432 | 432 | ||
| 433 | 433 | assert.NoError(t, err) | |
| 434 | 434 | } | |
| @@ -467,7 +467,7 @@ func TestManualReloadHandler_Handle_DeployErrors(t *testing.T) { | |||
| 467 | 467 | Type: domain.EventManualReload, | |
| 468 | 468 | } | |
| 469 | 469 | ||
| 470 | - | err := handler.Handle(event) | |
| 470 | + | err := handler.Handle(context.Background(), event) | |
| 471 | 471 | ||
| 472 | 472 | // Should return error indicating some failures | |
| 473 | 473 | assert.Error(t, err) | |
| @@ -497,7 +497,7 @@ func TestManualReloadHandler_Handle_DoesNotRestartRunningContainers(t *testing.T | |||
| 497 | 497 | Type: domain.EventManualReload, | |
| 498 | 498 | } | |
| 499 | 499 | ||
| 500 | - | err := handler.Handle(event) | |
| 500 | + | err := handler.Handle(context.Background(), event) | |
| 501 | 501 | ||
| 502 | 502 | assert.NoError(t, err) | |
| 503 | 503 | } | |
| @@ -539,7 +539,7 @@ func TestManualDeployHandler_Handle_DeploysSpecificRoute(t *testing.T) { | |||
| 539 | 539 | Data: &domain.ManualDeployPayload{Domain: "app1.example.com"}, | |
| 540 | 540 | } | |
| 541 | 541 | ||
| 542 | - | err := handler.Handle(event) | |
| 542 | + | err := handler.Handle(context.Background(), event) | |
| 543 | 543 | ||
| 544 | 544 | assert.NoError(t, err) | |
| 545 | 545 | } | |
| @@ -561,7 +561,7 @@ func TestManualDeployHandler_Handle_RouteNotFound(t *testing.T) { | |||
| 561 | 561 | Data: &domain.ManualDeployPayload{Domain: "unknown.example.com"}, | |
| 562 | 562 | } | |
| 563 | 563 | ||
| 564 | - | err := handler.Handle(event) | |
| 564 | + | err := handler.Handle(context.Background(), event) | |
| 565 | 565 | ||
| 566 | 566 | assert.Error(t, err) | |
| 567 | 567 | assert.Contains(t, err.Error(), "route not found") | |
| @@ -580,7 +580,7 @@ func TestManualDeployHandler_Handle_InvalidPayload(t *testing.T) { | |||
| 580 | 580 | Data: nil, | |
| 581 | 581 | } | |
| 582 | 582 | ||
| 583 | - | err := handler.Handle(event) | |
| 583 | + | err := handler.Handle(context.Background(), event) | |
| 584 | 584 | ||
| 585 | 585 | assert.Error(t, err) | |
| 586 | 586 | assert.Contains(t, err.Error(), "invalid manual deploy payload") | |
| @@ -599,7 +599,7 @@ func TestManualDeployHandler_Handle_EmptyDomain(t *testing.T) { | |||
| 599 | 599 | Data: &domain.ManualDeployPayload{Domain: ""}, | |
| 600 | 600 | } | |
| 601 | 601 | ||
| 602 | - | err := handler.Handle(event) | |
| 602 | + | err := handler.Handle(context.Background(), event) | |
| 603 | 603 | ||
| 604 | 604 | assert.Error(t, err) | |
| 605 | 605 | assert.Contains(t, err.Error(), "invalid manual deploy payload") | |
| @@ -623,7 +623,7 @@ func TestManualDeployHandler_Handle_DeployError(t *testing.T) { | |||
| 623 | 623 | Data: &domain.ManualDeployPayload{Domain: "app.example.com"}, | |
| 624 | 624 | } | |
| 625 | 625 | ||
| 626 | - | err := handler.Handle(event) | |
| 626 | + | err := handler.Handle(context.Background(), event) | |
| 627 | 627 | ||
| 628 | 628 | assert.Error(t, err) | |
| 629 | 629 | assert.Contains(t, err.Error(), "failed to deploy") | |
| @@ -63,7 +63,7 @@ func TestImagePushedHandler_Handle_DeploysMatchingRoutes(t *testing.T) { | |||||
| 63 | Tag: "latest", | 63 | Tag: "latest", | ||
| 64 | } | 64 | } | ||
| 65 | 65 | ||||
| 66 | - | err := handler.Handle(event) | 66 | + | err := handler.Handle(context.Background(), event) |
| 67 | 67 | ||||
| 68 | assert.NoError(t, err) | 68 | assert.NoError(t, err) | ||
| 69 | } | 69 | } | ||
| @@ -89,7 +89,7 @@ func TestImagePushedHandler_Handle_NoMatchingRoutes(t *testing.T) { | |||||
| 89 | Tag: "latest", | 89 | Tag: "latest", | ||
| 90 | } | 90 | } | ||
| 91 | 91 | ||||
| 92 | - | err := handler.Handle(event) | 92 | + | err := handler.Handle(context.Background(), event) |
| 93 | 93 | ||||
| 94 | assert.NoError(t, err) | 94 | assert.NoError(t, err) | ||
| 95 | } | 95 | } | ||
| @@ -107,7 +107,7 @@ func TestImagePushedHandler_Handle_EmptyImageName(t *testing.T) { | |||||
| 107 | Tag: "latest", | 107 | Tag: "latest", | ||
| 108 | } | 108 | } | ||
| 109 | 109 | ||||
| 110 | - | err := handler.Handle(event) | 110 | + | err := handler.Handle(context.Background(), event) |
| 111 | 111 | ||||
| 112 | assert.ErrorIs(t, err, domain.ErrInvalidImageFormat) | 112 | assert.ErrorIs(t, err, domain.ErrInvalidImageFormat) | ||
| 113 | } | 113 | } | ||
| @@ -137,7 +137,7 @@ func TestImagePushedHandler_Handle_DefaultTag(t *testing.T) { | |||||
| 137 | Tag: "", // Empty tag should default to "latest" | 137 | Tag: "", // Empty tag should default to "latest" | ||
| 138 | } | 138 | } | ||
| 139 | 139 | ||||
| 140 | - | err := handler.Handle(event) | 140 | + | err := handler.Handle(context.Background(), event) |
| 141 | 141 | ||||
| 142 | assert.NoError(t, err) | 142 | assert.NoError(t, err) | ||
| 143 | } | 143 | } | ||
| @@ -164,7 +164,7 @@ func TestImagePushedHandler_Handle_DeployError(t *testing.T) { | |||||
| 164 | } | 164 | } | ||
| 165 | 165 | ||||
| 166 | // Handler logs error but doesn't fail | 166 | // Handler logs error but doesn't fail | ||
| 167 | - | err := handler.Handle(event) | 167 | + | err := handler.Handle(context.Background(), event) |
| 168 | 168 | ||||
| 169 | assert.NoError(t, err) | 169 | assert.NoError(t, err) | ||
| 170 | } | 170 | } | ||
| @@ -192,7 +192,7 @@ func TestImagePushedHandler_Handle_StripsRegistryDomain(t *testing.T) { | |||||
| 192 | Tag: "latest", | 192 | Tag: "latest", | ||
| 193 | } | 193 | } | ||
| 194 | 194 | ||||
| 195 | - | err := handler.Handle(event) | 195 | + | err := handler.Handle(context.Background(), event) |
| 196 | 196 | ||||
| 197 | assert.NoError(t, err) | 197 | assert.NoError(t, err) | ||
| 198 | } | 198 | } | ||
| @@ -237,7 +237,7 @@ func TestConfigReloadHandler_Handle_DeploysNewRoutes(t *testing.T) { | |||||
| 237 | Type: domain.EventConfigReload, | 237 | Type: domain.EventConfigReload, | ||
| 238 | } | 238 | } | ||
| 239 | 239 | ||||
| 240 | - | err := handler.Handle(event) | 240 | + | err := handler.Handle(context.Background(), event) |
| 241 | 241 | ||||
| 242 | assert.NoError(t, err) | 242 | assert.NoError(t, err) | ||
| 243 | } | 243 | } | ||
| @@ -273,7 +273,7 @@ func TestConfigReloadHandler_Handle_StopsRemovedRoutes(t *testing.T) { | |||||
| 273 | Type: domain.EventConfigReload, | 273 | Type: domain.EventConfigReload, | ||
| 274 | } | 274 | } | ||
| 275 | 275 | ||||
| 276 | - | err := handler.Handle(event) | 276 | + | err := handler.Handle(context.Background(), event) |
| 277 | 277 | ||||
| 278 | assert.NoError(t, err) | 278 | assert.NoError(t, err) | ||
| 279 | } | 279 | } | ||
| @@ -313,7 +313,7 @@ func TestConfigReloadHandler_Handle_RedeploysChangedImage(t *testing.T) { | |||||
| 313 | Type: domain.EventConfigReload, | 313 | Type: domain.EventConfigReload, | ||
| 314 | } | 314 | } | ||
| 315 | 315 | ||||
| 316 | - | err := handler.Handle(event) | 316 | + | err := handler.Handle(context.Background(), event) |
| 317 | 317 | ||||
| 318 | assert.NoError(t, err) | 318 | assert.NoError(t, err) | ||
| 319 | } | 319 | } | ||
| @@ -349,7 +349,7 @@ func TestConfigReloadHandler_Handle_NoChanges(t *testing.T) { | |||||
| 349 | Type: domain.EventConfigReload, | 349 | Type: domain.EventConfigReload, | ||
| 350 | } | 350 | } | ||
| 351 | 351 | ||||
| 352 | - | err := handler.Handle(event) | 352 | + | err := handler.Handle(context.Background(), event) |
| 353 | 353 | ||||
| 354 | assert.NoError(t, err) | 354 | assert.NoError(t, err) | ||
| 355 | } | 355 | } | ||
| @@ -397,7 +397,7 @@ func TestManualReloadHandler_Handle_StartsOnlyMissingContainers(t *testing.T) { | |||||
| 397 | Type: domain.EventManualReload, | 397 | Type: domain.EventManualReload, | ||
| 398 | } | 398 | } | ||
| 399 | 399 | ||||
| 400 | - | err := handler.Handle(event) | 400 | + | err := handler.Handle(context.Background(), event) |
| 401 | 401 | ||||
| 402 | assert.NoError(t, err) | 402 | assert.NoError(t, err) | ||
| 403 | } | 403 | } | ||
| @@ -428,7 +428,7 @@ func TestManualReloadHandler_Handle_StartsMissingContainer(t *testing.T) { | |||||
| 428 | Type: domain.EventManualReload, | 428 | Type: domain.EventManualReload, | ||
| 429 | } | 429 | } | ||
| 430 | 430 | ||||
| 431 | - | err := handler.Handle(event) | 431 | + | err := handler.Handle(context.Background(), event) |
| 432 | 432 | ||||
| 433 | assert.NoError(t, err) | 433 | assert.NoError(t, err) | ||
| 434 | } | 434 | } | ||
| @@ -467,7 +467,7 @@ func TestManualReloadHandler_Handle_DeployErrors(t *testing.T) { | |||||
| 467 | Type: domain.EventManualReload, | 467 | Type: domain.EventManualReload, | ||
| 468 | } | 468 | } | ||
| 469 | 469 | ||||
| 470 | - | err := handler.Handle(event) | 470 | + | err := handler.Handle(context.Background(), event) |
| 471 | 471 | ||||
| 472 | // Should return error indicating some failures | 472 | // Should return error indicating some failures | ||
| 473 | assert.Error(t, err) | 473 | assert.Error(t, err) | ||
| @@ -497,7 +497,7 @@ func TestManualReloadHandler_Handle_DoesNotRestartRunningContainers(t *testing.T | |||||
| 497 | Type: domain.EventManualReload, | 497 | Type: domain.EventManualReload, | ||
| 498 | } | 498 | } | ||
| 499 | 499 | ||||
| 500 | - | err := handler.Handle(event) | 500 | + | err := handler.Handle(context.Background(), event) |
| 501 | 501 | ||||
| 502 | assert.NoError(t, err) | 502 | assert.NoError(t, err) | ||
| 503 | } | 503 | } | ||
| @@ -539,7 +539,7 @@ func TestManualDeployHandler_Handle_DeploysSpecificRoute(t *testing.T) { | |||||
| 539 | Data: &domain.ManualDeployPayload{Domain: "app1.example.com"}, | 539 | Data: &domain.ManualDeployPayload{Domain: "app1.example.com"}, | ||
| 540 | } | 540 | } | ||
| 541 | 541 | ||||
| 542 | - | err := handler.Handle(event) | 542 | + | err := handler.Handle(context.Background(), event) |
| 543 | 543 | ||||
| 544 | assert.NoError(t, err) | 544 | assert.NoError(t, err) | ||
| 545 | } | 545 | } | ||
| @@ -561,7 +561,7 @@ func TestManualDeployHandler_Handle_RouteNotFound(t *testing.T) { | |||||
| 561 | Data: &domain.ManualDeployPayload{Domain: "unknown.example.com"}, | 561 | Data: &domain.ManualDeployPayload{Domain: "unknown.example.com"}, | ||
| 562 | } | 562 | } | ||
| 563 | 563 | ||||
| 564 | - | err := handler.Handle(event) | 564 | + | err := handler.Handle(context.Background(), event) |
| 565 | 565 | ||||
| 566 | assert.Error(t, err) | 566 | assert.Error(t, err) | ||
| 567 | assert.Contains(t, err.Error(), "route not found") | 567 | assert.Contains(t, err.Error(), "route not found") | ||
| @@ -580,7 +580,7 @@ func TestManualDeployHandler_Handle_InvalidPayload(t *testing.T) { | |||||
| 580 | Data: nil, | 580 | Data: nil, | ||
| 581 | } | 581 | } | ||
| 582 | 582 | ||||
| 583 | - | err := handler.Handle(event) | 583 | + | err := handler.Handle(context.Background(), event) |
| 584 | 584 | ||||
| 585 | assert.Error(t, err) | 585 | assert.Error(t, err) | ||
| 586 | assert.Contains(t, err.Error(), "invalid manual deploy payload") | 586 | assert.Contains(t, err.Error(), "invalid manual deploy payload") | ||
| @@ -599,7 +599,7 @@ func TestManualDeployHandler_Handle_EmptyDomain(t *testing.T) { | |||||
| 599 | Data: &domain.ManualDeployPayload{Domain: ""}, | 599 | Data: &domain.ManualDeployPayload{Domain: ""}, | ||
| 600 | } | 600 | } | ||
| 601 | 601 | ||||
| 602 | - | err := handler.Handle(event) | 602 | + | err := handler.Handle(context.Background(), event) |
| 603 | 603 | ||||
| 604 | assert.Error(t, err) | 604 | assert.Error(t, err) | ||
| 605 | assert.Contains(t, err.Error(), "invalid manual deploy payload") | 605 | assert.Contains(t, err.Error(), "invalid manual deploy payload") | ||
| @@ -623,7 +623,7 @@ func TestManualDeployHandler_Handle_DeployError(t *testing.T) { | |||||
| 623 | Data: &domain.ManualDeployPayload{Domain: "app.example.com"}, | 623 | Data: &domain.ManualDeployPayload{Domain: "app.example.com"}, | ||
| 624 | } | 624 | } | ||
| 625 | 625 | ||||
| 626 | - | err := handler.Handle(event) | 626 | + | err := handler.Handle(context.Background(), event) |
| 627 | 627 | ||||
| 628 | assert.Error(t, err) | 628 | assert.Error(t, err) | ||
| 629 | assert.Contains(t, err.Error(), "failed to deploy") | 629 | assert.Contains(t, err.Error(), "failed to deploy") | ||
M internal/usecase/container/service.go +177 -46
| @@ -213,7 +213,7 @@ func (s *Service) Deploy(ctx context.Context, route domain.Route) (*domain.Conta | |||
| 213 | 213 | ||
| 214 | 214 | // Deploy attachments BEFORE main container (dependencies first) | |
| 215 | 215 | if err := s.deployAttachments(ctx, route.Domain, networkName); err != nil { | |
| 216 | - | log.WrapErr(err, "failed to deploy some attachments") | |
| 216 | + | return nil, log.WrapErr(err, "failed to deploy attachments") | |
| 217 | 217 | } | |
| 218 | 218 | ||
| 219 | 219 | // Get exposed ports (use actual image ref from pull) | |
| @@ -371,28 +371,23 @@ func (s *Service) Remove(ctx context.Context, containerID string, force bool) er | |||
| 371 | 371 | } | |
| 372 | 372 | } | |
| 373 | 373 | ||
| 374 | - | // Find domain for cleanup | |
| 375 | - | var containerDomain string | |
| 376 | - | s.mu.RLock() | |
| 377 | - | for d, c := range s.containers { | |
| 378 | - | if c.ID == containerID { | |
| 379 | - | containerDomain = d | |
| 380 | - | break | |
| 381 | - | } | |
| 382 | - | } | |
| 383 | - | s.mu.RUnlock() | |
| 374 | + | // Find domain and attachment IDs for cleanup | |
| 375 | + | containerDomain, attachmentIDs, cfg := s.findContainerContext(containerID) | |
| 384 | 376 | ||
| 385 | 377 | if err := s.runtime.RemoveContainer(ctx, containerID, force); err != nil { | |
| 386 | 378 | return log.WrapErr(err, "failed to remove container") | |
| 387 | 379 | } | |
| 388 | 380 | ||
| 389 | 381 | // Cleanup volumes | |
| 390 | - | if containerDomain != "" && !s.config.VolumePreserve { | |
| 382 | + | if containerDomain != "" && !cfg.VolumePreserve { | |
| 391 | 383 | if err := s.cleanupVolumesForDomain(ctx, containerDomain); err != nil { | |
| 392 | 384 | log.WrapErrWithFields(err, "failed to cleanup volumes", map[string]any{"domain": containerDomain}) | |
| 393 | 385 | } | |
| 394 | 386 | } | |
| 395 | 387 | ||
| 388 | + | // Clean up attachment containers | |
| 389 | + | s.removeAttachments(ctx, attachmentIDs) | |
| 390 | + | ||
| 396 | 391 | // Remove from tracking | |
| 397 | 392 | s.mu.Lock() | |
| 398 | 393 | var removedDomain string | |
| @@ -404,6 +399,9 @@ func (s *Service) Remove(ctx context.Context, containerID string, force bool) er | |||
| 404 | 399 | break | |
| 405 | 400 | } | |
| 406 | 401 | } | |
| 402 | + | if removedDomain != "" { | |
| 403 | + | delete(s.attachments, removedDomain) | |
| 404 | + | } | |
| 407 | 405 | s.mu.Unlock() | |
| 408 | 406 | ||
| 409 | 407 | // Note: We intentionally do NOT clean up deployMu entries to avoid a race condition: | |
| @@ -413,7 +411,7 @@ func (s *Service) Remove(ctx context.Context, containerID string, force bool) er | |||
| 413 | 411 | // domain ever deployed), and this is the safer choice for correctness. | |
| 414 | 412 | ||
| 415 | 413 | // Cleanup network | |
| 416 | - | if removedDomain != "" && s.config.NetworkIsolation { | |
| 414 | + | if removedDomain != "" && cfg.NetworkIsolation { | |
| 417 | 415 | networkName := s.getNetworkForApp(removedDomain) | |
| 418 | 416 | if err := s.cleanupNetworkIfEmpty(ctx, networkName); err != nil { | |
| 419 | 417 | log.WrapErrWithFields(err, "failed to cleanup network", map[string]any{"network": networkName}) | |
| @@ -423,6 +421,43 @@ func (s *Service) Remove(ctx context.Context, containerID string, force bool) er | |||
| 423 | 421 | return nil | |
| 424 | 422 | } | |
| 425 | 423 | ||
| 424 | + | // findContainerContext looks up the domain, attachment IDs, and config snapshot for a container. | |
| 425 | + | func (s *Service) findContainerContext(containerID string) (string, []string, Config) { | |
| 426 | + | s.mu.RLock() | |
| 427 | + | defer s.mu.RUnlock() | |
| 428 | + | cfg := s.config | |
| 429 | + | var containerDomain string | |
| 430 | + | for d, c := range s.containers { | |
| 431 | + | if c.ID == containerID { | |
| 432 | + | containerDomain = d | |
| 433 | + | break | |
| 434 | + | } | |
| 435 | + | } | |
| 436 | + | var attachmentIDs []string | |
| 437 | + | if containerDomain != "" { | |
| 438 | + | attachmentIDs = append(attachmentIDs, s.attachments[containerDomain]...) | |
| 439 | + | } | |
| 440 | + | return containerDomain, attachmentIDs, cfg | |
| 441 | + | } | |
| 442 | + | ||
| 443 | + | // removeAttachments stops and removes attachment containers best-effort. | |
| 444 | + | func (s *Service) removeAttachments(ctx context.Context, attachmentIDs []string) { | |
| 445 | + | log := zerowrap.FromCtx(ctx) | |
| 446 | + | for _, attachID := range attachmentIDs { | |
| 447 | + | if s.logWriter != nil { | |
| 448 | + | if err := s.logWriter.StopLogging(attachID); err != nil { | |
| 449 | + | log.Warn().Err(err).Str("attachment_id", attachID).Msg("failed to stop attachment log collection") | |
| 450 | + | } | |
| 451 | + | } | |
| 452 | + | if err := s.runtime.StopContainer(ctx, attachID); err != nil { | |
| 453 | + | log.Warn().Err(err).Str("attachment_id", attachID).Msg("failed to stop attachment container") | |
| 454 | + | } | |
| 455 | + | if err := s.runtime.RemoveContainer(ctx, attachID, true); err != nil { | |
| 456 | + | log.Warn().Err(err).Str("attachment_id", attachID).Msg("failed to remove attachment container") | |
| 457 | + | } | |
| 458 | + | } | |
| 459 | + | } | |
| 460 | + | ||
| 426 | 461 | // Get retrieves a container by domain. | |
| 427 | 462 | func (s *Service) Get(_ context.Context, domainName string) (*domain.Container, bool) { | |
| 428 | 463 | s.mu.RLock() | |
| @@ -492,10 +527,14 @@ func (s *Service) ListRoutesWithDetails(ctx context.Context) []domain.RouteInfo | |||
| 492 | 527 | // stripRegistryPrefix removes the configured registry domain prefix from an image reference. | |
| 493 | 528 | // For example, "reg.example.com/myapp:latest" becomes "myapp:latest" if registry domain is "reg.example.com". | |
| 494 | 529 | func (s *Service) stripRegistryPrefix(image string) string { | |
| 495 | - | if s.config.RegistryDomain == "" { | |
| 530 | + | s.mu.RLock() | |
| 531 | + | cfg := s.config | |
| 532 | + | s.mu.RUnlock() | |
| 533 | + | ||
| 534 | + | if cfg.RegistryDomain == "" { | |
| 496 | 535 | return image | |
| 497 | 536 | } | |
| 498 | - | prefix := strings.TrimSuffix(s.config.RegistryDomain, "/") + "/" | |
| 537 | + | prefix := strings.TrimSuffix(cfg.RegistryDomain, "/") + "/" | |
| 499 | 538 | if strings.HasPrefix(image, prefix) { | |
| 500 | 539 | return strings.TrimPrefix(image, prefix) | |
| 501 | 540 | } | |
| @@ -509,6 +548,10 @@ func (s *Service) ListAttachments(ctx context.Context, domainName string) []doma | |||
| 509 | 548 | ||
| 510 | 549 | // ListNetworks returns Gordon-managed networks. | |
| 511 | 550 | func (s *Service) ListNetworks(ctx context.Context) ([]*domain.NetworkInfo, error) { | |
| 551 | + | s.mu.RLock() | |
| 552 | + | cfg := s.config | |
| 553 | + | s.mu.RUnlock() | |
| 554 | + | ||
| 512 | 555 | networks, err := s.runtime.ListNetworks(ctx) | |
| 513 | 556 | if err != nil { | |
| 514 | 557 | return nil, err | |
| @@ -516,7 +559,7 @@ func (s *Service) ListNetworks(ctx context.Context) ([]*domain.NetworkInfo, erro | |||
| 516 | 559 | ||
| 517 | 560 | var filtered []*domain.NetworkInfo | |
| 518 | 561 | for _, network := range networks { | |
| 519 | - | if strings.HasPrefix(network.Name, s.config.NetworkPrefix+"-") { | |
| 562 | + | if strings.HasPrefix(network.Name, cfg.NetworkPrefix+"-") { | |
| 520 | 563 | filtered = append(filtered, network) | |
| 521 | 564 | } | |
| 522 | 565 | } | |
| @@ -532,14 +575,19 @@ func (s *Service) HealthCheck(ctx context.Context) map[string]bool { | |||
| 532 | 575 | }) | |
| 533 | 576 | log := zerowrap.FromCtx(ctx) | |
| 534 | 577 | ||
| 578 | + | // Copy container IDs under lock, then release before making Docker API calls | |
| 535 | 579 | s.mu.RLock() | |
| 536 | - | defer s.mu.RUnlock() | |
| 537 | - | ||
| 538 | - | health := make(map[string]bool, len(s.containers)) | |
| 580 | + | snapshot := make(map[string]string, len(s.containers)) | |
| 539 | 581 | for d, c := range s.containers { | |
| 540 | - | running, err := s.runtime.IsContainerRunning(ctx, c.ID) | |
| 582 | + | snapshot[d] = c.ID | |
| 583 | + | } | |
| 584 | + | s.mu.RUnlock() | |
| 585 | + | ||
| 586 | + | health := make(map[string]bool, len(snapshot)) | |
| 587 | + | for d, id := range snapshot { | |
| 588 | + | running, err := s.runtime.IsContainerRunning(ctx, id) | |
| 541 | 589 | if err != nil { | |
| 542 | - | log.WrapErrWithFields(err, "health check failed", map[string]any{"domain": d, zerowrap.FieldEntityID: c.ID}) | |
| 590 | + | log.WrapErrWithFields(err, "health check failed", map[string]any{"domain": d, zerowrap.FieldEntityID: id}) | |
| 543 | 591 | health[d] = false | |
| 544 | 592 | } else { | |
| 545 | 593 | health[d] = running | |
| @@ -556,9 +604,7 @@ func (s *Service) SyncContainers(ctx context.Context) error { | |||
| 556 | 604 | }) | |
| 557 | 605 | log := zerowrap.FromCtx(ctx) | |
| 558 | 606 | ||
| 559 | - | s.mu.Lock() | |
| 560 | - | defer s.mu.Unlock() | |
| 561 | - | ||
| 607 | + | // List containers without holding lock to avoid blocking during Docker API call | |
| 562 | 608 | allContainers, err := s.runtime.ListContainers(ctx, false) | |
| 563 | 609 | if err != nil { | |
| 564 | 610 | return log.WrapErr(err, "failed to list containers") | |
| @@ -573,7 +619,10 @@ func (s *Service) SyncContainers(ctx context.Context) error { | |||
| 573 | 619 | } | |
| 574 | 620 | } | |
| 575 | 621 | ||
| 622 | + | s.mu.Lock() | |
| 576 | 623 | s.containers = managed | |
| 624 | + | s.mu.Unlock() | |
| 625 | + | ||
| 577 | 626 | log.Info().Int(zerowrap.FieldCount, len(managed)).Msg("container state synchronized") | |
| 578 | 627 | return nil | |
| 579 | 628 | } | |
| @@ -649,7 +698,9 @@ func (s *Service) Shutdown(ctx context.Context) error { | |||
| 649 | 698 | ||
| 650 | 699 | // UpdateConfig updates the service configuration. | |
| 651 | 700 | func (s *Service) UpdateConfig(config Config) { | |
| 701 | + | s.mu.Lock() | |
| 652 | 702 | s.config = config | |
| 703 | + | s.mu.Unlock() | |
| 653 | 704 | } | |
| 654 | 705 | ||
| 655 | 706 | // Helper methods | |
| @@ -714,12 +765,16 @@ func (s *Service) startLogCollection(ctx context.Context, containerID, domainNam | |||
| 714 | 765 | } | |
| 715 | 766 | ||
| 716 | 767 | func (s *Service) buildImageRef(image string) string { | |
| 717 | - | if !s.config.RegistryAuthEnabled || s.config.RegistryDomain == "" { | |
| 768 | + | s.mu.RLock() | |
| 769 | + | cfg := s.config | |
| 770 | + | s.mu.RUnlock() | |
| 771 | + | ||
| 772 | + | if !cfg.RegistryAuthEnabled || cfg.RegistryDomain == "" { | |
| 718 | 773 | return image | |
| 719 | 774 | } | |
| 720 | 775 | ||
| 721 | 776 | // Normalize registry domain by trimming any trailing slash | |
| 722 | - | reg := strings.TrimSuffix(s.config.RegistryDomain, "/") | |
| 777 | + | reg := strings.TrimSuffix(cfg.RegistryDomain, "/") | |
| 723 | 778 | ||
| 724 | 779 | // Check if image already has the registry domain prefix | |
| 725 | 780 | prefix := reg + "/" | |
| @@ -815,7 +870,10 @@ func (s *Service) pullRefForDeploy(ctx context.Context, imageRef string) (string | |||
| 815 | 870 | if !domain.IsInternalDeploy(ctx) { | |
| 816 | 871 | return imageRef, false | |
| 817 | 872 | } | |
| 818 | - | return rewriteToLocalRegistry(imageRef, s.config.RegistryDomain, s.config.RegistryPort), true | |
| 873 | + | s.mu.RLock() | |
| 874 | + | cfg := s.config | |
| 875 | + | s.mu.RUnlock() | |
| 876 | + | return rewriteToLocalRegistry(imageRef, cfg.RegistryDomain, cfg.RegistryPort), true | |
| 819 | 877 | } | |
| 820 | 878 | ||
| 821 | 879 | // ensureImage ensures the image is available locally, pulling if needed. | |
| @@ -885,7 +943,11 @@ func (s *Service) ensureLocalImage(ctx context.Context, imageRef, pullRef string | |||
| 885 | 943 | return false, nil | |
| 886 | 944 | } | |
| 887 | 945 | ||
| 888 | - | pullPolicy := normalizePullPolicy(s.config.PullPolicy) | |
| 946 | + | s.mu.RLock() | |
| 947 | + | cfg := s.config | |
| 948 | + | s.mu.RUnlock() | |
| 949 | + | ||
| 950 | + | pullPolicy := normalizePullPolicy(cfg.PullPolicy) | |
| 889 | 951 | switch pullPolicy { | |
| 890 | 952 | case PullPolicyAlways: | |
| 891 | 953 | log.Info().Str("pull_policy", pullPolicy).Msg("pull policy forces image pull") | |
| @@ -930,25 +992,29 @@ func (s *Service) ensureLocalImage(ctx context.Context, imageRef, pullRef string | |||
| 930 | 992 | } | |
| 931 | 993 | ||
| 932 | 994 | func (s *Service) pullImage(ctx context.Context, pullRef string, isInternal bool) error { | |
| 995 | + | s.mu.RLock() | |
| 996 | + | cfg := s.config | |
| 997 | + | s.mu.RUnlock() | |
| 998 | + | ||
| 933 | 999 | log := zerowrap.FromCtx(ctx) | |
| 934 | 1000 | ||
| 935 | 1001 | switch { | |
| 936 | - | case isInternal && s.config.RegistryAuthEnabled: | |
| 937 | - | if s.config.InternalRegistryUsername == "" || s.config.InternalRegistryPassword == "" { | |
| 1002 | + | case isInternal && cfg.RegistryAuthEnabled: | |
| 1003 | + | if cfg.InternalRegistryUsername == "" || cfg.InternalRegistryPassword == "" { | |
| 938 | 1004 | return log.WrapErr(fmt.Errorf("internal registry auth not configured"), "failed to pull image for internal deploy") | |
| 939 | 1005 | } | |
| 940 | - | if err := s.runtime.PullImageWithAuth(ctx, pullRef, s.config.InternalRegistryUsername, s.config.InternalRegistryPassword); err != nil { | |
| 1006 | + | if err := s.runtime.PullImageWithAuth(ctx, pullRef, cfg.InternalRegistryUsername, cfg.InternalRegistryPassword); err != nil { | |
| 941 | 1007 | return log.WrapErr(err, "failed to pull image with internal auth") | |
| 942 | 1008 | } | |
| 943 | 1009 | case isInternal: | |
| 944 | 1010 | if err := s.runtime.PullImage(ctx, pullRef); err != nil { | |
| 945 | 1011 | return log.WrapErr(err, "failed to pull image") | |
| 946 | 1012 | } | |
| 947 | - | case s.config.RegistryAuthEnabled: | |
| 948 | - | if s.config.ServiceTokenUsername == "" || s.config.ServiceToken == "" { | |
| 1013 | + | case cfg.RegistryAuthEnabled: | |
| 1014 | + | if cfg.ServiceTokenUsername == "" || cfg.ServiceToken == "" { | |
| 949 | 1015 | return log.WrapErr(fmt.Errorf("registry service token not configured"), "failed to pull image for registry auth") | |
| 950 | 1016 | } | |
| 951 | - | if err := s.runtime.PullImageWithAuth(ctx, pullRef, s.config.ServiceTokenUsername, s.config.ServiceToken); err != nil { | |
| 1017 | + | if err := s.runtime.PullImageWithAuth(ctx, pullRef, cfg.ServiceTokenUsername, cfg.ServiceToken); err != nil { | |
| 952 | 1018 | return log.WrapErr(err, "failed to pull image with auth") | |
| 953 | 1019 | } | |
| 954 | 1020 | default: | |
| @@ -997,10 +1063,14 @@ func (s *Service) loadEnvironment(ctx context.Context, domainName, imageRef stri | |||
| 997 | 1063 | } | |
| 998 | 1064 | ||
| 999 | 1065 | func (s *Service) setupVolumes(ctx context.Context, domainName, imageRef string) (map[string]string, error) { | |
| 1066 | + | s.mu.RLock() | |
| 1067 | + | cfg := s.config | |
| 1068 | + | s.mu.RUnlock() | |
| 1069 | + | ||
| 1000 | 1070 | log := zerowrap.FromCtx(ctx) | |
| 1001 | 1071 | volumes := make(map[string]string) | |
| 1002 | 1072 | ||
| 1003 | - | if !s.config.VolumeAutoCreate { | |
| 1073 | + | if !cfg.VolumeAutoCreate { | |
| 1004 | 1074 | return volumes, nil | |
| 1005 | 1075 | } | |
| 1006 | 1076 | ||
| @@ -1011,7 +1081,7 @@ func (s *Service) setupVolumes(ctx context.Context, domainName, imageRef string) | |||
| 1011 | 1081 | } | |
| 1012 | 1082 | ||
| 1013 | 1083 | for _, path := range volumePaths { | |
| 1014 | - | name := generateVolumeName(s.config.VolumePrefix, domainName, path) | |
| 1084 | + | name := generateVolumeName(cfg.VolumePrefix, domainName, path) | |
| 1015 | 1085 | ||
| 1016 | 1086 | exists, err := s.runtime.VolumeExists(ctx, name) | |
| 1017 | 1087 | if err != nil { | |
| @@ -1034,11 +1104,15 @@ func (s *Service) setupVolumes(ctx context.Context, domainName, imageRef string) | |||
| 1034 | 1104 | } | |
| 1035 | 1105 | ||
| 1036 | 1106 | func (s *Service) getNetworkForApp(domainName string) string { | |
| 1037 | - | if !s.config.NetworkIsolation { | |
| 1107 | + | s.mu.RLock() | |
| 1108 | + | cfg := s.config | |
| 1109 | + | s.mu.RUnlock() | |
| 1110 | + | ||
| 1111 | + | if !cfg.NetworkIsolation { | |
| 1038 | 1112 | return "bridge" | |
| 1039 | 1113 | } | |
| 1040 | 1114 | ||
| 1041 | - | for groupName, domains := range s.config.NetworkGroups { | |
| 1115 | + | for groupName, domains := range cfg.NetworkGroups { | |
| 1042 | 1116 | if slices.Contains(domains, domainName) { | |
| 1043 | 1117 | return s.generateNetworkName(groupName) | |
| 1044 | 1118 | } | |
| @@ -1048,7 +1122,11 @@ func (s *Service) getNetworkForApp(domainName string) string { | |||
| 1048 | 1122 | } | |
| 1049 | 1123 | ||
| 1050 | 1124 | func (s *Service) generateNetworkName(identifier string) string { | |
| 1051 | - | return fmt.Sprintf("%s-%s", s.config.NetworkPrefix, strings.ReplaceAll(identifier, ".", "-")) | |
| 1125 | + | s.mu.RLock() | |
| 1126 | + | cfg := s.config | |
| 1127 | + | s.mu.RUnlock() | |
| 1128 | + | ||
| 1129 | + | return fmt.Sprintf("%s-%s", cfg.NetworkPrefix, strings.ReplaceAll(identifier, ".", "-")) | |
| 1052 | 1130 | } | |
| 1053 | 1131 | ||
| 1054 | 1132 | func (s *Service) createNetworkIfNeeded(ctx context.Context, networkName string) error { | |
| @@ -1138,12 +1216,49 @@ func (s *Service) deployAttachments(ctx context.Context, domainName, networkName | |||
| 1138 | 1216 | } | |
| 1139 | 1217 | ||
| 1140 | 1218 | log := zerowrap.FromCtx(ctx) | |
| 1219 | + | var deployed []string // track successfully deployed attachment IDs for rollback | |
| 1141 | 1220 | for _, svc := range attachments { | |
| 1142 | 1221 | if err := s.deployAttachedService(ctx, domainName, svc, networkName); err != nil { | |
| 1143 | 1222 | log.WrapErrWithFields(err, "failed to deploy attachment", map[string]any{zerowrap.FieldService: svc, "domain": domainName}) | |
| 1223 | + | ||
| 1224 | + | // Rollback: clean up already-deployed attachments | |
| 1225 | + | for _, id := range deployed { | |
| 1226 | + | if stopErr := s.runtime.StopContainer(ctx, id); stopErr != nil { | |
| 1227 | + | log.Warn().Err(stopErr).Str("attachment_id", id).Msg("rollback: failed to stop attachment") | |
| 1228 | + | } | |
| 1229 | + | if rmErr := s.runtime.RemoveContainer(ctx, id, true); rmErr != nil { | |
| 1230 | + | log.Warn().Err(rmErr).Str("attachment_id", id).Msg("rollback: failed to remove attachment") | |
| 1231 | + | } | |
| 1232 | + | } | |
| 1233 | + | // Deregister rolled-back attachments | |
| 1234 | + | if len(deployed) > 0 { | |
| 1235 | + | s.mu.Lock() | |
| 1236 | + | remaining := s.attachments[domainName] | |
| 1237 | + | filtered := remaining[:0] | |
| 1238 | + | rollbackSet := make(map[string]bool, len(deployed)) | |
| 1239 | + | for _, id := range deployed { | |
| 1240 | + | rollbackSet[id] = true | |
| 1241 | + | } | |
| 1242 | + | for _, id := range remaining { | |
| 1243 | + | if !rollbackSet[id] { | |
| 1244 | + | filtered = append(filtered, id) | |
| 1245 | + | } | |
| 1246 | + | } | |
| 1247 | + | s.attachments[domainName] = filtered | |
| 1248 | + | s.mu.Unlock() | |
| 1249 | + | } | |
| 1250 | + | ||
| 1251 | + | return fmt.Errorf("failed to deploy attachment %q (rolled back %d already-deployed)", svc, len(deployed)) | |
| 1252 | + | } | |
| 1253 | + | // Collect IDs of attachments tracked under this domain after successful deploy | |
| 1254 | + | s.mu.RLock() | |
| 1255 | + | ids := s.attachments[domainName] | |
| 1256 | + | if len(ids) > 0 { | |
| 1257 | + | latest := ids[len(ids)-1] | |
| 1258 | + | deployed = append(deployed, latest) | |
| 1144 | 1259 | } | |
| 1260 | + | s.mu.RUnlock() | |
| 1145 | 1261 | } | |
| 1146 | - | ||
| 1147 | 1262 | return nil | |
| 1148 | 1263 | } | |
| 1149 | 1264 | ||
| @@ -1151,11 +1266,15 @@ func (s *Service) deployAttachments(ctx context.Context, domainName, networkName | |||
| 1151 | 1266 | // 1. Direct domain attachments (attachments[domain]) | |
| 1152 | 1267 | // 2. Network group attachments (attachments[group] where domain is in network_groups[group]) | |
| 1153 | 1268 | func (s *Service) resolveAttachmentsForDomain(domainName string) []string { | |
| 1269 | + | s.mu.RLock() | |
| 1270 | + | cfg := s.config | |
| 1271 | + | s.mu.RUnlock() | |
| 1272 | + | ||
| 1154 | 1273 | seen := make(map[string]bool) | |
| 1155 | 1274 | var result []string | |
| 1156 | 1275 | ||
| 1157 | 1276 | // First, add domain-specific attachments | |
| 1158 | - | if domainAttachments, ok := s.config.Attachments[domainName]; ok { | |
| 1277 | + | if domainAttachments, ok := cfg.Attachments[domainName]; ok { | |
| 1159 | 1278 | for _, img := range domainAttachments { | |
| 1160 | 1279 | if !seen[img] { | |
| 1161 | 1280 | seen[img] = true | |
| @@ -1165,9 +1284,9 @@ func (s *Service) resolveAttachmentsForDomain(domainName string) []string { | |||
| 1165 | 1284 | } | |
| 1166 | 1285 | ||
| 1167 | 1286 | // Then, find which network group this domain belongs to and add group attachments | |
| 1168 | - | for groupName, domains := range s.config.NetworkGroups { | |
| 1287 | + | for groupName, domains := range cfg.NetworkGroups { | |
| 1169 | 1288 | if slices.Contains(domains, domainName) { | |
| 1170 | - | if groupAttachments, ok := s.config.Attachments[groupName]; ok { | |
| 1289 | + | if groupAttachments, ok := cfg.Attachments[groupName]; ok { | |
| 1171 | 1290 | for _, img := range groupAttachments { | |
| 1172 | 1291 | if !seen[img] { | |
| 1173 | 1292 | seen[img] = true | |
| @@ -1557,17 +1676,29 @@ func (s *Service) waitForReady(ctx context.Context, containerID string) error { | |||
| 1557 | 1676 | if i == 29 { | |
| 1558 | 1677 | return fmt.Errorf("container did not start within 30 seconds") | |
| 1559 | 1678 | } | |
| 1560 | - | time.Sleep(time.Second) | |
| 1679 | + | select { | |
| 1680 | + | case <-time.After(time.Second): | |
| 1681 | + | case <-ctx.Done(): | |
| 1682 | + | return ctx.Err() | |
| 1683 | + | } | |
| 1561 | 1684 | } | |
| 1562 | 1685 | ||
| 1686 | + | s.mu.RLock() | |
| 1687 | + | cfg := s.config | |
| 1688 | + | s.mu.RUnlock() | |
| 1689 | + | ||
| 1563 | 1690 | // Additional readiness delay (configurable, default 5 seconds) | |
| 1564 | - | delay := s.config.ReadinessDelay | |
| 1691 | + | delay := cfg.ReadinessDelay | |
| 1565 | 1692 | if delay == 0 { | |
| 1566 | 1693 | delay = 5 * time.Second | |
| 1567 | 1694 | } | |
| 1568 | 1695 | ||
| 1569 | 1696 | log.Debug().Dur("delay", delay).Msg("waiting for container readiness") | |
| 1570 | - | time.Sleep(delay) | |
| 1697 | + | select { | |
| 1698 | + | case <-time.After(delay): | |
| 1699 | + | case <-ctx.Done(): | |
| 1700 | + | return ctx.Err() | |
| 1701 | + | } | |
| 1571 | 1702 | ||
| 1572 | 1703 | // Verify still running after delay | |
| 1573 | 1704 | running, err := s.runtime.IsContainerRunning(ctx, containerID) | |
| @@ -213,7 +213,7 @@ func (s *Service) Deploy(ctx context.Context, route domain.Route) (*domain.Conta | |||||
| 213 | 213 | ||||
| 214 | // Deploy attachments BEFORE main container (dependencies first) | 214 | // Deploy attachments BEFORE main container (dependencies first) | ||
| 215 | if err := s.deployAttachments(ctx, route.Domain, networkName); err != nil { | 215 | if err := s.deployAttachments(ctx, route.Domain, networkName); err != nil { | ||
| 216 | - | log.WrapErr(err, "failed to deploy some attachments") | 216 | + | return nil, log.WrapErr(err, "failed to deploy attachments") |
| 217 | } | 217 | } | ||
| 218 | 218 | ||||
| 219 | // Get exposed ports (use actual image ref from pull) | 219 | // Get exposed ports (use actual image ref from pull) | ||
| @@ -371,28 +371,23 @@ func (s *Service) Remove(ctx context.Context, containerID string, force bool) er | |||||
| 371 | } | 371 | } | ||
| 372 | } | 372 | } | ||
| 373 | 373 | ||||
| 374 | - | // Find domain for cleanup | 374 | + | // Find domain and attachment IDs for cleanup |
| 375 | - | var containerDomain string | 375 | + | containerDomain, attachmentIDs, cfg := s.findContainerContext(containerID) |
| 376 | - | s.mu.RLock() | |||
| 377 | - | for d, c := range s.containers { | |||
| 378 | - | if c.ID == containerID { | |||
| 379 | - | containerDomain = d | |||
| 380 | - | break | |||
| 381 | - | } | |||
| 382 | - | } | |||
| 383 | - | s.mu.RUnlock() | |||
| 384 | 376 | ||||
| 385 | if err := s.runtime.RemoveContainer(ctx, containerID, force); err != nil { | 377 | if err := s.runtime.RemoveContainer(ctx, containerID, force); err != nil { | ||
| 386 | return log.WrapErr(err, "failed to remove container") | 378 | return log.WrapErr(err, "failed to remove container") | ||
| 387 | } | 379 | } | ||
| 388 | 380 | ||||
| 389 | // Cleanup volumes | 381 | // Cleanup volumes | ||
| 390 | - | if containerDomain != "" && !s.config.VolumePreserve { | 382 | + | if containerDomain != "" && !cfg.VolumePreserve { |
| 391 | if err := s.cleanupVolumesForDomain(ctx, containerDomain); err != nil { | 383 | if err := s.cleanupVolumesForDomain(ctx, containerDomain); err != nil { | ||
| 392 | log.WrapErrWithFields(err, "failed to cleanup volumes", map[string]any{"domain": containerDomain}) | 384 | log.WrapErrWithFields(err, "failed to cleanup volumes", map[string]any{"domain": containerDomain}) | ||
| 393 | } | 385 | } | ||
| 394 | } | 386 | } | ||
| 395 | 387 | ||||
| 388 | + | // Clean up attachment containers | |||
| 389 | + | s.removeAttachments(ctx, attachmentIDs) | |||
| 390 | + | ||||
| 396 | // Remove from tracking | 391 | // Remove from tracking | ||
| 397 | s.mu.Lock() | 392 | s.mu.Lock() | ||
| 398 | var removedDomain string | 393 | var removedDomain string | ||
| @@ -404,6 +399,9 @@ func (s *Service) Remove(ctx context.Context, containerID string, force bool) er | |||||
| 404 | break | 399 | break | ||
| 405 | } | 400 | } | ||
| 406 | } | 401 | } | ||
| 402 | + | if removedDomain != "" { | |||
| 403 | + | delete(s.attachments, removedDomain) | |||
| 404 | + | } | |||
| 407 | s.mu.Unlock() | 405 | s.mu.Unlock() | ||
| 408 | 406 | ||||
| 409 | // Note: We intentionally do NOT clean up deployMu entries to avoid a race condition: | 407 | // Note: We intentionally do NOT clean up deployMu entries to avoid a race condition: | ||
| @@ -413,7 +411,7 @@ func (s *Service) Remove(ctx context.Context, containerID string, force bool) er | |||||
| 413 | // domain ever deployed), and this is the safer choice for correctness. | 411 | // domain ever deployed), and this is the safer choice for correctness. | ||
| 414 | 412 | ||||
| 415 | // Cleanup network | 413 | // Cleanup network | ||
| 416 | - | if removedDomain != "" && s.config.NetworkIsolation { | 414 | + | if removedDomain != "" && cfg.NetworkIsolation { |
| 417 | networkName := s.getNetworkForApp(removedDomain) | 415 | networkName := s.getNetworkForApp(removedDomain) | ||
| 418 | if err := s.cleanupNetworkIfEmpty(ctx, networkName); err != nil { | 416 | if err := s.cleanupNetworkIfEmpty(ctx, networkName); err != nil { | ||
| 419 | log.WrapErrWithFields(err, "failed to cleanup network", map[string]any{"network": networkName}) | 417 | log.WrapErrWithFields(err, "failed to cleanup network", map[string]any{"network": networkName}) | ||
| @@ -423,6 +421,43 @@ func (s *Service) Remove(ctx context.Context, containerID string, force bool) er | |||||
| 423 | return nil | 421 | return nil | ||
| 424 | } | 422 | } | ||
| 425 | 423 | ||||
| 424 | + | // findContainerContext looks up the domain, attachment IDs, and config snapshot for a container. | |||
| 425 | + | func (s *Service) findContainerContext(containerID string) (string, []string, Config) { | |||
| 426 | + | s.mu.RLock() | |||
| 427 | + | defer s.mu.RUnlock() | |||
| 428 | + | cfg := s.config | |||
| 429 | + | var containerDomain string | |||
| 430 | + | for d, c := range s.containers { | |||
| 431 | + | if c.ID == containerID { | |||
| 432 | + | containerDomain = d | |||
| 433 | + | break | |||
| 434 | + | } | |||
| 435 | + | } | |||
| 436 | + | var attachmentIDs []string | |||
| 437 | + | if containerDomain != "" { | |||
| 438 | + | attachmentIDs = append(attachmentIDs, s.attachments[containerDomain]...) | |||
| 439 | + | } | |||
| 440 | + | return containerDomain, attachmentIDs, cfg | |||
| 441 | + | } | |||
| 442 | + | ||||
| 443 | + | // removeAttachments stops and removes attachment containers best-effort. | |||
| 444 | + | func (s *Service) removeAttachments(ctx context.Context, attachmentIDs []string) { | |||
| 445 | + | log := zerowrap.FromCtx(ctx) | |||
| 446 | + | for _, attachID := range attachmentIDs { | |||
| 447 | + | if s.logWriter != nil { | |||
| 448 | + | if err := s.logWriter.StopLogging(attachID); err != nil { | |||
| 449 | + | log.Warn().Err(err).Str("attachment_id", attachID).Msg("failed to stop attachment log collection") | |||
| 450 | + | } | |||
| 451 | + | } | |||
| 452 | + | if err := s.runtime.StopContainer(ctx, attachID); err != nil { | |||
| 453 | + | log.Warn().Err(err).Str("attachment_id", attachID).Msg("failed to stop attachment container") | |||
| 454 | + | } | |||
| 455 | + | if err := s.runtime.RemoveContainer(ctx, attachID, true); err != nil { | |||
| 456 | + | log.Warn().Err(err).Str("attachment_id", attachID).Msg("failed to remove attachment container") | |||
| 457 | + | } | |||
| 458 | + | } | |||
| 459 | + | } | |||
| 460 | + | ||||
| 426 | // Get retrieves a container by domain. | 461 | // Get retrieves a container by domain. | ||
| 427 | func (s *Service) Get(_ context.Context, domainName string) (*domain.Container, bool) { | 462 | func (s *Service) Get(_ context.Context, domainName string) (*domain.Container, bool) { | ||
| 428 | s.mu.RLock() | 463 | s.mu.RLock() | ||
| @@ -492,10 +527,14 @@ func (s *Service) ListRoutesWithDetails(ctx context.Context) []domain.RouteInfo | |||||
| 492 | // stripRegistryPrefix removes the configured registry domain prefix from an image reference. | 527 | // stripRegistryPrefix removes the configured registry domain prefix from an image reference. | ||
| 493 | // For example, "reg.example.com/myapp:latest" becomes "myapp:latest" if registry domain is "reg.example.com". | 528 | // For example, "reg.example.com/myapp:latest" becomes "myapp:latest" if registry domain is "reg.example.com". | ||
| 494 | func (s *Service) stripRegistryPrefix(image string) string { | 529 | func (s *Service) stripRegistryPrefix(image string) string { | ||
| 495 | - | if s.config.RegistryDomain == "" { | 530 | + | s.mu.RLock() |
| 531 | + | cfg := s.config | |||
| 532 | + | s.mu.RUnlock() | |||
| 533 | + | ||||
| 534 | + | if cfg.RegistryDomain == "" { | |||
| 496 | return image | 535 | return image | ||
| 497 | } | 536 | } | ||
| 498 | - | prefix := strings.TrimSuffix(s.config.RegistryDomain, "/") + "/" | 537 | + | prefix := strings.TrimSuffix(cfg.RegistryDomain, "/") + "/" |
| 499 | if strings.HasPrefix(image, prefix) { | 538 | if strings.HasPrefix(image, prefix) { | ||
| 500 | return strings.TrimPrefix(image, prefix) | 539 | return strings.TrimPrefix(image, prefix) | ||
| 501 | } | 540 | } | ||
| @@ -509,6 +548,10 @@ func (s *Service) ListAttachments(ctx context.Context, domainName string) []doma | |||||
| 509 | 548 | ||||
| 510 | // ListNetworks returns Gordon-managed networks. | 549 | // ListNetworks returns Gordon-managed networks. | ||
| 511 | func (s *Service) ListNetworks(ctx context.Context) ([]*domain.NetworkInfo, error) { | 550 | func (s *Service) ListNetworks(ctx context.Context) ([]*domain.NetworkInfo, error) { | ||
| 551 | + | s.mu.RLock() | |||
| 552 | + | cfg := s.config | |||
| 553 | + | s.mu.RUnlock() | |||
| 554 | + | ||||
| 512 | networks, err := s.runtime.ListNetworks(ctx) | 555 | networks, err := s.runtime.ListNetworks(ctx) | ||
| 513 | if err != nil { | 556 | if err != nil { | ||
| 514 | return nil, err | 557 | return nil, err | ||
| @@ -516,7 +559,7 @@ func (s *Service) ListNetworks(ctx context.Context) ([]*domain.NetworkInfo, erro | |||||
| 516 | 559 | ||||
| 517 | var filtered []*domain.NetworkInfo | 560 | var filtered []*domain.NetworkInfo | ||
| 518 | for _, network := range networks { | 561 | for _, network := range networks { | ||
| 519 | - | if strings.HasPrefix(network.Name, s.config.NetworkPrefix+"-") { | 562 | + | if strings.HasPrefix(network.Name, cfg.NetworkPrefix+"-") { |
| 520 | filtered = append(filtered, network) | 563 | filtered = append(filtered, network) | ||
| 521 | } | 564 | } | ||
| 522 | } | 565 | } | ||
| @@ -532,14 +575,19 @@ func (s *Service) HealthCheck(ctx context.Context) map[string]bool { | |||||
| 532 | }) | 575 | }) | ||
| 533 | log := zerowrap.FromCtx(ctx) | 576 | log := zerowrap.FromCtx(ctx) | ||
| 534 | 577 | ||||
| 578 | + | // Copy container IDs under lock, then release before making Docker API calls | |||
| 535 | s.mu.RLock() | 579 | s.mu.RLock() | ||
| 536 | - | defer s.mu.RUnlock() | 580 | + | snapshot := make(map[string]string, len(s.containers)) |
| 537 | - | ||||
| 538 | - | health := make(map[string]bool, len(s.containers)) | |||
| 539 | for d, c := range s.containers { | 581 | for d, c := range s.containers { | ||
| 540 | - | running, err := s.runtime.IsContainerRunning(ctx, c.ID) | 582 | + | snapshot[d] = c.ID |
| 583 | + | } | |||
| 584 | + | s.mu.RUnlock() | |||
| 585 | + | ||||
| 586 | + | health := make(map[string]bool, len(snapshot)) | |||
| 587 | + | for d, id := range snapshot { | |||
| 588 | + | running, err := s.runtime.IsContainerRunning(ctx, id) | |||
| 541 | if err != nil { | 589 | if err != nil { | ||
| 542 | - | log.WrapErrWithFields(err, "health check failed", map[string]any{"domain": d, zerowrap.FieldEntityID: c.ID}) | 590 | + | log.WrapErrWithFields(err, "health check failed", map[string]any{"domain": d, zerowrap.FieldEntityID: id}) |
| 543 | health[d] = false | 591 | health[d] = false | ||
| 544 | } else { | 592 | } else { | ||
| 545 | health[d] = running | 593 | health[d] = running | ||
| @@ -556,9 +604,7 @@ func (s *Service) SyncContainers(ctx context.Context) error { | |||||
| 556 | }) | 604 | }) | ||
| 557 | log := zerowrap.FromCtx(ctx) | 605 | log := zerowrap.FromCtx(ctx) | ||
| 558 | 606 | ||||
| 559 | - | s.mu.Lock() | 607 | + | // List containers without holding lock to avoid blocking during Docker API call |
| 560 | - | defer s.mu.Unlock() | |||
| 561 | - | ||||
| 562 | allContainers, err := s.runtime.ListContainers(ctx, false) | 608 | allContainers, err := s.runtime.ListContainers(ctx, false) | ||
| 563 | if err != nil { | 609 | if err != nil { | ||
| 564 | return log.WrapErr(err, "failed to list containers") | 610 | return log.WrapErr(err, "failed to list containers") | ||
| @@ -573,7 +619,10 @@ func (s *Service) SyncContainers(ctx context.Context) error { | |||||
| 573 | } | 619 | } | ||
| 574 | } | 620 | } | ||
| 575 | 621 | ||||
| 622 | + | s.mu.Lock() | |||
| 576 | s.containers = managed | 623 | s.containers = managed | ||
| 624 | + | s.mu.Unlock() | |||
| 625 | + | ||||
| 577 | log.Info().Int(zerowrap.FieldCount, len(managed)).Msg("container state synchronized") | 626 | log.Info().Int(zerowrap.FieldCount, len(managed)).Msg("container state synchronized") | ||
| 578 | return nil | 627 | return nil | ||
| 579 | } | 628 | } | ||
| @@ -649,7 +698,9 @@ func (s *Service) Shutdown(ctx context.Context) error { | |||||
| 649 | 698 | ||||
| 650 | // UpdateConfig updates the service configuration. | 699 | // UpdateConfig updates the service configuration. | ||
| 651 | func (s *Service) UpdateConfig(config Config) { | 700 | func (s *Service) UpdateConfig(config Config) { | ||
| 701 | + | s.mu.Lock() | |||
| 652 | s.config = config | 702 | s.config = config | ||
| 703 | + | s.mu.Unlock() | |||
| 653 | } | 704 | } | ||
| 654 | 705 | ||||
| 655 | // Helper methods | 706 | // Helper methods | ||
| @@ -714,12 +765,16 @@ func (s *Service) startLogCollection(ctx context.Context, containerID, domainNam | |||||
| 714 | } | 765 | } | ||
| 715 | 766 | ||||
| 716 | func (s *Service) buildImageRef(image string) string { | 767 | func (s *Service) buildImageRef(image string) string { | ||
| 717 | - | if !s.config.RegistryAuthEnabled || s.config.RegistryDomain == "" { | 768 | + | s.mu.RLock() |
| 769 | + | cfg := s.config | |||
| 770 | + | s.mu.RUnlock() | |||
| 771 | + | ||||
| 772 | + | if !cfg.RegistryAuthEnabled || cfg.RegistryDomain == "" { | |||
| 718 | return image | 773 | return image | ||
| 719 | } | 774 | } | ||
| 720 | 775 | ||||
| 721 | // Normalize registry domain by trimming any trailing slash | 776 | // Normalize registry domain by trimming any trailing slash | ||
| 722 | - | reg := strings.TrimSuffix(s.config.RegistryDomain, "/") | 777 | + | reg := strings.TrimSuffix(cfg.RegistryDomain, "/") |
| 723 | 778 | ||||
| 724 | // Check if image already has the registry domain prefix | 779 | // Check if image already has the registry domain prefix | ||
| 725 | prefix := reg + "/" | 780 | prefix := reg + "/" | ||
| @@ -815,7 +870,10 @@ func (s *Service) pullRefForDeploy(ctx context.Context, imageRef string) (string | |||||
| 815 | if !domain.IsInternalDeploy(ctx) { | 870 | if !domain.IsInternalDeploy(ctx) { | ||
| 816 | return imageRef, false | 871 | return imageRef, false | ||
| 817 | } | 872 | } | ||
| 818 | - | return rewriteToLocalRegistry(imageRef, s.config.RegistryDomain, s.config.RegistryPort), true | 873 | + | s.mu.RLock() |
| 874 | + | cfg := s.config | |||
| 875 | + | s.mu.RUnlock() | |||
| 876 | + | return rewriteToLocalRegistry(imageRef, cfg.RegistryDomain, cfg.RegistryPort), true | |||
| 819 | } | 877 | } | ||
| 820 | 878 | ||||
| 821 | // ensureImage ensures the image is available locally, pulling if needed. | 879 | // ensureImage ensures the image is available locally, pulling if needed. | ||
| @@ -885,7 +943,11 @@ func (s *Service) ensureLocalImage(ctx context.Context, imageRef, pullRef string | |||||
| 885 | return false, nil | 943 | return false, nil | ||
| 886 | } | 944 | } | ||
| 887 | 945 | ||||
| 888 | - | pullPolicy := normalizePullPolicy(s.config.PullPolicy) | 946 | + | s.mu.RLock() |
| 947 | + | cfg := s.config | |||
| 948 | + | s.mu.RUnlock() | |||
| 949 | + | ||||
| 950 | + | pullPolicy := normalizePullPolicy(cfg.PullPolicy) | |||
| 889 | switch pullPolicy { | 951 | switch pullPolicy { | ||
| 890 | case PullPolicyAlways: | 952 | case PullPolicyAlways: | ||
| 891 | log.Info().Str("pull_policy", pullPolicy).Msg("pull policy forces image pull") | 953 | log.Info().Str("pull_policy", pullPolicy).Msg("pull policy forces image pull") | ||
| @@ -930,25 +992,29 @@ func (s *Service) ensureLocalImage(ctx context.Context, imageRef, pullRef string | |||||
| 930 | } | 992 | } | ||
| 931 | 993 | ||||
| 932 | func (s *Service) pullImage(ctx context.Context, pullRef string, isInternal bool) error { | 994 | func (s *Service) pullImage(ctx context.Context, pullRef string, isInternal bool) error { | ||
| 995 | + | s.mu.RLock() | |||
| 996 | + | cfg := s.config | |||
| 997 | + | s.mu.RUnlock() | |||
| 998 | + | ||||
| 933 | log := zerowrap.FromCtx(ctx) | 999 | log := zerowrap.FromCtx(ctx) | ||
| 934 | 1000 | ||||
| 935 | switch { | 1001 | switch { | ||
| 936 | - | case isInternal && s.config.RegistryAuthEnabled: | 1002 | + | case isInternal && cfg.RegistryAuthEnabled: |
| 937 | - | if s.config.InternalRegistryUsername == "" || s.config.InternalRegistryPassword == "" { | 1003 | + | if cfg.InternalRegistryUsername == "" || cfg.InternalRegistryPassword == "" { |
| 938 | return log.WrapErr(fmt.Errorf("internal registry auth not configured"), "failed to pull image for internal deploy") | 1004 | return log.WrapErr(fmt.Errorf("internal registry auth not configured"), "failed to pull image for internal deploy") | ||
| 939 | } | 1005 | } | ||
| 940 | - | if err := s.runtime.PullImageWithAuth(ctx, pullRef, s.config.InternalRegistryUsername, s.config.InternalRegistryPassword); err != nil { | 1006 | + | if err := s.runtime.PullImageWithAuth(ctx, pullRef, cfg.InternalRegistryUsername, cfg.InternalRegistryPassword); err != nil { |
| 941 | return log.WrapErr(err, "failed to pull image with internal auth") | 1007 | return log.WrapErr(err, "failed to pull image with internal auth") | ||
| 942 | } | 1008 | } | ||
| 943 | case isInternal: | 1009 | case isInternal: | ||
| 944 | if err := s.runtime.PullImage(ctx, pullRef); err != nil { | 1010 | if err := s.runtime.PullImage(ctx, pullRef); err != nil { | ||
| 945 | return log.WrapErr(err, "failed to pull image") | 1011 | return log.WrapErr(err, "failed to pull image") | ||
| 946 | } | 1012 | } | ||
| 947 | - | case s.config.RegistryAuthEnabled: | 1013 | + | case cfg.RegistryAuthEnabled: |
| 948 | - | if s.config.ServiceTokenUsername == "" || s.config.ServiceToken == "" { | 1014 | + | if cfg.ServiceTokenUsername == "" || cfg.ServiceToken == "" { |
| 949 | return log.WrapErr(fmt.Errorf("registry service token not configured"), "failed to pull image for registry auth") | 1015 | return log.WrapErr(fmt.Errorf("registry service token not configured"), "failed to pull image for registry auth") | ||
| 950 | } | 1016 | } | ||
| 951 | - | if err := s.runtime.PullImageWithAuth(ctx, pullRef, s.config.ServiceTokenUsername, s.config.ServiceToken); err != nil { | 1017 | + | if err := s.runtime.PullImageWithAuth(ctx, pullRef, cfg.ServiceTokenUsername, cfg.ServiceToken); err != nil { |
| 952 | return log.WrapErr(err, "failed to pull image with auth") | 1018 | return log.WrapErr(err, "failed to pull image with auth") | ||
| 953 | } | 1019 | } | ||
| 954 | default: | 1020 | default: | ||
| @@ -997,10 +1063,14 @@ func (s *Service) loadEnvironment(ctx context.Context, domainName, imageRef stri | |||||
| 997 | } | 1063 | } | ||
| 998 | 1064 | ||||
| 999 | func (s *Service) setupVolumes(ctx context.Context, domainName, imageRef string) (map[string]string, error) { | 1065 | func (s *Service) setupVolumes(ctx context.Context, domainName, imageRef string) (map[string]string, error) { | ||
| 1066 | + | s.mu.RLock() | |||
| 1067 | + | cfg := s.config | |||
| 1068 | + | s.mu.RUnlock() | |||
| 1069 | + | ||||
| 1000 | log := zerowrap.FromCtx(ctx) | 1070 | log := zerowrap.FromCtx(ctx) | ||
| 1001 | volumes := make(map[string]string) | 1071 | volumes := make(map[string]string) | ||
| 1002 | 1072 | ||||
| 1003 | - | if !s.config.VolumeAutoCreate { | 1073 | + | if !cfg.VolumeAutoCreate { |
| 1004 | return volumes, nil | 1074 | return volumes, nil | ||
| 1005 | } | 1075 | } | ||
| 1006 | 1076 | ||||
| @@ -1011,7 +1081,7 @@ func (s *Service) setupVolumes(ctx context.Context, domainName, imageRef string) | |||||
| 1011 | } | 1081 | } | ||
| 1012 | 1082 | ||||
| 1013 | for _, path := range volumePaths { | 1083 | for _, path := range volumePaths { | ||
| 1014 | - | name := generateVolumeName(s.config.VolumePrefix, domainName, path) | 1084 | + | name := generateVolumeName(cfg.VolumePrefix, domainName, path) |
| 1015 | 1085 | ||||
| 1016 | exists, err := s.runtime.VolumeExists(ctx, name) | 1086 | exists, err := s.runtime.VolumeExists(ctx, name) | ||
| 1017 | if err != nil { | 1087 | if err != nil { | ||
| @@ -1034,11 +1104,15 @@ func (s *Service) setupVolumes(ctx context.Context, domainName, imageRef string) | |||||
| 1034 | } | 1104 | } | ||
| 1035 | 1105 | ||||
| 1036 | func (s *Service) getNetworkForApp(domainName string) string { | 1106 | func (s *Service) getNetworkForApp(domainName string) string { | ||
| 1037 | - | if !s.config.NetworkIsolation { | 1107 | + | s.mu.RLock() |
| 1108 | + | cfg := s.config | |||
| 1109 | + | s.mu.RUnlock() | |||
| 1110 | + | ||||
| 1111 | + | if !cfg.NetworkIsolation { | |||
| 1038 | return "bridge" | 1112 | return "bridge" | ||
| 1039 | } | 1113 | } | ||
| 1040 | 1114 | ||||
| 1041 | - | for groupName, domains := range s.config.NetworkGroups { | 1115 | + | for groupName, domains := range cfg.NetworkGroups { |
| 1042 | if slices.Contains(domains, domainName) { | 1116 | if slices.Contains(domains, domainName) { | ||
| 1043 | return s.generateNetworkName(groupName) | 1117 | return s.generateNetworkName(groupName) | ||
| 1044 | } | 1118 | } | ||
| @@ -1048,7 +1122,11 @@ func (s *Service) getNetworkForApp(domainName string) string { | |||||
| 1048 | } | 1122 | } | ||
| 1049 | 1123 | ||||
| 1050 | func (s *Service) generateNetworkName(identifier string) string { | 1124 | func (s *Service) generateNetworkName(identifier string) string { | ||
| 1051 | - | return fmt.Sprintf("%s-%s", s.config.NetworkPrefix, strings.ReplaceAll(identifier, ".", "-")) | 1125 | + | s.mu.RLock() |
| 1126 | + | cfg := s.config | |||
| 1127 | + | s.mu.RUnlock() | |||
| 1128 | + | ||||
| 1129 | + | return fmt.Sprintf("%s-%s", cfg.NetworkPrefix, strings.ReplaceAll(identifier, ".", "-")) | |||
| 1052 | } | 1130 | } | ||
| 1053 | 1131 | ||||
| 1054 | func (s *Service) createNetworkIfNeeded(ctx context.Context, networkName string) error { | 1132 | func (s *Service) createNetworkIfNeeded(ctx context.Context, networkName string) error { | ||
| @@ -1138,12 +1216,49 @@ func (s *Service) deployAttachments(ctx context.Context, domainName, networkName | |||||
| 1138 | } | 1216 | } | ||
| 1139 | 1217 | ||||
| 1140 | log := zerowrap.FromCtx(ctx) | 1218 | log := zerowrap.FromCtx(ctx) | ||
| 1219 | + | var deployed []string // track successfully deployed attachment IDs for rollback | |||
| 1141 | for _, svc := range attachments { | 1220 | for _, svc := range attachments { | ||
| 1142 | if err := s.deployAttachedService(ctx, domainName, svc, networkName); err != nil { | 1221 | if err := s.deployAttachedService(ctx, domainName, svc, networkName); err != nil { | ||
| 1143 | log.WrapErrWithFields(err, "failed to deploy attachment", map[string]any{zerowrap.FieldService: svc, "domain": domainName}) | 1222 | log.WrapErrWithFields(err, "failed to deploy attachment", map[string]any{zerowrap.FieldService: svc, "domain": domainName}) | ||
| 1223 | + | ||||
| 1224 | + | // Rollback: clean up already-deployed attachments | |||
| 1225 | + | for _, id := range deployed { | |||
| 1226 | + | if stopErr := s.runtime.StopContainer(ctx, id); stopErr != nil { | |||
| 1227 | + | log.Warn().Err(stopErr).Str("attachment_id", id).Msg("rollback: failed to stop attachment") | |||
| 1228 | + | } | |||
| 1229 | + | if rmErr := s.runtime.RemoveContainer(ctx, id, true); rmErr != nil { | |||
| 1230 | + | log.Warn().Err(rmErr).Str("attachment_id", id).Msg("rollback: failed to remove attachment") | |||
| 1231 | + | } | |||
| 1232 | + | } | |||
| 1233 | + | // Deregister rolled-back attachments | |||
| 1234 | + | if len(deployed) > 0 { | |||
| 1235 | + | s.mu.Lock() | |||
| 1236 | + | remaining := s.attachments[domainName] | |||
| 1237 | + | filtered := remaining[:0] | |||
| 1238 | + | rollbackSet := make(map[string]bool, len(deployed)) | |||
| 1239 | + | for _, id := range deployed { | |||
| 1240 | + | rollbackSet[id] = true | |||
| 1241 | + | } | |||
| 1242 | + | for _, id := range remaining { | |||
| 1243 | + | if !rollbackSet[id] { | |||
| 1244 | + | filtered = append(filtered, id) | |||
| 1245 | + | } | |||
| 1246 | + | } | |||
| 1247 | + | s.attachments[domainName] = filtered | |||
| 1248 | + | s.mu.Unlock() | |||
| 1249 | + | } | |||
| 1250 | + | ||||
| 1251 | + | return fmt.Errorf("failed to deploy attachment %q (rolled back %d already-deployed)", svc, len(deployed)) | |||
| 1252 | + | } | |||
| 1253 | + | // Collect IDs of attachments tracked under this domain after successful deploy | |||
| 1254 | + | s.mu.RLock() | |||
| 1255 | + | ids := s.attachments[domainName] | |||
| 1256 | + | if len(ids) > 0 { | |||
| 1257 | + | latest := ids[len(ids)-1] | |||
| 1258 | + | deployed = append(deployed, latest) | |||
| 1144 | } | 1259 | } | ||
| 1260 | + | s.mu.RUnlock() | |||
| 1145 | } | 1261 | } | ||
| 1146 | - | ||||
| 1147 | return nil | 1262 | return nil | ||
| 1148 | } | 1263 | } | ||
| 1149 | 1264 | ||||
| @@ -1151,11 +1266,15 @@ func (s *Service) deployAttachments(ctx context.Context, domainName, networkName | |||||
| 1151 | // 1. Direct domain attachments (attachments[domain]) | 1266 | // 1. Direct domain attachments (attachments[domain]) | ||
| 1152 | // 2. Network group attachments (attachments[group] where domain is in network_groups[group]) | 1267 | // 2. Network group attachments (attachments[group] where domain is in network_groups[group]) | ||
| 1153 | func (s *Service) resolveAttachmentsForDomain(domainName string) []string { | 1268 | func (s *Service) resolveAttachmentsForDomain(domainName string) []string { | ||
| 1269 | + | s.mu.RLock() | |||
| 1270 | + | cfg := s.config | |||
| 1271 | + | s.mu.RUnlock() | |||
| 1272 | + | ||||
| 1154 | seen := make(map[string]bool) | 1273 | seen := make(map[string]bool) | ||
| 1155 | var result []string | 1274 | var result []string | ||
| 1156 | 1275 | ||||
| 1157 | // First, add domain-specific attachments | 1276 | // First, add domain-specific attachments | ||
| 1158 | - | if domainAttachments, ok := s.config.Attachments[domainName]; ok { | 1277 | + | if domainAttachments, ok := cfg.Attachments[domainName]; ok { |
| 1159 | for _, img := range domainAttachments { | 1278 | for _, img := range domainAttachments { | ||
| 1160 | if !seen[img] { | 1279 | if !seen[img] { | ||
| 1161 | seen[img] = true | 1280 | seen[img] = true | ||
| @@ -1165,9 +1284,9 @@ func (s *Service) resolveAttachmentsForDomain(domainName string) []string { | |||||
| 1165 | } | 1284 | } | ||
| 1166 | 1285 | ||||
| 1167 | // Then, find which network group this domain belongs to and add group attachments | 1286 | // Then, find which network group this domain belongs to and add group attachments | ||
| 1168 | - | for groupName, domains := range s.config.NetworkGroups { | 1287 | + | for groupName, domains := range cfg.NetworkGroups { |
| 1169 | if slices.Contains(domains, domainName) { | 1288 | if slices.Contains(domains, domainName) { | ||
| 1170 | - | if groupAttachments, ok := s.config.Attachments[groupName]; ok { | 1289 | + | if groupAttachments, ok := cfg.Attachments[groupName]; ok { |
| 1171 | for _, img := range groupAttachments { | 1290 | for _, img := range groupAttachments { | ||
| 1172 | if !seen[img] { | 1291 | if !seen[img] { | ||
| 1173 | seen[img] = true | 1292 | seen[img] = true | ||
| @@ -1557,17 +1676,29 @@ func (s *Service) waitForReady(ctx context.Context, containerID string) error { | |||||
| 1557 | if i == 29 { | 1676 | if i == 29 { | ||
| 1558 | return fmt.Errorf("container did not start within 30 seconds") | 1677 | return fmt.Errorf("container did not start within 30 seconds") | ||
| 1559 | } | 1678 | } | ||
| 1560 | - | time.Sleep(time.Second) | 1679 | + | select { |
| 1680 | + | case <-time.After(time.Second): | |||
| 1681 | + | case <-ctx.Done(): | |||
| 1682 | + | return ctx.Err() | |||
| 1683 | + | } | |||
| 1561 | } | 1684 | } | ||
| 1562 | 1685 | ||||
| 1686 | + | s.mu.RLock() | |||
| 1687 | + | cfg := s.config | |||
| 1688 | + | s.mu.RUnlock() | |||
| 1689 | + | ||||
| 1563 | // Additional readiness delay (configurable, default 5 seconds) | 1690 | // Additional readiness delay (configurable, default 5 seconds) | ||
| 1564 | - | delay := s.config.ReadinessDelay | 1691 | + | delay := cfg.ReadinessDelay |
| 1565 | if delay == 0 { | 1692 | if delay == 0 { | ||
| 1566 | delay = 5 * time.Second | 1693 | delay = 5 * time.Second | ||
| 1567 | } | 1694 | } | ||
| 1568 | 1695 | ||||
| 1569 | log.Debug().Dur("delay", delay).Msg("waiting for container readiness") | 1696 | log.Debug().Dur("delay", delay).Msg("waiting for container readiness") | ||
| 1570 | - | time.Sleep(delay) | 1697 | + | select { |
| 1698 | + | case <-time.After(delay): | |||
| 1699 | + | case <-ctx.Done(): | |||
| 1700 | + | return ctx.Err() | |||
| 1701 | + | } | |||
| 1571 | 1702 | ||||
| 1572 | // Verify still running after delay | 1703 | // Verify still running after delay | ||
| 1573 | running, err := s.runtime.IsContainerRunning(ctx, containerID) | 1704 | running, err := s.runtime.IsContainerRunning(ctx, containerID) | ||
M internal/usecase/container/service_test.go +42 -0
| @@ -1137,6 +1137,48 @@ func TestService_AutoStart_HandlesDeployErrors(t *testing.T) { | |||
| 1137 | 1137 | assert.Contains(t, err.Error(), "auto-start completed with 1 errors") | |
| 1138 | 1138 | } | |
| 1139 | 1139 | ||
| 1140 | + | func TestService_AutoStart_UsesInternalDeployContext(t *testing.T) { | |
| 1141 | + | runtime := mocks.NewMockContainerRuntime(t) | |
| 1142 | + | envLoader := mocks.NewMockEnvLoader(t) | |
| 1143 | + | eventBus := mocks.NewMockEventPublisher(t) | |
| 1144 | + | ||
| 1145 | + | config := Config{ | |
| 1146 | + | RegistryDomain: "reg.example.com", | |
| 1147 | + | RegistryPort: 5000, | |
| 1148 | + | ReadinessDelay: time.Millisecond, | |
| 1149 | + | } | |
| 1150 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |
| 1151 | + | ||
| 1152 | + | // Mark context as internal deploy — this is what syncAndAutoStart should do | |
| 1153 | + | ctx := domain.WithInternalDeploy(testContext()) | |
| 1154 | + | ||
| 1155 | + | routes := []domain.Route{ | |
| 1156 | + | {Domain: "app1.example.com", Image: "reg.example.com/myapp:latest"}, | |
| 1157 | + | } | |
| 1158 | + | ||
| 1159 | + | // Key assertion: PullImage should be called with localhost:5000 rewrite, | |
| 1160 | + | // NOT the original reg.example.com/myapp:latest | |
| 1161 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil) | |
| 1162 | + | runtime.EXPECT().PullImage(mock.Anything, "localhost:5000/myapp:latest").Return(nil) | |
| 1163 | + | runtime.EXPECT().TagImage(mock.Anything, "localhost:5000/myapp:latest", "reg.example.com/myapp:latest").Return(nil) | |
| 1164 | + | runtime.EXPECT().UntagImage(mock.Anything, "localhost:5000/myapp:latest").Return(nil) | |
| 1165 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "reg.example.com/myapp:latest").Return([]int{8080}, nil) | |
| 1166 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "app1.example.com").Return([]string{}, nil) | |
| 1167 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "reg.example.com/myapp:latest").Return([]string{}, nil) | |
| 1168 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.AnythingOfType("*domain.ContainerConfig")).Return(&domain.Container{ | |
| 1169 | + | ID: "container-1", Name: "gordon-app1.example.com", Status: "created", | |
| 1170 | + | }, nil) | |
| 1171 | + | runtime.EXPECT().StartContainer(mock.Anything, "container-1").Return(nil) | |
| 1172 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "container-1").Return(true, nil).Times(2) | |
| 1173 | + | runtime.EXPECT().InspectContainer(mock.Anything, "container-1").Return(&domain.Container{ | |
| 1174 | + | Status: "running", | |
| 1175 | + | }, nil) | |
| 1176 | + | eventBus.EXPECT().Publish(domain.EventContainerDeployed, mock.AnythingOfType("*domain.ContainerEventPayload")).Return(nil) | |
| 1177 | + | ||
| 1178 | + | err := svc.AutoStart(ctx, routes) | |
| 1179 | + | assert.NoError(t, err) | |
| 1180 | + | } | |
| 1181 | + | ||
| 1140 | 1182 | // TestService_Deploy_OrphanCleanupSkipsTrackedContainer verifies that the orphan cleanup | |
| 1141 | 1183 | // does not remove the currently tracked container during zero-downtime deployment. | |
| 1142 | 1184 | // This is critical for preventing downtime - the old container must stay running | |
| @@ -1137,6 +1137,48 @@ func TestService_AutoStart_HandlesDeployErrors(t *testing.T) { | |||||
| 1137 | assert.Contains(t, err.Error(), "auto-start completed with 1 errors") | 1137 | assert.Contains(t, err.Error(), "auto-start completed with 1 errors") | ||
| 1138 | } | 1138 | } | ||
| 1139 | 1139 | ||||
| 1140 | + | func TestService_AutoStart_UsesInternalDeployContext(t *testing.T) { | |||
| 1141 | + | runtime := mocks.NewMockContainerRuntime(t) | |||
| 1142 | + | envLoader := mocks.NewMockEnvLoader(t) | |||
| 1143 | + | eventBus := mocks.NewMockEventPublisher(t) | |||
| 1144 | + | ||||
| 1145 | + | config := Config{ | |||
| 1146 | + | RegistryDomain: "reg.example.com", | |||
| 1147 | + | RegistryPort: 5000, | |||
| 1148 | + | ReadinessDelay: time.Millisecond, | |||
| 1149 | + | } | |||
| 1150 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |||
| 1151 | + | ||||
| 1152 | + | // Mark context as internal deploy — this is what syncAndAutoStart should do | |||
| 1153 | + | ctx := domain.WithInternalDeploy(testContext()) | |||
| 1154 | + | ||||
| 1155 | + | routes := []domain.Route{ | |||
| 1156 | + | {Domain: "app1.example.com", Image: "reg.example.com/myapp:latest"}, | |||
| 1157 | + | } | |||
| 1158 | + | ||||
| 1159 | + | // Key assertion: PullImage should be called with localhost:5000 rewrite, | |||
| 1160 | + | // NOT the original reg.example.com/myapp:latest | |||
| 1161 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil) | |||
| 1162 | + | runtime.EXPECT().PullImage(mock.Anything, "localhost:5000/myapp:latest").Return(nil) | |||
| 1163 | + | runtime.EXPECT().TagImage(mock.Anything, "localhost:5000/myapp:latest", "reg.example.com/myapp:latest").Return(nil) | |||
| 1164 | + | runtime.EXPECT().UntagImage(mock.Anything, "localhost:5000/myapp:latest").Return(nil) | |||
| 1165 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "reg.example.com/myapp:latest").Return([]int{8080}, nil) | |||
| 1166 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "app1.example.com").Return([]string{}, nil) | |||
| 1167 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "reg.example.com/myapp:latest").Return([]string{}, nil) | |||
| 1168 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.AnythingOfType("*domain.ContainerConfig")).Return(&domain.Container{ | |||
| 1169 | + | ID: "container-1", Name: "gordon-app1.example.com", Status: "created", | |||
| 1170 | + | }, nil) | |||
| 1171 | + | runtime.EXPECT().StartContainer(mock.Anything, "container-1").Return(nil) | |||
| 1172 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "container-1").Return(true, nil).Times(2) | |||
| 1173 | + | runtime.EXPECT().InspectContainer(mock.Anything, "container-1").Return(&domain.Container{ | |||
| 1174 | + | Status: "running", | |||
| 1175 | + | }, nil) | |||
| 1176 | + | eventBus.EXPECT().Publish(domain.EventContainerDeployed, mock.AnythingOfType("*domain.ContainerEventPayload")).Return(nil) | |||
| 1177 | + | ||||
| 1178 | + | err := svc.AutoStart(ctx, routes) | |||
| 1179 | + | assert.NoError(t, err) | |||
| 1180 | + | } | |||
| 1181 | + | ||||
| 1140 | // TestService_Deploy_OrphanCleanupSkipsTrackedContainer verifies that the orphan cleanup | 1182 | // TestService_Deploy_OrphanCleanupSkipsTrackedContainer verifies that the orphan cleanup | ||
| 1141 | // does not remove the currently tracked container during zero-downtime deployment. | 1183 | // does not remove the currently tracked container during zero-downtime deployment. | ||
| 1142 | // This is critical for preventing downtime - the old container must stay running | 1184 | // This is critical for preventing downtime - the old container must stay running | ||
M internal/usecase/proxy/events.go +42 -2
| @@ -28,8 +28,8 @@ func NewContainerDeployedHandler(ctx context.Context, invalidator TargetInvalida | |||
| 28 | 28 | } | |
| 29 | 29 | ||
| 30 | 30 | // Handle handles a container.deployed event by invalidating the proxy cache. | |
| 31 | - | func (h *ContainerDeployedHandler) Handle(event domain.Event) error { | |
| 32 | - | ctx := zerowrap.CtxWithFields(h.ctx, map[string]any{ | |
| 31 | + | func (h *ContainerDeployedHandler) Handle(ctx context.Context, event domain.Event) error { | |
| 32 | + | ctx = zerowrap.CtxWithFields(ctx, map[string]any{ | |
| 33 | 33 | zerowrap.FieldLayer: "usecase", | |
| 34 | 34 | zerowrap.FieldHandler: "ContainerDeployedHandler", | |
| 35 | 35 | zerowrap.FieldEvent: string(event.Type), | |
| @@ -61,3 +61,43 @@ func (h *ContainerDeployedHandler) Handle(event domain.Event) error { | |||
| 61 | 61 | func (h *ContainerDeployedHandler) CanHandle(eventType domain.EventType) bool { | |
| 62 | 62 | return eventType == domain.EventContainerDeployed | |
| 63 | 63 | } | |
| 64 | + | ||
| 65 | + | // TargetRefresher defines the interface for refreshing all proxy targets. | |
| 66 | + | type TargetRefresher interface { | |
| 67 | + | RefreshTargets(ctx context.Context) error | |
| 68 | + | } | |
| 69 | + | ||
| 70 | + | // ConfigReloadProxyHandler handles config.reload events to clear the proxy target cache. | |
| 71 | + | // When routes are removed during a config reload, cached targets for those routes | |
| 72 | + | // become stale. This handler clears all cached targets so they are re-resolved. | |
| 73 | + | type ConfigReloadProxyHandler struct { | |
| 74 | + | refresher TargetRefresher | |
| 75 | + | ctx context.Context | |
| 76 | + | } | |
| 77 | + | ||
| 78 | + | // NewConfigReloadProxyHandler creates a new ConfigReloadProxyHandler. | |
| 79 | + | func NewConfigReloadProxyHandler(ctx context.Context, refresher TargetRefresher) *ConfigReloadProxyHandler { | |
| 80 | + | return &ConfigReloadProxyHandler{ | |
| 81 | + | refresher: refresher, | |
| 82 | + | ctx: ctx, | |
| 83 | + | } | |
| 84 | + | } | |
| 85 | + | ||
| 86 | + | // Handle clears all cached proxy targets so they are re-resolved from current state. | |
| 87 | + | func (h *ConfigReloadProxyHandler) Handle(ctx context.Context, event domain.Event) error { | |
| 88 | + | ctx = zerowrap.CtxWithFields(ctx, map[string]any{ | |
| 89 | + | zerowrap.FieldLayer: "usecase", | |
| 90 | + | zerowrap.FieldHandler: "ConfigReloadProxyHandler", | |
| 91 | + | zerowrap.FieldEvent: string(event.Type), | |
| 92 | + | "event_id": event.ID, | |
| 93 | + | }) | |
| 94 | + | log := zerowrap.FromCtx(ctx) | |
| 95 | + | ||
| 96 | + | log.Debug().Msg("clearing proxy target cache after config reload") | |
| 97 | + | return h.refresher.RefreshTargets(ctx) | |
| 98 | + | } | |
| 99 | + | ||
| 100 | + | // CanHandle returns whether this handler can handle the given event type. | |
| 101 | + | func (h *ConfigReloadProxyHandler) CanHandle(eventType domain.EventType) bool { | |
| 102 | + | return eventType == domain.EventConfigReload | |
| 103 | + | } | |
| @@ -28,8 +28,8 @@ func NewContainerDeployedHandler(ctx context.Context, invalidator TargetInvalida | |||||
| 28 | } | 28 | } | ||
| 29 | 29 | ||||
| 30 | // Handle handles a container.deployed event by invalidating the proxy cache. | 30 | // Handle handles a container.deployed event by invalidating the proxy cache. | ||
| 31 | - | func (h *ContainerDeployedHandler) Handle(event domain.Event) error { | 31 | + | func (h *ContainerDeployedHandler) Handle(ctx context.Context, event domain.Event) error { |
| 32 | - | ctx := zerowrap.CtxWithFields(h.ctx, map[string]any{ | 32 | + | ctx = zerowrap.CtxWithFields(ctx, map[string]any{ |
| 33 | zerowrap.FieldLayer: "usecase", | 33 | zerowrap.FieldLayer: "usecase", | ||
| 34 | zerowrap.FieldHandler: "ContainerDeployedHandler", | 34 | zerowrap.FieldHandler: "ContainerDeployedHandler", | ||
| 35 | zerowrap.FieldEvent: string(event.Type), | 35 | zerowrap.FieldEvent: string(event.Type), | ||
| @@ -61,3 +61,43 @@ func (h *ContainerDeployedHandler) Handle(event domain.Event) error { | |||||
| 61 | func (h *ContainerDeployedHandler) CanHandle(eventType domain.EventType) bool { | 61 | func (h *ContainerDeployedHandler) CanHandle(eventType domain.EventType) bool { | ||
| 62 | return eventType == domain.EventContainerDeployed | 62 | return eventType == domain.EventContainerDeployed | ||
| 63 | } | 63 | } | ||
| 64 | + | ||||
| 65 | + | // TargetRefresher defines the interface for refreshing all proxy targets. | |||
| 66 | + | type TargetRefresher interface { | |||
| 67 | + | RefreshTargets(ctx context.Context) error | |||
| 68 | + | } | |||
| 69 | + | ||||
| 70 | + | // ConfigReloadProxyHandler handles config.reload events to clear the proxy target cache. | |||
| 71 | + | // When routes are removed during a config reload, cached targets for those routes | |||
| 72 | + | // become stale. This handler clears all cached targets so they are re-resolved. | |||
| 73 | + | type ConfigReloadProxyHandler struct { | |||
| 74 | + | refresher TargetRefresher | |||
| 75 | + | ctx context.Context | |||
| 76 | + | } | |||
| 77 | + | ||||
| 78 | + | // NewConfigReloadProxyHandler creates a new ConfigReloadProxyHandler. | |||
| 79 | + | func NewConfigReloadProxyHandler(ctx context.Context, refresher TargetRefresher) *ConfigReloadProxyHandler { | |||
| 80 | + | return &ConfigReloadProxyHandler{ | |||
| 81 | + | refresher: refresher, | |||
| 82 | + | ctx: ctx, | |||
| 83 | + | } | |||
| 84 | + | } | |||
| 85 | + | ||||
| 86 | + | // Handle clears all cached proxy targets so they are re-resolved from current state. | |||
| 87 | + | func (h *ConfigReloadProxyHandler) Handle(ctx context.Context, event domain.Event) error { | |||
| 88 | + | ctx = zerowrap.CtxWithFields(ctx, map[string]any{ | |||
| 89 | + | zerowrap.FieldLayer: "usecase", | |||
| 90 | + | zerowrap.FieldHandler: "ConfigReloadProxyHandler", | |||
| 91 | + | zerowrap.FieldEvent: string(event.Type), | |||
| 92 | + | "event_id": event.ID, | |||
| 93 | + | }) | |||
| 94 | + | log := zerowrap.FromCtx(ctx) | |||
| 95 | + | ||||
| 96 | + | log.Debug().Msg("clearing proxy target cache after config reload") | |||
| 97 | + | return h.refresher.RefreshTargets(ctx) | |||
| 98 | + | } | |||
| 99 | + | ||||
| 100 | + | // CanHandle returns whether this handler can handle the given event type. | |||
| 101 | + | func (h *ConfigReloadProxyHandler) CanHandle(eventType domain.EventType) bool { | |||
| 102 | + | return eventType == domain.EventConfigReload | |||
| 103 | + | } | |||
M internal/usecase/proxy/service.go +18 -6
| @@ -70,9 +70,14 @@ func NewService( | |||
| 70 | 70 | ||
| 71 | 71 | // ServeHTTP handles incoming HTTP requests and proxies them to the appropriate backend. | |
| 72 | 72 | func (s *Service) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |
| 73 | + | // Copy config under RLock to avoid data race with UpdateConfig | |
| 74 | + | s.mu.RLock() | |
| 75 | + | cfg := s.config | |
| 76 | + | s.mu.RUnlock() | |
| 77 | + | ||
| 73 | 78 | // SECURITY: Limit request body size to prevent resource exhaustion. | |
| 74 | - | if s.config.MaxBodySize > 0 { | |
| 75 | - | r.Body = http.MaxBytesReader(w, r.Body, s.config.MaxBodySize) | |
| 79 | + | if cfg.MaxBodySize > 0 { | |
| 80 | + | r.Body = http.MaxBytesReader(w, r.Body, cfg.MaxBodySize) | |
| 76 | 81 | } | |
| 77 | 82 | ||
| 78 | 83 | // Enrich request context with fields for downstream logging | |
| @@ -88,7 +93,7 @@ func (s *Service) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |||
| 88 | 93 | log := zerowrap.FromCtx(ctx) | |
| 89 | 94 | ||
| 90 | 95 | // Check if this is the registry domain | |
| 91 | - | if s.config.RegistryDomain != "" && r.Host == s.config.RegistryDomain { | |
| 96 | + | if cfg.RegistryDomain != "" && r.Host == cfg.RegistryDomain { | |
| 92 | 97 | log.Info().Msg("routing request to registry") | |
| 93 | 98 | s.proxyToRegistry(w, r) | |
| 94 | 99 | return | |
| @@ -281,7 +286,9 @@ func (s *Service) RefreshTargets(ctx context.Context) error { | |||
| 281 | 286 | ||
| 282 | 287 | // UpdateConfig updates the service configuration. | |
| 283 | 288 | func (s *Service) UpdateConfig(config Config) { | |
| 289 | + | s.mu.Lock() | |
| 284 | 290 | s.config = config | |
| 291 | + | s.mu.Unlock() | |
| 285 | 292 | } | |
| 286 | 293 | ||
| 287 | 294 | // Helper methods | |
| @@ -346,9 +353,14 @@ func (s *Service) proxyToTarget(w http.ResponseWriter, r *http.Request, target * | |||
| 346 | 353 | } | |
| 347 | 354 | ||
| 348 | 355 | func (s *Service) proxyToRegistry(w http.ResponseWriter, r *http.Request) { | |
| 356 | + | // Copy config under RLock to avoid data race with UpdateConfig | |
| 357 | + | s.mu.RLock() | |
| 358 | + | cfg := s.config | |
| 359 | + | s.mu.RUnlock() | |
| 360 | + | ||
| 349 | 361 | log := zerowrap.FromCtx(r.Context()) | |
| 350 | 362 | ||
| 351 | - | targetURL, err := url.Parse(fmt.Sprintf("http://localhost:%d", s.config.RegistryPort)) | |
| 363 | + | targetURL, err := url.Parse(fmt.Sprintf("http://localhost:%d", cfg.RegistryPort)) | |
| 352 | 364 | if err != nil { | |
| 353 | 365 | log.WrapErr(err, "failed to parse registry target URL") | |
| 354 | 366 | http.Error(w, "Internal Server Error", http.StatusInternalServerError) | |
| @@ -359,11 +371,11 @@ func (s *Service) proxyToRegistry(w http.ResponseWriter, r *http.Request) { | |||
| 359 | 371 | func(w http.ResponseWriter, _ *http.Request, err error) { | |
| 360 | 372 | // Check if this is a MaxBytesError (request body too large) | |
| 361 | 373 | if err.Error() == "http: request body too large" { | |
| 362 | - | log.Warn().Err(err).Int("registry_port", s.config.RegistryPort).Msg("registry proxy error: request body too large") | |
| 374 | + | log.Warn().Err(err).Int("registry_port", cfg.RegistryPort).Msg("registry proxy error: request body too large") | |
| 363 | 375 | http.Error(w, "Request Entity Too Large", http.StatusRequestEntityTooLarge) | |
| 364 | 376 | return | |
| 365 | 377 | } | |
| 366 | - | log.Error().Err(err).Int("registry_port", s.config.RegistryPort).Msg("registry proxy error") | |
| 378 | + | log.Error().Err(err).Int("registry_port", cfg.RegistryPort).Msg("registry proxy error") | |
| 367 | 379 | http.Error(w, "Registry Unavailable", http.StatusServiceUnavailable) | |
| 368 | 380 | }, | |
| 369 | 381 | func(resp *http.Response) error { | |
| @@ -70,9 +70,14 @@ func NewService( | |||||
| 70 | 70 | ||||
| 71 | // ServeHTTP handles incoming HTTP requests and proxies them to the appropriate backend. | 71 | // ServeHTTP handles incoming HTTP requests and proxies them to the appropriate backend. | ||
| 72 | func (s *Service) ServeHTTP(w http.ResponseWriter, r *http.Request) { | 72 | func (s *Service) ServeHTTP(w http.ResponseWriter, r *http.Request) { | ||
| 73 | + | // Copy config under RLock to avoid data race with UpdateConfig | |||
| 74 | + | s.mu.RLock() | |||
| 75 | + | cfg := s.config | |||
| 76 | + | s.mu.RUnlock() | |||
| 77 | + | ||||
| 73 | // SECURITY: Limit request body size to prevent resource exhaustion. | 78 | // SECURITY: Limit request body size to prevent resource exhaustion. | ||
| 74 | - | if s.config.MaxBodySize > 0 { | 79 | + | if cfg.MaxBodySize > 0 { |
| 75 | - | r.Body = http.MaxBytesReader(w, r.Body, s.config.MaxBodySize) | 80 | + | r.Body = http.MaxBytesReader(w, r.Body, cfg.MaxBodySize) |
| 76 | } | 81 | } | ||
| 77 | 82 | ||||
| 78 | // Enrich request context with fields for downstream logging | 83 | // Enrich request context with fields for downstream logging | ||
| @@ -88,7 +93,7 @@ func (s *Service) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |||||
| 88 | log := zerowrap.FromCtx(ctx) | 93 | log := zerowrap.FromCtx(ctx) | ||
| 89 | 94 | ||||
| 90 | // Check if this is the registry domain | 95 | // Check if this is the registry domain | ||
| 91 | - | if s.config.RegistryDomain != "" && r.Host == s.config.RegistryDomain { | 96 | + | if cfg.RegistryDomain != "" && r.Host == cfg.RegistryDomain { |
| 92 | log.Info().Msg("routing request to registry") | 97 | log.Info().Msg("routing request to registry") | ||
| 93 | s.proxyToRegistry(w, r) | 98 | s.proxyToRegistry(w, r) | ||
| 94 | return | 99 | return | ||
| @@ -281,7 +286,9 @@ func (s *Service) RefreshTargets(ctx context.Context) error { | |||||
| 281 | 286 | ||||
| 282 | // UpdateConfig updates the service configuration. | 287 | // UpdateConfig updates the service configuration. | ||
| 283 | func (s *Service) UpdateConfig(config Config) { | 288 | func (s *Service) UpdateConfig(config Config) { | ||
| 289 | + | s.mu.Lock() | |||
| 284 | s.config = config | 290 | s.config = config | ||
| 291 | + | s.mu.Unlock() | |||
| 285 | } | 292 | } | ||
| 286 | 293 | ||||
| 287 | // Helper methods | 294 | // Helper methods | ||
| @@ -346,9 +353,14 @@ func (s *Service) proxyToTarget(w http.ResponseWriter, r *http.Request, target * | |||||
| 346 | } | 353 | } | ||
| 347 | 354 | ||||
| 348 | func (s *Service) proxyToRegistry(w http.ResponseWriter, r *http.Request) { | 355 | func (s *Service) proxyToRegistry(w http.ResponseWriter, r *http.Request) { | ||
| 356 | + | // Copy config under RLock to avoid data race with UpdateConfig | |||
| 357 | + | s.mu.RLock() | |||
| 358 | + | cfg := s.config | |||
| 359 | + | s.mu.RUnlock() | |||
| 360 | + | ||||
| 349 | log := zerowrap.FromCtx(r.Context()) | 361 | log := zerowrap.FromCtx(r.Context()) | ||
| 350 | 362 | ||||
| 351 | - | targetURL, err := url.Parse(fmt.Sprintf("http://localhost:%d", s.config.RegistryPort)) | 363 | + | targetURL, err := url.Parse(fmt.Sprintf("http://localhost:%d", cfg.RegistryPort)) |
| 352 | if err != nil { | 364 | if err != nil { | ||
| 353 | log.WrapErr(err, "failed to parse registry target URL") | 365 | log.WrapErr(err, "failed to parse registry target URL") | ||
| 354 | http.Error(w, "Internal Server Error", http.StatusInternalServerError) | 366 | http.Error(w, "Internal Server Error", http.StatusInternalServerError) | ||
| @@ -359,11 +371,11 @@ func (s *Service) proxyToRegistry(w http.ResponseWriter, r *http.Request) { | |||||
| 359 | func(w http.ResponseWriter, _ *http.Request, err error) { | 371 | func(w http.ResponseWriter, _ *http.Request, err error) { | ||
| 360 | // Check if this is a MaxBytesError (request body too large) | 372 | // Check if this is a MaxBytesError (request body too large) | ||
| 361 | if err.Error() == "http: request body too large" { | 373 | if err.Error() == "http: request body too large" { | ||
| 362 | - | log.Warn().Err(err).Int("registry_port", s.config.RegistryPort).Msg("registry proxy error: request body too large") | 374 | + | log.Warn().Err(err).Int("registry_port", cfg.RegistryPort).Msg("registry proxy error: request body too large") |
| 363 | http.Error(w, "Request Entity Too Large", http.StatusRequestEntityTooLarge) | 375 | http.Error(w, "Request Entity Too Large", http.StatusRequestEntityTooLarge) | ||
| 364 | return | 376 | return | ||
| 365 | } | 377 | } | ||
| 366 | - | log.Error().Err(err).Int("registry_port", s.config.RegistryPort).Msg("registry proxy error") | 378 | + | log.Error().Err(err).Int("registry_port", cfg.RegistryPort).Msg("registry proxy error") |
| 367 | http.Error(w, "Registry Unavailable", http.StatusServiceUnavailable) | 379 | http.Error(w, "Registry Unavailable", http.StatusServiceUnavailable) | ||
| 368 | }, | 380 | }, | ||
| 369 | func(resp *http.Response) error { | 381 | func(resp *http.Response) error { | ||
M internal/usecase/proxy/service_test.go +2 -2
| @@ -361,7 +361,7 @@ func TestContainerDeployedHandler_Handle_InvalidatesCache(t *testing.T) { | |||
| 361 | 361 | }, | |
| 362 | 362 | } | |
| 363 | 363 | ||
| 364 | - | err := handler.Handle(event) | |
| 364 | + | err := handler.Handle(context.Background(), event) | |
| 365 | 365 | ||
| 366 | 366 | assert.NoError(t, err) | |
| 367 | 367 | ||
| @@ -393,7 +393,7 @@ func TestContainerDeployedHandler_Handle_NoDomain(t *testing.T) { | |||
| 393 | 393 | } | |
| 394 | 394 | ||
| 395 | 395 | // Should not error, just skip | |
| 396 | - | err := handler.Handle(event) | |
| 396 | + | err := handler.Handle(context.Background(), event) | |
| 397 | 397 | assert.NoError(t, err) | |
| 398 | 398 | } | |
| 399 | 399 | ||
| @@ -361,7 +361,7 @@ func TestContainerDeployedHandler_Handle_InvalidatesCache(t *testing.T) { | |||||
| 361 | }, | 361 | }, | ||
| 362 | } | 362 | } | ||
| 363 | 363 | ||||
| 364 | - | err := handler.Handle(event) | 364 | + | err := handler.Handle(context.Background(), event) |
| 365 | 365 | ||||
| 366 | assert.NoError(t, err) | 366 | assert.NoError(t, err) | ||
| 367 | 367 | ||||
| @@ -393,7 +393,7 @@ func TestContainerDeployedHandler_Handle_NoDomain(t *testing.T) { | |||||
| 393 | } | 393 | } | ||
| 394 | 394 | ||||
| 395 | // Should not error, just skip | 395 | // Should not error, just skip | ||
| 396 | - | err := handler.Handle(event) | 396 | + | err := handler.Handle(context.Background(), event) |
| 397 | assert.NoError(t, err) | 397 | assert.NoError(t, err) | ||
| 398 | } | 398 | } | ||
| 399 | 399 | ||||
M pkg/validation/path.go +2 -2
| @@ -28,8 +28,8 @@ var referenceRegex = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}$`) | |||
| 28 | 28 | var digestRegex = regexp.MustCompile(`^(sha256:[a-f0-9]{64}|sha512:[a-f0-9]{128})$`) | |
| 29 | 29 | ||
| 30 | 30 | // UUID validation for blob uploads: | |
| 31 | - | // - Format: timestamp-name (where name has / replaced with _) | |
| 32 | - | var uuidRegex = regexp.MustCompile(`^[0-9]+-[a-zA-Z0-9_-]+$`) | |
| 31 | + | // - Format: standard UUID v4 (e.g., 550e8400-e29b-41d4-a716-446655440000) | |
| 32 | + | var uuidRegex = regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$`) | |
| 33 | 33 | ||
| 34 | 34 | // MaxRepositoryNameLength is the maximum allowed length for repository names. | |
| 35 | 35 | const MaxRepositoryNameLength = 256 | |
| @@ -28,8 +28,8 @@ var referenceRegex = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}$`) | |||||
| 28 | var digestRegex = regexp.MustCompile(`^(sha256:[a-f0-9]{64}|sha512:[a-f0-9]{128})$`) | 28 | var digestRegex = regexp.MustCompile(`^(sha256:[a-f0-9]{64}|sha512:[a-f0-9]{128})$`) | ||
| 29 | 29 | ||||
| 30 | // UUID validation for blob uploads: | 30 | // UUID validation for blob uploads: | ||
| 31 | - | // - Format: timestamp-name (where name has / replaced with _) | 31 | + | // - Format: standard UUID v4 (e.g., 550e8400-e29b-41d4-a716-446655440000) |
| 32 | - | var uuidRegex = regexp.MustCompile(`^[0-9]+-[a-zA-Z0-9_-]+$`) | 32 | + | var uuidRegex = regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$`) |
| 33 | 33 | ||||
| 34 | // MaxRepositoryNameLength is the maximum allowed length for repository names. | 34 | // MaxRepositoryNameLength is the maximum allowed length for repository names. | ||
| 35 | const MaxRepositoryNameLength = 256 | 35 | const MaxRepositoryNameLength = 256 | ||
M pkg/validation/path_test.go +15 -12
| @@ -163,23 +163,26 @@ func TestValidateUUID(t *testing.T) { | |||
| 163 | 163 | wantErr bool | |
| 164 | 164 | errMsg string | |
| 165 | 165 | }{ | |
| 166 | - | // Valid cases | |
| 167 | - | {"valid uuid", "1234567890-myapp", false, ""}, | |
| 168 | - | {"with underscore", "1234567890-my_app", false, ""}, | |
| 169 | - | {"with hyphen in name", "1234567890-my-app", false, ""}, | |
| 170 | - | {"nested repo uuid", "1234567890-myorg_myapp", false, ""}, | |
| 166 | + | // Valid cases - standard UUID v4 | |
| 167 | + | {"valid uuid v4", "550e8400-e29b-41d4-a716-446655440000", false, ""}, | |
| 168 | + | {"another valid uuid v4", "f47ac10b-58cc-4372-a567-0e02b2c3d479", false, ""}, | |
| 169 | + | ||
| 170 | + | // Invalid cases - not UUID v4 | |
| 171 | + | {"uuid v1 wrong version", "6ba7b810-9dad-11d1-80b4-00c04fd430c8", true, "invalid UUID format"}, | |
| 172 | + | {"all zeros not v4", "00000000-0000-0000-0000-000000000000", true, "invalid UUID format"}, | |
| 173 | + | {"wrong variant nibble", "550e8400-e29b-41d4-7716-446655440000", true, "invalid UUID format"}, | |
| 171 | 174 | ||
| 172 | 175 | // Invalid cases - path traversal | |
| 173 | - | {"path traversal", "1234567890-../etc/passwd", true, "path traversal"}, | |
| 174 | - | {"traversal in middle", "1234567890-app/../etc", true, "path traversal"}, | |
| 176 | + | {"path traversal", "../etc/passwd", true, "path traversal"}, | |
| 177 | + | {"traversal in middle", "app/../etc", true, "path traversal"}, | |
| 175 | 178 | ||
| 176 | 179 | // Invalid cases - format | |
| 177 | 180 | {"empty", "", true, "cannot be empty"}, | |
| 178 | - | {"no timestamp", "myapp", true, "invalid UUID format"}, | |
| 179 | - | {"no hyphen", "1234567890myapp", true, "invalid UUID format"}, | |
| 180 | - | {"special chars", "1234567890-my@app", true, "invalid UUID format"}, | |
| 181 | - | {"spaces", "1234567890-my app", true, "invalid UUID format"}, | |
| 182 | - | {"slash not replaced", "1234567890-myorg/myapp", true, "invalid UUID format"}, | |
| 181 | + | {"random string", "myapp", true, "invalid UUID format"}, | |
| 182 | + | {"old timestamp format", "1234567890-myapp", true, "invalid UUID format"}, | |
| 183 | + | {"uppercase hex", "550E8400-E29B-41D4-A716-446655440000", true, "invalid UUID format"}, | |
| 184 | + | {"spaces", "550e8400 e29b 41d4 a716 446655440000", true, "invalid UUID format"}, | |
| 185 | + | {"missing section", "550e8400-e29b-41d4-a716", true, "invalid UUID format"}, | |
| 183 | 186 | } | |
| 184 | 187 | ||
| 185 | 188 | for _, tt := range tests { | |
| @@ -163,23 +163,26 @@ func TestValidateUUID(t *testing.T) { | |||||
| 163 | wantErr bool | 163 | wantErr bool | ||
| 164 | errMsg string | 164 | errMsg string | ||
| 165 | }{ | 165 | }{ | ||
| 166 | - | // Valid cases | 166 | + | // Valid cases - standard UUID v4 |
| 167 | - | {"valid uuid", "1234567890-myapp", false, ""}, | 167 | + | {"valid uuid v4", "550e8400-e29b-41d4-a716-446655440000", false, ""}, |
| 168 | - | {"with underscore", "1234567890-my_app", false, ""}, | 168 | + | {"another valid uuid v4", "f47ac10b-58cc-4372-a567-0e02b2c3d479", false, ""}, |
| 169 | - | {"with hyphen in name", "1234567890-my-app", false, ""}, | 169 | + | |
| 170 | - | {"nested repo uuid", "1234567890-myorg_myapp", false, ""}, | 170 | + | // Invalid cases - not UUID v4 |
| 171 | + | {"uuid v1 wrong version", "6ba7b810-9dad-11d1-80b4-00c04fd430c8", true, "invalid UUID format"}, | |||
| 172 | + | {"all zeros not v4", "00000000-0000-0000-0000-000000000000", true, "invalid UUID format"}, | |||
| 173 | + | {"wrong variant nibble", "550e8400-e29b-41d4-7716-446655440000", true, "invalid UUID format"}, | |||
| 171 | 174 | ||||
| 172 | // Invalid cases - path traversal | 175 | // Invalid cases - path traversal | ||
| 173 | - | {"path traversal", "1234567890-../etc/passwd", true, "path traversal"}, | 176 | + | {"path traversal", "../etc/passwd", true, "path traversal"}, |
| 174 | - | {"traversal in middle", "1234567890-app/../etc", true, "path traversal"}, | 177 | + | {"traversal in middle", "app/../etc", true, "path traversal"}, |
| 175 | 178 | ||||
| 176 | // Invalid cases - format | 179 | // Invalid cases - format | ||
| 177 | {"empty", "", true, "cannot be empty"}, | 180 | {"empty", "", true, "cannot be empty"}, | ||
| 178 | - | {"no timestamp", "myapp", true, "invalid UUID format"}, | 181 | + | {"random string", "myapp", true, "invalid UUID format"}, |
| 179 | - | {"no hyphen", "1234567890myapp", true, "invalid UUID format"}, | 182 | + | {"old timestamp format", "1234567890-myapp", true, "invalid UUID format"}, |
| 180 | - | {"special chars", "1234567890-my@app", true, "invalid UUID format"}, | 183 | + | {"uppercase hex", "550E8400-E29B-41D4-A716-446655440000", true, "invalid UUID format"}, |
| 181 | - | {"spaces", "1234567890-my app", true, "invalid UUID format"}, | 184 | + | {"spaces", "550e8400 e29b 41d4 a716 446655440000", true, "invalid UUID format"}, |
| 182 | - | {"slash not replaced", "1234567890-myorg/myapp", true, "invalid UUID format"}, | 185 | + | {"missing section", "550e8400-e29b-41d4-a716", true, "invalid UUID format"}, |
| 183 | } | 186 | } | ||
| 184 | 187 | ||||
| 185 | for _, tt := range tests { | 188 | for _, tt := range tests { | ||