fix(certificatee): remove legacy duplicate certificate files from the sync loop

README.md

@@ -117,6 +117,8 @@ Certificator and certificatee expose Prometheus metrics for monitoring:
 | `certificatee_certificates_wildcard_total` | Gauge | `endpoint` | Number of certificates with wildcard storage filenames |
 | `certificatee_certificate_not_after_timestamp_seconds` | Gauge | `endpoint`, `domain` | Live certificate expiry reported by the HAProxy Data Plane API runtime endpoint |
 | `certificatee_certificate_metadata_lookup_failures_total` | Counter | `endpoint`, `domain` | Per-certificate Data Plane API runtime metadata lookups that failed |
+| `certificatee_legacy_certificates_removed_total` | Counter | `endpoint`, `domain` | Legacy duplicate certificate files removed from HAProxy storage |
+| `certificatee_legacy_certificates_removal_failures_total` | Counter | `endpoint`, `domain` | Failed attempts to remove a legacy duplicate certificate file |
 | `certificatee_dataplaneapi_version` | Gauge | `endpoint`, `version` | Detected HAProxy Data Plane API version for certificatee endpoints (`1` = detected version) |
 | `certificatee_last_sync_timestamp_seconds` | Gauge | `endpoint` | Unix timestamp of the last successful endpoint sync |
 

cmd/certificatee/main.go

@@ -37,6 +37,23 @@ type certificateSyncResult struct {
 	skipReason     string
 }
 
