diff --git a/cmd/certificatee/main.go b/cmd/certificatee/main.go index 6299453..e18c93b 100644 --- a/cmd/certificatee/main.go +++ b/cmd/certificatee/main.go @@ -38,12 +38,11 @@ type certificateSyncResult struct { } // 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. +// during this sync cycle, so legacy-duplicate cleanup can tell whether the +// certificate it would keep is confirmed healthy. type processedCertificateOutcome struct { ref haproxy.CertificateRef domain string - san []string err error result certificateSyncResult } @@ -54,6 +53,17 @@ func (o processedCertificateOutcome) isStable() bool { return o.err == nil && o.result.skipReason == "" && !o.result.runtimeUpdated } +// certRefDetail is a certificate ref together with the live HAProxy metadata +// fetched for it this cycle. Fetching every ref's detail before any writes +// happen lets legacy-duplicate classification see every ref's live SAN up +// front, instead of discovering duplicates mid-loop after some may already +// have been synced. +type certRefDetail struct { + ref haproxy.CertificateRef + haproxyCert *haproxy.CertificateDetail + fallbackDomain string +} + type noUsableVaultCertificateError struct { domains []string cause error @@ -171,92 +181,58 @@ func processHAProxyEndpoint(logger *logrus.Logger, cfg config.Config, vaultClien var errs []error var expiringCount int var skippedVaultCount int - var processed []processedCertificateOutcome + // Phase 1: fetch every ref's live metadata before doing any writes, so + // legacy-duplicate classification (which needs every ref's live SAN) sees + // the whole picture up front instead of discovering a duplicate mid-loop + // after one half of the pair has already been synced. + details := make([]certRefDetail, 0, len(certRefs)) for _, ref := range certRefs { - displayName := ref.DisplayName - apiName := ref.APIName - logger.Infof("[%s] Checking certificate: %s", endpoint, displayName) - - fallbackDomain := domainFromCertificateName(displayName) + fallbackDomain := domainFromCertificateName(ref.DisplayName) - haproxyCert, err := haproxyClient.GetCertificateDetail(apiName) + haproxyCert, err := haproxyClient.GetCertificateDetail(ref.APIName) if err != nil { certmetrics.CertificateMetadataLookupFailures.WithLabelValues(endpoint, fallbackDomain).Inc() errs = append(errs, err) - logger.Errorf("[%s] failed to get live dataplane metadata for %s: %v", endpoint, displayName, err) + logger.Errorf("[%s] failed to get live dataplane metadata for %s: %v", endpoint, ref.DisplayName, err) 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, ", ")) - - // Use HAProxy's live certificate metadata for expiry. Vault provides the - // replacement payload persisted to storage and, when needed, runtime. - syncResult, err := syncCertificate(apiName, candidateDomains, vaultClient, haproxyClient, haproxyCert, cfg.Certificatee.RenewBeforeDays) - domain := syncResult.domain - if domain == "" { - domain = fallbackDomain - } - - if haproxyCert != nil && !haproxyCert.NotAfter.IsZero() { - certmetrics.CertificateNotAfterTimestamp.WithLabelValues(endpoint, domain).Set(float64(haproxyCert.NotAfter.Unix())) - } + details = append(details, certRefDetail{ref: ref, haproxyCert: haproxyCert, fallbackDomain: fallbackDomain}) + } - // Track expiring certificates even when Vault replacement material is invalid. - if syncResult.isExpiring { - expiringCount++ - } + legacyKeeperAPIName := classifyLegacyDuplicates(logger, endpoint, details) - if err != nil { - errs = append(errs, err) - logger.Errorf("[%s] %v", endpoint, err) - certmetrics.CertificatesUpdateFailures.WithLabelValues(endpoint, domain).Inc() - record(err, syncResult) + // Phase 2: sync every ref that isn't a confirmed legacy duplicate, exactly + // as before. Confirmed legacy duplicates are handled in phase 3 instead, + // once we know whether their keeper turned out stable this cycle. + outcomes := make(map[string]processedCertificateOutcome, len(details)) + for _, d := range details { + if _, isLegacy := legacyKeeperAPIName[d.ref.APIName]; isLegacy { continue } + outcomes[d.ref.APIName] = syncOneCertificate(logger, cfg, vaultClient, haproxyClient, endpoint, d, &errs, &expiringCount, &skippedVaultCount) + } - if syncResult.skipReason != "" { - skippedVaultCount++ - logger.Debugf("[%s] Skipping certificate %s: %s", endpoint, displayName, syncResult.skipReason) - record(nil, syncResult) + // Phase 3: for each confirmed legacy duplicate, remove it once its keeper + // is confirmed healthy this cycle. If the keeper isn't stable yet, keep + // maintaining the duplicate normally rather than risk an unrenewed + // certificate - cleanup can wait for a calmer cycle. + for _, d := range details { + keeperAPIName, isLegacy := legacyKeeperAPIName[d.ref.APIName] + if !isLegacy { continue } - if syncResult.storageSynced { - logger.Infof("[%s] Certificate %s persisted to storage", endpoint, displayName) - } - - if syncResult.runtimeUpdated { - certmetrics.CertificatesUpdated.WithLabelValues(endpoint, domain).Inc() - logger.Infof("[%s] Certificate %s updated successfully: %s", endpoint, displayName, syncResult.reason) - record(nil, syncResult) + keeperOutcome, ok := outcomes[keeperAPIName] + if !ok || !keeperOutcome.isStable() { + outcomes[d.ref.APIName] = syncOneCertificate(logger, cfg, vaultClient, haproxyClient, endpoint, d, &errs, &expiringCount, &skippedVaultCount) continue } - logger.Infof("[%s] Certificate %s is up to date", endpoint, displayName) - record(nil, syncResult) + removeLegacyDuplicateCertificate(logger, endpoint, haproxyClient, d.ref, keeperOutcome.domain) } - cleanupLegacyDuplicateCertificates(logger, endpoint, haproxyClient, processed) - // Record expiring certificates count certmetrics.CertificatesExpiring.WithLabelValues(endpoint).Set(float64(expiringCount)) if skippedVaultCount > 0 { @@ -266,6 +242,74 @@ func processHAProxyEndpoint(logger *logrus.Logger, cfg config.Config, vaultClien return errors.Join(errs...) } +// syncOneCertificate runs the normal Vault-to-HAProxy sync for a single +// certificate ref and returns its outcome. Shared by the main sync pass and +// the legacy-duplicate fallback path, so both go through identical logic. +func syncOneCertificate( + logger *logrus.Logger, + cfg config.Config, + vaultClient certificateStore, + haproxyClient *haproxy.Client, + endpoint string, + d certRefDetail, + errs *[]error, + expiringCount *int, + skippedVaultCount *int, +) processedCertificateOutcome { + ref := d.ref + displayName := ref.DisplayName + apiName := ref.APIName + haproxyCert := d.haproxyCert + + logger.Infof("[%s] Checking certificate: %s", endpoint, displayName) + + candidateDomains := domainsForVault(displayName, haproxyCert) + logger.Debugf("[%s] Candidate Vault domains for certificate '%s': %s", endpoint, displayName, strings.Join(candidateDomains, ", ")) + + // Use HAProxy's live certificate metadata for expiry. Vault provides the + // replacement payload persisted to storage and, when needed, runtime. + syncResult, err := syncCertificate(apiName, candidateDomains, vaultClient, haproxyClient, haproxyCert, cfg.Certificatee.RenewBeforeDays) + domain := syncResult.domain + if domain == "" { + domain = d.fallbackDomain + } + + if haproxyCert != nil && !haproxyCert.NotAfter.IsZero() { + certmetrics.CertificateNotAfterTimestamp.WithLabelValues(endpoint, domain).Set(float64(haproxyCert.NotAfter.Unix())) + } + + // Track expiring certificates even when Vault replacement material is invalid. + if syncResult.isExpiring { + *expiringCount++ + } + + if err != nil { + *errs = append(*errs, err) + logger.Errorf("[%s] %v", endpoint, err) + certmetrics.CertificatesUpdateFailures.WithLabelValues(endpoint, domain).Inc() + return processedCertificateOutcome{ref: ref, domain: domain, err: err, result: syncResult} + } + + if syncResult.skipReason != "" { + *skippedVaultCount++ + logger.Debugf("[%s] Skipping certificate %s: %s", endpoint, displayName, syncResult.skipReason) + return processedCertificateOutcome{ref: ref, domain: domain, result: syncResult} + } + + if syncResult.storageSynced { + logger.Infof("[%s] Certificate %s persisted to storage", endpoint, displayName) + } + + if syncResult.runtimeUpdated { + certmetrics.CertificatesUpdated.WithLabelValues(endpoint, domain).Inc() + logger.Infof("[%s] Certificate %s updated successfully: %s", endpoint, displayName, syncResult.reason) + return processedCertificateOutcome{ref: ref, domain: domain, result: syncResult} + } + + logger.Infof("[%s] Certificate %s is up to date", endpoint, displayName) + return processedCertificateOutcome{ref: ref, domain: domain, result: syncResult} +} + // 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 @@ -277,37 +321,37 @@ 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 { +// classifyLegacyDuplicates groups refs by live SAN and, for any group of two +// or more, decides whether it resolves unambiguously to one current-format +// certificate (the keeper) and one or more legacy-named duplicates. It +// returns a map from a legacy duplicate's APIName to its keeper's APIName. +// Ambiguous groups (no keeper, or more than one) are logged and left alone +// rather than guessed at. +func classifyLegacyDuplicates(logger *logrus.Logger, endpoint string, details []certRefDetail) map[string]string { + groups := make(map[string][]certRefDetail) + for _, d := range details { + san := certificateDomains(d.haproxyCert) + if len(san) == 0 { continue } - key := strings.Join(outcome.san, ",") - groups[key] = append(groups[key], outcome) + key := strings.Join(san, ",") + groups[key] = append(groups[key], d) } + legacyKeeperAPIName := make(map[string]string) for san, group := range groups { if len(group) < 2 { continue } - var keeper *processedCertificateOutcome - var legacy []processedCertificateOutcome + var keeper *certRefDetail + var legacy []certRefDetail ambiguous := false for i := range group { - outcome := group[i] - if isLegacyCertificateName(outcome.ref.DisplayName) { - legacy = append(legacy, outcome) + d := group[i] + if isLegacyCertificateName(d.ref.DisplayName) { + legacy = append(legacy, d) continue } if keeper != nil { @@ -322,28 +366,41 @@ func cleanupLegacyDuplicateCertificates(logger *logrus.Logger, endpoint string, 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 _, l := range legacy { + legacyKeeperAPIName[l.ref.APIName] = keeper.ref.APIName } + } - 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 - } + return legacyKeeperAPIName +} - 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) +// removeLegacyDuplicateCertificate deletes a confirmed legacy duplicate's +// storage file. Runtime-level deletion isn't viable here: HAProxy refuses to +// remove a certificate still referenced by a bind ("del ssl cert" returns a +// 500 "in use" error), which every file loaded from a directory crt-store is, +// regardless of whether it's the one actually selected for a live SNI match. +// Storage deletion (skip_reload=true) does succeed and removes the on-disk +// file for good, but HAProxy's live runtime listing is intentionally +// disconnected from disk state without a reload, so the deleted name keeps +// reappearing in ListCertificateRefs until some later reload happens for +// unrelated reasons - at which point it drops out on its own. Until then, +// repeating this delete is a harmless no-op (or a 404): the caller is +// responsible for never handing this ref back through the normal sync path +// once it's classified as a legacy duplicate, or the delete and the ordinary +// sync's storage write would fight each other every cycle. +func removeLegacyDuplicateCertificate(logger *logrus.Logger, endpoint string, haproxyClient *haproxy.Client, ref haproxy.CertificateRef, keeperDomain string) { + storageCertName := haproxy.StorageCertificateName(ref.APIName) + if err := haproxyClient.DeleteCertificate(storageCertName); err != nil { + if haproxy.IsHTTPStatus(err, http.StatusNotFound) { + return } + certmetrics.LegacyCertificatesRemovalFailures.WithLabelValues(endpoint, keeperDomain).Inc() + logger.Warnf("[%s] failed to remove legacy duplicate certificate %s: %v", endpoint, ref.DisplayName, err) + return } + + certmetrics.LegacyCertificatesRemoved.WithLabelValues(endpoint, keeperDomain).Inc() + logger.Infof("[%s] Removed legacy duplicate certificate %s from storage (HAProxy will drop it from its live listing on the next reload)", endpoint, ref.DisplayName) } func setDataPlaneAPIVersion(endpoint, version string) { diff --git a/cmd/certificatee/main_test.go b/cmd/certificatee/main_test.go index efbacff..8c8752b 100644 --- a/cmd/certificatee/main_test.go +++ b/cmd/certificatee/main_test.go @@ -701,8 +701,92 @@ func TestCleanupLegacyDuplicateCertificatesRemovesConfirmedDuplicate(t *testing. 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) + // Regression check for the delete/recreate thrash loop: once a legacy + // duplicate is classified, it must never go through the normal storage + // sync again in the same cycle, or the next cycle's rediscovery (HAProxy + // never drops it from ListCertificateRefs without a reload) would recreate + // the very file we just deleted. + if len(storageWrites) != 1 || !strings.HasSuffix(storageWrites[0], "/_.devnet.rpcpool.com.pem") { + t.Fatalf("storageWrites = %v, want exactly one write, for the keeper only", storageWrites) + } +} + +// TestCleanupLegacyDuplicateCertificatesDoesNotThrashAcrossCycles is the +// direct regression test for the delete/recreate loop: HAProxy's runtime +// listing keeps reporting the legacy duplicate even after its storage file is +// deleted (confirmed against a real Data Plane API - deleting from storage +// with skip_reload=true never touches the live runtime listing). Running +// processHAProxyEndpoint twice against that unchanging listing must not +// recreate the deleted file on the second pass. +func TestCleanupLegacyDuplicateCertificatesDoesNotThrashAcrossCycles(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 legacyStorageWrites []string + var keeperStorageWrites []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": + // HAProxy keeps listing both names every cycle: this is the + // stale, disconnected-from-disk runtime state that drove the + // original bug. + 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 && r.URL.Path == "/v3/services/haproxy/storage/ssl_certificates/__devnet_rpcpool_com.pem": + legacyStorageWrites = append(legacyStorageWrites, r.URL.Path) + w.WriteHeader(http.StatusOK) + case r.Method == http.MethodPut && strings.HasPrefix(r.URL.Path, "/v3/services/haproxy/storage/ssl_certificates/"): + keeperStorageWrites = append(keeperStorageWrites, r.URL.Path) + w.WriteHeader(http.StatusOK) + case r.Method == http.MethodDelete && strings.HasPrefix(r.URL.Path, "/v3/services/haproxy/storage/ssl_certificates/"): + 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) + } + + cfg := config.Config{Certificatee: config.Certificatee{RenewBeforeDays: 30}} + store := fakeCertificateStore{ + secretsByPath: map[string]map[string]any{ + "certificates/*.devnet.rpcpool.com": {"certificate": certPEM, "private_key": keyPEM}, + }, + } + healthChecker := newCertificateeHealthChecker(nil, time.Minute) + + for cycle := 1; cycle <= 2; cycle++ { + if err := processHAProxyEndpoint(logger, cfg, store, haproxyClient, healthChecker); err != nil { + t.Fatalf("processHAProxyEndpoint() cycle %d error = %v", cycle, err) + } + } + + if len(legacyStorageWrites) != 0 { + t.Fatalf("legacyStorageWrites = %v, want zero across both cycles - this is the thrash bug", legacyStorageWrites) + } + if len(keeperStorageWrites) != 2 { + t.Fatalf("keeperStorageWrites = %v, want one per cycle", keeperStorageWrites) + } + if len(deletes) == 0 { + t.Fatal("deletes is empty, want at least one delete attempt for the legacy duplicate") } } @@ -715,6 +799,8 @@ func TestCleanupLegacyDuplicateCertificatesSkipsWhenKeeperUnstable(t *testing.T) vaultSerial := cert.SerialNumber.Text(16) var deletes []string + var storageWrites []string + var runtimeWrites []string server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch { @@ -731,8 +817,10 @@ func TestCleanupLegacyDuplicateCertificatesSkipsWhenKeeperUnstable(t *testing.T) 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/"): + storageWrites = append(storageWrites, r.URL.Path) w.WriteHeader(http.StatusOK) case r.Method == http.MethodPut && strings.HasPrefix(r.URL.Path, "/v3/services/haproxy/runtime/ssl_certs/"): + runtimeWrites = append(runtimeWrites, r.URL.Path) w.WriteHeader(http.StatusOK) case r.Method == http.MethodDelete: deletes = append(deletes, r.URL.Path) @@ -767,38 +855,55 @@ func TestCleanupLegacyDuplicateCertificatesSkipsWhenKeeperUnstable(t *testing.T) if len(deletes) != 0 { t.Fatalf("deletes = %v, want no cleanup while the keeper still needs a runtime push", deletes) } + // Falling back to the normal sync for the legacy duplicate this cycle + // means both files get persisted to storage, and the keeper's runtime + // gets the fresh push it needed - unmaintained certs are never left + // behind just because we're deferring cleanup. + if len(storageWrites) != 2 { + t.Fatalf("storageWrites = %v, want both duplicates persisted while cleanup is deferred", storageWrites) + } + if len(runtimeWrites) != 1 || !strings.Contains(runtimeWrites[0], "_.devnet.rpcpool.com.pem") { + t.Fatalf("runtimeWrites = %v, want exactly one runtime push, for the keeper", runtimeWrites) + } } -func TestCleanupLegacyDuplicateCertificatesSkipsAmbiguousGroups(t *testing.T) { +func TestClassifyLegacyDuplicatesSkipsAmbiguousGroups(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{} + sanCert := &haproxy.CertificateDetail{SubjectAlternativeNames: "DNS:*.devnet.rpcpool.com"} 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}, + details := []certRefDetail{ + {ref: haproxy.CertificateRef{DisplayName: "__devnet_rpcpool_com.pem", APIName: "certs/__devnet_rpcpool_com.pem"}, haproxyCert: sanCert}, + {ref: haproxy.CertificateRef{DisplayName: "__devnet_rpcpool_com_v2.pem", APIName: "certs/__devnet_rpcpool_com_v2.pem"}, haproxyCert: sanCert}, + } + got := classifyLegacyDuplicates(logger, "test-endpoint", details) + if len(got) != 0 { + t.Fatalf("classifyLegacyDuplicates() = %v, want empty: no current-format certificate to keep", got) } - 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}, + details := []certRefDetail{ + {ref: haproxy.CertificateRef{DisplayName: "_.devnet.rpcpool.com.pem", APIName: "certs/_.devnet.rpcpool.com.pem"}, haproxyCert: sanCert}, + {ref: haproxy.CertificateRef{DisplayName: "devnet.rpcpool.com.pem", APIName: "certs/devnet.rpcpool.com.pem"}, haproxyCert: sanCert}, + } + got := classifyLegacyDuplicates(logger, "test-endpoint", details) + if len(got) != 0 { + t.Fatalf("classifyLegacyDuplicates() = %v, want empty: ambiguous which entry to keep", got) + } + }) + + t.Run("unambiguous pair still resolves", func(t *testing.T) { + details := []certRefDetail{ + {ref: haproxy.CertificateRef{DisplayName: "_.devnet.rpcpool.com.pem", APIName: "certs/_.devnet.rpcpool.com.pem"}, haproxyCert: sanCert}, + {ref: haproxy.CertificateRef{DisplayName: "__devnet_rpcpool_com.pem", APIName: "certs/__devnet_rpcpool_com.pem"}, haproxyCert: sanCert}, + } + got := classifyLegacyDuplicates(logger, "test-endpoint", details) + want := map[string]string{"certs/__devnet_rpcpool_com.pem": "certs/_.devnet.rpcpool.com.pem"} + if len(got) != len(want) || got["certs/__devnet_rpcpool_com.pem"] != want["certs/__devnet_rpcpool_com.pem"] { + t.Fatalf("classifyLegacyDuplicates() = %v, want %v", got, want) } - cleanupLegacyDuplicateCertificates(logger, "test-endpoint", haproxyClient, processed) }) }