From 8cda8600268714973412c29d70306acbe941c591 Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Sat, 2 May 2026 19:13:18 +0000 Subject: [PATCH] caddy: fix duration metric + file-mode PEM validate + api-mode idempotency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes Bundle 9 of the 2026-05-02 deployment-target coverage audit (see cowork/deployment-target-audit-2026-05-02/RESULTS.md). Three small independent fixes that share one connector file: 1. Duration metric (caddy.go L176). Pre-fix: "duration_ms": fmt.Sprintf("%d", time.Since(time.Now()).Milliseconds()) This always returned ~0ms because time.Now() was called twice — the second call captured a baseline immediately before time.Since computed the delta. The intended baseline is `startTime` declared at L113 and threaded through deployViaFile correctly. Post-fix: "duration_ms": fmt.Sprintf("%d", time.Since(startTime).Milliseconds()) deployViaAPI's signature evolves to take startTime time.Time so the api-mode path uses the same baseline as the file-mode path. 2. File-mode ValidateDeployment now validates PEM syntax. Pre-fix (caddy.go L266-293) checked file existence only via os.Stat. A cert file containing garbage bytes passed validation; Caddy's file-watcher silently failed to load it; operators saw "validation green" + "TLS handshake fails" with no obvious connection. Post-fix: after the os.Stat checks succeed, os.ReadFile + parse the first PEM block as an x509 cert via the shared certutil.ParseCertificatePEM helper. Failure surfaces as Valid=false with a clear "not valid PEM/x509" message. 3. API-mode idempotency short-circuit. Pre-fix, every deploy POSTed to /config/apps/tls/certificates/load even when the active cert was already what we wanted to deploy. Caddy reloads TLS state on every POST, briefly bumping CPU and possibly disrupting connections in flight. Post-fix: idempotencySkipPOST runs a GET first, parses the response (handles BOTH the array-of-objects and single-object shapes Caddy admin can return), SHA-256 compares the entry's `cert` field to the deploy payload's cert bytes, and skips the POST when match. Result.Metadata["idempotent"]="true" surfaces the no-op. Conservative: any GET failure (network, non-200, parse error, no matching entry, hash mismatch) silently falls through to the POST, preserving today's behavior. Idempotency is a fast path, not a correctness boundary — false negatives are safe; false positives are dangerous. Tests added to caddy_test.go (6 new tests, ~290 LOC): - TestCaddy_API_DurationMetric_NonZero (httptest server with a 10ms sleep in the POST handler; asserts duration_ms parses as int >= 5). - TestCaddy_ValidateDeployment_FileMode_MalformedPEM_Rejected (writes garbage to cert.pem; asserts Valid=false with PEM/x509 in message). - TestCaddy_ValidateDeployment_FileMode_ValidPEM_Accepted (writes a real ECDSA P-256 self-signed cert; asserts Valid=true). - TestCaddy_API_Idempotent_SkipsPOSTWhenCertHashMatches (GET response contains the same cert as the deploy payload; POST counter remains 0; metadata.idempotent=true; exactly 1 GET probe ran). - TestCaddy_API_Idempotent_RunsPOSTWhenCertHashDiffers (GET response contains a DIFFERENT cert; POST counter is 1; idempotent absent). - TestCaddy_API_Idempotent_GETFails_FallsThroughToPOST (GET returns 500; POST still runs; deploy succeeds; idempotent absent). Two existing tests updated to match the new contracts: - TestCaddyConnector_DeployViaAPI_Success: mock handler now serves BOTH GET (returns "[]" so the comparison falls through) and POST (the original 200-OK path). The dispatch is a method-switch inside the path-match branch. - TestCaddyConnector_ValidateDeployment_Success: the placeholder cert "MIIC..." used to pass the old existence-only check; post-Fix-2 it fails the PEM-parse check. Test now uses generateTestCertAndKey to produce a real self-signed ECDSA P-256 cert. generateTestCertAndKey helper added to the test file — same pattern the javakeystore + wincertstore tests use, kept local because the caddy package has no other test in the certutil family that would make a shared helper cleaner. Verified locally: - gofmt -l ./internal/connector/target/caddy/ clean - go vet ./internal/connector/target/caddy/ clean - go build ./cmd/agent/... clean (factory wiring unchanged) - go test -race -count=1 ./internal/connector/target/caddy/ green (16 tests total: 11 pre-existing including the two updated + 6 new) Audit reference: cowork/deployment-target-audit-2026-05-02/RESULTS.md Bundle 9. --- internal/connector/target/caddy/caddy.go | 138 +++++- internal/connector/target/caddy/caddy_test.go | 424 +++++++++++++++++- 2 files changed, 538 insertions(+), 24 deletions(-) diff --git a/internal/connector/target/caddy/caddy.go b/internal/connector/target/caddy/caddy.go index 7368eb8..55cc75f 100644 --- a/internal/connector/target/caddy/caddy.go +++ b/internal/connector/target/caddy/caddy.go @@ -3,6 +3,7 @@ package caddy import ( "bytes" "context" + "crypto/sha256" "encoding/json" "fmt" "io" @@ -13,6 +14,7 @@ import ( "time" "github.com/shankar0123/certctl/internal/connector/target" + "github.com/shankar0123/certctl/internal/connector/target/certutil" "github.com/shankar0123/certctl/internal/deploy" ) @@ -114,7 +116,7 @@ func (c *Connector) DeployCertificate(ctx context.Context, request target.Deploy // Try API mode if configured if c.config.Mode == "api" { - result, err := c.deployViaAPI(ctx, request) + result, err := c.deployViaAPI(ctx, request, startTime) if err == nil { c.logger.Info("certificate deployed to Caddy via API", "duration", time.Since(startTime).String()) @@ -128,7 +130,15 @@ func (c *Connector) DeployCertificate(ctx context.Context, request target.Deploy } // deployViaAPI deploys a certificate using Caddy's admin API. -func (c *Connector) deployViaAPI(ctx context.Context, request target.DeploymentRequest) (*target.DeploymentResult, error) { +// +// Bundle 9 of the 2026-05-02 deployment-target audit: +// - Fix 1: duration_ms metric uses the caller-passed startTime (was +// time.Since(time.Now()) which always rounded to ~0ms). +// - Fix 3: SHA-256 idempotency short-circuit — GET the current loaded +// certs and compare to the deploy payload's cert bytes; skip the POST +// when they match. Conservative: any GET failure (network, non-200, +// parse error, hash mismatch) falls through to today's POST behavior. +func (c *Connector) deployViaAPI(ctx context.Context, request target.DeploymentRequest, startTime time.Time) (*target.DeploymentResult, error) { c.logger.Debug("attempting API deployment", "url", c.config.AdminAPI) // Build the certificate payload with combined cert and chain @@ -137,13 +147,41 @@ func (c *Connector) deployViaAPI(ctx context.Context, request target.DeploymentR certData += request.ChainPEM + "\n" } + apiURL := c.config.AdminAPI + "/config/apps/tls/certificates/load" + + // Bundle 9 Fix 3: idempotency short-circuit. GET the current loaded + // certs and SHA-256 compare to the deploy payload — if they match, + // skip the POST and return success with metadata.idempotent=true. + // Caddy's response shape varies between operators / config versions; + // handle BOTH the array-of-objects and single-object shapes. Any + // failure during the GET (network, non-200, JSON parse error, hash + // mismatch) silently falls through to the POST below — false + // negatives are safe; false positives are dangerous. + deployHash := sha256.Sum256([]byte(certData)) + if c.idempotencySkipPOST(ctx, apiURL, deployHash) { + c.logger.Debug("Caddy already has this certificate loaded; skipping POST", + "admin_url", c.config.AdminAPI) + return &target.DeploymentResult{ + Success: true, + TargetAddress: c.config.AdminAPI, + DeploymentID: fmt.Sprintf("caddy-api-idem-%d", time.Now().Unix()), + Message: "Caddy already has this certificate loaded (idempotent skip)", + DeployedAt: time.Now(), + Metadata: map[string]string{ + "method": "api", + "admin_url": c.config.AdminAPI, + "idempotent": "true", + "duration_ms": fmt.Sprintf("%d", time.Since(startTime).Milliseconds()), + }, + }, nil + } + payload := map[string]string{ "cert": certData, "key": request.KeyPEM, } bodyBytes, _ := json.Marshal(payload) - apiURL := c.config.AdminAPI + "/config/apps/tls/certificates/load" req, err := http.NewRequestWithContext(ctx, "POST", apiURL, bytes.NewReader(bodyBytes)) if err != nil { @@ -173,11 +211,65 @@ func (c *Connector) deployViaAPI(ctx context.Context, request target.DeploymentR Metadata: map[string]string{ "method": "api", "admin_url": c.config.AdminAPI, - "duration_ms": fmt.Sprintf("%d", time.Since(time.Now()).Milliseconds()), + "duration_ms": fmt.Sprintf("%d", time.Since(startTime).Milliseconds()), }, }, nil } +// idempotencySkipPOST returns true iff GET-ing the Caddy admin API at the +// same path returns a cert whose SHA-256 matches the deploy payload's +// SHA-256. The function is best-effort: it returns false (= "fall through +// to the POST") on any error along the way (network, non-200, JSON parse, +// no matching entry, hash mismatch). +// +// Caddy's response shape can be either: +// - a single object: {"cert": "...", "key": "..."} +// - an array of objects: [{"cert": "...", "key": "..."}, ...] +// +// We try both shapes; the first that decodes AND has a matching hash wins. +// +// Bundle 9 Fix 3 of the 2026-05-02 deployment-target audit. +func (c *Connector) idempotencySkipPOST(ctx context.Context, apiURL string, deployHash [32]byte) bool { + getReq, err := http.NewRequestWithContext(ctx, "GET", apiURL, nil) + if err != nil { + return false + } + getReq.Header.Set("Accept", "application/json") + resp, err := c.client.Do(getReq) + if err != nil { + return false + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return false + } + body, err := io.ReadAll(resp.Body) + if err != nil { + return false + } + // Try array-of-objects shape first. + var asArray []map[string]string + if err := json.Unmarshal(body, &asArray); err == nil { + for _, entry := range asArray { + if entry["cert"] == "" { + continue + } + if sha256.Sum256([]byte(entry["cert"])) == deployHash { + return true + } + } + return false + } + // Fall back to single-object shape. + var asObject map[string]string + if err := json.Unmarshal(body, &asObject); err == nil { + if asObject["cert"] != "" && sha256.Sum256([]byte(asObject["cert"])) == deployHash { + return true + } + } + return false +} + // deployViaFile deploys a certificate by writing files to the cert directory. func (c *Connector) deployViaFile(ctx context.Context, request target.DeploymentRequest, startTime time.Time) (*target.DeploymentResult, error) { c.logger.Debug("deploying via file mode", "cert_dir", c.config.CertDir) @@ -262,7 +354,15 @@ func (c *Connector) ValidateDeployment(ctx context.Context, request target.Valid startTime := time.Now() - // For file mode, verify files exist + // For file mode, verify files exist AND the cert is valid PEM/x509. + // + // Bundle 9 Fix 2 of the 2026-05-02 deployment-target audit: pre-fix, + // this branch checked file existence only (os.Stat). A cert file that + // existed but contained malformed bytes passed validation; Caddy's + // file-watcher silently failed to load it; operators saw "validation + // green" + "TLS handshake fails" with no obvious connection. Post-fix, + // the cert file is read and parsed via certutil.ParseCertificatePEM so + // only files that Caddy can actually load pass validation. if c.config.Mode == "file" || c.config.CertDir != "" { certPath := filepath.Join(c.config.CertDir, c.config.CertFile) keyPath := filepath.Join(c.config.CertDir, c.config.KeyFile) @@ -290,6 +390,34 @@ func (c *Connector) ValidateDeployment(ctx context.Context, request target.Valid ValidatedAt: time.Now(), }, fmt.Errorf("%s", errMsg) } + + // Bundle 9 Fix 2: PEM-parse the cert file. A malformed cert that + // Caddy's watcher would silently reject must fail validation here + // instead of bubbling up as a confusing "TLS handshake fails" + // later. + certBytes, err := os.ReadFile(certPath) + if err != nil { + errMsg := fmt.Sprintf("failed to read certificate file %s: %v", certPath, err) + c.logger.Error("validation failed", "error", err) + return &target.ValidationResult{ + Valid: false, + Serial: request.Serial, + TargetAddress: certPath, + Message: errMsg, + ValidatedAt: time.Now(), + }, fmt.Errorf("%s", errMsg) + } + if _, err := certutil.ParseCertificatePEM(string(certBytes)); err != nil { + errMsg := fmt.Sprintf("certificate file %s is not valid PEM/x509: %v", certPath, err) + c.logger.Error("validation failed", "error", err) + return &target.ValidationResult{ + Valid: false, + Serial: request.Serial, + TargetAddress: certPath, + Message: errMsg, + ValidatedAt: time.Now(), + }, fmt.Errorf("%s", errMsg) + } } validationDuration := time.Since(startTime) diff --git a/internal/connector/target/caddy/caddy_test.go b/internal/connector/target/caddy/caddy_test.go index 7d954b0..25ede5b 100644 --- a/internal/connector/target/caddy/caddy_test.go +++ b/internal/connector/target/caddy/caddy_test.go @@ -2,20 +2,64 @@ package caddy_test import ( "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" "encoding/json" + "encoding/pem" + "fmt" "io" "log/slog" + "math/big" "net/http" "net/http/httptest" "os" "path/filepath" + "strconv" "strings" + "sync/atomic" "testing" + "time" "github.com/shankar0123/certctl/internal/connector/target" "github.com/shankar0123/certctl/internal/connector/target/caddy" ) +// generateTestCertAndKey creates a self-signed cert + ECDSA key for tests +// that exercise the file-mode PEM-validation path added in Bundle 9 (the +// 2026-05-02 deployment-target audit). Pre-Bundle-9 the placeholder +// "-----BEGIN CERTIFICATE-----\nMIIC...\n-----END CERTIFICATE-----" was +// enough because ValidateDeployment only checked file existence; Fix 2 +// of Bundle 9 PEM-parses the file via certutil.ParseCertificatePEM, so +// real test certs are required wherever the test deploys-then-validates. +func generateTestCertAndKey(t *testing.T) (string, string) { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generate key: %v", err) + } + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "caddy-test.example.com"}, + NotBefore: time.Now().Add(-1 * time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + } + certDER, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key) + if err != nil { + t.Fatalf("create cert: %v", err) + } + certPEM := string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER})) + keyDER, err := x509.MarshalPKCS8PrivateKey(key) + if err != nil { + t.Fatalf("marshal key: %v", err) + } + keyPEM := string(pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: keyDER})) + return certPEM, keyPEM +} + func TestCaddyConnector_ValidateConfig_Success(t *testing.T) { logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug})) ctx := context.Background() @@ -107,24 +151,37 @@ func TestCaddyConnector_DeployViaAPI_Success(t *testing.T) { logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug})) ctx := context.Background() - // Create a mock Caddy admin API server + // Create a mock Caddy admin API server. + // + // Bundle 9 Fix 3 of the 2026-05-02 deployment-target audit added an + // idempotency short-circuit: the connector now GETs the load endpoint + // first to compare the active cert hash with the deploy payload. The + // GET returns an empty array so the comparison falls through to the + // POST (which is what this test exercises). server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if strings.Contains(r.URL.Path, "/config/apps/tls/certificates/load") { - // Verify POST request with JSON body - if r.Method != "POST" { - t.Fatalf("expected POST, got %s", r.Method) + switch r.Method { + case "GET": + // Idempotency probe — return empty so the connector falls + // through to the POST path that this test asserts on. + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + w.Write([]byte("[]")) + return + case "POST": + body, _ := io.ReadAll(r.Body) + var payload map[string]string + json.Unmarshal(body, &payload) + if payload["cert"] == "" { + t.Fatal("cert field missing in payload") + } + if payload["key"] == "" { + t.Fatal("key field missing in payload") + } + w.WriteHeader(http.StatusOK) + return } - body, _ := io.ReadAll(r.Body) - var payload map[string]string - json.Unmarshal(body, &payload) - if payload["cert"] == "" { - t.Fatal("cert field missing in payload") - } - if payload["key"] == "" { - t.Fatal("key field missing in payload") - } - w.WriteHeader(http.StatusOK) - return + t.Fatalf("unexpected method: %s", r.Method) } w.WriteHeader(http.StatusNotFound) })) @@ -298,11 +355,13 @@ func TestCaddyConnector_ValidateDeployment_Success(t *testing.T) { rawConfig, _ := json.Marshal(cfg) _ = connector.ValidateConfig(ctx, rawConfig) - // Deploy a certificate + // Bundle 9 Fix 2: ValidateDeployment now PEM-parses the cert file, so + // the deploy-then-validate flow needs a real test cert (placeholder + // "MIIC..." would fail the new PEM-parse check). + certPEM, keyPEM := generateTestCertAndKey(t) deployRequest := target.DeploymentRequest{ - CertPEM: "-----BEGIN CERTIFICATE-----\nMIIC...\n-----END CERTIFICATE-----", - KeyPEM: "-----BEGIN PRIVATE KEY-----\nMIIE...\n-----END PRIVATE KEY-----", - ChainPEM: "-----BEGIN CERTIFICATE-----\nMIIC...\n-----END CERTIFICATE-----", + CertPEM: certPEM, + KeyPEM: keyPEM, } connector.DeployCertificate(ctx, deployRequest) @@ -396,3 +455,330 @@ func TestCaddyConnector_APIMode_NoChain(t *testing.T) { t.Fatalf("deployment should succeed, got: %s", result.Message) } } + +// --- Bundle 9: duration metric + file-mode PEM validate + api-mode idempotency --- +// +// Six tests pin the three independent fixes added in Bundle 9 of the +// 2026-05-02 deployment-target audit: +// - Fix 1 (duration metric L176): TestCaddy_API_DurationMetric_NonZero. +// - Fix 2 (file-mode PEM validate): +// TestCaddy_ValidateDeployment_FileMode_MalformedPEM_Rejected, +// TestCaddy_ValidateDeployment_FileMode_ValidPEM_Accepted. +// - Fix 3 (api-mode idempotency short-circuit): +// TestCaddy_API_Idempotent_SkipsPOSTWhenCertHashMatches, +// TestCaddy_API_Idempotent_RunsPOSTWhenCertHashDiffers, +// TestCaddy_API_Idempotent_GETFails_FallsThroughToPOST. + +func TestCaddy_API_DurationMetric_NonZero(t *testing.T) { + logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug})) + ctx := context.Background() + + // Bundle 9 Fix 1: pre-fix the api-mode duration_ms metric was + // computed as time.Since(time.Now()).Milliseconds() which always + // rounded to ~0ms. Post-fix it uses the startTime captured in + // DeployCertificate. Add a small artificial delay in the handler so + // the asserted duration_ms is unambiguously non-zero. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.URL.Path, "/config/apps/tls/certificates/load") { + w.WriteHeader(http.StatusNotFound) + return + } + switch r.Method { + case "GET": + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + w.Write([]byte("[]")) + case "POST": + // Simulate a slow Caddy admin reload — 10ms is enough to + // produce a measurable duration_ms. + time.Sleep(10 * time.Millisecond) + w.WriteHeader(http.StatusOK) + default: + w.WriteHeader(http.StatusMethodNotAllowed) + } + })) + defer server.Close() + + cfg := caddy.Config{AdminAPI: server.URL, Mode: "api"} + connector := caddy.New(&cfg, logger) + rawConfig, _ := json.Marshal(cfg) + _ = connector.ValidateConfig(ctx, rawConfig) + + certPEM, keyPEM := generateTestCertAndKey(t) + result, err := connector.DeployCertificate(ctx, target.DeploymentRequest{ + CertPEM: certPEM, + KeyPEM: keyPEM, + }) + if err != nil { + t.Fatalf("deploy failed: %v", err) + } + if !result.Success { + t.Fatalf("expected success, got: %s", result.Message) + } + + // duration_ms must parse as int >= 5 (we slept 10ms in the handler; + // allow some headroom for clock granularity on slow CI hosts). + durationStr := result.Metadata["duration_ms"] + if durationStr == "" { + t.Fatal("expected duration_ms in metadata") + } + durationMs, err := strconv.Atoi(durationStr) + if err != nil { + t.Fatalf("duration_ms is not int-parseable: %q (%v)", durationStr, err) + } + if durationMs < 5 { + t.Errorf("duration_ms = %d, expected >= 5 (handler slept 10ms; pre-Bundle-9 bug rounded this to 0)", durationMs) + } +} + +func TestCaddy_ValidateDeployment_FileMode_MalformedPEM_Rejected(t *testing.T) { + logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug})) + ctx := context.Background() + + // Bundle 9 Fix 2: ValidateDeployment now PEM-parses the cert file + // (was only os.Stat existence check). A cert file containing garbage + // passes existence-check but fails PEM-decode → Valid=false. + tmpDir := t.TempDir() + certPath := filepath.Join(tmpDir, "cert.pem") + keyPath := filepath.Join(tmpDir, "key.pem") + if err := os.WriteFile(certPath, []byte("this is not a PEM cert"), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(keyPath, []byte("this is not a key either"), 0600); err != nil { + t.Fatal(err) + } + + cfg := caddy.Config{ + AdminAPI: "http://localhost:2019", + CertDir: tmpDir, + CertFile: "cert.pem", + KeyFile: "key.pem", + Mode: "file", + } + connector := caddy.New(&cfg, logger) + rawConfig, _ := json.Marshal(cfg) + _ = connector.ValidateConfig(ctx, rawConfig) + + result, err := connector.ValidateDeployment(ctx, target.ValidationRequest{ + CertificateID: "mc-test", + Serial: "0xDEADBEEF", + }) + if err == nil { + t.Fatal("expected error when cert file is malformed PEM") + } + if result.Valid { + t.Fatal("expected Valid=false for malformed PEM") + } + // Error message must reference the PEM/x509 failure so operators see + // what's wrong rather than a confusing downstream symptom. + if !strings.Contains(result.Message, "PEM") && !strings.Contains(result.Message, "x509") { + t.Errorf("expected error message to mention PEM/x509, got: %s", result.Message) + } +} + +func TestCaddy_ValidateDeployment_FileMode_ValidPEM_Accepted(t *testing.T) { + logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug})) + ctx := context.Background() + + // Bundle 9 Fix 2: a real test cert + key passes both the os.Stat + // existence check and the new certutil.ParseCertificatePEM check. + tmpDir := t.TempDir() + certPath := filepath.Join(tmpDir, "cert.pem") + keyPath := filepath.Join(tmpDir, "key.pem") + certPEM, keyPEM := generateTestCertAndKey(t) + if err := os.WriteFile(certPath, []byte(certPEM), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(keyPath, []byte(keyPEM), 0600); err != nil { + t.Fatal(err) + } + + cfg := caddy.Config{ + AdminAPI: "http://localhost:2019", + CertDir: tmpDir, + CertFile: "cert.pem", + KeyFile: "key.pem", + Mode: "file", + } + connector := caddy.New(&cfg, logger) + rawConfig, _ := json.Marshal(cfg) + _ = connector.ValidateConfig(ctx, rawConfig) + + result, err := connector.ValidateDeployment(ctx, target.ValidationRequest{ + CertificateID: "mc-test", + Serial: "1", + }) + if err != nil { + t.Fatalf("ValidateDeployment failed unexpectedly: %v (msg: %s)", err, result.Message) + } + if !result.Valid { + t.Errorf("expected Valid=true for a real PEM cert, got: %s", result.Message) + } +} + +func TestCaddy_API_Idempotent_SkipsPOSTWhenCertHashMatches(t *testing.T) { + logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug})) + ctx := context.Background() + + // Bundle 9 Fix 3: the connector GETs first, hashes the active cert, + // and skips the POST when SHA-256 matches. Build the deploy payload + // (cert + trailing newline; no chain in this test) and seed the mock + // GET response with an identical cert string so the hash matches and + // the POST counter stays at 0. + certPEM, keyPEM := generateTestCertAndKey(t) + expectedCertField := certPEM + "\n" // matches deployViaAPI's "request.CertPEM + \"\\n\"" build step + + var postCount, getCount atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.URL.Path, "/config/apps/tls/certificates/load") { + w.WriteHeader(http.StatusNotFound) + return + } + switch r.Method { + case "GET": + getCount.Add(1) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + payload, _ := json.Marshal([]map[string]string{ + {"cert": expectedCertField, "key": keyPEM}, + }) + w.Write(payload) + case "POST": + postCount.Add(1) + w.WriteHeader(http.StatusOK) + default: + w.WriteHeader(http.StatusMethodNotAllowed) + } + })) + defer server.Close() + + cfg := caddy.Config{AdminAPI: server.URL, Mode: "api"} + connector := caddy.New(&cfg, logger) + rawConfig, _ := json.Marshal(cfg) + _ = connector.ValidateConfig(ctx, rawConfig) + + result, err := connector.DeployCertificate(ctx, target.DeploymentRequest{ + CertPEM: certPEM, + KeyPEM: keyPEM, + }) + if err != nil { + t.Fatalf("deploy failed: %v", err) + } + if !result.Success { + t.Fatalf("expected success, got: %s", result.Message) + } + if got := postCount.Load(); got != 0 { + t.Errorf("expected 0 POST calls (idempotent skip), got %d", got) + } + if got := getCount.Load(); got != 1 { + t.Errorf("expected exactly 1 GET call (idempotency probe), got %d", got) + } + if result.Metadata["idempotent"] != "true" { + t.Errorf("expected metadata.idempotent=true on the skip path, got: %q", result.Metadata["idempotent"]) + } +} + +func TestCaddy_API_Idempotent_RunsPOSTWhenCertHashDiffers(t *testing.T) { + logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug})) + ctx := context.Background() + + // Bundle 9 Fix 3: when the GET response's cert hash differs from + // the deploy payload, fall through to the POST. metadata.idempotent + // must NOT be set on the POST path (only on the skip path). + certPEM, keyPEM := generateTestCertAndKey(t) + differentCert, _ := generateTestCertAndKey(t) // a DIFFERENT cert in the GET response + + var postCount atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.URL.Path, "/config/apps/tls/certificates/load") { + w.WriteHeader(http.StatusNotFound) + return + } + switch r.Method { + case "GET": + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + payload, _ := json.Marshal([]map[string]string{ + {"cert": differentCert, "key": "different-key"}, + }) + w.Write(payload) + case "POST": + postCount.Add(1) + w.WriteHeader(http.StatusOK) + } + })) + defer server.Close() + + cfg := caddy.Config{AdminAPI: server.URL, Mode: "api"} + connector := caddy.New(&cfg, logger) + rawConfig, _ := json.Marshal(cfg) + _ = connector.ValidateConfig(ctx, rawConfig) + + result, err := connector.DeployCertificate(ctx, target.DeploymentRequest{ + CertPEM: certPEM, + KeyPEM: keyPEM, + }) + if err != nil { + t.Fatalf("deploy failed: %v", err) + } + if !result.Success { + t.Fatalf("expected success, got: %s", result.Message) + } + if got := postCount.Load(); got != 1 { + t.Errorf("expected exactly 1 POST call (cert-hash mismatch fell through), got %d", got) + } + if result.Metadata["idempotent"] == "true" { + t.Error("expected metadata.idempotent absent or false on the non-idempotent POST path") + } +} + +func TestCaddy_API_Idempotent_GETFails_FallsThroughToPOST(t *testing.T) { + logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug})) + ctx := context.Background() + + // Bundle 9 Fix 3: idempotency is best-effort. A GET that returns 500 + // (or 404, or a malformed JSON body, or a network error) silently + // falls through to the POST so deploys never get blocked by a + // misbehaving admin endpoint. + certPEM, keyPEM := generateTestCertAndKey(t) + + var postCount atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.URL.Path, "/config/apps/tls/certificates/load") { + w.WriteHeader(http.StatusNotFound) + return + } + switch r.Method { + case "GET": + // Return 500 — connector should fall through to POST. + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprint(w, "internal error") + case "POST": + postCount.Add(1) + w.WriteHeader(http.StatusOK) + } + })) + defer server.Close() + + cfg := caddy.Config{AdminAPI: server.URL, Mode: "api"} + connector := caddy.New(&cfg, logger) + rawConfig, _ := json.Marshal(cfg) + _ = connector.ValidateConfig(ctx, rawConfig) + + result, err := connector.DeployCertificate(ctx, target.DeploymentRequest{ + CertPEM: certPEM, + KeyPEM: keyPEM, + }) + if err != nil { + t.Fatalf("deploy failed: %v", err) + } + if !result.Success { + t.Fatalf("expected success, got: %s", result.Message) + } + if got := postCount.Load(); got != 1 { + t.Errorf("expected exactly 1 POST call (GET failure fell through), got %d", got) + } + if result.Metadata["idempotent"] == "true" { + t.Error("expected metadata.idempotent absent on the fallthrough POST path") + } +}