+// processedCertificateOutcome records what happened to one certificate ref
+// during this sync cycle, so a later pass can safely identify and remove
+// legacy duplicate certificate files sharing the same live SAN.
+type processedCertificateOutcome struct {
+	ref    haproxy.CertificateRef
+	domain string
+	san    []string
+	err    error
+	result certificateSyncResult
+}
+
+// isStable reports whether this cycle's sync needed no corrective action for
+// this certificate: no error, nothing skipped, and no fresh runtime push.
+func (o processedCertificateOutcome) isStable() bool {
+	return o.err == nil && o.result.skipReason == "" && !o.result.runtimeUpdated
+}
+
 type noUsableVaultCertificateError struct {
 	domains []string
 	cause   error
@@ -154,6 +171,7 @@ func processHAProxyEndpoint(logger *logrus.Logger, cfg config.Config, vaultClien
 	var errs []error
 	var expiringCount int
 	var skippedVaultCount int
+	var processed []processedCertificateOutcome
 
 	for _, ref := range certRefs {
 		displayName := ref.DisplayName
@@ -170,6 +188,23 @@ func processHAProxyEndpoint(logger *logrus.Logger, cfg config.Config, vaultClien
 			continue
 		}
 
+		// record captures this ref's outcome for the legacy-duplicate cleanup
+		// pass below. It must run on every exit path once haproxyCert is known,
+		// since duplicates can land in any of the branches below.
+		record := func(err error, result certificateSyncResult) {
+			domain := result.domain
+			if domain == "" {
+				domain = fallbackDomain
+			}
+			processed = append(processed, processedCertificateOutcome{
+				ref:    ref,
+				domain: domain,
+				san:    certificateDomains(haproxyCert),
+				err:    err,
+				result: result,
+			})
+		}
+
 		candidateDomains := domainsForVault(displayName, haproxyCert)
 		logger.Debugf("[%s] Candidate Vault domains for certificate '%s': %s", endpoint, displayName, strings.Join(candidateDomains, ", "))
 
@@ -194,12 +229,14 @@ func processHAProxyEndpoint(logger *logrus.Logger, cfg config.Config, vaultClien
 			errs = append(errs, err)
 			logger.Errorf("[%s] %v", endpoint, err)
 			certmetrics.CertificatesUpdateFailures.WithLabelValues(endpoint, domain).Inc()
+			record(err, syncResult)
 			continue
 		}
 
 		if syncResult.skipReason != "" {
 			skippedVaultCount++
 			logger.Debugf("[%s] Skipping certificate %s: %s", endpoint, displayName, syncResult.skipReason)
+			record(nil, syncResult)
 			continue
 		}
 
@@ -210,12 +247,16 @@ func processHAProxyEndpoint(logger *logrus.Logger, cfg config.Config, vaultClien
 		if syncResult.runtimeUpdated {
 			certmetrics.CertificatesUpdated.WithLabelValues(endpoint, domain).Inc()
 			logger.Infof("[%s] Certificate %s updated successfully: %s", endpoint, displayName, syncResult.reason)
+			record(nil, syncResult)
 			continue
 		}
 
 		logger.Infof("[%s] Certificate %s is up to date", endpoint, displayName)
+		record(nil, syncResult)
 	}
 
+	cleanupLegacyDuplicateCertificates(logger, endpoint, haproxyClient, processed)
+
 	// Record expiring certificates count
 	certmetrics.CertificatesExpiring.WithLabelValues(endpoint).Set(float64(expiringCount))
 	if skippedVaultCount > 0 {
@@ -225,6 +266,86 @@ func processHAProxyEndpoint(logger *logrus.Logger, cfg config.Config, vaultClien
 	return errors.Join(errs...)
 }
 
+// isLegacyCertificateName reports whether displayName uses the pre-migration
+// naming convention, where every "." in the domain (including the leading
+// wildcard "*.") was sanitized to "_" (e.g. "__devnet_rpcpool_com.pem" or
+// "api_mainnet-beta_solana_com.pem"). The current convention only sanitizes
+// the leading "*." to "_." and otherwise keeps the domain's dots literal
+// (e.g. "_.devnet.rpcpool.com.pem"), so a real domain name with zero dots
+// after stripping the extension can only be the legacy encoding.
+func isLegacyCertificateName(displayName string) bool {
+	return !strings.Contains(haproxy.ExtractDomainFromPath(displayName), ".")
+}
+
+// cleanupLegacyDuplicateCertificates removes certificate files left behind by
+// the pre-migration naming convention once we're sure it's safe: HAProxy is
+// still carrying both the legacy file and its modern replacement for the
+// exact same live SAN, and the replacement synced cleanly this cycle with no
+// error, no skip, and no fresh runtime push. Deletion only ever targets the
+// storage layer with skip_reload=true, so it never forces an HAProxy reload.
+// Cleanup failures are logged and metered, never treated as sync failures:
+// this is best-effort housekeeping, not certificate delivery.
+func cleanupLegacyDuplicateCertificates(logger *logrus.Logger, endpoint string, haproxyClient *haproxy.Client, processed []processedCertificateOutcome) {
+	groups := make(map[string][]processedCertificateOutcome)
+	for _, outcome := range processed {
+		if len(outcome.san) == 0 {
+			continue
+		}
+		key := strings.Join(outcome.san, ",")
+		groups[key] = append(groups[key], outcome)
+	}
+
+	for san, group := range groups {
+		if len(group) < 2 {
+			continue
+		}
+
+		var keeper *processedCertificateOutcome
+		var legacy []processedCertificateOutcome
+		ambiguous := false
+
+		for i := range group {
+			outcome := group[i]
+			if isLegacyCertificateName(outcome.ref.DisplayName) {
+				legacy = append(legacy, outcome)
+				continue
+			}
+			if keeper != nil {
+				ambiguous = true
+				continue
+			}
+			keeper = &group[i]
+		}
+
+		if ambiguous || keeper == nil || len(legacy) == 0 {
+			logger.Warnf("[%s] %d certificate(s) share SAN %q but do not resolve to exactly one current-format certificate and one or more legacy duplicates; skipping cleanup", endpoint, len(group), san)
+			continue
+		}
+
+		if !keeper.isStable() {
+			// The certificate we'd keep wasn't confirmed healthy this cycle
+			// (error, skip, or a runtime push just happened) - leave the
+			// legacy duplicate alone and re-evaluate next cycle.
+			continue
+		}
+
+		for _, dup := range legacy {
+			storageCertName := haproxy.StorageCertificateName(dup.ref.APIName)
+			if err := haproxyClient.DeleteCertificate(storageCertName); err != nil {
+				if haproxy.IsHTTPStatus(err, http.StatusNotFound) {
+					continue
+				}
+				certmetrics.LegacyCertificatesRemovalFailures.WithLabelValues(endpoint, keeper.domain).Inc()
+				logger.Warnf("[%s] failed to remove legacy duplicate certificate %s: %v", endpoint, dup.ref.DisplayName, err)
+				continue
+			}
+
+			certmetrics.LegacyCertificatesRemoved.WithLabelValues(endpoint, keeper.domain).Inc()
+			logger.Infof("[%s] Removed legacy duplicate certificate %s (superseded by %s)", endpoint, dup.ref.DisplayName, keeper.ref.DisplayName)
+		}
+	}
+}
+
 func setDataPlaneAPIVersion(endpoint, version string) {
 	for _, candidate := range []string{"v2", "v3"} {
 		value := 0.0

cmd/certificatee/main_test.go

@@ -600,3 +600,205 @@ func TestProcessHAProxyEndpointUsesSANAndExistingCertificateName(t *testing.T) {
 		t.Fatal("test certificate serial unexpectedly matches live serial")
 	}
 }
+
+func TestIsLegacyCertificateName(t *testing.T) {
+	tests := []struct {
+		name        string
+		displayName string
+		want        bool
+	}{
+		{"legacy wildcard", "__devnet_rpcpool_com.pem", true},
+		{"legacy non-wildcard", "api_mainnet-beta_solana_com.pem", true},
+		{"current wildcard", "_.devnet.rpcpool.com.pem", false},
+		{"current non-wildcard", "api.mainnet-beta.solana.com.pem", false},
+		{"current wildcard other extension", "_.devnet.rpcpool.com.crt", false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := isLegacyCertificateName(tt.displayName); got != tt.want {
+				t.Errorf("isLegacyCertificateName(%q) = %v, want %v", tt.displayName, got, tt.want)
+			}
+		})
+	}
+}
+
+// devnetCertDetailHandler builds a runtime ssl_certs detail response shared by
+// a legacy/current duplicate pair: same live SAN, same serial, so the current
+// duplicate is "stable" (no expiry, no serial mismatch) unless overridden.
+func devnetCertDetailHandler(storageName, serial string, notAfter time.Time) func(w http.ResponseWriter, r *http.Request) {
+	return func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = fmt.Fprintf(w, `{
+			"storage_name":%q,
+			"not_after":%q,
+			"not_before":"2026-05-08T00:00:00.000Z",
+			"serial":%q,
+			"subject_alternative_names":"DNS:*.devnet.rpcpool.com"
+		}`, storageName, notAfter.UTC().Format(time.RFC3339Nano), serial)
+	}
+}
+
+func TestCleanupLegacyDuplicateCertificatesRemovesConfirmedDuplicate(t *testing.T) {
+	logger := logrus.New()
+	logger.SetLevel(logrus.PanicLevel)
+
+	certPEM, keyPEM, cert := testCertificateBundleForDomain(t, "*.devnet.rpcpool.com", 0x0B74A913, time.Now().AddDate(0, 0, 90))
+	liveNotAfter := time.Now().AddDate(0, 0, 60)
+	serial := cert.SerialNumber.Text(16)
+
+	var deletes []string
+	var storageWrites []string
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch {
+		case r.Method == http.MethodGet && r.URL.Path == "/v3/services/haproxy/runtime/ssl_certs":
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = w.Write([]byte(`[
+				{"description":"_.devnet.rpcpool.com.pem","storage_name":"certs/_.devnet.rpcpool.com.pem"},
+				{"description":"__devnet_rpcpool_com.pem","storage_name":"certs/__devnet_rpcpool_com.pem"}
+			]`))
+		case r.Method == http.MethodGet && r.URL.Path == "/v3/services/haproxy/runtime/ssl_certs/certs/_.devnet.rpcpool.com.pem":
+			devnetCertDetailHandler("certs/_.devnet.rpcpool.com.pem", serial, liveNotAfter)(w, r)
+		case r.Method == http.MethodGet && r.URL.Path == "/v3/services/haproxy/runtime/ssl_certs/certs/__devnet_rpcpool_com.pem":
+			devnetCertDetailHandler("certs/__devnet_rpcpool_com.pem", serial, liveNotAfter)(w, r)
+		case r.Method == http.MethodPut && strings.HasPrefix(r.URL.Path, "/v3/services/haproxy/storage/ssl_certificates/"):
+			storageWrites = append(storageWrites, r.URL.Path)
+			w.WriteHeader(http.StatusOK)
+		case r.Method == http.MethodDelete && strings.HasPrefix(r.URL.Path, "/v3/services/haproxy/storage/ssl_certificates/"):
+			if got := r.URL.Query().Get("skip_reload"); got != "true" {
+				t.Errorf("skip_reload = %q, want true", got)
+			}
+			deletes = append(deletes, r.URL.Path)
+			w.WriteHeader(http.StatusNoContent)
+		default:
+			t.Fatalf("unexpected %s %q", r.Method, r.URL.String())
+		}
+	}))
+	defer server.Close()
+
+	haproxyClient, err := haproxy.NewClient(haproxy.ClientConfig{BaseURL: server.URL}, logger)
+	if err != nil {
+		t.Fatalf("NewClient() error = %v", err)
+	}
+
+	healthChecker := newCertificateeHealthChecker(nil, time.Minute)
+	err = processHAProxyEndpoint(
+		logger,
+		config.Config{Certificatee: config.Certificatee{RenewBeforeDays: 30}},
+		fakeCertificateStore{
+			secretsByPath: map[string]map[string]any{
+				"certificates/*.devnet.rpcpool.com": {"certificate": certPEM, "private_key": keyPEM},
+			},
+		},
+		haproxyClient,
+		healthChecker,
+	)
+	if err != nil {
+		t.Fatalf("processHAProxyEndpoint() error = %v", err)
+	}
+
+	if len(deletes) != 1 || !strings.HasSuffix(deletes[0], "/__devnet_rpcpool_com.pem") {
+		t.Fatalf("deletes = %v, want exactly one delete of the legacy duplicate", deletes)
+	}
+	if len(storageWrites) != 2 {
+		t.Fatalf("storageWrites = %v, want both duplicates persisted to storage", storageWrites)
+	}
+}
+
+func TestCleanupLegacyDuplicateCertificatesSkipsWhenKeeperUnstable(t *testing.T) {
+	logger := logrus.New()
+	logger.SetLevel(logrus.PanicLevel)
+
+	certPEM, keyPEM, cert := testCertificateBundleForDomain(t, "*.devnet.rpcpool.com", 0x0B74A913, time.Now().AddDate(0, 0, 90))
+	liveNotAfter := time.Now().AddDate(0, 0, 60)
+	vaultSerial := cert.SerialNumber.Text(16)
+
+	var deletes []string
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch {
+		case r.Method == http.MethodGet && r.URL.Path == "/v3/services/haproxy/runtime/ssl_certs":
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = w.Write([]byte(`[
+				{"description":"_.devnet.rpcpool.com.pem","storage_name":"certs/_.devnet.rpcpool.com.pem"},
+				{"description":"__devnet_rpcpool_com.pem","storage_name":"certs/__devnet_rpcpool_com.pem"}
+			]`))
+		case r.Method == http.MethodGet && r.URL.Path == "/v3/services/haproxy/runtime/ssl_certs/certs/_.devnet.rpcpool.com.pem":
+			// Keeper's live serial does not match Vault's: this cycle needs a
+			// runtime push, so it is not yet "stable".
+			devnetCertDetailHandler("certs/_.devnet.rpcpool.com.pem", "DEADBEEF0000", liveNotAfter)(w, r)
+		case r.Method == http.MethodGet && r.URL.Path == "/v3/services/haproxy/runtime/ssl_certs/certs/__devnet_rpcpool_com.pem":
+			devnetCertDetailHandler("certs/__devnet_rpcpool_com.pem", vaultSerial, liveNotAfter)(w, r)
+		case r.Method == http.MethodPut && strings.HasPrefix(r.URL.Path, "/v3/services/haproxy/storage/ssl_certificates/"):
+			w.WriteHeader(http.StatusOK)
+		case r.Method == http.MethodPut && strings.HasPrefix(r.URL.Path, "/v3/services/haproxy/runtime/ssl_certs/"):
+			w.WriteHeader(http.StatusOK)
+		case r.Method == http.MethodDelete:
+			deletes = append(deletes, r.URL.Path)
+			w.WriteHeader(http.StatusNoContent)
+		default:
+			t.Fatalf("unexpected %s %q", r.Method, r.URL.String())
+		}
+	}))
+	defer server.Close()
+
+	haproxyClient, err := haproxy.NewClient(haproxy.ClientConfig{BaseURL: server.URL}, logger)
+	if err != nil {
+		t.Fatalf("NewClient() error = %v", err)
+	}
+
+	healthChecker := newCertificateeHealthChecker(nil, time.Minute)
+	err = processHAProxyEndpoint(
+		logger,
+		config.Config{Certificatee: config.Certificatee{RenewBeforeDays: 30}},
+		fakeCertificateStore{
+			secretsByPath: map[string]map[string]any{
+				"certificates/*.devnet.rpcpool.com": {"certificate": certPEM, "private_key": keyPEM},
+			},
+		},
+		haproxyClient,
+		healthChecker,
+	)
+	if err != nil {
+		t.Fatalf("processHAProxyEndpoint() error = %v", err)
+	}
+
+	if len(deletes) != 0 {
+		t.Fatalf("deletes = %v, want no cleanup while the keeper still needs a runtime push", deletes)
+	}
+}
+
+func TestCleanupLegacyDuplicateCertificatesSkipsAmbiguousGroups(t *testing.T) {
+	logger := logrus.New()
+	logger.SetLevel(logrus.PanicLevel)
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		t.Fatalf("unexpected request %s %q; cleanup should not call the API for an ambiguous group", r.Method, r.URL.String())
+	}))
+	defer server.Close()
+
+	haproxyClient, err := haproxy.NewClient(haproxy.ClientConfig{BaseURL: server.URL}, logger)
+	if err != nil {
+		t.Fatalf("NewClient() error = %v", err)
+	}
+
+	san := []string{"*.devnet.rpcpool.com"}
+	stable := certificateSyncResult{}
+
+	t.Run("no current-format keeper", func(t *testing.T) {
+		processed := []processedCertificateOutcome{
+			{ref: haproxy.CertificateRef{DisplayName: "__devnet_rpcpool_com.pem", APIName: "certs/__devnet_rpcpool_com.pem"}, san: san, result: stable},
+			{ref: haproxy.CertificateRef{DisplayName: "__devnet_rpcpool_com_v2.pem", APIName: "certs/__devnet_rpcpool_com_v2.pem"}, san: san, result: stable},
+		}
+		cleanupLegacyDuplicateCertificates(logger, "test-endpoint", haproxyClient, processed)
+	})
+
+	t.Run("two current-format entries", func(t *testing.T) {
+		processed := []processedCertificateOutcome{
+			{ref: haproxy.CertificateRef{DisplayName: "_.devnet.rpcpool.com.pem", APIName: "certs/_.devnet.rpcpool.com.pem"}, san: san, result: stable},
+			{ref: haproxy.CertificateRef{DisplayName: "devnet.rpcpool.com.pem", APIName: "certs/devnet.rpcpool.com.pem"}, san: san, result: stable},
+		}
+		cleanupLegacyDuplicateCertificates(logger, "test-endpoint", haproxyClient, processed)
+	})
+}