address CodeRabbit public ACME TLS findings
- commit
- a7df376bc574107c18535c38a8f052728f2a3a28
- author
- brice · · 2026-04-29 09:42 UTC
- parent
- b85bb4e
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
20 files · +380 -353
M docs/cli/tls.md +1 -1
| @@ -34,7 +34,7 @@ gordon tls status --remote https://gordon.mydomain.com --token $TOKEN | |||
| 34 | 34 | ||
| 35 | 35 | ### Human Output | |
| 36 | 36 | ||
| 37 | - | ``` | |
| 37 | + | ```text | |
| 38 | 38 | Public TLS / ACME Status | |
| 39 | 39 | ||
| 40 | 40 | ACME: enabled | |
| @@ -34,7 +34,7 @@ gordon tls status --remote https://gordon.mydomain.com --token $TOKEN | |||||
| 34 | 34 | ||||
| 35 | ### Human Output | 35 | ### Human Output | ||
| 36 | 36 | ||||
| 37 | - | ``` | 37 | + | ```text |
| 38 | Public TLS / ACME Status | 38 | Public TLS / ACME Status | ||
| 39 | 39 | ||||
| 40 | ACME: enabled | 40 | ACME: enabled | ||
M internal/adapters/in/cli/tls_test.go +14 -2
| @@ -272,6 +272,15 @@ func TestTLSStatus_JSONOutput(t *testing.T) { | |||
| 272 | 272 | ConfiguredMode: "acme", | |
| 273 | 273 | EffectiveMode: "acme-staging", | |
| 274 | 274 | TokenSource: "env", | |
| 275 | + | Certificates: []dto.TLSCertificateEntry{ | |
| 276 | + | { | |
| 277 | + | ID: "cert-abc123", | |
| 278 | + | Names: []string{"example.com", "www.example.com"}, | |
| 279 | + | Status: "valid", | |
| 280 | + | NotAfter: notAfter, | |
| 281 | + | LastError: "", | |
| 282 | + | }, | |
| 283 | + | }, | |
| 275 | 284 | }, nil | |
| 276 | 285 | }, | |
| 277 | 286 | } | |
| @@ -288,6 +297,9 @@ func TestTLSStatus_JSONOutput(t *testing.T) { | |||
| 288 | 297 | assert.Equal(t, "acme", result.ConfiguredMode) | |
| 289 | 298 | assert.Equal(t, "acme-staging", result.EffectiveMode) | |
| 290 | 299 | assert.Equal(t, "env", result.TokenSource) | |
| 291 | - | ||
| 292 | - | _ = notAfter | |
| 300 | + | require.Len(t, result.Certificates, 1) | |
| 301 | + | assert.Equal(t, "cert-abc123", result.Certificates[0].ID) | |
| 302 | + | assert.Equal(t, []string{"example.com", "www.example.com"}, result.Certificates[0].Names) | |
| 303 | + | assert.Equal(t, "valid", result.Certificates[0].Status) | |
| 304 | + | assert.Equal(t, notAfter.Format(time.RFC3339), result.Certificates[0].NotAfter.Format(time.RFC3339)) | |
| 293 | 305 | } | |
| @@ -272,6 +272,15 @@ func TestTLSStatus_JSONOutput(t *testing.T) { | |||||
| 272 | ConfiguredMode: "acme", | 272 | ConfiguredMode: "acme", | ||
| 273 | EffectiveMode: "acme-staging", | 273 | EffectiveMode: "acme-staging", | ||
| 274 | TokenSource: "env", | 274 | TokenSource: "env", | ||
| 275 | + | Certificates: []dto.TLSCertificateEntry{ | |||
| 276 | + | { | |||
| 277 | + | ID: "cert-abc123", | |||
| 278 | + | Names: []string{"example.com", "www.example.com"}, | |||
| 279 | + | Status: "valid", | |||
| 280 | + | NotAfter: notAfter, | |||
| 281 | + | LastError: "", | |||
| 282 | + | }, | |||
| 283 | + | }, | |||
| 275 | }, nil | 284 | }, nil | ||
| 276 | }, | 285 | }, | ||
| 277 | } | 286 | } | ||
| @@ -288,6 +297,9 @@ func TestTLSStatus_JSONOutput(t *testing.T) { | |||||
| 288 | assert.Equal(t, "acme", result.ConfiguredMode) | 297 | assert.Equal(t, "acme", result.ConfiguredMode) | ||
| 289 | assert.Equal(t, "acme-staging", result.EffectiveMode) | 298 | assert.Equal(t, "acme-staging", result.EffectiveMode) | ||
| 290 | assert.Equal(t, "env", result.TokenSource) | 299 | assert.Equal(t, "env", result.TokenSource) | ||
| 291 | - | 300 | + | require.Len(t, result.Certificates, 1) | |
| 292 | - | _ = notAfter | 301 | + | assert.Equal(t, "cert-abc123", result.Certificates[0].ID) |
| 302 | + | assert.Equal(t, []string{"example.com", "www.example.com"}, result.Certificates[0].Names) | |||
| 303 | + | assert.Equal(t, "valid", result.Certificates[0].Status) | |||
| 304 | + | assert.Equal(t, notAfter.Format(time.RFC3339), result.Certificates[0].NotAfter.Format(time.RFC3339)) | |||
| 293 | } | 305 | } | ||
M internal/adapters/in/http/acme/handler_test.go +15 -62
| @@ -5,6 +5,8 @@ import ( | |||
| 5 | 5 | "net/http" | |
| 6 | 6 | "net/http/httptest" | |
| 7 | 7 | "testing" | |
| 8 | + | ||
| 9 | + | "github.com/stretchr/testify/assert" | |
| 8 | 10 | ) | |
| 9 | 11 | ||
| 10 | 12 | // challengeSvc is a simple map-backed implementation of ChallengeService for testing. | |
| @@ -27,22 +29,10 @@ func TestHandlerServesGETChallenge(t *testing.T) { | |||
| 27 | 29 | resp := rec.Result() | |
| 28 | 30 | defer resp.Body.Close() | |
| 29 | 31 | ||
| 30 | - | if resp.StatusCode != http.StatusOK { | |
| 31 | - | t.Errorf("expected status 200, got %d", resp.StatusCode) | |
| 32 | - | } | |
| 33 | - | ||
| 34 | - | if ct := resp.Header.Get("Content-Type"); ct != "text/plain; charset=utf-8" { | |
| 35 | - | t.Errorf("expected Content-Type text/plain; charset=utf-8, got %q", ct) | |
| 36 | - | } | |
| 37 | - | ||
| 38 | - | if cc := resp.Header.Get("Cache-Control"); cc != "no-store" { | |
| 39 | - | t.Errorf("expected Cache-Control no-store, got %q", cc) | |
| 40 | - | } | |
| 41 | - | ||
| 42 | - | body := rec.Body.String() | |
| 43 | - | if body != "key-auth" { | |
| 44 | - | t.Errorf("expected body %q, got %q", "key-auth", body) | |
| 45 | - | } | |
| 32 | + | assert.Equal(t, http.StatusOK, resp.StatusCode) | |
| 33 | + | assert.Equal(t, "text/plain; charset=utf-8", resp.Header.Get("Content-Type")) | |
| 34 | + | assert.Equal(t, "no-store", resp.Header.Get("Cache-Control")) | |
| 35 | + | assert.Equal(t, "key-auth", rec.Body.String()) | |
| 46 | 36 | } | |
| 47 | 37 | ||
| 48 | 38 | func TestHandlerServesHEADChallenge(t *testing.T) { | |
| @@ -57,22 +47,10 @@ func TestHandlerServesHEADChallenge(t *testing.T) { | |||
| 57 | 47 | resp := rec.Result() | |
| 58 | 48 | defer resp.Body.Close() | |
| 59 | 49 | ||
| 60 | - | if resp.StatusCode != http.StatusOK { | |
| 61 | - | t.Errorf("expected status 200, got %d", resp.StatusCode) | |
| 62 | - | } | |
| 63 | - | ||
| 64 | - | if ct := resp.Header.Get("Content-Type"); ct != "text/plain; charset=utf-8" { | |
| 65 | - | t.Errorf("expected Content-Type text/plain; charset=utf-8, got %q", ct) | |
| 66 | - | } | |
| 67 | - | ||
| 68 | - | if cc := resp.Header.Get("Cache-Control"); cc != "no-store" { | |
| 69 | - | t.Errorf("expected Cache-Control no-store, got %q", cc) | |
| 70 | - | } | |
| 71 | - | ||
| 72 | - | body := rec.Body.String() | |
| 73 | - | if body != "" { | |
| 74 | - | t.Errorf("expected empty body for HEAD, got %q", body) | |
| 75 | - | } | |
| 50 | + | assert.Equal(t, http.StatusOK, resp.StatusCode) | |
| 51 | + | assert.Equal(t, "text/plain; charset=utf-8", resp.Header.Get("Content-Type")) | |
| 52 | + | assert.Equal(t, "no-store", resp.Header.Get("Cache-Control")) | |
| 53 | + | assert.Empty(t, rec.Body.String()) | |
| 76 | 54 | } | |
| 77 | 55 | ||
| 78 | 56 | func TestHandlerRejectsUnsafeToken(t *testing.T) { | |
| @@ -95,12 +73,7 @@ func TestHandlerRejectsUnsafeToken(t *testing.T) { | |||
| 95 | 73 | ||
| 96 | 74 | h.ServeHTTP(rec, req) | |
| 97 | 75 | ||
| 98 | - | resp := rec.Result() | |
| 99 | - | defer resp.Body.Close() | |
| 100 | - | ||
| 101 | - | if resp.StatusCode != http.StatusNotFound { | |
| 102 | - | t.Errorf("expected status 404, got %d for path %q", resp.StatusCode, tt.path) | |
| 103 | - | } | |
| 76 | + | assert.Equal(t, http.StatusNotFound, rec.Code, "path: %s", tt.path) | |
| 104 | 77 | }) | |
| 105 | 78 | } | |
| 106 | 79 | } | |
| @@ -114,12 +87,7 @@ func TestHandlerRejectsPOST(t *testing.T) { | |||
| 114 | 87 | ||
| 115 | 88 | h.ServeHTTP(rec, req) | |
| 116 | 89 | ||
| 117 | - | resp := rec.Result() | |
| 118 | - | defer resp.Body.Close() | |
| 119 | - | ||
| 120 | - | if resp.StatusCode != http.StatusMethodNotAllowed { | |
| 121 | - | t.Errorf("expected status 405, got %d", resp.StatusCode) | |
| 122 | - | } | |
| 90 | + | assert.Equal(t, http.StatusMethodNotAllowed, rec.Code) | |
| 123 | 91 | } | |
| 124 | 92 | ||
| 125 | 93 | func TestHandlerRejectsMissingPrefix(t *testing.T) { | |
| @@ -130,12 +98,7 @@ func TestHandlerRejectsMissingPrefix(t *testing.T) { | |||
| 130 | 98 | ||
| 131 | 99 | h.ServeHTTP(rec, req) | |
| 132 | 100 | ||
| 133 | - | resp := rec.Result() | |
| 134 | - | defer resp.Body.Close() | |
| 135 | - | ||
| 136 | - | if resp.StatusCode != http.StatusNotFound { | |
| 137 | - | t.Errorf("expected status 404 for non-matching prefix, got %d", resp.StatusCode) | |
| 138 | - | } | |
| 101 | + | assert.Equal(t, http.StatusNotFound, rec.Code) | |
| 139 | 102 | } | |
| 140 | 103 | ||
| 141 | 104 | func TestHandlerRejectsUnknownToken(t *testing.T) { | |
| @@ -146,12 +109,7 @@ func TestHandlerRejectsUnknownToken(t *testing.T) { | |||
| 146 | 109 | ||
| 147 | 110 | h.ServeHTTP(rec, req) | |
| 148 | 111 | ||
| 149 | - | resp := rec.Result() | |
| 150 | - | defer resp.Body.Close() | |
| 151 | - | ||
| 152 | - | if resp.StatusCode != http.StatusNotFound { | |
| 153 | - | t.Errorf("expected status 404 for unknown token, got %d", resp.StatusCode) | |
| 154 | - | } | |
| 112 | + | assert.Equal(t, http.StatusNotFound, rec.Code) | |
| 155 | 113 | } | |
| 156 | 114 | ||
| 157 | 115 | func TestHandlerRejectsNilService(t *testing.T) { | |
| @@ -162,10 +120,5 @@ func TestHandlerRejectsNilService(t *testing.T) { | |||
| 162 | 120 | ||
| 163 | 121 | h.ServeHTTP(rec, req) | |
| 164 | 122 | ||
| 165 | - | resp := rec.Result() | |
| 166 | - | defer resp.Body.Close() | |
| 167 | - | ||
| 168 | - | if resp.StatusCode != http.StatusNotFound { | |
| 169 | - | t.Errorf("expected status 404 for nil service, got %d", resp.StatusCode) | |
| 170 | - | } | |
| 123 | + | assert.Equal(t, http.StatusNotFound, rec.Code) | |
| 171 | 124 | } | |
| @@ -5,6 +5,8 @@ import ( | |||||
| 5 | "net/http" | 5 | "net/http" | ||
| 6 | "net/http/httptest" | 6 | "net/http/httptest" | ||
| 7 | "testing" | 7 | "testing" | ||
| 8 | + | ||||
| 9 | + | "github.com/stretchr/testify/assert" | |||
| 8 | ) | 10 | ) | ||
| 9 | 11 | ||||
| 10 | // challengeSvc is a simple map-backed implementation of ChallengeService for testing. | 12 | // challengeSvc is a simple map-backed implementation of ChallengeService for testing. | ||
| @@ -27,22 +29,10 @@ func TestHandlerServesGETChallenge(t *testing.T) { | |||||
| 27 | resp := rec.Result() | 29 | resp := rec.Result() | ||
| 28 | defer resp.Body.Close() | 30 | defer resp.Body.Close() | ||
| 29 | 31 | ||||
| 30 | - | if resp.StatusCode != http.StatusOK { | 32 | + | assert.Equal(t, http.StatusOK, resp.StatusCode) |
| 31 | - | t.Errorf("expected status 200, got %d", resp.StatusCode) | 33 | + | assert.Equal(t, "text/plain; charset=utf-8", resp.Header.Get("Content-Type")) |
| 32 | - | } | 34 | + | assert.Equal(t, "no-store", resp.Header.Get("Cache-Control")) |
| 33 | - | 35 | + | assert.Equal(t, "key-auth", rec.Body.String()) | |
| 34 | - | if ct := resp.Header.Get("Content-Type"); ct != "text/plain; charset=utf-8" { | |||
| 35 | - | t.Errorf("expected Content-Type text/plain; charset=utf-8, got %q", ct) | |||
| 36 | - | } | |||
| 37 | - | ||||
| 38 | - | if cc := resp.Header.Get("Cache-Control"); cc != "no-store" { | |||
| 39 | - | t.Errorf("expected Cache-Control no-store, got %q", cc) | |||
| 40 | - | } | |||
| 41 | - | ||||
| 42 | - | body := rec.Body.String() | |||
| 43 | - | if body != "key-auth" { | |||
| 44 | - | t.Errorf("expected body %q, got %q", "key-auth", body) | |||
| 45 | - | } | |||
| 46 | } | 36 | } | ||
| 47 | 37 | ||||
| 48 | func TestHandlerServesHEADChallenge(t *testing.T) { | 38 | func TestHandlerServesHEADChallenge(t *testing.T) { | ||
| @@ -57,22 +47,10 @@ func TestHandlerServesHEADChallenge(t *testing.T) { | |||||
| 57 | resp := rec.Result() | 47 | resp := rec.Result() | ||
| 58 | defer resp.Body.Close() | 48 | defer resp.Body.Close() | ||
| 59 | 49 | ||||
| 60 | - | if resp.StatusCode != http.StatusOK { | 50 | + | assert.Equal(t, http.StatusOK, resp.StatusCode) |
| 61 | - | t.Errorf("expected status 200, got %d", resp.StatusCode) | 51 | + | assert.Equal(t, "text/plain; charset=utf-8", resp.Header.Get("Content-Type")) |
| 62 | - | } | 52 | + | assert.Equal(t, "no-store", resp.Header.Get("Cache-Control")) |
| 63 | - | 53 | + | assert.Empty(t, rec.Body.String()) | |
| 64 | - | if ct := resp.Header.Get("Content-Type"); ct != "text/plain; charset=utf-8" { | |||
| 65 | - | t.Errorf("expected Content-Type text/plain; charset=utf-8, got %q", ct) | |||
| 66 | - | } | |||
| 67 | - | ||||
| 68 | - | if cc := resp.Header.Get("Cache-Control"); cc != "no-store" { | |||
| 69 | - | t.Errorf("expected Cache-Control no-store, got %q", cc) | |||
| 70 | - | } | |||
| 71 | - | ||||
| 72 | - | body := rec.Body.String() | |||
| 73 | - | if body != "" { | |||
| 74 | - | t.Errorf("expected empty body for HEAD, got %q", body) | |||
| 75 | - | } | |||
| 76 | } | 54 | } | ||
| 77 | 55 | ||||
| 78 | func TestHandlerRejectsUnsafeToken(t *testing.T) { | 56 | func TestHandlerRejectsUnsafeToken(t *testing.T) { | ||
| @@ -95,12 +73,7 @@ func TestHandlerRejectsUnsafeToken(t *testing.T) { | |||||
| 95 | 73 | ||||
| 96 | h.ServeHTTP(rec, req) | 74 | h.ServeHTTP(rec, req) | ||
| 97 | 75 | ||||
| 98 | - | resp := rec.Result() | 76 | + | assert.Equal(t, http.StatusNotFound, rec.Code, "path: %s", tt.path) |
| 99 | - | defer resp.Body.Close() | |||
| 100 | - | ||||
| 101 | - | if resp.StatusCode != http.StatusNotFound { | |||
| 102 | - | t.Errorf("expected status 404, got %d for path %q", resp.StatusCode, tt.path) | |||
| 103 | - | } | |||
| 104 | }) | 77 | }) | ||
| 105 | } | 78 | } | ||
| 106 | } | 79 | } | ||
| @@ -114,12 +87,7 @@ func TestHandlerRejectsPOST(t *testing.T) { | |||||
| 114 | 87 | ||||
| 115 | h.ServeHTTP(rec, req) | 88 | h.ServeHTTP(rec, req) | ||
| 116 | 89 | ||||
| 117 | - | resp := rec.Result() | 90 | + | assert.Equal(t, http.StatusMethodNotAllowed, rec.Code) |
| 118 | - | defer resp.Body.Close() | |||
| 119 | - | ||||
| 120 | - | if resp.StatusCode != http.StatusMethodNotAllowed { | |||
| 121 | - | t.Errorf("expected status 405, got %d", resp.StatusCode) | |||
| 122 | - | } | |||
| 123 | } | 91 | } | ||
| 124 | 92 | ||||
| 125 | func TestHandlerRejectsMissingPrefix(t *testing.T) { | 93 | func TestHandlerRejectsMissingPrefix(t *testing.T) { | ||
| @@ -130,12 +98,7 @@ func TestHandlerRejectsMissingPrefix(t *testing.T) { | |||||
| 130 | 98 | ||||
| 131 | h.ServeHTTP(rec, req) | 99 | h.ServeHTTP(rec, req) | ||
| 132 | 100 | ||||
| 133 | - | resp := rec.Result() | 101 | + | assert.Equal(t, http.StatusNotFound, rec.Code) |
| 134 | - | defer resp.Body.Close() | |||
| 135 | - | ||||
| 136 | - | if resp.StatusCode != http.StatusNotFound { | |||
| 137 | - | t.Errorf("expected status 404 for non-matching prefix, got %d", resp.StatusCode) | |||
| 138 | - | } | |||
| 139 | } | 102 | } | ||
| 140 | 103 | ||||
| 141 | func TestHandlerRejectsUnknownToken(t *testing.T) { | 104 | func TestHandlerRejectsUnknownToken(t *testing.T) { | ||
| @@ -146,12 +109,7 @@ func TestHandlerRejectsUnknownToken(t *testing.T) { | |||||
| 146 | 109 | ||||
| 147 | h.ServeHTTP(rec, req) | 110 | h.ServeHTTP(rec, req) | ||
| 148 | 111 | ||||
| 149 | - | resp := rec.Result() | 112 | + | assert.Equal(t, http.StatusNotFound, rec.Code) |
| 150 | - | defer resp.Body.Close() | |||
| 151 | - | ||||
| 152 | - | if resp.StatusCode != http.StatusNotFound { | |||
| 153 | - | t.Errorf("expected status 404 for unknown token, got %d", resp.StatusCode) | |||
| 154 | - | } | |||
| 155 | } | 113 | } | ||
| 156 | 114 | ||||
| 157 | func TestHandlerRejectsNilService(t *testing.T) { | 115 | func TestHandlerRejectsNilService(t *testing.T) { | ||
| @@ -162,10 +120,5 @@ func TestHandlerRejectsNilService(t *testing.T) { | |||||
| 162 | 120 | ||||
| 163 | h.ServeHTTP(rec, req) | 121 | h.ServeHTTP(rec, req) | ||
| 164 | 122 | ||||
| 165 | - | resp := rec.Result() | 123 | + | assert.Equal(t, http.StatusNotFound, rec.Code) |
| 166 | - | defer resp.Body.Close() | |||
| 167 | - | ||||
| 168 | - | if resp.StatusCode != http.StatusNotFound { | |||
| 169 | - | t.Errorf("expected status 404 for nil service, got %d", resp.StatusCode) | |||
| 170 | - | } | |||
| 171 | } | 124 | } | ||
M internal/adapters/out/acmelego/cloudflare.go +40 -6
| @@ -3,12 +3,14 @@ package acmelego | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "encoding/json" | |
| 6 | + | "errors" | |
| 6 | 7 | "fmt" | |
| 7 | 8 | "io" | |
| 8 | 9 | "net/http" | |
| 9 | 10 | "net/url" | |
| 10 | 11 | "strings" | |
| 11 | 12 | "sync" | |
| 13 | + | "time" | |
| 12 | 14 | ||
| 13 | 15 | "github.com/bnema/gordon/internal/boundaries/out" | |
| 14 | 16 | ) | |
| @@ -45,10 +47,17 @@ type CloudflareZoneResolver struct { | |||
| 45 | 47 | } | |
| 46 | 48 | ||
| 47 | 49 | type zoneCacheEntry struct { | |
| 48 | - | zone out.CloudflareZone | |
| 49 | - | err error | |
| 50 | + | zone out.CloudflareZone | |
| 51 | + | err error | |
| 52 | + | createdAt time.Time | |
| 50 | 53 | } | |
| 51 | 54 | ||
| 55 | + | const ( | |
| 56 | + | defaultCacheTTL = 10 * time.Minute | |
| 57 | + | defaultCacheMaxSize = 100 | |
| 58 | + | defaultMaxResponseSize = 1 << 20 // 1 MB | |
| 59 | + | ) | |
| 60 | + | ||
| 52 | 61 | // CloudflareZoneResolverOption configures a CloudflareZoneResolver. | |
| 53 | 62 | type CloudflareZoneResolverOption func(*CloudflareZoneResolver) | |
| 54 | 63 | ||
| @@ -112,24 +121,27 @@ func (r *CloudflareZoneResolver) FindZone(ctx context.Context, domainName string | |||
| 112 | 121 | candidates = append(candidates, candidate) | |
| 113 | 122 | } | |
| 114 | 123 | ||
| 124 | + | var candidateErrs []error | |
| 115 | 125 | for _, candidate := range candidates { | |
| 116 | 126 | if cached, ok := r.cached(candidate); ok { | |
| 117 | 127 | if cached.err == nil { | |
| 118 | 128 | r.storeCache(domainName, cached.zone, nil) | |
| 119 | 129 | return cached.zone, nil | |
| 120 | 130 | } | |
| 131 | + | candidateErrs = append(candidateErrs, fmt.Errorf("%s: %w", candidate, cached.err)) | |
| 121 | 132 | continue | |
| 122 | 133 | } | |
| 123 | 134 | zone, err := r.findZoneByName(ctx, candidate) | |
| 124 | 135 | r.storeCache(candidate, zone, err) | |
| 125 | 136 | if err != nil { | |
| 137 | + | candidateErrs = append(candidateErrs, fmt.Errorf("%s: %w", candidate, err)) | |
| 126 | 138 | continue | |
| 127 | 139 | } | |
| 128 | 140 | r.storeCache(domainName, zone, nil) | |
| 129 | 141 | return zone, nil | |
| 130 | 142 | } | |
| 131 | 143 | ||
| 132 | - | err := fmt.Errorf("cloudflare zone resolver: no active zone found for %q", domainName) | |
| 144 | + | err := fmt.Errorf("cloudflare zone resolver: no active zone found for %q: %w", domainName, errors.Join(candidateErrs...)) | |
| 133 | 145 | r.storeCache(domainName, out.CloudflareZone{}, err) | |
| 134 | 146 | return out.CloudflareZone{}, err | |
| 135 | 147 | } | |
| @@ -138,13 +150,31 @@ func (r *CloudflareZoneResolver) cached(name string) (zoneCacheEntry, bool) { | |||
| 138 | 150 | r.mu.RLock() | |
| 139 | 151 | defer r.mu.RUnlock() | |
| 140 | 152 | entry, ok := r.cache[name] | |
| 141 | - | return entry, ok | |
| 153 | + | if !ok { | |
| 154 | + | return entry, false | |
| 155 | + | } | |
| 156 | + | if time.Since(entry.createdAt) > defaultCacheTTL { | |
| 157 | + | return zoneCacheEntry{}, false | |
| 158 | + | } | |
| 159 | + | return entry, true | |
| 142 | 160 | } | |
| 143 | 161 | ||
| 144 | 162 | func (r *CloudflareZoneResolver) storeCache(name string, zone out.CloudflareZone, err error) { | |
| 145 | 163 | r.mu.Lock() | |
| 146 | 164 | defer r.mu.Unlock() | |
| 147 | - | r.cache[name] = zoneCacheEntry{zone: zone, err: err} | |
| 165 | + | if len(r.cache) >= defaultCacheMaxSize { | |
| 166 | + | // Evict oldest entry | |
| 167 | + | var oldestKey string | |
| 168 | + | var oldestTime time.Time | |
| 169 | + | for k, v := range r.cache { | |
| 170 | + | if oldestKey == "" || v.createdAt.Before(oldestTime) { | |
| 171 | + | oldestKey = k | |
| 172 | + | oldestTime = v.createdAt | |
| 173 | + | } | |
| 174 | + | } | |
| 175 | + | delete(r.cache, oldestKey) | |
| 176 | + | } | |
| 177 | + | r.cache[name] = zoneCacheEntry{zone: zone, err: err, createdAt: time.Now()} | |
| 148 | 178 | } | |
| 149 | 179 | ||
| 150 | 180 | // findZoneByName queries the Cloudflare API for an active zone with the exact given name. | |
| @@ -172,10 +202,14 @@ func (r *CloudflareZoneResolver) findZoneByName(ctx context.Context, name string | |||
| 172 | 202 | } | |
| 173 | 203 | defer resp.Body.Close() | |
| 174 | 204 | ||
| 175 | - | body, err := io.ReadAll(resp.Body) | |
| 205 | + | lr := io.LimitReader(resp.Body, defaultMaxResponseSize+1) | |
| 206 | + | body, err := io.ReadAll(lr) | |
| 176 | 207 | if err != nil { | |
| 177 | 208 | return out.CloudflareZone{}, fmt.Errorf("read response: %w", err) | |
| 178 | 209 | } | |
| 210 | + | if len(body) > defaultMaxResponseSize { | |
| 211 | + | return out.CloudflareZone{}, fmt.Errorf("cloudflare api: response body too large (%d bytes)", len(body)) | |
| 212 | + | } | |
| 179 | 213 | ||
| 180 | 214 | if resp.StatusCode != http.StatusOK { | |
| 181 | 215 | return out.CloudflareZone{}, fmt.Errorf("cloudflare api: status %d", resp.StatusCode) | |
| @@ -3,12 +3,14 @@ package acmelego | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "encoding/json" | 5 | "encoding/json" | ||
| 6 | + | "errors" | |||
| 6 | "fmt" | 7 | "fmt" | ||
| 7 | "io" | 8 | "io" | ||
| 8 | "net/http" | 9 | "net/http" | ||
| 9 | "net/url" | 10 | "net/url" | ||
| 10 | "strings" | 11 | "strings" | ||
| 11 | "sync" | 12 | "sync" | ||
| 13 | + | "time" | |||
| 12 | 14 | ||||
| 13 | "github.com/bnema/gordon/internal/boundaries/out" | 15 | "github.com/bnema/gordon/internal/boundaries/out" | ||
| 14 | ) | 16 | ) | ||
| @@ -45,10 +47,17 @@ type CloudflareZoneResolver struct { | |||||
| 45 | } | 47 | } | ||
| 46 | 48 | ||||
| 47 | type zoneCacheEntry struct { | 49 | type zoneCacheEntry struct { | ||
| 48 | - | zone out.CloudflareZone | 50 | + | zone out.CloudflareZone |
| 49 | - | err error | 51 | + | err error |
| 52 | + | createdAt time.Time | |||
| 50 | } | 53 | } | ||
| 51 | 54 | ||||
| 55 | + | const ( | |||
| 56 | + | defaultCacheTTL = 10 * time.Minute | |||
| 57 | + | defaultCacheMaxSize = 100 | |||
| 58 | + | defaultMaxResponseSize = 1 << 20 // 1 MB | |||
| 59 | + | ) | |||
| 60 | + | ||||
| 52 | // CloudflareZoneResolverOption configures a CloudflareZoneResolver. | 61 | // CloudflareZoneResolverOption configures a CloudflareZoneResolver. | ||
| 53 | type CloudflareZoneResolverOption func(*CloudflareZoneResolver) | 62 | type CloudflareZoneResolverOption func(*CloudflareZoneResolver) | ||
| 54 | 63 | ||||
| @@ -112,24 +121,27 @@ func (r *CloudflareZoneResolver) FindZone(ctx context.Context, domainName string | |||||
| 112 | candidates = append(candidates, candidate) | 121 | candidates = append(candidates, candidate) | ||
| 113 | } | 122 | } | ||
| 114 | 123 | ||||
| 124 | + | var candidateErrs []error | |||
| 115 | for _, candidate := range candidates { | 125 | for _, candidate := range candidates { | ||
| 116 | if cached, ok := r.cached(candidate); ok { | 126 | if cached, ok := r.cached(candidate); ok { | ||
| 117 | if cached.err == nil { | 127 | if cached.err == nil { | ||
| 118 | r.storeCache(domainName, cached.zone, nil) | 128 | r.storeCache(domainName, cached.zone, nil) | ||
| 119 | return cached.zone, nil | 129 | return cached.zone, nil | ||
| 120 | } | 130 | } | ||
| 131 | + | candidateErrs = append(candidateErrs, fmt.Errorf("%s: %w", candidate, cached.err)) | |||
| 121 | continue | 132 | continue | ||
| 122 | } | 133 | } | ||
| 123 | zone, err := r.findZoneByName(ctx, candidate) | 134 | zone, err := r.findZoneByName(ctx, candidate) | ||
| 124 | r.storeCache(candidate, zone, err) | 135 | r.storeCache(candidate, zone, err) | ||
| 125 | if err != nil { | 136 | if err != nil { | ||
| 137 | + | candidateErrs = append(candidateErrs, fmt.Errorf("%s: %w", candidate, err)) | |||
| 126 | continue | 138 | continue | ||
| 127 | } | 139 | } | ||
| 128 | r.storeCache(domainName, zone, nil) | 140 | r.storeCache(domainName, zone, nil) | ||
| 129 | return zone, nil | 141 | return zone, nil | ||
| 130 | } | 142 | } | ||
| 131 | 143 | ||||
| 132 | - | err := fmt.Errorf("cloudflare zone resolver: no active zone found for %q", domainName) | 144 | + | err := fmt.Errorf("cloudflare zone resolver: no active zone found for %q: %w", domainName, errors.Join(candidateErrs...)) |
| 133 | r.storeCache(domainName, out.CloudflareZone{}, err) | 145 | r.storeCache(domainName, out.CloudflareZone{}, err) | ||
| 134 | return out.CloudflareZone{}, err | 146 | return out.CloudflareZone{}, err | ||
| 135 | } | 147 | } | ||
| @@ -138,13 +150,31 @@ func (r *CloudflareZoneResolver) cached(name string) (zoneCacheEntry, bool) { | |||||
| 138 | r.mu.RLock() | 150 | r.mu.RLock() | ||
| 139 | defer r.mu.RUnlock() | 151 | defer r.mu.RUnlock() | ||
| 140 | entry, ok := r.cache[name] | 152 | entry, ok := r.cache[name] | ||
| 141 | - | return entry, ok | 153 | + | if !ok { |
| 154 | + | return entry, false | |||
| 155 | + | } | |||
| 156 | + | if time.Since(entry.createdAt) > defaultCacheTTL { | |||
| 157 | + | return zoneCacheEntry{}, false | |||
| 158 | + | } | |||
| 159 | + | return entry, true | |||
| 142 | } | 160 | } | ||
| 143 | 161 | ||||
| 144 | func (r *CloudflareZoneResolver) storeCache(name string, zone out.CloudflareZone, err error) { | 162 | func (r *CloudflareZoneResolver) storeCache(name string, zone out.CloudflareZone, err error) { | ||
| 145 | r.mu.Lock() | 163 | r.mu.Lock() | ||
| 146 | defer r.mu.Unlock() | 164 | defer r.mu.Unlock() | ||
| 147 | - | r.cache[name] = zoneCacheEntry{zone: zone, err: err} | 165 | + | if len(r.cache) >= defaultCacheMaxSize { |
| 166 | + | // Evict oldest entry | |||
| 167 | + | var oldestKey string | |||
| 168 | + | var oldestTime time.Time | |||
| 169 | + | for k, v := range r.cache { | |||
| 170 | + | if oldestKey == "" || v.createdAt.Before(oldestTime) { | |||
| 171 | + | oldestKey = k | |||
| 172 | + | oldestTime = v.createdAt | |||
| 173 | + | } | |||
| 174 | + | } | |||
| 175 | + | delete(r.cache, oldestKey) | |||
| 176 | + | } | |||
| 177 | + | r.cache[name] = zoneCacheEntry{zone: zone, err: err, createdAt: time.Now()} | |||
| 148 | } | 178 | } | ||
| 149 | 179 | ||||
| 150 | // findZoneByName queries the Cloudflare API for an active zone with the exact given name. | 180 | // findZoneByName queries the Cloudflare API for an active zone with the exact given name. | ||
| @@ -172,10 +202,14 @@ func (r *CloudflareZoneResolver) findZoneByName(ctx context.Context, name string | |||||
| 172 | } | 202 | } | ||
| 173 | defer resp.Body.Close() | 203 | defer resp.Body.Close() | ||
| 174 | 204 | ||||
| 175 | - | body, err := io.ReadAll(resp.Body) | 205 | + | lr := io.LimitReader(resp.Body, defaultMaxResponseSize+1) |
| 206 | + | body, err := io.ReadAll(lr) | |||
| 176 | if err != nil { | 207 | if err != nil { | ||
| 177 | return out.CloudflareZone{}, fmt.Errorf("read response: %w", err) | 208 | return out.CloudflareZone{}, fmt.Errorf("read response: %w", err) | ||
| 178 | } | 209 | } | ||
| 210 | + | if len(body) > defaultMaxResponseSize { | |||
| 211 | + | return out.CloudflareZone{}, fmt.Errorf("cloudflare api: response body too large (%d bytes)", len(body)) | |||
| 212 | + | } | |||
| 179 | 213 | ||||
| 180 | if resp.StatusCode != http.StatusOK { | 214 | if resp.StatusCode != http.StatusOK { | ||
| 181 | return out.CloudflareZone{}, fmt.Errorf("cloudflare api: status %d", resp.StatusCode) | 215 | return out.CloudflareZone{}, fmt.Errorf("cloudflare api: status %d", resp.StatusCode) | ||
M internal/adapters/out/acmelego/issuer.go +32 -30
| @@ -67,6 +67,9 @@ func NewIssuer(cfg Config) (*Issuer, error) { | |||
| 67 | 67 | if cfg.Challenge == "" { | |
| 68 | 68 | cfg.Challenge = domain.ACMEChallengeHTTP01 | |
| 69 | 69 | } | |
| 70 | + | if cfg.Challenge == domain.ACMEChallengeHTTP01 && cfg.HTTPChallengeSink == nil { | |
| 71 | + | return nil, errors.New("acmelego: HTTP-01 challenge requires a non-nil HTTPChallengeSink") | |
| 72 | + | } | |
| 70 | 73 | if cfg.Challenge == domain.ACMEChallengeCloudflareDNS01 && cfg.Token == "" { | |
| 71 | 74 | return nil, fmt.Errorf("acmelego: %w", domain.ErrCloudflareTokenMissing) | |
| 72 | 75 | } | |
| @@ -92,7 +95,7 @@ func (i *Issuer) Obtain(ctx context.Context, order out.CertificateOrder) (*out.S | |||
| 92 | 95 | return nil, fmt.Errorf("obtain certificate: %w", err) | |
| 93 | 96 | } | |
| 94 | 97 | ||
| 95 | - | return resourceToStored(order, resource), nil | |
| 98 | + | return resourceToStored(order, resource) | |
| 96 | 99 | } | |
| 97 | 100 | ||
| 98 | 101 | // Renew renews an existing certificate. The ACME client is initialized lazily | |
| @@ -133,7 +136,7 @@ func (i *Issuer) Renew(ctx context.Context, cert out.StoredCertificate) (*out.St | |||
| 133 | 136 | Challenge: cert.Challenge, | |
| 134 | 137 | } | |
| 135 | 138 | ||
| 136 | - | return resourceToStored(order, renewed), nil | |
| 139 | + | return resourceToStored(order, renewed) | |
| 137 | 140 | } | |
| 138 | 141 | ||
| 139 | 142 | // ensureClient initializes the lego client and ACME account if not yet done. | |
| @@ -152,18 +155,7 @@ func (i *Issuer) ensureClient(ctx context.Context) error { | |||
| 152 | 155 | } | |
| 153 | 156 | i.user = user | |
| 154 | 157 | ||
| 155 | - | // Build lego config | |
| 156 | - | legoCfg := lego.NewConfig(user) | |
| 157 | - | legoCfg.Certificate.KeyType = certcrypto.EC256 // ECDSA P-256 | |
| 158 | - | ||
| 159 | - | if i.cfg.CADirectoryURL != "" { | |
| 160 | - | legoCfg.CADirURL = i.cfg.CADirectoryURL | |
| 161 | - | } | |
| 162 | - | if i.cfg.HTTPClient != nil { | |
| 163 | - | legoCfg.HTTPClient = i.cfg.HTTPClient | |
| 164 | - | } | |
| 165 | - | ||
| 166 | - | client, err := lego.NewClient(legoCfg) | |
| 158 | + | client, err := lego.NewClient(i.newLegoConfig(user)) | |
| 167 | 159 | if err != nil { | |
| 168 | 160 | return fmt.Errorf("create lego client: %w", err) | |
| 169 | 161 | } | |
| @@ -213,16 +205,7 @@ func (i *Issuer) loadOrCreateAccount(ctx context.Context) (*AccountUser, error) | |||
| 213 | 205 | user := NewAccountUser(i.cfg.Email, privateKey, nil) | |
| 214 | 206 | ||
| 215 | 207 | // We need a temporary client to register the account | |
| 216 | - | legoCfg := lego.NewConfig(user) | |
| 217 | - | legoCfg.Certificate.KeyType = certcrypto.EC256 | |
| 218 | - | if i.cfg.CADirectoryURL != "" { | |
| 219 | - | legoCfg.CADirURL = i.cfg.CADirectoryURL | |
| 220 | - | } | |
| 221 | - | if i.cfg.HTTPClient != nil { | |
| 222 | - | legoCfg.HTTPClient = i.cfg.HTTPClient | |
| 223 | - | } | |
| 224 | - | ||
| 225 | - | tmpClient, err := lego.NewClient(legoCfg) | |
| 208 | + | tmpClient, err := lego.NewClient(i.newLegoConfig(user)) | |
| 226 | 209 | if err != nil { | |
| 227 | 210 | return nil, fmt.Errorf("create temporary lego client: %w", err) | |
| 228 | 211 | } | |
| @@ -278,8 +261,21 @@ func restoreAccount(stored *out.ACMEAccount) (*AccountUser, error) { | |||
| 278 | 261 | return NewAccountUser(stored.Email, privateKey, reg), nil | |
| 279 | 262 | } | |
| 280 | 263 | ||
| 264 | + | // newLegoConfig creates a lego.Config with common settings from the Issuer config. | |
| 265 | + | func (i *Issuer) newLegoConfig(user *AccountUser) *lego.Config { | |
| 266 | + | legoCfg := lego.NewConfig(user) | |
| 267 | + | legoCfg.Certificate.KeyType = certcrypto.EC256 // ECDSA P-256 | |
| 268 | + | if i.cfg.CADirectoryURL != "" { | |
| 269 | + | legoCfg.CADirURL = i.cfg.CADirectoryURL | |
| 270 | + | } | |
| 271 | + | if i.cfg.HTTPClient != nil { | |
| 272 | + | legoCfg.HTTPClient = i.cfg.HTTPClient | |
| 273 | + | } | |
| 274 | + | return legoCfg | |
| 275 | + | } | |
| 276 | + | ||
| 281 | 277 | // resourceToStored converts a lego certificate.Resource to out.StoredCertificate. | |
| 282 | - | func resourceToStored(order out.CertificateOrder, res *certificate.Resource) *out.StoredCertificate { | |
| 278 | + | func resourceToStored(order out.CertificateOrder, res *certificate.Resource) (*out.StoredCertificate, error) { | |
| 283 | 279 | var fullchainPEM []byte | |
| 284 | 280 | if len(res.IssuerCertificate) > 0 { | |
| 285 | 281 | fullchainPEM = append(append([]byte{}, res.Certificate...), res.IssuerCertificate...) | |
| @@ -290,11 +286,17 @@ func resourceToStored(order out.CertificateOrder, res *certificate.Resource) *ou | |||
| 290 | 286 | // Parse the certificate to extract NotAfter and populate tls.Certificate. | |
| 291 | 287 | var tlsCert tls.Certificate | |
| 292 | 288 | notAfter := time.Time{} | |
| 293 | - | if parsedTLSCert, err := tls.X509KeyPair(fullchainPEM, res.PrivateKey); err == nil && len(parsedTLSCert.Certificate) > 0 { | |
| 294 | - | tlsCert = parsedTLSCert | |
| 295 | - | if parsed, parseErr := x509.ParseCertificate(parsedTLSCert.Certificate[0]); parseErr == nil { | |
| 296 | - | notAfter = parsed.NotAfter | |
| 289 | + | parsedTLSCert, tlsErr := tls.X509KeyPair(fullchainPEM, res.PrivateKey) | |
| 290 | + | if tlsErr != nil { | |
| 291 | + | return nil, fmt.Errorf("parse tls certificate: %w", tlsErr) | |
| 292 | + | } | |
| 293 | + | tlsCert = parsedTLSCert | |
| 294 | + | if len(parsedTLSCert.Certificate) > 0 { | |
| 295 | + | parsed, parseErr := x509.ParseCertificate(parsedTLSCert.Certificate[0]) | |
| 296 | + | if parseErr != nil { | |
| 297 | + | return nil, fmt.Errorf("parse x509 certificate: %w", parseErr) | |
| 297 | 298 | } | |
| 299 | + | notAfter = parsed.NotAfter | |
| 298 | 300 | } | |
| 299 | 301 | ||
| 300 | 302 | names := order.Names | |
| @@ -312,5 +314,5 @@ func resourceToStored(order out.CertificateOrder, res *certificate.Resource) *ou | |||
| 312 | 314 | FullchainPEM: fullchainPEM, | |
| 313 | 315 | PrivateKeyPEM: res.PrivateKey, | |
| 314 | 316 | NotAfter: notAfter, | |
| 315 | - | } | |
| 317 | + | }, nil | |
| 316 | 318 | } | |
| @@ -67,6 +67,9 @@ func NewIssuer(cfg Config) (*Issuer, error) { | |||||
| 67 | if cfg.Challenge == "" { | 67 | if cfg.Challenge == "" { | ||
| 68 | cfg.Challenge = domain.ACMEChallengeHTTP01 | 68 | cfg.Challenge = domain.ACMEChallengeHTTP01 | ||
| 69 | } | 69 | } | ||
| 70 | + | if cfg.Challenge == domain.ACMEChallengeHTTP01 && cfg.HTTPChallengeSink == nil { | |||
| 71 | + | return nil, errors.New("acmelego: HTTP-01 challenge requires a non-nil HTTPChallengeSink") | |||
| 72 | + | } | |||
| 70 | if cfg.Challenge == domain.ACMEChallengeCloudflareDNS01 && cfg.Token == "" { | 73 | if cfg.Challenge == domain.ACMEChallengeCloudflareDNS01 && cfg.Token == "" { | ||
| 71 | return nil, fmt.Errorf("acmelego: %w", domain.ErrCloudflareTokenMissing) | 74 | return nil, fmt.Errorf("acmelego: %w", domain.ErrCloudflareTokenMissing) | ||
| 72 | } | 75 | } | ||
| @@ -92,7 +95,7 @@ func (i *Issuer) Obtain(ctx context.Context, order out.CertificateOrder) (*out.S | |||||
| 92 | return nil, fmt.Errorf("obtain certificate: %w", err) | 95 | return nil, fmt.Errorf("obtain certificate: %w", err) | ||
| 93 | } | 96 | } | ||
| 94 | 97 | ||||
| 95 | - | return resourceToStored(order, resource), nil | 98 | + | return resourceToStored(order, resource) |
| 96 | } | 99 | } | ||
| 97 | 100 | ||||
| 98 | // Renew renews an existing certificate. The ACME client is initialized lazily | 101 | // Renew renews an existing certificate. The ACME client is initialized lazily | ||
| @@ -133,7 +136,7 @@ func (i *Issuer) Renew(ctx context.Context, cert out.StoredCertificate) (*out.St | |||||
| 133 | Challenge: cert.Challenge, | 136 | Challenge: cert.Challenge, | ||
| 134 | } | 137 | } | ||
| 135 | 138 | ||||
| 136 | - | return resourceToStored(order, renewed), nil | 139 | + | return resourceToStored(order, renewed) |
| 137 | } | 140 | } | ||
| 138 | 141 | ||||
| 139 | // ensureClient initializes the lego client and ACME account if not yet done. | 142 | // ensureClient initializes the lego client and ACME account if not yet done. | ||
| @@ -152,18 +155,7 @@ func (i *Issuer) ensureClient(ctx context.Context) error { | |||||
| 152 | } | 155 | } | ||
| 153 | i.user = user | 156 | i.user = user | ||
| 154 | 157 | ||||
| 155 | - | // Build lego config | 158 | + | client, err := lego.NewClient(i.newLegoConfig(user)) |
| 156 | - | legoCfg := lego.NewConfig(user) | |||
| 157 | - | legoCfg.Certificate.KeyType = certcrypto.EC256 // ECDSA P-256 | |||
| 158 | - | ||||
| 159 | - | if i.cfg.CADirectoryURL != "" { | |||
| 160 | - | legoCfg.CADirURL = i.cfg.CADirectoryURL | |||
| 161 | - | } | |||
| 162 | - | if i.cfg.HTTPClient != nil { | |||
| 163 | - | legoCfg.HTTPClient = i.cfg.HTTPClient | |||
| 164 | - | } | |||
| 165 | - | ||||
| 166 | - | client, err := lego.NewClient(legoCfg) | |||
| 167 | if err != nil { | 159 | if err != nil { | ||
| 168 | return fmt.Errorf("create lego client: %w", err) | 160 | return fmt.Errorf("create lego client: %w", err) | ||
| 169 | } | 161 | } | ||
| @@ -213,16 +205,7 @@ func (i *Issuer) loadOrCreateAccount(ctx context.Context) (*AccountUser, error) | |||||
| 213 | user := NewAccountUser(i.cfg.Email, privateKey, nil) | 205 | user := NewAccountUser(i.cfg.Email, privateKey, nil) | ||
| 214 | 206 | ||||
| 215 | // We need a temporary client to register the account | 207 | // We need a temporary client to register the account | ||
| 216 | - | legoCfg := lego.NewConfig(user) | 208 | + | tmpClient, err := lego.NewClient(i.newLegoConfig(user)) |
| 217 | - | legoCfg.Certificate.KeyType = certcrypto.EC256 | |||
| 218 | - | if i.cfg.CADirectoryURL != "" { | |||
| 219 | - | legoCfg.CADirURL = i.cfg.CADirectoryURL | |||
| 220 | - | } | |||
| 221 | - | if i.cfg.HTTPClient != nil { | |||
| 222 | - | legoCfg.HTTPClient = i.cfg.HTTPClient | |||
| 223 | - | } | |||
| 224 | - | ||||
| 225 | - | tmpClient, err := lego.NewClient(legoCfg) | |||
| 226 | if err != nil { | 209 | if err != nil { | ||
| 227 | return nil, fmt.Errorf("create temporary lego client: %w", err) | 210 | return nil, fmt.Errorf("create temporary lego client: %w", err) | ||
| 228 | } | 211 | } | ||
| @@ -278,8 +261,21 @@ func restoreAccount(stored *out.ACMEAccount) (*AccountUser, error) { | |||||
| 278 | return NewAccountUser(stored.Email, privateKey, reg), nil | 261 | return NewAccountUser(stored.Email, privateKey, reg), nil | ||
| 279 | } | 262 | } | ||
| 280 | 263 | ||||
| 264 | + | // newLegoConfig creates a lego.Config with common settings from the Issuer config. | |||
| 265 | + | func (i *Issuer) newLegoConfig(user *AccountUser) *lego.Config { | |||
| 266 | + | legoCfg := lego.NewConfig(user) | |||
| 267 | + | legoCfg.Certificate.KeyType = certcrypto.EC256 // ECDSA P-256 | |||
| 268 | + | if i.cfg.CADirectoryURL != "" { | |||
| 269 | + | legoCfg.CADirURL = i.cfg.CADirectoryURL | |||
| 270 | + | } | |||
| 271 | + | if i.cfg.HTTPClient != nil { | |||
| 272 | + | legoCfg.HTTPClient = i.cfg.HTTPClient | |||
| 273 | + | } | |||
| 274 | + | return legoCfg | |||
| 275 | + | } | |||
| 276 | + | ||||
| 281 | // resourceToStored converts a lego certificate.Resource to out.StoredCertificate. | 277 | // resourceToStored converts a lego certificate.Resource to out.StoredCertificate. | ||
| 282 | - | func resourceToStored(order out.CertificateOrder, res *certificate.Resource) *out.StoredCertificate { | 278 | + | func resourceToStored(order out.CertificateOrder, res *certificate.Resource) (*out.StoredCertificate, error) { |
| 283 | var fullchainPEM []byte | 279 | var fullchainPEM []byte | ||
| 284 | if len(res.IssuerCertificate) > 0 { | 280 | if len(res.IssuerCertificate) > 0 { | ||
| 285 | fullchainPEM = append(append([]byte{}, res.Certificate...), res.IssuerCertificate...) | 281 | fullchainPEM = append(append([]byte{}, res.Certificate...), res.IssuerCertificate...) | ||
| @@ -290,11 +286,17 @@ func resourceToStored(order out.CertificateOrder, res *certificate.Resource) *ou | |||||
| 290 | // Parse the certificate to extract NotAfter and populate tls.Certificate. | 286 | // Parse the certificate to extract NotAfter and populate tls.Certificate. | ||
| 291 | var tlsCert tls.Certificate | 287 | var tlsCert tls.Certificate | ||
| 292 | notAfter := time.Time{} | 288 | notAfter := time.Time{} | ||
| 293 | - | if parsedTLSCert, err := tls.X509KeyPair(fullchainPEM, res.PrivateKey); err == nil && len(parsedTLSCert.Certificate) > 0 { | 289 | + | parsedTLSCert, tlsErr := tls.X509KeyPair(fullchainPEM, res.PrivateKey) |
| 294 | - | tlsCert = parsedTLSCert | 290 | + | if tlsErr != nil { |
| 295 | - | if parsed, parseErr := x509.ParseCertificate(parsedTLSCert.Certificate[0]); parseErr == nil { | 291 | + | return nil, fmt.Errorf("parse tls certificate: %w", tlsErr) |
| 296 | - | notAfter = parsed.NotAfter | 292 | + | } |
| 293 | + | tlsCert = parsedTLSCert | |||
| 294 | + | if len(parsedTLSCert.Certificate) > 0 { | |||
| 295 | + | parsed, parseErr := x509.ParseCertificate(parsedTLSCert.Certificate[0]) | |||
| 296 | + | if parseErr != nil { | |||
| 297 | + | return nil, fmt.Errorf("parse x509 certificate: %w", parseErr) | |||
| 297 | } | 298 | } | ||
| 299 | + | notAfter = parsed.NotAfter | |||
| 298 | } | 300 | } | ||
| 299 | 301 | ||||
| 300 | names := order.Names | 302 | names := order.Names | ||
| @@ -312,5 +314,5 @@ func resourceToStored(order out.CertificateOrder, res *certificate.Resource) *ou | |||||
| 312 | FullchainPEM: fullchainPEM, | 314 | FullchainPEM: fullchainPEM, | ||
| 313 | PrivateKeyPEM: res.PrivateKey, | 315 | PrivateKeyPEM: res.PrivateKey, | ||
| 314 | NotAfter: notAfter, | 316 | NotAfter: notAfter, | ||
| 315 | - | } | 317 | + | }, nil |
| 316 | } | 318 | } | ||
M internal/adapters/out/acmelego/issuer_test.go +18 -7
| @@ -11,6 +11,7 @@ import ( | |||
| 11 | 11 | "crypto/x509/pkix" | |
| 12 | 12 | "encoding/json" | |
| 13 | 13 | "encoding/pem" | |
| 14 | + | "errors" | |
| 14 | 15 | "math/big" | |
| 15 | 16 | "testing" | |
| 16 | 17 | "time" | |
| @@ -49,14 +50,21 @@ func TestNewIssuerValidatesCloudflareToken(t *testing.T) { | |||
| 49 | 50 | ||
| 50 | 51 | func TestNewIssuerValidWithDefaults(t *testing.T) { | |
| 51 | 52 | issuer, err := NewIssuer(Config{ | |
| 52 | - | Email: "test@example.com", | |
| 53 | - | Store: &mockStore{}, | |
| 53 | + | Email: "test@example.com", | |
| 54 | + | Store: &mockStore{}, | |
| 55 | + | HTTPChallengeSink: &mockHTTPChallengeSink{}, | |
| 54 | 56 | }) | |
| 55 | 57 | require.NoError(t, err) | |
| 56 | 58 | require.NotNil(t, issuer) | |
| 57 | 59 | assert.Equal(t, domain.ACMEChallengeHTTP01, issuer.cfg.Challenge) | |
| 58 | 60 | } | |
| 59 | 61 | ||
| 62 | + | // mockHTTPChallengeSink implements HTTPChallengeSink for testing. | |
| 63 | + | type mockHTTPChallengeSink struct{} | |
| 64 | + | ||
| 65 | + | func (m *mockHTTPChallengeSink) Present(token, keyAuth string) {} | |
| 66 | + | func (m *mockHTTPChallengeSink) CleanUp(token string) {} | |
| 67 | + | ||
| 60 | 68 | // mockStore implements out.CertificateStore for testing. | |
| 61 | 69 | type mockStore struct { | |
| 62 | 70 | account *out.ACMEAccount | |
| @@ -151,7 +159,8 @@ func TestResourceConversionToStoredCertificate(t *testing.T) { | |||
| 151 | 159 | Challenge: domain.ACMEChallengeHTTP01, | |
| 152 | 160 | } | |
| 153 | 161 | ||
| 154 | - | stored := resourceToStored(order, resource) | |
| 162 | + | stored, err := resourceToStored(order, resource) | |
| 163 | + | require.NoError(t, err) | |
| 155 | 164 | require.NotNil(t, stored) | |
| 156 | 165 | assert.Equal(t, "test-cert", stored.ID) | |
| 157 | 166 | assert.Equal(t, []string{"example.com"}, stored.Names) | |
| @@ -187,7 +196,8 @@ func TestResourceConversionNoIssuer(t *testing.T) { | |||
| 187 | 196 | Names: []string{"example.com"}, | |
| 188 | 197 | } | |
| 189 | 198 | ||
| 190 | - | stored := resourceToStored(order, resource) | |
| 199 | + | stored, err := resourceToStored(order, resource) | |
| 200 | + | require.NoError(t, err) | |
| 191 | 201 | require.NotNil(t, stored) | |
| 192 | 202 | assert.Equal(t, certPEM, stored.FullchainPEM, "fullchain should equal cert when no issuer") | |
| 193 | 203 | assert.Empty(t, stored.ChainPEM) | |
| @@ -248,7 +258,7 @@ func generateSelfSignedCert(domain string) (certPEM, keyPEM []byte, err error) { | |||
| 248 | 258 | ||
| 249 | 259 | certPEM = pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}) | |
| 250 | 260 | if certPEM == nil { | |
| 251 | - | return nil, nil, err | |
| 261 | + | return nil, nil, errors.New("pem encode certificate returned nil") | |
| 252 | 262 | } | |
| 253 | 263 | ||
| 254 | 264 | keyDER, err := x509.MarshalECPrivateKey(privateKey) | |
| @@ -258,7 +268,7 @@ func generateSelfSignedCert(domain string) (certPEM, keyPEM []byte, err error) { | |||
| 258 | 268 | ||
| 259 | 269 | keyPEM = pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: keyDER}) | |
| 260 | 270 | if keyPEM == nil { | |
| 261 | - | return nil, nil, err | |
| 271 | + | return nil, nil, errors.New("pem encode private key returned nil") | |
| 262 | 272 | } | |
| 263 | 273 | ||
| 264 | 274 | return certPEM, keyPEM, nil | |
| @@ -280,7 +290,8 @@ func TestParsedCertificateNotAfter(t *testing.T) { | |||
| 280 | 290 | Names: []string{"test.example.com"}, | |
| 281 | 291 | } | |
| 282 | 292 | ||
| 283 | - | stored := resourceToStored(order, resource) | |
| 293 | + | stored, err := resourceToStored(order, resource) | |
| 294 | + | require.NoError(t, err) | |
| 284 | 295 | require.NotNil(t, stored) | |
| 285 | 296 | assert.False(t, stored.NotAfter.IsZero()) | |
| 286 | 297 | assert.True(t, stored.NotAfter.After(time.Now())) | |
| @@ -11,6 +11,7 @@ import ( | |||||
| 11 | "crypto/x509/pkix" | 11 | "crypto/x509/pkix" | ||
| 12 | "encoding/json" | 12 | "encoding/json" | ||
| 13 | "encoding/pem" | 13 | "encoding/pem" | ||
| 14 | + | "errors" | |||
| 14 | "math/big" | 15 | "math/big" | ||
| 15 | "testing" | 16 | "testing" | ||
| 16 | "time" | 17 | "time" | ||
| @@ -49,14 +50,21 @@ func TestNewIssuerValidatesCloudflareToken(t *testing.T) { | |||||
| 49 | 50 | ||||
| 50 | func TestNewIssuerValidWithDefaults(t *testing.T) { | 51 | func TestNewIssuerValidWithDefaults(t *testing.T) { | ||
| 51 | issuer, err := NewIssuer(Config{ | 52 | issuer, err := NewIssuer(Config{ | ||
| 52 | - | Email: "test@example.com", | 53 | + | Email: "test@example.com", |
| 53 | - | Store: &mockStore{}, | 54 | + | Store: &mockStore{}, |
| 55 | + | HTTPChallengeSink: &mockHTTPChallengeSink{}, | |||
| 54 | }) | 56 | }) | ||
| 55 | require.NoError(t, err) | 57 | require.NoError(t, err) | ||
| 56 | require.NotNil(t, issuer) | 58 | require.NotNil(t, issuer) | ||
| 57 | assert.Equal(t, domain.ACMEChallengeHTTP01, issuer.cfg.Challenge) | 59 | assert.Equal(t, domain.ACMEChallengeHTTP01, issuer.cfg.Challenge) | ||
| 58 | } | 60 | } | ||
| 59 | 61 | ||||
| 62 | + | // mockHTTPChallengeSink implements HTTPChallengeSink for testing. | |||
| 63 | + | type mockHTTPChallengeSink struct{} | |||
| 64 | + | ||||
| 65 | + | func (m *mockHTTPChallengeSink) Present(token, keyAuth string) {} | |||
| 66 | + | func (m *mockHTTPChallengeSink) CleanUp(token string) {} | |||
| 67 | + | ||||
| 60 | // mockStore implements out.CertificateStore for testing. | 68 | // mockStore implements out.CertificateStore for testing. | ||
| 61 | type mockStore struct { | 69 | type mockStore struct { | ||
| 62 | account *out.ACMEAccount | 70 | account *out.ACMEAccount | ||
| @@ -151,7 +159,8 @@ func TestResourceConversionToStoredCertificate(t *testing.T) { | |||||
| 151 | Challenge: domain.ACMEChallengeHTTP01, | 159 | Challenge: domain.ACMEChallengeHTTP01, | ||
| 152 | } | 160 | } | ||
| 153 | 161 | ||||
| 154 | - | stored := resourceToStored(order, resource) | 162 | + | stored, err := resourceToStored(order, resource) |
| 163 | + | require.NoError(t, err) | |||
| 155 | require.NotNil(t, stored) | 164 | require.NotNil(t, stored) | ||
| 156 | assert.Equal(t, "test-cert", stored.ID) | 165 | assert.Equal(t, "test-cert", stored.ID) | ||
| 157 | assert.Equal(t, []string{"example.com"}, stored.Names) | 166 | assert.Equal(t, []string{"example.com"}, stored.Names) | ||
| @@ -187,7 +196,8 @@ func TestResourceConversionNoIssuer(t *testing.T) { | |||||
| 187 | Names: []string{"example.com"}, | 196 | Names: []string{"example.com"}, | ||
| 188 | } | 197 | } | ||
| 189 | 198 | ||||
| 190 | - | stored := resourceToStored(order, resource) | 199 | + | stored, err := resourceToStored(order, resource) |
| 200 | + | require.NoError(t, err) | |||
| 191 | require.NotNil(t, stored) | 201 | require.NotNil(t, stored) | ||
| 192 | assert.Equal(t, certPEM, stored.FullchainPEM, "fullchain should equal cert when no issuer") | 202 | assert.Equal(t, certPEM, stored.FullchainPEM, "fullchain should equal cert when no issuer") | ||
| 193 | assert.Empty(t, stored.ChainPEM) | 203 | assert.Empty(t, stored.ChainPEM) | ||
| @@ -248,7 +258,7 @@ func generateSelfSignedCert(domain string) (certPEM, keyPEM []byte, err error) { | |||||
| 248 | 258 | ||||
| 249 | certPEM = pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}) | 259 | certPEM = pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}) | ||
| 250 | if certPEM == nil { | 260 | if certPEM == nil { | ||
| 251 | - | return nil, nil, err | 261 | + | return nil, nil, errors.New("pem encode certificate returned nil") |
| 252 | } | 262 | } | ||
| 253 | 263 | ||||
| 254 | keyDER, err := x509.MarshalECPrivateKey(privateKey) | 264 | keyDER, err := x509.MarshalECPrivateKey(privateKey) | ||
| @@ -258,7 +268,7 @@ func generateSelfSignedCert(domain string) (certPEM, keyPEM []byte, err error) { | |||||
| 258 | 268 | ||||
| 259 | keyPEM = pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: keyDER}) | 269 | keyPEM = pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: keyDER}) | ||
| 260 | if keyPEM == nil { | 270 | if keyPEM == nil { | ||
| 261 | - | return nil, nil, err | 271 | + | return nil, nil, errors.New("pem encode private key returned nil") |
| 262 | } | 272 | } | ||
| 263 | 273 | ||||
| 264 | return certPEM, keyPEM, nil | 274 | return certPEM, keyPEM, nil | ||
| @@ -280,7 +290,8 @@ func TestParsedCertificateNotAfter(t *testing.T) { | |||||
| 280 | Names: []string{"test.example.com"}, | 290 | Names: []string{"test.example.com"}, | ||
| 281 | } | 291 | } | ||
| 282 | 292 | ||||
| 283 | - | stored := resourceToStored(order, resource) | 293 | + | stored, err := resourceToStored(order, resource) |
| 294 | + | require.NoError(t, err) | |||
| 284 | require.NotNil(t, stored) | 295 | require.NotNil(t, stored) | ||
| 285 | assert.False(t, stored.NotAfter.IsZero()) | 296 | assert.False(t, stored.NotAfter.IsZero()) | ||
| 286 | assert.True(t, stored.NotAfter.After(time.Now())) | 297 | assert.True(t, stored.NotAfter.After(time.Now())) | ||
M internal/adapters/out/secrets/public_tls_resolver.go +37 -2
| @@ -6,6 +6,7 @@ import ( | |||
| 6 | 6 | "fmt" | |
| 7 | 7 | "os" | |
| 8 | 8 | "strings" | |
| 9 | + | "time" | |
| 9 | 10 | ||
| 10 | 11 | "github.com/bnema/zerowrap" | |
| 11 | 12 | ||
| @@ -15,6 +16,10 @@ import ( | |||
| 15 | 16 | ||
| 16 | 17 | const defaultCloudflareTokenPassName = "gordon/cloudflare/api-token" | |
| 17 | 18 | ||
| 19 | + | // Package-level disabled logger and pass provider reused across calls. | |
| 20 | + | var disabledLogger = zerowrap.New(zerowrap.Config{Level: "disabled"}) | |
| 21 | + | var defaultPassProvider = NewPassProvider(disabledLogger) | |
| 22 | + | ||
| 18 | 23 | // PublicTLSResolverConfig holds optional overrides for PublicTLSResolver. | |
| 19 | 24 | type PublicTLSResolverConfig struct { | |
| 20 | 25 | PassLookup func(ctx context.Context, name string) (string, error) | |
| @@ -62,7 +67,7 @@ func (r *PublicTLSResolver) ResolveCloudflareToken(ctx context.Context) (out.Sec | |||
| 62 | 67 | ||
| 63 | 68 | // 2. Try token file env | |
| 64 | 69 | if filePath := r.envLookup("GORDON_CLOUDFLARE_API_TOKEN_FILE"); filePath != "" { | |
| 65 | - | if data, err := os.ReadFile(filePath); err == nil { | |
| 70 | + | if data, err := readFileWithTimeout(ctx, filePath, 5*time.Second); err == nil { | |
| 66 | 71 | if trimmed := strings.TrimSpace(string(data)); trimmed != "" { | |
| 67 | 72 | return out.SecretValue{Value: trimmed, Source: domain.ACMETokenSourceFile}, nil | |
| 68 | 73 | } | |
| @@ -96,5 +101,35 @@ func defaultPassLookup(ctx context.Context, name string) (string, error) { | |||
| 96 | 101 | return "", fmt.Errorf("pass lookup: disallowed name %q", name) | |
| 97 | 102 | } | |
| 98 | 103 | ||
| 99 | - | return NewPassProvider(zerowrap.New(zerowrap.Config{Level: "disabled"})).GetSecret(ctx, name) | |
| 104 | + | return defaultPassProvider.GetSecret(ctx, name) | |
| 105 | + | } | |
| 106 | + | ||
| 107 | + | // readFileWithTimeout reads a file with a context-aware deadline. | |
| 108 | + | // It runs os.ReadFile in a goroutine and selects on context expiry. | |
| 109 | + | // If the context has a deadline, it is used; otherwise a timeout is applied. | |
| 110 | + | func readFileWithTimeout(ctx context.Context, path string, timeout time.Duration) ([]byte, error) { | |
| 111 | + | readCtx := ctx | |
| 112 | + | if _, hasDeadline := ctx.Deadline(); !hasDeadline { | |
| 113 | + | var cancel context.CancelFunc | |
| 114 | + | readCtx, cancel = context.WithTimeout(ctx, timeout) | |
| 115 | + | defer cancel() | |
| 116 | + | } | |
| 117 | + | ||
| 118 | + | type result struct { | |
| 119 | + | data []byte | |
| 120 | + | err error | |
| 121 | + | } | |
| 122 | + | ||
| 123 | + | ch := make(chan result, 1) | |
| 124 | + | go func() { | |
| 125 | + | data, err := os.ReadFile(path) | |
| 126 | + | ch <- result{data: data, err: err} | |
| 127 | + | }() | |
| 128 | + | ||
| 129 | + | select { | |
| 130 | + | case <-readCtx.Done(): | |
| 131 | + | return nil, readCtx.Err() | |
| 132 | + | case r := <-ch: | |
| 133 | + | return r.data, r.err | |
| 134 | + | } | |
| 100 | 135 | } | |
| @@ -6,6 +6,7 @@ import ( | |||||
| 6 | "fmt" | 6 | "fmt" | ||
| 7 | "os" | 7 | "os" | ||
| 8 | "strings" | 8 | "strings" | ||
| 9 | + | "time" | |||
| 9 | 10 | ||||
| 10 | "github.com/bnema/zerowrap" | 11 | "github.com/bnema/zerowrap" | ||
| 11 | 12 | ||||
| @@ -15,6 +16,10 @@ import ( | |||||
| 15 | 16 | ||||
| 16 | const defaultCloudflareTokenPassName = "gordon/cloudflare/api-token" | 17 | const defaultCloudflareTokenPassName = "gordon/cloudflare/api-token" | ||
| 17 | 18 | ||||
| 19 | + | // Package-level disabled logger and pass provider reused across calls. | |||
| 20 | + | var disabledLogger = zerowrap.New(zerowrap.Config{Level: "disabled"}) | |||
| 21 | + | var defaultPassProvider = NewPassProvider(disabledLogger) | |||
| 22 | + | ||||
| 18 | // PublicTLSResolverConfig holds optional overrides for PublicTLSResolver. | 23 | // PublicTLSResolverConfig holds optional overrides for PublicTLSResolver. | ||
| 19 | type PublicTLSResolverConfig struct { | 24 | type PublicTLSResolverConfig struct { | ||
| 20 | PassLookup func(ctx context.Context, name string) (string, error) | 25 | PassLookup func(ctx context.Context, name string) (string, error) | ||
| @@ -62,7 +67,7 @@ func (r *PublicTLSResolver) ResolveCloudflareToken(ctx context.Context) (out.Sec | |||||
| 62 | 67 | ||||
| 63 | // 2. Try token file env | 68 | // 2. Try token file env | ||
| 64 | if filePath := r.envLookup("GORDON_CLOUDFLARE_API_TOKEN_FILE"); filePath != "" { | 69 | if filePath := r.envLookup("GORDON_CLOUDFLARE_API_TOKEN_FILE"); filePath != "" { | ||
| 65 | - | if data, err := os.ReadFile(filePath); err == nil { | 70 | + | if data, err := readFileWithTimeout(ctx, filePath, 5*time.Second); err == nil { |
| 66 | if trimmed := strings.TrimSpace(string(data)); trimmed != "" { | 71 | if trimmed := strings.TrimSpace(string(data)); trimmed != "" { | ||
| 67 | return out.SecretValue{Value: trimmed, Source: domain.ACMETokenSourceFile}, nil | 72 | return out.SecretValue{Value: trimmed, Source: domain.ACMETokenSourceFile}, nil | ||
| 68 | } | 73 | } | ||
| @@ -96,5 +101,35 @@ func defaultPassLookup(ctx context.Context, name string) (string, error) { | |||||
| 96 | return "", fmt.Errorf("pass lookup: disallowed name %q", name) | 101 | return "", fmt.Errorf("pass lookup: disallowed name %q", name) | ||
| 97 | } | 102 | } | ||
| 98 | 103 | ||||
| 99 | - | return NewPassProvider(zerowrap.New(zerowrap.Config{Level: "disabled"})).GetSecret(ctx, name) | 104 | + | return defaultPassProvider.GetSecret(ctx, name) |
| 105 | + | } | |||
| 106 | + | ||||
| 107 | + | // readFileWithTimeout reads a file with a context-aware deadline. | |||
| 108 | + | // It runs os.ReadFile in a goroutine and selects on context expiry. | |||
| 109 | + | // If the context has a deadline, it is used; otherwise a timeout is applied. | |||
| 110 | + | func readFileWithTimeout(ctx context.Context, path string, timeout time.Duration) ([]byte, error) { | |||
| 111 | + | readCtx := ctx | |||
| 112 | + | if _, hasDeadline := ctx.Deadline(); !hasDeadline { | |||
| 113 | + | var cancel context.CancelFunc | |||
| 114 | + | readCtx, cancel = context.WithTimeout(ctx, timeout) | |||
| 115 | + | defer cancel() | |||
| 116 | + | } | |||
| 117 | + | ||||
| 118 | + | type result struct { | |||
| 119 | + | data []byte | |||
| 120 | + | err error | |||
| 121 | + | } | |||
| 122 | + | ||||
| 123 | + | ch := make(chan result, 1) | |||
| 124 | + | go func() { | |||
| 125 | + | data, err := os.ReadFile(path) | |||
| 126 | + | ch <- result{data: data, err: err} | |||
| 127 | + | }() | |||
| 128 | + | ||||
| 129 | + | select { | |||
| 130 | + | case <-readCtx.Done(): | |||
| 131 | + | return nil, readCtx.Err() | |||
| 132 | + | case r := <-ch: | |||
| 133 | + | return r.data, r.err | |||
| 134 | + | } | |||
| 100 | } | 135 | } | ||
M internal/app/run.go +3 -5
| @@ -43,9 +43,6 @@ import ( | |||
| 43 | 43 | "github.com/bnema/gordon/internal/adapters/out/telemetry" | |
| 44 | 44 | "github.com/bnema/gordon/internal/adapters/out/tokenstore" | |
| 45 | 45 | ||
| 46 | - | // Boundaries | |
| 47 | - | "github.com/bnema/gordon/internal/boundaries/in" | |
| 48 | - | ||
| 49 | 46 | // OTel | |
| 50 | 47 | "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" | |
| 51 | 48 | ||
| @@ -61,6 +58,7 @@ import ( | |||
| 61 | 58 | "github.com/bnema/gordon/internal/adapters/in/http/registry" | |
| 62 | 59 | ||
| 63 | 60 | // Boundaries | |
| 61 | + | "github.com/bnema/gordon/internal/boundaries/in" | |
| 64 | 62 | "github.com/bnema/gordon/internal/boundaries/out" | |
| 65 | 63 | ||
| 66 | 64 | // Domain | |
| @@ -1681,10 +1679,10 @@ func (c *reloadCoordinator) applyLoadedConfig(ctx context.Context, now time.Time | |||
| 1681 | 1679 | c.lastRun = now | |
| 1682 | 1680 | ||
| 1683 | 1681 | // Reconcile public TLS certificates after config reload. | |
| 1682 | + | // Log and continue on failure — a transient ACME issue should not abort the entire reload. | |
| 1684 | 1683 | if c.publicTLS != nil { | |
| 1685 | 1684 | if err := c.publicTLS.Reconcile(ctx); err != nil { | |
| 1686 | - | c.log.Error().Err(err).Msg("failed to reconcile public TLS certificates after reload") | |
| 1687 | - | return fmt.Errorf("reconcile public TLS: %w", err) | |
| 1685 | + | c.log.Warn().Err(err).Msg("failed to reconcile public TLS certificates after reload, continuing") | |
| 1688 | 1686 | } | |
| 1689 | 1687 | } | |
| 1690 | 1688 | ||
| @@ -43,9 +43,6 @@ import ( | |||||
| 43 | "github.com/bnema/gordon/internal/adapters/out/telemetry" | 43 | "github.com/bnema/gordon/internal/adapters/out/telemetry" | ||
| 44 | "github.com/bnema/gordon/internal/adapters/out/tokenstore" | 44 | "github.com/bnema/gordon/internal/adapters/out/tokenstore" | ||
| 45 | 45 | ||||
| 46 | - | // Boundaries | |||
| 47 | - | "github.com/bnema/gordon/internal/boundaries/in" | |||
| 48 | - | ||||
| 49 | // OTel | 46 | // OTel | ||
| 50 | "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" | 47 | "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" | ||
| 51 | 48 | ||||
| @@ -61,6 +58,7 @@ import ( | |||||
| 61 | "github.com/bnema/gordon/internal/adapters/in/http/registry" | 58 | "github.com/bnema/gordon/internal/adapters/in/http/registry" | ||
| 62 | 59 | ||||
| 63 | // Boundaries | 60 | // Boundaries | ||
| 61 | + | "github.com/bnema/gordon/internal/boundaries/in" | |||
| 64 | "github.com/bnema/gordon/internal/boundaries/out" | 62 | "github.com/bnema/gordon/internal/boundaries/out" | ||
| 65 | 63 | ||||
| 66 | // Domain | 64 | // Domain | ||
| @@ -1681,10 +1679,10 @@ func (c *reloadCoordinator) applyLoadedConfig(ctx context.Context, now time.Time | |||||
| 1681 | c.lastRun = now | 1679 | c.lastRun = now | ||
| 1682 | 1680 | ||||
| 1683 | // Reconcile public TLS certificates after config reload. | 1681 | // Reconcile public TLS certificates after config reload. | ||
| 1682 | + | // Log and continue on failure — a transient ACME issue should not abort the entire reload. | |||
| 1684 | if c.publicTLS != nil { | 1683 | if c.publicTLS != nil { | ||
| 1685 | if err := c.publicTLS.Reconcile(ctx); err != nil { | 1684 | if err := c.publicTLS.Reconcile(ctx); err != nil { | ||
| 1686 | - | c.log.Error().Err(err).Msg("failed to reconcile public TLS certificates after reload") | 1685 | + | c.log.Warn().Err(err).Msg("failed to reconcile public TLS certificates after reload, continuing") |
| 1687 | - | return fmt.Errorf("reconcile public TLS: %w", err) | |||
| 1688 | } | 1686 | } | ||
| 1689 | } | 1687 | } | ||
| 1690 | 1688 | ||||
D internal/boundaries/in/public_tls_test.go +0 -37
| @@ -1,37 +0,0 @@ | |||
| 1 | - | package in | |
| 2 | - | ||
| 3 | - | import ( | |
| 4 | - | "context" | |
| 5 | - | "crypto/tls" | |
| 6 | - | "testing" | |
| 7 | - | ||
| 8 | - | "github.com/bnema/gordon/internal/domain" | |
| 9 | - | ) | |
| 10 | - | ||
| 11 | - | // publicTLSStub is a compile-time stub for PublicTLSService. | |
| 12 | - | type publicTLSStub struct{} | |
| 13 | - | ||
| 14 | - | func (s publicTLSStub) GetCertificate(hello *tls.ClientHelloInfo) (*tls.Certificate, error) { | |
| 15 | - | return nil, nil | |
| 16 | - | } | |
| 17 | - | ||
| 18 | - | func (s publicTLSStub) GetHTTP01Challenge(ctx context.Context, token string) (string, bool) { | |
| 19 | - | return "", false | |
| 20 | - | } | |
| 21 | - | ||
| 22 | - | func (s publicTLSStub) Status(ctx context.Context) domain.PublicTLSStatus { | |
| 23 | - | return domain.PublicTLSStatus{} | |
| 24 | - | } | |
| 25 | - | ||
| 26 | - | func (s publicTLSStub) Reconcile(ctx context.Context) error { | |
| 27 | - | return nil | |
| 28 | - | } | |
| 29 | - | ||
| 30 | - | func (s publicTLSStub) Stop(ctx context.Context) error { | |
| 31 | - | return nil | |
| 32 | - | } | |
| 33 | - | ||
| 34 | - | // TestPublicTLSServiceInterface asserts that publicTLSStub satisfies PublicTLSService. | |
| 35 | - | func TestPublicTLSServiceInterface(t *testing.T) { | |
| 36 | - | var _ PublicTLSService = publicTLSStub{} | |
| 37 | - | } | |
| @@ -1,37 +0,0 @@ | |||||
| 1 | - | package in | |||
| 2 | - | ||||
| 3 | - | import ( | |||
| 4 | - | "context" | |||
| 5 | - | "crypto/tls" | |||
| 6 | - | "testing" | |||
| 7 | - | ||||
| 8 | - | "github.com/bnema/gordon/internal/domain" | |||
| 9 | - | ) | |||
| 10 | - | ||||
| 11 | - | // publicTLSStub is a compile-time stub for PublicTLSService. | |||
| 12 | - | type publicTLSStub struct{} | |||
| 13 | - | ||||
| 14 | - | func (s publicTLSStub) GetCertificate(hello *tls.ClientHelloInfo) (*tls.Certificate, error) { | |||
| 15 | - | return nil, nil | |||
| 16 | - | } | |||
| 17 | - | ||||
| 18 | - | func (s publicTLSStub) GetHTTP01Challenge(ctx context.Context, token string) (string, bool) { | |||
| 19 | - | return "", false | |||
| 20 | - | } | |||
| 21 | - | ||||
| 22 | - | func (s publicTLSStub) Status(ctx context.Context) domain.PublicTLSStatus { | |||
| 23 | - | return domain.PublicTLSStatus{} | |||
| 24 | - | } | |||
| 25 | - | ||||
| 26 | - | func (s publicTLSStub) Reconcile(ctx context.Context) error { | |||
| 27 | - | return nil | |||
| 28 | - | } | |||
| 29 | - | ||||
| 30 | - | func (s publicTLSStub) Stop(ctx context.Context) error { | |||
| 31 | - | return nil | |||
| 32 | - | } | |||
| 33 | - | ||||
| 34 | - | // TestPublicTLSServiceInterface asserts that publicTLSStub satisfies PublicTLSService. | |||
| 35 | - | func TestPublicTLSServiceInterface(t *testing.T) { | |||
| 36 | - | var _ PublicTLSService = publicTLSStub{} | |||
| 37 | - | } | |||
D internal/boundaries/out/public_tls_test.go +0 -72
| @@ -1,72 +0,0 @@ | |||
| 1 | - | package out | |
| 2 | - | ||
| 3 | - | import ( | |
| 4 | - | "context" | |
| 5 | - | "crypto/tls" | |
| 6 | - | "testing" | |
| 7 | - | "time" | |
| 8 | - | ||
| 9 | - | "github.com/bnema/gordon/internal/domain" | |
| 10 | - | ) | |
| 11 | - | ||
| 12 | - | // issuerStub implements PublicCertificateIssuer. | |
| 13 | - | type issuerStub struct{} | |
| 14 | - | ||
| 15 | - | func (s issuerStub) Obtain(ctx context.Context, order CertificateOrder) (*StoredCertificate, error) { | |
| 16 | - | return nil, nil | |
| 17 | - | } | |
| 18 | - | ||
| 19 | - | func (s issuerStub) Renew(ctx context.Context, cert StoredCertificate) (*StoredCertificate, error) { | |
| 20 | - | return nil, nil | |
| 21 | - | } | |
| 22 | - | ||
| 23 | - | // storeStub implements CertificateStore. | |
| 24 | - | type storeStub struct{} | |
| 25 | - | ||
| 26 | - | func (s storeStub) LoadAccount(ctx context.Context) (*ACMEAccount, error) { | |
| 27 | - | return nil, nil | |
| 28 | - | } | |
| 29 | - | ||
| 30 | - | func (s storeStub) SaveAccount(ctx context.Context, account ACMEAccount) error { | |
| 31 | - | return nil | |
| 32 | - | } | |
| 33 | - | ||
| 34 | - | func (s storeStub) LoadAll(ctx context.Context) ([]StoredCertificate, error) { | |
| 35 | - | return nil, nil | |
| 36 | - | } | |
| 37 | - | ||
| 38 | - | func (s storeStub) Save(ctx context.Context, cert StoredCertificate) error { | |
| 39 | - | return nil | |
| 40 | - | } | |
| 41 | - | ||
| 42 | - | func (s storeStub) Lock(ctx context.Context) (func() error, error) { | |
| 43 | - | return func() error { return nil }, nil | |
| 44 | - | } | |
| 45 | - | ||
| 46 | - | // secretStub implements SecretResolver. | |
| 47 | - | type secretStub struct{} | |
| 48 | - | ||
| 49 | - | func (s secretStub) ResolveCloudflareToken(ctx context.Context) (SecretValue, error) { | |
| 50 | - | return SecretValue{}, nil | |
| 51 | - | } | |
| 52 | - | ||
| 53 | - | // zoneStub implements CloudflareZoneResolver. | |
| 54 | - | type zoneStub struct{} | |
| 55 | - | ||
| 56 | - | func (s zoneStub) FindZone(ctx context.Context, domainName string) (CloudflareZone, error) { | |
| 57 | - | return CloudflareZone{}, nil | |
| 58 | - | } | |
| 59 | - | ||
| 60 | - | // TestPublicTLSOutputInterfaces asserts that all stubs satisfy their respective interfaces | |
| 61 | - | // and that the required types are importable. | |
| 62 | - | func TestPublicTLSOutputInterfaces(t *testing.T) { | |
| 63 | - | var _ PublicCertificateIssuer = issuerStub{} | |
| 64 | - | var _ CertificateStore = storeStub{} | |
| 65 | - | var _ SecretResolver = secretStub{} | |
| 66 | - | var _ CloudflareZoneResolver = zoneStub{} | |
| 67 | - | ||
| 68 | - | // Verify that referenced types are importable | |
| 69 | - | var _ tls.Certificate | |
| 70 | - | var _ time.Time | |
| 71 | - | _ = domain.ACMEChallengeHTTP01 | |
| 72 | - | } | |
| @@ -1,72 +0,0 @@ | |||||
| 1 | - | package out | |||
| 2 | - | ||||
| 3 | - | import ( | |||
| 4 | - | "context" | |||
| 5 | - | "crypto/tls" | |||
| 6 | - | "testing" | |||
| 7 | - | "time" | |||
| 8 | - | ||||
| 9 | - | "github.com/bnema/gordon/internal/domain" | |||
| 10 | - | ) | |||
| 11 | - | ||||
| 12 | - | // issuerStub implements PublicCertificateIssuer. | |||
| 13 | - | type issuerStub struct{} | |||
| 14 | - | ||||
| 15 | - | func (s issuerStub) Obtain(ctx context.Context, order CertificateOrder) (*StoredCertificate, error) { | |||
| 16 | - | return nil, nil | |||
| 17 | - | } | |||
| 18 | - | ||||
| 19 | - | func (s issuerStub) Renew(ctx context.Context, cert StoredCertificate) (*StoredCertificate, error) { | |||
| 20 | - | return nil, nil | |||
| 21 | - | } | |||
| 22 | - | ||||
| 23 | - | // storeStub implements CertificateStore. | |||
| 24 | - | type storeStub struct{} | |||
| 25 | - | ||||
| 26 | - | func (s storeStub) LoadAccount(ctx context.Context) (*ACMEAccount, error) { | |||
| 27 | - | return nil, nil | |||
| 28 | - | } | |||
| 29 | - | ||||
| 30 | - | func (s storeStub) SaveAccount(ctx context.Context, account ACMEAccount) error { | |||
| 31 | - | return nil | |||
| 32 | - | } | |||
| 33 | - | ||||
| 34 | - | func (s storeStub) LoadAll(ctx context.Context) ([]StoredCertificate, error) { | |||
| 35 | - | return nil, nil | |||
| 36 | - | } | |||
| 37 | - | ||||
| 38 | - | func (s storeStub) Save(ctx context.Context, cert StoredCertificate) error { | |||
| 39 | - | return nil | |||
| 40 | - | } | |||
| 41 | - | ||||
| 42 | - | func (s storeStub) Lock(ctx context.Context) (func() error, error) { | |||
| 43 | - | return func() error { return nil }, nil | |||
| 44 | - | } | |||
| 45 | - | ||||
| 46 | - | // secretStub implements SecretResolver. | |||
| 47 | - | type secretStub struct{} | |||
| 48 | - | ||||
| 49 | - | func (s secretStub) ResolveCloudflareToken(ctx context.Context) (SecretValue, error) { | |||
| 50 | - | return SecretValue{}, nil | |||
| 51 | - | } | |||
| 52 | - | ||||
| 53 | - | // zoneStub implements CloudflareZoneResolver. | |||
| 54 | - | type zoneStub struct{} | |||
| 55 | - | ||||
| 56 | - | func (s zoneStub) FindZone(ctx context.Context, domainName string) (CloudflareZone, error) { | |||
| 57 | - | return CloudflareZone{}, nil | |||
| 58 | - | } | |||
| 59 | - | ||||
| 60 | - | // TestPublicTLSOutputInterfaces asserts that all stubs satisfy their respective interfaces | |||
| 61 | - | // and that the required types are importable. | |||
| 62 | - | func TestPublicTLSOutputInterfaces(t *testing.T) { | |||
| 63 | - | var _ PublicCertificateIssuer = issuerStub{} | |||
| 64 | - | var _ CertificateStore = storeStub{} | |||
| 65 | - | var _ SecretResolver = secretStub{} | |||
| 66 | - | var _ CloudflareZoneResolver = zoneStub{} | |||
| 67 | - | ||||
| 68 | - | // Verify that referenced types are importable | |||
| 69 | - | var _ tls.Certificate | |||
| 70 | - | var _ time.Time | |||
| 71 | - | _ = domain.ACMEChallengeHTTP01 | |||
| 72 | - | } | |||
M internal/domain/public_tls.go +1 -1
| @@ -71,7 +71,7 @@ func (c ManagedCertificate) Covers(host string) bool { | |||
| 71 | 71 | ||
| 72 | 72 | // SafePathComponent reports whether value is a single safe path component. | |
| 73 | 73 | func SafePathComponent(value string) bool { | |
| 74 | - | return value != "" && !strings.Contains(value, "/") && !strings.Contains(value, "\\") && !strings.Contains(value, "..") | |
| 74 | + | return value != "" && !strings.Contains(value, "/") && !strings.Contains(value, "\\") && !strings.Contains(value, "..") && !strings.Contains(value, "\x00") | |
| 75 | 75 | } | |
| 76 | 76 | ||
| 77 | 77 | // CertificateNamesCoverHost reports whether any certificate name covers host. | |
| @@ -71,7 +71,7 @@ func (c ManagedCertificate) Covers(host string) bool { | |||||
| 71 | 71 | ||||
| 72 | // SafePathComponent reports whether value is a single safe path component. | 72 | // SafePathComponent reports whether value is a single safe path component. | ||
| 73 | func SafePathComponent(value string) bool { | 73 | func SafePathComponent(value string) bool { | ||
| 74 | - | return value != "" && !strings.Contains(value, "/") && !strings.Contains(value, "\\") && !strings.Contains(value, "..") | 74 | + | return value != "" && !strings.Contains(value, "/") && !strings.Contains(value, "\\") && !strings.Contains(value, "..") && !strings.Contains(value, "\x00") |
| 75 | } | 75 | } | ||
| 76 | 76 | ||||
| 77 | // CertificateNamesCoverHost reports whether any certificate name covers host. | 77 | // CertificateNamesCoverHost reports whether any certificate name covers host. | ||
M internal/domain/public_tls_test.go +63 -4
| @@ -44,9 +44,68 @@ func TestManagedCertificateCovers(t *testing.T) { | |||
| 44 | 44 | } | |
| 45 | 45 | ||
| 46 | 46 | func TestManagedCertificateHealth(t *testing.T) { | |
| 47 | - | valid := ManagedCertificate{NotAfter: time.Now().Add(60 * 24 * time.Hour)} | |
| 48 | - | expired := ManagedCertificate{NotAfter: time.Now().Add(-time.Hour)} | |
| 47 | + | // Fixed reference time for deterministic tests. | |
| 48 | + | now := time.Date(2026, 4, 29, 12, 0, 0, 0, time.UTC) | |
| 49 | 49 | ||
| 50 | - | assert.Equal(t, TLSCertificateStatusValid, valid.Health(time.Now())) | |
| 51 | - | assert.Equal(t, TLSCertificateStatusExpired, expired.Health(time.Now())) | |
| 50 | + | tests := []struct { | |
| 51 | + | name string | |
| 52 | + | cert ManagedCertificate | |
| 53 | + | want TLSCertificateStatus | |
| 54 | + | }{ | |
| 55 | + | { | |
| 56 | + | name: "valid certificate far from expiry", | |
| 57 | + | cert: ManagedCertificate{NotAfter: now.Add(60 * 24 * time.Hour)}, | |
| 58 | + | want: TLSCertificateStatusValid, | |
| 59 | + | }, | |
| 60 | + | { | |
| 61 | + | name: "expired certificate", | |
| 62 | + | cert: ManagedCertificate{NotAfter: now.Add(-time.Hour)}, | |
| 63 | + | want: TLSCertificateStatusExpired, | |
| 64 | + | }, | |
| 65 | + | { | |
| 66 | + | name: "warning when less than 30 days remain", | |
| 67 | + | cert: ManagedCertificate{NotAfter: now.Add(15 * 24 * time.Hour)}, | |
| 68 | + | want: TLSCertificateStatusWarning, | |
| 69 | + | }, | |
| 70 | + | { | |
| 71 | + | name: "warning exactly at 30 day boundary in the past", | |
| 72 | + | cert: ManagedCertificate{NotAfter: now.Add(29 * 24 * time.Hour)}, | |
| 73 | + | want: TLSCertificateStatusWarning, | |
| 74 | + | }, | |
| 75 | + | { | |
| 76 | + | name: "missing when NotAfter is zero", | |
| 77 | + | cert: ManagedCertificate{}, | |
| 78 | + | want: TLSCertificateStatusMissing, | |
| 79 | + | }, | |
| 80 | + | { | |
| 81 | + | name: "error when LastError is set", | |
| 82 | + | cert: ManagedCertificate{ | |
| 83 | + | NotAfter: now.Add(60 * 24 * time.Hour), | |
| 84 | + | LastError: "issuer unavailable", | |
| 85 | + | }, | |
| 86 | + | want: TLSCertificateStatusError, | |
| 87 | + | }, | |
| 88 | + | { | |
| 89 | + | name: "error takes priority over expired", | |
| 90 | + | cert: ManagedCertificate{ | |
| 91 | + | NotAfter: now.Add(-time.Hour), | |
| 92 | + | LastError: "something went wrong", | |
| 93 | + | }, | |
| 94 | + | want: TLSCertificateStatusError, | |
| 95 | + | }, | |
| 96 | + | { | |
| 97 | + | name: "error takes priority over zero NotAfter", | |
| 98 | + | cert: ManagedCertificate{ | |
| 99 | + | LastError: "something went wrong", | |
| 100 | + | }, | |
| 101 | + | want: TLSCertificateStatusError, | |
| 102 | + | }, | |
| 103 | + | } | |
| 104 | + | ||
| 105 | + | for _, tt := range tests { | |
| 106 | + | t.Run(tt.name, func(t *testing.T) { | |
| 107 | + | got := tt.cert.Health(now) | |
| 108 | + | assert.Equal(t, tt.want, got) | |
| 109 | + | }) | |
| 110 | + | } | |
| 52 | 111 | } | |
| @@ -44,9 +44,68 @@ func TestManagedCertificateCovers(t *testing.T) { | |||||
| 44 | } | 44 | } | ||
| 45 | 45 | ||||
| 46 | func TestManagedCertificateHealth(t *testing.T) { | 46 | func TestManagedCertificateHealth(t *testing.T) { | ||
| 47 | - | valid := ManagedCertificate{NotAfter: time.Now().Add(60 * 24 * time.Hour)} | 47 | + | // Fixed reference time for deterministic tests. |
| 48 | - | expired := ManagedCertificate{NotAfter: time.Now().Add(-time.Hour)} | 48 | + | now := time.Date(2026, 4, 29, 12, 0, 0, 0, time.UTC) |
| 49 | 49 | ||||
| 50 | - | assert.Equal(t, TLSCertificateStatusValid, valid.Health(time.Now())) | 50 | + | tests := []struct { |
| 51 | - | assert.Equal(t, TLSCertificateStatusExpired, expired.Health(time.Now())) | 51 | + | name string |
| 52 | + | cert ManagedCertificate | |||
| 53 | + | want TLSCertificateStatus | |||
| 54 | + | }{ | |||
| 55 | + | { | |||
| 56 | + | name: "valid certificate far from expiry", | |||
| 57 | + | cert: ManagedCertificate{NotAfter: now.Add(60 * 24 * time.Hour)}, | |||
| 58 | + | want: TLSCertificateStatusValid, | |||
| 59 | + | }, | |||
| 60 | + | { | |||
| 61 | + | name: "expired certificate", | |||
| 62 | + | cert: ManagedCertificate{NotAfter: now.Add(-time.Hour)}, | |||
| 63 | + | want: TLSCertificateStatusExpired, | |||
| 64 | + | }, | |||
| 65 | + | { | |||
| 66 | + | name: "warning when less than 30 days remain", | |||
| 67 | + | cert: ManagedCertificate{NotAfter: now.Add(15 * 24 * time.Hour)}, | |||
| 68 | + | want: TLSCertificateStatusWarning, | |||
| 69 | + | }, | |||
| 70 | + | { | |||
| 71 | + | name: "warning exactly at 30 day boundary in the past", | |||
| 72 | + | cert: ManagedCertificate{NotAfter: now.Add(29 * 24 * time.Hour)}, | |||
| 73 | + | want: TLSCertificateStatusWarning, | |||
| 74 | + | }, | |||
| 75 | + | { | |||
| 76 | + | name: "missing when NotAfter is zero", | |||
| 77 | + | cert: ManagedCertificate{}, | |||
| 78 | + | want: TLSCertificateStatusMissing, | |||
| 79 | + | }, | |||
| 80 | + | { | |||
| 81 | + | name: "error when LastError is set", | |||
| 82 | + | cert: ManagedCertificate{ | |||
| 83 | + | NotAfter: now.Add(60 * 24 * time.Hour), | |||
| 84 | + | LastError: "issuer unavailable", | |||
| 85 | + | }, | |||
| 86 | + | want: TLSCertificateStatusError, | |||
| 87 | + | }, | |||
| 88 | + | { | |||
| 89 | + | name: "error takes priority over expired", | |||
| 90 | + | cert: ManagedCertificate{ | |||
| 91 | + | NotAfter: now.Add(-time.Hour), | |||
| 92 | + | LastError: "something went wrong", | |||
| 93 | + | }, | |||
| 94 | + | want: TLSCertificateStatusError, | |||
| 95 | + | }, | |||
| 96 | + | { | |||
| 97 | + | name: "error takes priority over zero NotAfter", | |||
| 98 | + | cert: ManagedCertificate{ | |||
| 99 | + | LastError: "something went wrong", | |||
| 100 | + | }, | |||
| 101 | + | want: TLSCertificateStatusError, | |||
| 102 | + | }, | |||
| 103 | + | } | |||
| 104 | + | ||||
| 105 | + | for _, tt := range tests { | |||
| 106 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 107 | + | got := tt.cert.Health(now) | |||
| 108 | + | assert.Equal(t, tt.want, got) | |||
| 109 | + | }) | |||
| 110 | + | } | |||
| 52 | } | 111 | } | ||
M internal/usecase/publictls/http01.go +5 -3
| @@ -2,6 +2,7 @@ package publictls | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "log/slog" | |
| 5 | 6 | "sync" | |
| 6 | 7 | ||
| 7 | 8 | "github.com/bnema/gordon/internal/domain" | |
| @@ -10,7 +11,7 @@ import ( | |||
| 10 | 11 | // HTTP01Challenges stores ACME HTTP-01 challenge tokens and their key | |
| 11 | 12 | // authorizations. It is safe for concurrent use. | |
| 12 | 13 | type HTTP01Challenges struct { | |
| 13 | - | mu sync.Mutex | |
| 14 | + | mu sync.RWMutex | |
| 14 | 15 | data map[string]string | |
| 15 | 16 | } | |
| 16 | 17 | ||
| @@ -25,6 +26,7 @@ func NewHTTP01Challenges() *HTTP01Challenges { | |||
| 25 | 26 | // ignores unsafe tokens (see safeHTTP01Token) and empty keyAuth values. | |
| 26 | 27 | func (c *HTTP01Challenges) Present(token, keyAuth string) { | |
| 27 | 28 | if !safeHTTP01Token(token) || keyAuth == "" { | |
| 29 | + | slog.Debug("http01: rejected Present for unsafe token or empty keyAuth") | |
| 28 | 30 | return | |
| 29 | 31 | } | |
| 30 | 32 | c.mu.Lock() | |
| @@ -46,8 +48,8 @@ func (c *HTTP01Challenges) Get(_ context.Context, token string) (string, bool) { | |||
| 46 | 48 | if !safeHTTP01Token(token) { | |
| 47 | 49 | return "", false | |
| 48 | 50 | } | |
| 49 | - | c.mu.Lock() | |
| 50 | - | defer c.mu.Unlock() | |
| 51 | + | c.mu.RLock() | |
| 52 | + | defer c.mu.RUnlock() | |
| 51 | 53 | keyAuth, ok := c.data[token] | |
| 52 | 54 | return keyAuth, ok | |
| 53 | 55 | } | |
| @@ -2,6 +2,7 @@ package publictls | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "log/slog" | |||
| 5 | "sync" | 6 | "sync" | ||
| 6 | 7 | ||||
| 7 | "github.com/bnema/gordon/internal/domain" | 8 | "github.com/bnema/gordon/internal/domain" | ||
| @@ -10,7 +11,7 @@ import ( | |||||
| 10 | // HTTP01Challenges stores ACME HTTP-01 challenge tokens and their key | 11 | // HTTP01Challenges stores ACME HTTP-01 challenge tokens and their key | ||
| 11 | // authorizations. It is safe for concurrent use. | 12 | // authorizations. It is safe for concurrent use. | ||
| 12 | type HTTP01Challenges struct { | 13 | type HTTP01Challenges struct { | ||
| 13 | - | mu sync.Mutex | 14 | + | mu sync.RWMutex |
| 14 | data map[string]string | 15 | data map[string]string | ||
| 15 | } | 16 | } | ||
| 16 | 17 | ||||
| @@ -25,6 +26,7 @@ func NewHTTP01Challenges() *HTTP01Challenges { | |||||
| 25 | // ignores unsafe tokens (see safeHTTP01Token) and empty keyAuth values. | 26 | // ignores unsafe tokens (see safeHTTP01Token) and empty keyAuth values. | ||
| 26 | func (c *HTTP01Challenges) Present(token, keyAuth string) { | 27 | func (c *HTTP01Challenges) Present(token, keyAuth string) { | ||
| 27 | if !safeHTTP01Token(token) || keyAuth == "" { | 28 | if !safeHTTP01Token(token) || keyAuth == "" { | ||
| 29 | + | slog.Debug("http01: rejected Present for unsafe token or empty keyAuth") | |||
| 28 | return | 30 | return | ||
| 29 | } | 31 | } | ||
| 30 | c.mu.Lock() | 32 | c.mu.Lock() | ||
| @@ -46,8 +48,8 @@ func (c *HTTP01Challenges) Get(_ context.Context, token string) (string, bool) { | |||||
| 46 | if !safeHTTP01Token(token) { | 48 | if !safeHTTP01Token(token) { | ||
| 47 | return "", false | 49 | return "", false | ||
| 48 | } | 50 | } | ||
| 49 | - | c.mu.Lock() | 51 | + | c.mu.RLock() |
| 50 | - | defer c.mu.Unlock() | 52 | + | defer c.mu.RUnlock() |
| 51 | keyAuth, ok := c.data[token] | 53 | keyAuth, ok := c.data[token] | ||
| 52 | return keyAuth, ok | 54 | return keyAuth, ok | ||
| 53 | } | 55 | } | ||
M internal/usecase/publictls/http01_test.go +66 -13
| @@ -2,6 +2,8 @@ package publictls | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "fmt" | |
| 6 | + | "sync" | |
| 5 | 7 | "testing" | |
| 6 | 8 | ||
| 7 | 9 | "github.com/stretchr/testify/assert" | |
| @@ -27,20 +29,71 @@ func TestHTTP01Challenges(t *testing.T) { | |||
| 27 | 29 | assert.False(t, ok) | |
| 28 | 30 | }) | |
| 29 | 31 | ||
| 30 | - | t.Run("concurrent access does not panic", func(t *testing.T) { | |
| 32 | + | t.Run("concurrent access is correct", func(t *testing.T) { | |
| 31 | 33 | ch := NewHTTP01Challenges() | |
| 32 | - | done := make(chan struct{}) | |
| 33 | - | go func() { | |
| 34 | - | ch.Present("t1", "ka1") | |
| 35 | - | done <- struct{}{} | |
| 36 | - | }() | |
| 37 | - | go func() { | |
| 38 | - | ch.Get(ctx, "t1") | |
| 39 | - | done <- struct{}{} | |
| 40 | - | }() | |
| 41 | - | <-done | |
| 42 | - | <-done | |
| 43 | - | // If we reach here without a race, the test passes. | |
| 34 | + | ||
| 35 | + | // Pre-populate some tokens. | |
| 36 | + | ch.Present("existing-token", "existing-keyauth") | |
| 37 | + | ||
| 38 | + | const numGoroutines = 20 | |
| 39 | + | var wg sync.WaitGroup | |
| 40 | + | errCh := make(chan error, numGoroutines) | |
| 41 | + | ||
| 42 | + | // Spawn concurrent readers and writers. | |
| 43 | + | for i := range numGoroutines { | |
| 44 | + | wg.Add(1) | |
| 45 | + | go func(id int) { | |
| 46 | + | defer wg.Done() | |
| 47 | + | token := fmt.Sprintf("token-%d", id) | |
| 48 | + | keyAuth := fmt.Sprintf("ka-%d", id) | |
| 49 | + | ||
| 50 | + | // Every goroutine presents its own token. | |
| 51 | + | ch.Present(token, keyAuth) | |
| 52 | + | ||
| 53 | + | // Every goroutine reads back its own token. | |
| 54 | + | got, ok := ch.Get(ctx, token) | |
| 55 | + | if !ok { | |
| 56 | + | errCh <- fmt.Errorf("token %q not found after Present", token) | |
| 57 | + | return | |
| 58 | + | } | |
| 59 | + | if got != keyAuth { | |
| 60 | + | errCh <- fmt.Errorf("token %q: got %q, want %q", token, got, keyAuth) | |
| 61 | + | return | |
| 62 | + | } | |
| 63 | + | ||
| 64 | + | // Every goroutine reads the pre-populated existing token. | |
| 65 | + | existing, ok := ch.Get(ctx, "existing-token") | |
| 66 | + | if !ok { | |
| 67 | + | errCh <- fmt.Errorf("existing-token not found by goroutine %d", id) | |
| 68 | + | return | |
| 69 | + | } | |
| 70 | + | if existing != "existing-keyauth" { | |
| 71 | + | errCh <- fmt.Errorf("existing-token: got %q, want existing-keyauth", existing) | |
| 72 | + | return | |
| 73 | + | } | |
| 74 | + | ||
| 75 | + | // Every goroutine cleans up its own token. | |
| 76 | + | ch.CleanUp(token) | |
| 77 | + | _, ok = ch.Get(ctx, token) | |
| 78 | + | if ok { | |
| 79 | + | errCh <- fmt.Errorf("token %q should be gone after CleanUp", token) | |
| 80 | + | return | |
| 81 | + | } | |
| 82 | + | }(i) | |
| 83 | + | } | |
| 84 | + | ||
| 85 | + | wg.Wait() | |
| 86 | + | close(errCh) | |
| 87 | + | ||
| 88 | + | // Collect any errors from goroutines. | |
| 89 | + | for err := range errCh { | |
| 90 | + | t.Error(err) | |
| 91 | + | } | |
| 92 | + | ||
| 93 | + | // Verify that pre-populated token is still present after all concurrent access. | |
| 94 | + | existing, ok := ch.Get(ctx, "existing-token") | |
| 95 | + | require.True(t, ok, "existing-token should survive concurrent access") | |
| 96 | + | assert.Equal(t, "existing-keyauth", existing) | |
| 44 | 97 | }) | |
| 45 | 98 | } | |
| 46 | 99 | ||
| @@ -2,6 +2,8 @@ package publictls | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "fmt" | |||
| 6 | + | "sync" | |||
| 5 | "testing" | 7 | "testing" | ||
| 6 | 8 | ||||
| 7 | "github.com/stretchr/testify/assert" | 9 | "github.com/stretchr/testify/assert" | ||
| @@ -27,20 +29,71 @@ func TestHTTP01Challenges(t *testing.T) { | |||||
| 27 | assert.False(t, ok) | 29 | assert.False(t, ok) | ||
| 28 | }) | 30 | }) | ||
| 29 | 31 | ||||
| 30 | - | t.Run("concurrent access does not panic", func(t *testing.T) { | 32 | + | t.Run("concurrent access is correct", func(t *testing.T) { |
| 31 | ch := NewHTTP01Challenges() | 33 | ch := NewHTTP01Challenges() | ||
| 32 | - | done := make(chan struct{}) | 34 | + | |
| 33 | - | go func() { | 35 | + | // Pre-populate some tokens. |
| 34 | - | ch.Present("t1", "ka1") | 36 | + | ch.Present("existing-token", "existing-keyauth") |
| 35 | - | done <- struct{}{} | 37 | + | |
| 36 | - | }() | 38 | + | const numGoroutines = 20 |
| 37 | - | go func() { | 39 | + | var wg sync.WaitGroup |
| 38 | - | ch.Get(ctx, "t1") | 40 | + | errCh := make(chan error, numGoroutines) |
| 39 | - | done <- struct{}{} | 41 | + | |
| 40 | - | }() | 42 | + | // Spawn concurrent readers and writers. |
| 41 | - | <-done | 43 | + | for i := range numGoroutines { |
| 42 | - | <-done | 44 | + | wg.Add(1) |
| 43 | - | // If we reach here without a race, the test passes. | 45 | + | go func(id int) { |
| 46 | + | defer wg.Done() | |||
| 47 | + | token := fmt.Sprintf("token-%d", id) | |||
| 48 | + | keyAuth := fmt.Sprintf("ka-%d", id) | |||
| 49 | + | ||||
| 50 | + | // Every goroutine presents its own token. | |||
| 51 | + | ch.Present(token, keyAuth) | |||
| 52 | + | ||||
| 53 | + | // Every goroutine reads back its own token. | |||
| 54 | + | got, ok := ch.Get(ctx, token) | |||
| 55 | + | if !ok { | |||
| 56 | + | errCh <- fmt.Errorf("token %q not found after Present", token) | |||
| 57 | + | return | |||
| 58 | + | } | |||
| 59 | + | if got != keyAuth { | |||
| 60 | + | errCh <- fmt.Errorf("token %q: got %q, want %q", token, got, keyAuth) | |||
| 61 | + | return | |||
| 62 | + | } | |||
| 63 | + | ||||
| 64 | + | // Every goroutine reads the pre-populated existing token. | |||
| 65 | + | existing, ok := ch.Get(ctx, "existing-token") | |||
| 66 | + | if !ok { | |||
| 67 | + | errCh <- fmt.Errorf("existing-token not found by goroutine %d", id) | |||
| 68 | + | return | |||
| 69 | + | } | |||
| 70 | + | if existing != "existing-keyauth" { | |||
| 71 | + | errCh <- fmt.Errorf("existing-token: got %q, want existing-keyauth", existing) | |||
| 72 | + | return | |||
| 73 | + | } | |||
| 74 | + | ||||
| 75 | + | // Every goroutine cleans up its own token. | |||
| 76 | + | ch.CleanUp(token) | |||
| 77 | + | _, ok = ch.Get(ctx, token) | |||
| 78 | + | if ok { | |||
| 79 | + | errCh <- fmt.Errorf("token %q should be gone after CleanUp", token) | |||
| 80 | + | return | |||
| 81 | + | } | |||
| 82 | + | }(i) | |||
| 83 | + | } | |||
| 84 | + | ||||
| 85 | + | wg.Wait() | |||
| 86 | + | close(errCh) | |||
| 87 | + | ||||
| 88 | + | // Collect any errors from goroutines. | |||
| 89 | + | for err := range errCh { | |||
| 90 | + | t.Error(err) | |||
| 91 | + | } | |||
| 92 | + | ||||
| 93 | + | // Verify that pre-populated token is still present after all concurrent access. | |||
| 94 | + | existing, ok := ch.Get(ctx, "existing-token") | |||
| 95 | + | require.True(t, ok, "existing-token should survive concurrent access") | |||
| 96 | + | assert.Equal(t, "existing-keyauth", existing) | |||
| 44 | }) | 97 | }) | ||
| 45 | } | 98 | } | ||
| 46 | 99 | ||||
M internal/usecase/publictls/integration_test.go +6 -6
| @@ -100,13 +100,13 @@ func TestPublicTLSIntegration_DNS01WildcardStatusAndCertificateLookup(t *testing | |||
| 100 | 100 | routeMap[r.Domain] = r | |
| 101 | 101 | } | |
| 102 | 102 | ||
| 103 | - | for _, domain := range []string{"app.example.com", "api.prod.example.com", "example.com"} { | |
| 104 | - | rc, ok := routeMap[domain] | |
| 105 | - | require.True(t, ok, "route %s should be present in status", domain) | |
| 106 | - | assert.True(t, rc.Covered, "route %s should be covered", domain) | |
| 103 | + | for _, routeDomain := range []string{"app.example.com", "api.prod.example.com", "example.com"} { | |
| 104 | + | rc, ok := routeMap[routeDomain] | |
| 105 | + | require.True(t, ok, "route %s should be present in status", routeDomain) | |
| 106 | + | assert.True(t, rc.Covered, "route %s should be covered", routeDomain) | |
| 107 | 107 | assert.True(t, rc.RequiredACME) | |
| 108 | - | assert.NotEmpty(t, rc.CoveredBy, "route %s should have CoveredBy set", domain) | |
| 109 | - | assert.Empty(t, rc.Error, "route %s should have no error", domain) | |
| 108 | + | assert.NotEmpty(t, rc.CoveredBy, "route %s should have CoveredBy set", routeDomain) | |
| 109 | + | assert.Empty(t, rc.Error, "route %s should have no error", routeDomain) | |
| 110 | 110 | } | |
| 111 | 111 | ||
| 112 | 112 | // app.example.com should be covered by the wildcard cert (dns01-example.com). | |
| @@ -100,13 +100,13 @@ func TestPublicTLSIntegration_DNS01WildcardStatusAndCertificateLookup(t *testing | |||||
| 100 | routeMap[r.Domain] = r | 100 | routeMap[r.Domain] = r | ||
| 101 | } | 101 | } | ||
| 102 | 102 | ||||
| 103 | - | for _, domain := range []string{"app.example.com", "api.prod.example.com", "example.com"} { | 103 | + | for _, routeDomain := range []string{"app.example.com", "api.prod.example.com", "example.com"} { |
| 104 | - | rc, ok := routeMap[domain] | 104 | + | rc, ok := routeMap[routeDomain] |
| 105 | - | require.True(t, ok, "route %s should be present in status", domain) | 105 | + | require.True(t, ok, "route %s should be present in status", routeDomain) |
| 106 | - | assert.True(t, rc.Covered, "route %s should be covered", domain) | 106 | + | assert.True(t, rc.Covered, "route %s should be covered", routeDomain) |
| 107 | assert.True(t, rc.RequiredACME) | 107 | assert.True(t, rc.RequiredACME) | ||
| 108 | - | assert.NotEmpty(t, rc.CoveredBy, "route %s should have CoveredBy set", domain) | 108 | + | assert.NotEmpty(t, rc.CoveredBy, "route %s should have CoveredBy set", routeDomain) |
| 109 | - | assert.Empty(t, rc.Error, "route %s should have no error", domain) | 109 | + | assert.Empty(t, rc.Error, "route %s should have no error", routeDomain) |
| 110 | } | 110 | } | ||
| 111 | 111 | ||||
| 112 | // app.example.com should be covered by the wildcard cert (dns01-example.com). | 112 | // app.example.com should be covered by the wildcard cert (dns01-example.com). | ||
M internal/usecase/publictls/renewal.go +4 -7
| @@ -35,11 +35,13 @@ func (s *Service) StartRenewalLoop(ctx context.Context, interval time.Duration) | |||
| 35 | 35 | // Derive a cancellable context so Stop can terminate the loop. | |
| 36 | 36 | loopCtx, cancel := context.WithCancel(ctx) | |
| 37 | 37 | ||
| 38 | - | // Cancel any previous loop and wait for it to fully exit before | |
| 39 | - | // overwriting s.done, preventing overlapping loop goroutines. | |
| 38 | + | // Atomically swap loop state under lock before cancelling the old loop, | |
| 39 | + | // preventing overlapping loop goroutines and race conditions. | |
| 40 | 40 | s.mu.Lock() | |
| 41 | 41 | oldCancel := s.cancel | |
| 42 | 42 | oldDone := s.done | |
| 43 | + | s.cancel = cancel | |
| 44 | + | s.done = done | |
| 43 | 45 | s.mu.Unlock() | |
| 44 | 46 | ||
| 45 | 47 | if oldCancel != nil { | |
| @@ -49,11 +51,6 @@ func (s *Service) StartRenewalLoop(ctx context.Context, interval time.Duration) | |||
| 49 | 51 | <-oldDone | |
| 50 | 52 | } | |
| 51 | 53 | ||
| 52 | - | s.mu.Lock() | |
| 53 | - | s.cancel = cancel | |
| 54 | - | s.done = done | |
| 55 | - | s.mu.Unlock() | |
| 56 | - | ||
| 57 | 54 | go func() { | |
| 58 | 55 | defer close(done) | |
| 59 | 56 | defer cancel() | |
| @@ -35,11 +35,13 @@ func (s *Service) StartRenewalLoop(ctx context.Context, interval time.Duration) | |||||
| 35 | // Derive a cancellable context so Stop can terminate the loop. | 35 | // Derive a cancellable context so Stop can terminate the loop. | ||
| 36 | loopCtx, cancel := context.WithCancel(ctx) | 36 | loopCtx, cancel := context.WithCancel(ctx) | ||
| 37 | 37 | ||||
| 38 | - | // Cancel any previous loop and wait for it to fully exit before | 38 | + | // Atomically swap loop state under lock before cancelling the old loop, |
| 39 | - | // overwriting s.done, preventing overlapping loop goroutines. | 39 | + | // preventing overlapping loop goroutines and race conditions. |
| 40 | s.mu.Lock() | 40 | s.mu.Lock() | ||
| 41 | oldCancel := s.cancel | 41 | oldCancel := s.cancel | ||
| 42 | oldDone := s.done | 42 | oldDone := s.done | ||
| 43 | + | s.cancel = cancel | |||
| 44 | + | s.done = done | |||
| 43 | s.mu.Unlock() | 45 | s.mu.Unlock() | ||
| 44 | 46 | ||||
| 45 | if oldCancel != nil { | 47 | if oldCancel != nil { | ||
| @@ -49,11 +51,6 @@ func (s *Service) StartRenewalLoop(ctx context.Context, interval time.Duration) | |||||
| 49 | <-oldDone | 51 | <-oldDone | ||
| 50 | } | 52 | } | ||
| 51 | 53 | ||||
| 52 | - | s.mu.Lock() | |||
| 53 | - | s.cancel = cancel | |||
| 54 | - | s.done = done | |||
| 55 | - | s.mu.Unlock() | |||
| 56 | - | ||||
| 57 | go func() { | 54 | go func() { | ||
| 58 | defer close(done) | 55 | defer close(done) | ||
| 59 | defer cancel() | 56 | defer cancel() | ||
M internal/usecase/publictls/renewal_test.go +16 -18
| @@ -164,12 +164,12 @@ func TestRenewDueCertificatesSavesAndUpdatesCache(t *testing.T) { | |||
| 164 | 164 | err = svc.Load(ctx) | |
| 165 | 165 | require.NoError(t, err) | |
| 166 | 166 | ||
| 167 | - | // Verify cert is in cache with original NotAfter. | |
| 168 | - | svc.mu.Lock() | |
| 169 | - | cached, ok := svc.certs[certID] | |
| 170 | - | svc.mu.Unlock() | |
| 171 | - | require.True(t, ok, "cert should be in cache after Load") | |
| 172 | - | assert.True(t, cached.NotAfter.Equal(notAfterSoon), "cache should have original NotAfter") | |
| 167 | + | // Verify cert is in cache with original NotAfter via Status. | |
| 168 | + | status := svc.Status(ctx) | |
| 169 | + | require.Len(t, status.Certificates, 1, "expected 1 managed certificate after Load") | |
| 170 | + | assert.True(t, status.Certificates[0].NotAfter.Equal(notAfterSoon), | |
| 171 | + | "cache should have original NotAfter (got %v, want %v)", | |
| 172 | + | status.Certificates[0].NotAfter, notAfterSoon) | |
| 173 | 173 | ||
| 174 | 174 | // Run renewDueCertificates. | |
| 175 | 175 | err = svc.renewDueCertificates(ctx, now) | |
| @@ -181,16 +181,14 @@ func TestRenewDueCertificatesSavesAndUpdatesCache(t *testing.T) { | |||
| 181 | 181 | require.Len(t, all, 1) | |
| 182 | 182 | assert.True(t, all[0].NotAfter.Equal(notAfterRenewed), "store should have renewed NotAfter") | |
| 183 | 183 | ||
| 184 | - | // Verify the cache has the updated certificate. | |
| 185 | - | svc.mu.Lock() | |
| 186 | - | cached, ok = svc.certs[certID] | |
| 187 | - | svc.mu.Unlock() | |
| 188 | - | require.True(t, ok, "cert should still be in cache after renewal") | |
| 189 | - | assert.True(t, cached.NotAfter.Equal(notAfterRenewed), "cache should have renewed NotAfter") | |
| 190 | - | ||
| 191 | - | // Verify lastErr is cleared for the renewed certificate. | |
| 192 | - | svc.mu.Lock() | |
| 193 | - | _, hasErr := svc.lastErr[certID] | |
| 194 | - | svc.mu.Unlock() | |
| 195 | - | assert.False(t, hasErr, "lastErr should be cleared for renewed cert") | |
| 184 | + | // Verify the cache has the updated certificate via Status. | |
| 185 | + | status = svc.Status(ctx) | |
| 186 | + | require.Len(t, status.Certificates, 1, "expected 1 managed certificate after renewal") | |
| 187 | + | assert.True(t, status.Certificates[0].NotAfter.Equal(notAfterRenewed), | |
| 188 | + | "cache should have renewed NotAfter (got %v, want %v)", | |
| 189 | + | status.Certificates[0].NotAfter, notAfterRenewed) | |
| 190 | + | ||
| 191 | + | // Verify lastErr is empty for the renewed certificate. | |
| 192 | + | assert.Empty(t, status.Certificates[0].LastError, | |
| 193 | + | "LastError should be empty for renewed cert") | |
| 196 | 194 | } | |
| @@ -164,12 +164,12 @@ func TestRenewDueCertificatesSavesAndUpdatesCache(t *testing.T) { | |||||
| 164 | err = svc.Load(ctx) | 164 | err = svc.Load(ctx) | ||
| 165 | require.NoError(t, err) | 165 | require.NoError(t, err) | ||
| 166 | 166 | ||||
| 167 | - | // Verify cert is in cache with original NotAfter. | 167 | + | // Verify cert is in cache with original NotAfter via Status. |
| 168 | - | svc.mu.Lock() | 168 | + | status := svc.Status(ctx) |
| 169 | - | cached, ok := svc.certs[certID] | 169 | + | require.Len(t, status.Certificates, 1, "expected 1 managed certificate after Load") |
| 170 | - | svc.mu.Unlock() | 170 | + | assert.True(t, status.Certificates[0].NotAfter.Equal(notAfterSoon), |
| 171 | - | require.True(t, ok, "cert should be in cache after Load") | 171 | + | "cache should have original NotAfter (got %v, want %v)", |
| 172 | - | assert.True(t, cached.NotAfter.Equal(notAfterSoon), "cache should have original NotAfter") | 172 | + | status.Certificates[0].NotAfter, notAfterSoon) |
| 173 | 173 | ||||
| 174 | // Run renewDueCertificates. | 174 | // Run renewDueCertificates. | ||
| 175 | err = svc.renewDueCertificates(ctx, now) | 175 | err = svc.renewDueCertificates(ctx, now) | ||
| @@ -181,16 +181,14 @@ func TestRenewDueCertificatesSavesAndUpdatesCache(t *testing.T) { | |||||
| 181 | require.Len(t, all, 1) | 181 | require.Len(t, all, 1) | ||
| 182 | assert.True(t, all[0].NotAfter.Equal(notAfterRenewed), "store should have renewed NotAfter") | 182 | assert.True(t, all[0].NotAfter.Equal(notAfterRenewed), "store should have renewed NotAfter") | ||
| 183 | 183 | ||||
| 184 | - | // Verify the cache has the updated certificate. | 184 | + | // Verify the cache has the updated certificate via Status. |
| 185 | - | svc.mu.Lock() | 185 | + | status = svc.Status(ctx) |
| 186 | - | cached, ok = svc.certs[certID] | 186 | + | require.Len(t, status.Certificates, 1, "expected 1 managed certificate after renewal") |
| 187 | - | svc.mu.Unlock() | 187 | + | assert.True(t, status.Certificates[0].NotAfter.Equal(notAfterRenewed), |
| 188 | - | require.True(t, ok, "cert should still be in cache after renewal") | 188 | + | "cache should have renewed NotAfter (got %v, want %v)", |
| 189 | - | assert.True(t, cached.NotAfter.Equal(notAfterRenewed), "cache should have renewed NotAfter") | 189 | + | status.Certificates[0].NotAfter, notAfterRenewed) |
| 190 | - | 190 | + | ||
| 191 | - | // Verify lastErr is cleared for the renewed certificate. | 191 | + | // Verify lastErr is empty for the renewed certificate. |
| 192 | - | svc.mu.Lock() | 192 | + | assert.Empty(t, status.Certificates[0].LastError, |
| 193 | - | _, hasErr := svc.lastErr[certID] | 193 | + | "LastError should be empty for renewed cert") |
| 194 | - | svc.mu.Unlock() | |||
| 195 | - | assert.False(t, hasErr, "lastErr should be cleared for renewed cert") | |||
| 196 | } | 194 | } | ||
M internal/usecase/publictls/service.go +33 -5
| @@ -4,12 +4,14 @@ import ( | |||
| 4 | 4 | "context" | |
| 5 | 5 | "crypto/tls" | |
| 6 | 6 | "fmt" | |
| 7 | + | "reflect" | |
| 7 | 8 | "strings" | |
| 8 | 9 | "sync" | |
| 9 | 10 | "time" | |
| 10 | 11 | ||
| 11 | 12 | "github.com/bnema/gordon/internal/boundaries/out" | |
| 12 | 13 | "github.com/bnema/gordon/internal/domain" | |
| 14 | + | "github.com/bnema/zerowrap" | |
| 13 | 15 | ) | |
| 14 | 16 | ||
| 15 | 17 | // RouteSource provides routes from which certificate targets are derived. | |
| @@ -27,13 +29,15 @@ type ServiceDeps struct { | |||
| 27 | 29 | ZoneResolver zoneResolver | |
| 28 | 30 | Challenges *HTTP01Challenges | |
| 29 | 31 | Effective EffectiveChallenge | |
| 32 | + | Log zerowrap.Logger | |
| 30 | 33 | } | |
| 31 | 34 | ||
| 32 | 35 | // Service manages public TLS certificates via ACME. | |
| 33 | 36 | type Service struct { | |
| 34 | - | mu sync.Mutex | |
| 37 | + | mu sync.RWMutex | |
| 35 | 38 | cfg Config | |
| 36 | 39 | deps ServiceDeps | |
| 40 | + | log zerowrap.Logger | |
| 37 | 41 | certs map[string]*out.StoredCertificate // indexed by cert ID | |
| 38 | 42 | lastErr map[string]string // indexed by cert ID | |
| 39 | 43 | routeErr map[string]string // indexed by host | |
| @@ -52,9 +56,13 @@ func NewService(cfg Config, deps ServiceDeps) *Service { | |||
| 52 | 56 | if deps.Challenges == nil { | |
| 53 | 57 | deps.Challenges = NewHTTP01Challenges() | |
| 54 | 58 | } | |
| 59 | + | if reflect.ValueOf(deps.Log).IsZero() { | |
| 60 | + | deps.Log = zerowrap.Default() | |
| 61 | + | } | |
| 55 | 62 | return &Service{ | |
| 56 | 63 | cfg: cfg, | |
| 57 | 64 | deps: deps, | |
| 65 | + | log: deps.Log, | |
| 58 | 66 | certs: make(map[string]*out.StoredCertificate), | |
| 59 | 67 | lastErr: make(map[string]string), | |
| 60 | 68 | routeErr: make(map[string]string), | |
| @@ -99,13 +107,19 @@ func (s *Service) Load(ctx context.Context) error { | |||
| 99 | 107 | // If ACME is disabled, it is a no-op. | |
| 100 | 108 | func (s *Service) Reconcile(ctx context.Context) error { | |
| 101 | 109 | if !s.cfg.Enabled { | |
| 110 | + | s.log.Debug().Msg("acme disabled, skipping reconcile") | |
| 102 | 111 | return nil | |
| 103 | 112 | } | |
| 104 | 113 | ||
| 114 | + | log := s.log.With().Str("component", "Reconcile").Logger() | |
| 115 | + | log.Debug().Msg("starting certificate reconciliation") | |
| 116 | + | ||
| 105 | 117 | if s.deps.Store == nil { | |
| 118 | + | log.Warn().Msg("certificate store is nil, cannot reconcile") | |
| 106 | 119 | return fmt.Errorf("certificate store is nil") | |
| 107 | 120 | } | |
| 108 | 121 | if s.deps.Issuer == nil { | |
| 122 | + | log.Warn().Msg("certificate issuer is nil, cannot reconcile") | |
| 109 | 123 | return fmt.Errorf("certificate issuer is nil") | |
| 110 | 124 | } | |
| 111 | 125 | ||
| @@ -187,6 +201,7 @@ func (s *Service) Reconcile(ctx context.Context) error { | |||
| 187 | 201 | // are not blocked during network I/O (Obtain) or storage writes (Save). | |
| 188 | 202 | s.reconcileMissingTargets(ctx, missing) | |
| 189 | 203 | ||
| 204 | + | log.Debug().Int("missing_count", len(missing)).Msg("reconciled missing targets") | |
| 190 | 205 | return nil | |
| 191 | 206 | } | |
| 192 | 207 | ||
| @@ -203,14 +218,14 @@ func (s *Service) reconcileMissingTargets(ctx context.Context, missing []Certifi | |||
| 203 | 218 | stored, err := s.deps.Issuer.Obtain(ctx, order) | |
| 204 | 219 | if err != nil { | |
| 205 | 220 | s.mu.Lock() | |
| 206 | - | s.lastErr[target.ID] = err.Error() | |
| 221 | + | s.lastErr[target.ID] = "obtain certificate failed" | |
| 207 | 222 | s.mu.Unlock() | |
| 208 | 223 | continue | |
| 209 | 224 | } | |
| 210 | 225 | ||
| 211 | 226 | if err := s.deps.Store.Save(ctx, *stored); err != nil { | |
| 212 | 227 | s.mu.Lock() | |
| 213 | - | s.lastErr[target.ID] = fmt.Sprintf("save certificate: %v", err) | |
| 228 | + | s.lastErr[target.ID] = "save certificate failed" | |
| 214 | 229 | s.mu.Unlock() | |
| 215 | 230 | continue | |
| 216 | 231 | } | |
| @@ -270,8 +285,8 @@ func (s *Service) GetCertificate(hello *tls.ClientHelloInfo) (*tls.Certificate, | |||
| 270 | 285 | return nil, nil | |
| 271 | 286 | } | |
| 272 | 287 | ||
| 273 | - | s.mu.Lock() | |
| 274 | - | defer s.mu.Unlock() | |
| 288 | + | s.mu.RLock() | |
| 289 | + | defer s.mu.RUnlock() | |
| 275 | 290 | ||
| 276 | 291 | // Check if this host requires ACME coverage. | |
| 277 | 292 | if !s.isRequiredHostLocked(host) { | |
| @@ -313,6 +328,19 @@ func (s *Service) GetHTTP01Challenge(ctx context.Context, token string) (string, | |||
| 313 | 328 | return s.deps.Challenges.Get(ctx, token) | |
| 314 | 329 | } | |
| 315 | 330 | ||
| 331 | + | // GetStoredCertificate returns a copy of the stored certificate for the given | |
| 332 | + | // ID, or nil if not found. Exposed for testing. | |
| 333 | + | func (s *Service) GetStoredCertificate(id string) *out.StoredCertificate { | |
| 334 | + | s.mu.RLock() | |
| 335 | + | defer s.mu.RUnlock() | |
| 336 | + | cert := s.certs[id] | |
| 337 | + | if cert == nil { | |
| 338 | + | return nil | |
| 339 | + | } | |
| 340 | + | cpy := *cert | |
| 341 | + | return &cpy | |
| 342 | + | } | |
| 343 | + | ||
| 316 | 344 | // Stop gracefully stops the service. If a renewal loop is running, it is | |
| 317 | 345 | // cancelled and Stop waits for it to finish (subject to ctx deadline). | |
| 318 | 346 | func (s *Service) Stop(ctx context.Context) error { | |
| @@ -4,12 +4,14 @@ import ( | |||||
| 4 | "context" | 4 | "context" | ||
| 5 | "crypto/tls" | 5 | "crypto/tls" | ||
| 6 | "fmt" | 6 | "fmt" | ||
| 7 | + | "reflect" | |||
| 7 | "strings" | 8 | "strings" | ||
| 8 | "sync" | 9 | "sync" | ||
| 9 | "time" | 10 | "time" | ||
| 10 | 11 | ||||
| 11 | "github.com/bnema/gordon/internal/boundaries/out" | 12 | "github.com/bnema/gordon/internal/boundaries/out" | ||
| 12 | "github.com/bnema/gordon/internal/domain" | 13 | "github.com/bnema/gordon/internal/domain" | ||
| 14 | + | "github.com/bnema/zerowrap" | |||
| 13 | ) | 15 | ) | ||
| 14 | 16 | ||||
| 15 | // RouteSource provides routes from which certificate targets are derived. | 17 | // RouteSource provides routes from which certificate targets are derived. | ||
| @@ -27,13 +29,15 @@ type ServiceDeps struct { | |||||
| 27 | ZoneResolver zoneResolver | 29 | ZoneResolver zoneResolver | ||
| 28 | Challenges *HTTP01Challenges | 30 | Challenges *HTTP01Challenges | ||
| 29 | Effective EffectiveChallenge | 31 | Effective EffectiveChallenge | ||
| 32 | + | Log zerowrap.Logger | |||
| 30 | } | 33 | } | ||
| 31 | 34 | ||||
| 32 | // Service manages public TLS certificates via ACME. | 35 | // Service manages public TLS certificates via ACME. | ||
| 33 | type Service struct { | 36 | type Service struct { | ||
| 34 | - | mu sync.Mutex | 37 | + | mu sync.RWMutex |
| 35 | cfg Config | 38 | cfg Config | ||
| 36 | deps ServiceDeps | 39 | deps ServiceDeps | ||
| 40 | + | log zerowrap.Logger | |||
| 37 | certs map[string]*out.StoredCertificate // indexed by cert ID | 41 | certs map[string]*out.StoredCertificate // indexed by cert ID | ||
| 38 | lastErr map[string]string // indexed by cert ID | 42 | lastErr map[string]string // indexed by cert ID | ||
| 39 | routeErr map[string]string // indexed by host | 43 | routeErr map[string]string // indexed by host | ||
| @@ -52,9 +56,13 @@ func NewService(cfg Config, deps ServiceDeps) *Service { | |||||
| 52 | if deps.Challenges == nil { | 56 | if deps.Challenges == nil { | ||
| 53 | deps.Challenges = NewHTTP01Challenges() | 57 | deps.Challenges = NewHTTP01Challenges() | ||
| 54 | } | 58 | } | ||
| 59 | + | if reflect.ValueOf(deps.Log).IsZero() { | |||
| 60 | + | deps.Log = zerowrap.Default() | |||
| 61 | + | } | |||
| 55 | return &Service{ | 62 | return &Service{ | ||
| 56 | cfg: cfg, | 63 | cfg: cfg, | ||
| 57 | deps: deps, | 64 | deps: deps, | ||
| 65 | + | log: deps.Log, | |||
| 58 | certs: make(map[string]*out.StoredCertificate), | 66 | certs: make(map[string]*out.StoredCertificate), | ||
| 59 | lastErr: make(map[string]string), | 67 | lastErr: make(map[string]string), | ||
| 60 | routeErr: make(map[string]string), | 68 | routeErr: make(map[string]string), | ||
| @@ -99,13 +107,19 @@ func (s *Service) Load(ctx context.Context) error { | |||||
| 99 | // If ACME is disabled, it is a no-op. | 107 | // If ACME is disabled, it is a no-op. | ||
| 100 | func (s *Service) Reconcile(ctx context.Context) error { | 108 | func (s *Service) Reconcile(ctx context.Context) error { | ||
| 101 | if !s.cfg.Enabled { | 109 | if !s.cfg.Enabled { | ||
| 110 | + | s.log.Debug().Msg("acme disabled, skipping reconcile") | |||
| 102 | return nil | 111 | return nil | ||
| 103 | } | 112 | } | ||
| 104 | 113 | ||||
| 114 | + | log := s.log.With().Str("component", "Reconcile").Logger() | |||
| 115 | + | log.Debug().Msg("starting certificate reconciliation") | |||
| 116 | + | ||||
| 105 | if s.deps.Store == nil { | 117 | if s.deps.Store == nil { | ||
| 118 | + | log.Warn().Msg("certificate store is nil, cannot reconcile") | |||
| 106 | return fmt.Errorf("certificate store is nil") | 119 | return fmt.Errorf("certificate store is nil") | ||
| 107 | } | 120 | } | ||
| 108 | if s.deps.Issuer == nil { | 121 | if s.deps.Issuer == nil { | ||
| 122 | + | log.Warn().Msg("certificate issuer is nil, cannot reconcile") | |||
| 109 | return fmt.Errorf("certificate issuer is nil") | 123 | return fmt.Errorf("certificate issuer is nil") | ||
| 110 | } | 124 | } | ||
| 111 | 125 | ||||
| @@ -187,6 +201,7 @@ func (s *Service) Reconcile(ctx context.Context) error { | |||||
| 187 | // are not blocked during network I/O (Obtain) or storage writes (Save). | 201 | // are not blocked during network I/O (Obtain) or storage writes (Save). | ||
| 188 | s.reconcileMissingTargets(ctx, missing) | 202 | s.reconcileMissingTargets(ctx, missing) | ||
| 189 | 203 | ||||
| 204 | + | log.Debug().Int("missing_count", len(missing)).Msg("reconciled missing targets") | |||
| 190 | return nil | 205 | return nil | ||
| 191 | } | 206 | } | ||
| 192 | 207 | ||||
| @@ -203,14 +218,14 @@ func (s *Service) reconcileMissingTargets(ctx context.Context, missing []Certifi | |||||
| 203 | stored, err := s.deps.Issuer.Obtain(ctx, order) | 218 | stored, err := s.deps.Issuer.Obtain(ctx, order) | ||
| 204 | if err != nil { | 219 | if err != nil { | ||
| 205 | s.mu.Lock() | 220 | s.mu.Lock() | ||
| 206 | - | s.lastErr[target.ID] = err.Error() | 221 | + | s.lastErr[target.ID] = "obtain certificate failed" |
| 207 | s.mu.Unlock() | 222 | s.mu.Unlock() | ||
| 208 | continue | 223 | continue | ||
| 209 | } | 224 | } | ||
| 210 | 225 | ||||
| 211 | if err := s.deps.Store.Save(ctx, *stored); err != nil { | 226 | if err := s.deps.Store.Save(ctx, *stored); err != nil { | ||
| 212 | s.mu.Lock() | 227 | s.mu.Lock() | ||
| 213 | - | s.lastErr[target.ID] = fmt.Sprintf("save certificate: %v", err) | 228 | + | s.lastErr[target.ID] = "save certificate failed" |
| 214 | s.mu.Unlock() | 229 | s.mu.Unlock() | ||
| 215 | continue | 230 | continue | ||
| 216 | } | 231 | } | ||
| @@ -270,8 +285,8 @@ func (s *Service) GetCertificate(hello *tls.ClientHelloInfo) (*tls.Certificate, | |||||
| 270 | return nil, nil | 285 | return nil, nil | ||
| 271 | } | 286 | } | ||
| 272 | 287 | ||||
| 273 | - | s.mu.Lock() | 288 | + | s.mu.RLock() |
| 274 | - | defer s.mu.Unlock() | 289 | + | defer s.mu.RUnlock() |
| 275 | 290 | ||||
| 276 | // Check if this host requires ACME coverage. | 291 | // Check if this host requires ACME coverage. | ||
| 277 | if !s.isRequiredHostLocked(host) { | 292 | if !s.isRequiredHostLocked(host) { | ||
| @@ -313,6 +328,19 @@ func (s *Service) GetHTTP01Challenge(ctx context.Context, token string) (string, | |||||
| 313 | return s.deps.Challenges.Get(ctx, token) | 328 | return s.deps.Challenges.Get(ctx, token) | ||
| 314 | } | 329 | } | ||
| 315 | 330 | ||||
| 331 | + | // GetStoredCertificate returns a copy of the stored certificate for the given | |||
| 332 | + | // ID, or nil if not found. Exposed for testing. | |||
| 333 | + | func (s *Service) GetStoredCertificate(id string) *out.StoredCertificate { | |||
| 334 | + | s.mu.RLock() | |||
| 335 | + | defer s.mu.RUnlock() | |||
| 336 | + | cert := s.certs[id] | |||
| 337 | + | if cert == nil { | |||
| 338 | + | return nil | |||
| 339 | + | } | |||
| 340 | + | cpy := *cert | |||
| 341 | + | return &cpy | |||
| 342 | + | } | |||
| 343 | + | ||||
| 316 | // Stop gracefully stops the service. If a renewal loop is running, it is | 344 | // Stop gracefully stops the service. If a renewal loop is running, it is | ||
| 317 | // cancelled and Stop waits for it to finish (subject to ctx deadline). | 345 | // cancelled and Stop waits for it to finish (subject to ctx deadline). | ||
| 318 | func (s *Service) Stop(ctx context.Context) error { | 346 | func (s *Service) Stop(ctx context.Context) error { | ||
M internal/usecase/publictls/service_test.go +17 -11
| @@ -25,7 +25,11 @@ import ( | |||
| 25 | 25 | // Fakes | |
| 26 | 26 | // --------------------------------------------------------------------------- | |
| 27 | 27 | ||
| 28 | - | // fakeRoutes implements RouteSource for testing. | |
| 28 | + | // fakeRoutes models a stateful RouteSource using an internal mutex and | |
| 29 | + | // copy-on-read semantics. Generated mocks cannot easily replicate this | |
| 30 | + | // pattern because the interface methods return slices/maps by value and | |
| 31 | + | // callers may modify the returned data; a hand-rolled fake ensures each | |
| 32 | + | // GetRoutes/GetExternalRoutes call returns a defensive copy. | |
| 29 | 33 | type fakeRoutes struct { | |
| 30 | 34 | mu sync.Mutex | |
| 31 | 35 | routes []domain.Route | |
| @@ -50,7 +54,11 @@ func (f *fakeRoutes) GetExternalRoutes() map[string]string { | |||
| 50 | 54 | return out | |
| 51 | 55 | } | |
| 52 | 56 | ||
| 53 | - | // fakeIssuer implements out.PublicCertificateIssuer for testing. | |
| 57 | + | // fakeIssuer is a hand-rolled implementation of out.PublicCertificateIssuer | |
| 58 | + | // that records issued orders and delegates to pluggable obtain/renew | |
| 59 | + | // functions. Generated mocks are less convenient here because the test needs | |
| 60 | + | // to inspect captured orders after the call completes — a real slice on the | |
| 61 | + | // fake avoids mock expectation boilerplate. | |
| 54 | 62 | type fakeIssuer struct { | |
| 55 | 63 | mu sync.Mutex | |
| 56 | 64 | orders []out.CertificateOrder // recorded orders | |
| @@ -98,7 +106,10 @@ func defaultStoredCert(order out.CertificateOrder) (*out.StoredCertificate, erro | |||
| 98 | 106 | }, nil | |
| 99 | 107 | } | |
| 100 | 108 | ||
| 101 | - | // fakeStore implements out.CertificateStore for testing. | |
| 109 | + | // fakeStore is a hand-rolled implementation of out.CertificateStore that | |
| 110 | + | // manages certificates in memory with copy-on-read semantics and a separate | |
| 111 | + | // lock for Lock() to avoid deadlock with the data mutex. Generated mocks | |
| 112 | + | // cannot easily replicate the Lock/unlock lifecycle used in Reconcile. | |
| 102 | 113 | type fakeStore struct { | |
| 103 | 114 | mu sync.Mutex | |
| 104 | 115 | lockMu sync.Mutex // separate lock for Lock()/unlock to avoid deadlock with mu | |
| @@ -493,14 +504,9 @@ func TestServiceLoadParsesPEMIntoTLSCertificate(t *testing.T) { | |||
| 493 | 504 | err = svc.Load(ctx) | |
| 494 | 505 | require.NoError(t, err) | |
| 495 | 506 | ||
| 496 | - | // Check that the cached cert has a valid tls.Certificate. | |
| 497 | - | //nolint:unused // accessing the cert through the map is sufficient | |
| 498 | - | svc.mu.Lock() | |
| 499 | - | cached, ok := svc.certs["http01-test.example.com"] | |
| 500 | - | svc.mu.Unlock() | |
| 501 | - | ||
| 502 | - | require.True(t, ok, "cert should be cached") | |
| 503 | - | require.NotNil(t, cached) | |
| 507 | + | // Verify the cached cert has a valid tls.Certificate via the accessor. | |
| 508 | + | cached := svc.GetStoredCertificate("http01-test.example.com") | |
| 509 | + | require.NotNil(t, cached, "cert should be cached") | |
| 504 | 510 | assert.NotEmpty(t, cached.Certificate.Certificate, "tls.Certificate should be populated") | |
| 505 | 511 | assert.NotNil(t, cached.Certificate.PrivateKey, "private key should be populated") | |
| 506 | 512 | ||
| @@ -25,7 +25,11 @@ import ( | |||||
| 25 | // Fakes | 25 | // Fakes | ||
| 26 | // --------------------------------------------------------------------------- | 26 | // --------------------------------------------------------------------------- | ||
| 27 | 27 | ||||
| 28 | - | // fakeRoutes implements RouteSource for testing. | 28 | + | // fakeRoutes models a stateful RouteSource using an internal mutex and |
| 29 | + | // copy-on-read semantics. Generated mocks cannot easily replicate this | |||
| 30 | + | // pattern because the interface methods return slices/maps by value and | |||
| 31 | + | // callers may modify the returned data; a hand-rolled fake ensures each | |||
| 32 | + | // GetRoutes/GetExternalRoutes call returns a defensive copy. | |||
| 29 | type fakeRoutes struct { | 33 | type fakeRoutes struct { | ||
| 30 | mu sync.Mutex | 34 | mu sync.Mutex | ||
| 31 | routes []domain.Route | 35 | routes []domain.Route | ||
| @@ -50,7 +54,11 @@ func (f *fakeRoutes) GetExternalRoutes() map[string]string { | |||||
| 50 | return out | 54 | return out | ||
| 51 | } | 55 | } | ||
| 52 | 56 | ||||
| 53 | - | // fakeIssuer implements out.PublicCertificateIssuer for testing. | 57 | + | // fakeIssuer is a hand-rolled implementation of out.PublicCertificateIssuer |
| 58 | + | // that records issued orders and delegates to pluggable obtain/renew | |||
| 59 | + | // functions. Generated mocks are less convenient here because the test needs | |||
| 60 | + | // to inspect captured orders after the call completes — a real slice on the | |||
| 61 | + | // fake avoids mock expectation boilerplate. | |||
| 54 | type fakeIssuer struct { | 62 | type fakeIssuer struct { | ||
| 55 | mu sync.Mutex | 63 | mu sync.Mutex | ||
| 56 | orders []out.CertificateOrder // recorded orders | 64 | orders []out.CertificateOrder // recorded orders | ||
| @@ -98,7 +106,10 @@ func defaultStoredCert(order out.CertificateOrder) (*out.StoredCertificate, erro | |||||
| 98 | }, nil | 106 | }, nil | ||
| 99 | } | 107 | } | ||
| 100 | 108 | ||||
| 101 | - | // fakeStore implements out.CertificateStore for testing. | 109 | + | // fakeStore is a hand-rolled implementation of out.CertificateStore that |
| 110 | + | // manages certificates in memory with copy-on-read semantics and a separate | |||
| 111 | + | // lock for Lock() to avoid deadlock with the data mutex. Generated mocks | |||
| 112 | + | // cannot easily replicate the Lock/unlock lifecycle used in Reconcile. | |||
| 102 | type fakeStore struct { | 113 | type fakeStore struct { | ||
| 103 | mu sync.Mutex | 114 | mu sync.Mutex | ||
| 104 | lockMu sync.Mutex // separate lock for Lock()/unlock to avoid deadlock with mu | 115 | lockMu sync.Mutex // separate lock for Lock()/unlock to avoid deadlock with mu | ||
| @@ -493,14 +504,9 @@ func TestServiceLoadParsesPEMIntoTLSCertificate(t *testing.T) { | |||||
| 493 | err = svc.Load(ctx) | 504 | err = svc.Load(ctx) | ||
| 494 | require.NoError(t, err) | 505 | require.NoError(t, err) | ||
| 495 | 506 | ||||
| 496 | - | // Check that the cached cert has a valid tls.Certificate. | 507 | + | // Verify the cached cert has a valid tls.Certificate via the accessor. |
| 497 | - | //nolint:unused // accessing the cert through the map is sufficient | 508 | + | cached := svc.GetStoredCertificate("http01-test.example.com") |
| 498 | - | svc.mu.Lock() | 509 | + | require.NotNil(t, cached, "cert should be cached") |
| 499 | - | cached, ok := svc.certs["http01-test.example.com"] | |||
| 500 | - | svc.mu.Unlock() | |||
| 501 | - | ||||
| 502 | - | require.True(t, ok, "cert should be cached") | |||
| 503 | - | require.NotNil(t, cached) | |||
| 504 | assert.NotEmpty(t, cached.Certificate.Certificate, "tls.Certificate should be populated") | 510 | assert.NotEmpty(t, cached.Certificate.Certificate, "tls.Certificate should be populated") | ||
| 505 | assert.NotNil(t, cached.Certificate.PrivateKey, "private key should be populated") | 511 | assert.NotNil(t, cached.Certificate.PrivateKey, "private key should be populated") | ||
| 506 | 512 | ||||
M internal/usecase/publictls/status.go +9 -61
| @@ -2,8 +2,8 @@ package publictls | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "regexp" | |
| 5 | 6 | "sort" | |
| 6 | - | "strings" | |
| 7 | 7 | "time" | |
| 8 | 8 | ||
| 9 | 9 | "github.com/bnema/gordon/internal/domain" | |
| @@ -130,74 +130,22 @@ func (s *Service) findCoverageLocked(routeDomain string, managedCerts []domain.M | |||
| 130 | 130 | return false, "" | |
| 131 | 131 | } | |
| 132 | 132 | ||
| 133 | + | // keyEqValuePattern matches key=value patterns for known sensitive keys | |
| 134 | + | // in a case-insensitive manner. | |
| 135 | + | var keyEqValuePattern = regexp.MustCompile(`(?i)(token|secret|key|password|pass|api_key|apikey|auth)=[^\s]+`) | |
| 136 | + | ||
| 133 | 137 | // sanitizeError removes obvious sensitive strings from error messages. | |
| 134 | - | // At minimum it ensures that the literal "secret" and patterns like | |
| 135 | - | // "token=secret" are not present in the output. | |
| 138 | + | // It redacts key=value patterns for known sensitive keys while preserving | |
| 139 | + | // safe context. It does not blanket-redact messages just because they | |
| 140 | + | // contain the word "secret" or similar. | |
| 136 | 141 | func sanitizeError(err string) string { | |
| 137 | 142 | if err == "" { | |
| 138 | 143 | return "" | |
| 139 | 144 | } | |
| 140 | 145 | ||
| 141 | - | result := err | |
| 142 | - | ||
| 143 | - | // Remove key=value patterns for well-known sensitive keys. | |
| 144 | - | result = sanitizeKeyEqValue(result, "token") | |
| 145 | - | result = sanitizeKeyEqValue(result, "secret") | |
| 146 | - | result = sanitizeKeyEqValue(result, "key") | |
| 147 | - | result = sanitizeKeyEqValue(result, "password") | |
| 148 | - | result = sanitizeKeyEqValue(result, "pass") | |
| 149 | - | result = sanitizeKeyEqValue(result, "api_key") | |
| 150 | - | result = sanitizeKeyEqValue(result, "apikey") | |
| 151 | - | result = sanitizeKeyEqValue(result, "auth") | |
| 152 | - | ||
| 153 | - | // Replace any remaining occurrences of "secret" (case-insensitive). | |
| 154 | - | lower := strings.ToLower(result) | |
| 155 | - | if strings.Contains(lower, "secret") { | |
| 156 | - | result = "redacted" | |
| 157 | - | return result | |
| 158 | - | } | |
| 159 | - | ||
| 146 | + | result := keyEqValuePattern.ReplaceAllString(err, "$1=redacted") | |
| 160 | 147 | if result == "" { | |
| 161 | 148 | return "certificate error" | |
| 162 | 149 | } | |
| 163 | 150 | return result | |
| 164 | 151 | } | |
| 165 | - | ||
| 166 | - | // sanitizeKeyEqValue removes patterns like "key=value" from the string. | |
| 167 | - | // It replaces the value portion with "redacted". | |
| 168 | - | func sanitizeKeyEqValue(s, key string) string { | |
| 169 | - | prefix := key + "=" | |
| 170 | - | lowerPrefix := strings.ToLower(prefix) | |
| 171 | - | result := s | |
| 172 | - | pos := 0 | |
| 173 | - | ||
| 174 | - | for { | |
| 175 | - | lowerResult := strings.ToLower(result) | |
| 176 | - | if pos >= len(lowerResult) { | |
| 177 | - | break | |
| 178 | - | } | |
| 179 | - | idx := strings.Index(lowerResult[pos:], lowerPrefix) | |
| 180 | - | if idx == -1 { | |
| 181 | - | break | |
| 182 | - | } | |
| 183 | - | idx += pos // make absolute | |
| 184 | - | ||
| 185 | - | // Find end of value (space, newline, or end of string). | |
| 186 | - | valStart := idx + len(prefix) | |
| 187 | - | rest := result[valStart:] | |
| 188 | - | end := strings.IndexAny(rest, " \n") | |
| 189 | - | ||
| 190 | - | replacement := key + "=redacted" | |
| 191 | - | var after string | |
| 192 | - | if end == -1 { | |
| 193 | - | after = "" | |
| 194 | - | } else { | |
| 195 | - | after = rest[end:] | |
| 196 | - | } | |
| 197 | - | result = result[:idx] + replacement + after | |
| 198 | - | ||
| 199 | - | // Advance search position past the replacement to avoid re-matching. | |
| 200 | - | pos = idx + len(replacement) | |
| 201 | - | } | |
| 202 | - | return result | |
| 203 | - | } | |
| @@ -2,8 +2,8 @@ package publictls | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "regexp" | |||
| 5 | "sort" | 6 | "sort" | ||
| 6 | - | "strings" | |||
| 7 | "time" | 7 | "time" | ||
| 8 | 8 | ||||
| 9 | "github.com/bnema/gordon/internal/domain" | 9 | "github.com/bnema/gordon/internal/domain" | ||
| @@ -130,74 +130,22 @@ func (s *Service) findCoverageLocked(routeDomain string, managedCerts []domain.M | |||||
| 130 | return false, "" | 130 | return false, "" | ||
| 131 | } | 131 | } | ||
| 132 | 132 | ||||
| 133 | + | // keyEqValuePattern matches key=value patterns for known sensitive keys | |||
| 134 | + | // in a case-insensitive manner. | |||
| 135 | + | var keyEqValuePattern = regexp.MustCompile(`(?i)(token|secret|key|password|pass|api_key|apikey|auth)=[^\s]+`) | |||
| 136 | + | ||||
| 133 | // sanitizeError removes obvious sensitive strings from error messages. | 137 | // sanitizeError removes obvious sensitive strings from error messages. | ||
| 134 | - | // At minimum it ensures that the literal "secret" and patterns like | 138 | + | // It redacts key=value patterns for known sensitive keys while preserving |
| 135 | - | // "token=secret" are not present in the output. | 139 | + | // safe context. It does not blanket-redact messages just because they |
| 140 | + | // contain the word "secret" or similar. | |||
| 136 | func sanitizeError(err string) string { | 141 | func sanitizeError(err string) string { | ||
| 137 | if err == "" { | 142 | if err == "" { | ||
| 138 | return "" | 143 | return "" | ||
| 139 | } | 144 | } | ||
| 140 | 145 | ||||
| 141 | - | result := err | 146 | + | result := keyEqValuePattern.ReplaceAllString(err, "$1=redacted") |
| 142 | - | ||||
| 143 | - | // Remove key=value patterns for well-known sensitive keys. | |||
| 144 | - | result = sanitizeKeyEqValue(result, "token") | |||
| 145 | - | result = sanitizeKeyEqValue(result, "secret") | |||
| 146 | - | result = sanitizeKeyEqValue(result, "key") | |||
| 147 | - | result = sanitizeKeyEqValue(result, "password") | |||
| 148 | - | result = sanitizeKeyEqValue(result, "pass") | |||
| 149 | - | result = sanitizeKeyEqValue(result, "api_key") | |||
| 150 | - | result = sanitizeKeyEqValue(result, "apikey") | |||
| 151 | - | result = sanitizeKeyEqValue(result, "auth") | |||
| 152 | - | ||||
| 153 | - | // Replace any remaining occurrences of "secret" (case-insensitive). | |||
| 154 | - | lower := strings.ToLower(result) | |||
| 155 | - | if strings.Contains(lower, "secret") { | |||
| 156 | - | result = "redacted" | |||
| 157 | - | return result | |||
| 158 | - | } | |||
| 159 | - | ||||
| 160 | if result == "" { | 147 | if result == "" { | ||
| 161 | return "certificate error" | 148 | return "certificate error" | ||
| 162 | } | 149 | } | ||
| 163 | return result | 150 | return result | ||
| 164 | } | 151 | } | ||
| 165 | - | ||||
| 166 | - | // sanitizeKeyEqValue removes patterns like "key=value" from the string. | |||
| 167 | - | // It replaces the value portion with "redacted". | |||
| 168 | - | func sanitizeKeyEqValue(s, key string) string { | |||
| 169 | - | prefix := key + "=" | |||
| 170 | - | lowerPrefix := strings.ToLower(prefix) | |||
| 171 | - | result := s | |||
| 172 | - | pos := 0 | |||
| 173 | - | ||||
| 174 | - | for { | |||
| 175 | - | lowerResult := strings.ToLower(result) | |||
| 176 | - | if pos >= len(lowerResult) { | |||
| 177 | - | break | |||
| 178 | - | } | |||
| 179 | - | idx := strings.Index(lowerResult[pos:], lowerPrefix) | |||
| 180 | - | if idx == -1 { | |||
| 181 | - | break | |||
| 182 | - | } | |||
| 183 | - | idx += pos // make absolute | |||
| 184 | - | ||||
| 185 | - | // Find end of value (space, newline, or end of string). | |||
| 186 | - | valStart := idx + len(prefix) | |||
| 187 | - | rest := result[valStart:] | |||
| 188 | - | end := strings.IndexAny(rest, " \n") | |||
| 189 | - | ||||
| 190 | - | replacement := key + "=redacted" | |||
| 191 | - | var after string | |||
| 192 | - | if end == -1 { | |||
| 193 | - | after = "" | |||
| 194 | - | } else { | |||
| 195 | - | after = rest[end:] | |||
| 196 | - | } | |||
| 197 | - | result = result[:idx] + replacement + after | |||
| 198 | - | ||||
| 199 | - | // Advance search position past the replacement to avoid re-matching. | |||
| 200 | - | pos = idx + len(replacement) | |||
| 201 | - | } | |||
| 202 | - | return result | |||
| 203 | - | } | |||