Skip to content

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

Merged
ananthb merged 1 commit into
masterfrom
certificatee-legacy-cert-cleanup
Aug 12, 2026
Merged

fix(certificatee): remove legacy duplicate certificate files from the sync loop#42
ananthb merged 1 commit into
masterfrom
certificatee-legacy-cert-cleanup

Conversation

@ananthb

@ananthb ananthb commented Aug 12, 2026

Copy link
Copy Markdown
Member
AI description, click for details

Summary

While checking on the SAN-match (#38) and storage-persist (#37) fixes in staging/canary, found that HAProxy's certs/ directory carries two files per wildcard domain: a pre-migration underscore-sanitized name (__devnet_rpcpool_com.pem) alongside the current dot-preserving name (_.devnet.rpcpool.com.pem), both with the identical live SAN. Confirmed via the canary-AP dataplane API directly: all 19 wildcard domains on that node have this exact duplicate pair, both marked "status": "Used".

certificatee already discovers and syncs both names independently (correctly — it can't assume the legacy one is dead just because a newer one exists). This duplication is the most likely explanation for a runtime-cert-push convergence delay observed on canary (*.devnet.rpcpool.com needed 3 identical-mismatch retries over ~1hr before HAProxy reflected the pushed serial).

  • Add a cleanup pass in processHAProxyEndpoint that removes the legacy duplicate once its current-format sibling has synced cleanly this cycle (no error, no skip, no fresh runtime push) — never in the same cycle we just changed something.
  • Only acts on unambiguous groups: exactly one non-legacy-shaped name and one or more legacy-shaped names sharing the exact same live SAN. Anything else (e.g. two names both look current, or none do) is logged and skipped rather than guessed at.
  • DeleteCertificate now passes skip_reload=true, so cleanup never forces an HAProxy reload. It also drops a configuration/version round-trip and version query param this Data Plane API endpoint doesn't accept for this route (confirmed against the API's own OpenAPI spec) — as written before this change, calling it would have forced a reload on every delete.
  • Adds certificatee_legacy_certificates_removed_total / ..._removal_failures_total metrics for observability, documented in the README metrics table.

Cleanup failures are logged/metered, never treated as sync failures — this is best-effort housekeeping, not certificate delivery.

Test plan

  • go test ./pkg/haproxy/... ./cmd/certificatee/... ./pkg/certmetrics/... — all pass
  • go vet + gofmt clean on changed files
  • New tests cover: confirmed-duplicate removal, skip when the keeper needs a runtime push this cycle, skip on ambiguous SAN groups (no keeper / multiple keepers), and DeleteCertificate sending skip_reload=true with no version param
  • Verify on canary before promoting to prod — same duplicate-file pattern exists in prod's certs/ directories

No Linear ticket — this is a follow-up from manually investigating staging/canary cert-sync health.

🤖 Generated with Claude Code

… sync loop

Every wildcard domain we checked on canary carries two HAProxy storage
files for the same SAN: a pre-migration underscore-sanitized name
(__devnet_rpcpool_com.pem) alongside the current dot-preserving name
(_.devnet.rpcpool.com.pem). Both get synced independently every cycle,
and the leftover legacy file appears to be what makes runtime cert
pushes to its sibling take a few extra cycles to converge.

Add a cleanup pass to processHAProxyEndpoint that removes the legacy
duplicate once its current-format sibling has synced cleanly this
cycle (no error, no skip, no fresh runtime push), and only when the
group resolves unambiguously to exactly one keeper. Deletion always
passes skip_reload=true so it never forces an HAProxy reload.

Also fixes DeleteCertificate: it was calling a configuration/version
endpoint and passing a version query param that this Data Plane API
endpoint doesn't accept, and it never set skip_reload, so calling it
as-is would have forced a reload on every cleanup.
@github-actions

Copy link
Copy Markdown

Code Coverage Report

Total Coverage: total: (statements) 52.3%

Coverage by function
github.com/vinted/certificator/cmd/certificatee/health.go:27:		newCertificateeHealthChecker		100.0%
github.com/vinted/certificator/cmd/certificatee/health.go:38:		certificateeHealthMaxSyncAge		75.0%
github.com/vinted/certificator/cmd/certificatee/health.go:46:		Check					87.5%
github.com/vinted/certificator/cmd/certificatee/health.go:78:		MarkEndpointSyncSuccess			80.0%
github.com/vinted/certificator/cmd/certificatee/health.go:88:		lastSync				100.0%
github.com/vinted/certificator/cmd/certificatee/helpers.go:9:		createHAProxyClients			0.0%
github.com/vinted/certificator/cmd/certificatee/main.go:53:		isStable				100.0%
github.com/vinted/certificator/cmd/certificatee/main.go:62:		Error					100.0%
github.com/vinted/certificator/cmd/certificatee/main.go:66:		Unwrap					100.0%
github.com/vinted/certificator/cmd/certificatee/main.go:70:		main					0.0%
github.com/vinted/certificator/cmd/certificatee/main.go:122:		maybeUpdateCertificates			0.0%
github.com/vinted/certificator/cmd/certificatee/main.go:138:		processHAProxyEndpoint			73.0%
github.com/vinted/certificator/cmd/certificatee/main.go:276:		isLegacyCertificateName			100.0%
github.com/vinted/certificator/cmd/certificatee/main.go:288:		cleanupLegacyDuplicateCertificates	83.3%
github.com/vinted/certificator/cmd/certificatee/main.go:349:		setDataPlaneAPIVersion			100.0%
github.com/vinted/certificator/cmd/certificatee/main.go:359:		domainFromCertificateName		100.0%
github.com/vinted/certificator/cmd/certificatee/main.go:364:		domainsForVault				91.7%
github.com/vinted/certificator/cmd/certificatee/main.go:385:		certificateDomains			66.7%
github.com/vinted/certificator/cmd/certificatee/main.go:393:		parseCertificateDomainList		77.8%
github.com/vinted/certificator/cmd/certificatee/main.go:425:		shouldUpdateForLiveExpiry		100.0%
github.com/vinted/certificator/cmd/certificatee/main.go:443:		shouldUpdateForSerialMismatch		100.0%
github.com/vinted/certificator/cmd/certificatee/main.go:453:		validateVaultCertificateForUpdate	100.0%
github.com/vinted/certificator/cmd/certificatee/main.go:457:		validateVaultCertificateForUpdateAt	91.7%
github.com/vinted/certificator/cmd/certificatee/main.go:482:		expectedVaultCertificateDomains		83.3%
github.com/vinted/certificator/cmd/certificatee/main.go:495:		vaultCertificateMatchesDomains		100.0%
github.com/vinted/certificator/cmd/certificatee/main.go:508:		parseVaultLeafCertificate		100.0%
github.com/vinted/certificator/cmd/certificatee/main.go:517:		readVaultCertificateBundle		85.7%
github.com/vinted/certificator/cmd/certificatee/main.go:531:		readValidVaultCertificateBundle		92.3%
github.com/vinted/certificator/cmd/certificatee/main.go:559:		firstDomain				66.7%
github.com/vinted/certificator/cmd/certificatee/main.go:567:		syncCertificate				85.7%
github.com/vinted/certificator/cmd/certificatee/main.go:616:		buildPEMBundle				70.0%
github.com/vinted/certificator/cmd/certificatee/main.go:642:		endsWith				66.7%
github.com/vinted/certificator/cmd/certificator/main.go:28:		main					0.0%
github.com/vinted/certificator/cmd/certificator/main.go:112:		deleteExpiredVaultCertificate		0.0%
github.com/vinted/certificator/cmd/certificator/main.go:116:		deleteExpiredVaultCertificateAt		100.0%
github.com/vinted/certificator/pkg/acme/acme.go:27:			GetEmail				0.0%
github.com/vinted/certificator/pkg/acme/acme.go:32:			GetRegistration				0.0%
github.com/vinted/certificator/pkg/acme/acme.go:37:			GetPrivateKey				0.0%
github.com/vinted/certificator/pkg/acme/acme.go:42:			NewClient				0.0%
github.com/vinted/certificator/pkg/acme/acme.go:61:			setupClient				0.0%
github.com/vinted/certificator/pkg/acme/acme.go:80:			setupAccount				0.0%
github.com/vinted/certificator/pkg/acme/acme.go:118:			newAccount				0.0%
github.com/vinted/certificator/pkg/acme/acme.go:130:			getAccountKey				0.0%
github.com/vinted/certificator/pkg/acme/acme.go:159:			registerAccount				0.0%
github.com/vinted/certificator/pkg/acme/acme.go:177:			recoverAccount				0.0%
github.com/vinted/certificator/pkg/acme/acme.go:211:			saveAccount				0.0%
github.com/vinted/certificator/pkg/acme/acme.go:221:			saveKey					0.0%
github.com/vinted/certificator/pkg/certificate/certificate.go:18:	ObtainCertificate			0.0%
github.com/vinted/certificator/pkg/certificate/certificate.go:50:	GetCertificate				0.0%
github.com/vinted/certificator/pkg/certificate/certificate.go:63:	ParsePEMCertificate			0.0%
github.com/vinted/certificator/pkg/certificate/certificate.go:80:	DeleteCertificate			0.0%
github.com/vinted/certificator/pkg/certificate/certificate.go:85:	IsExpired				0.0%
github.com/vinted/certificator/pkg/certificate/certificate.go:91:	NeedsReissuing				0.0%
github.com/vinted/certificator/pkg/certificate/certificate.go:119:	arraysEqual				0.0%
github.com/vinted/certificator/pkg/certificate/certificate.go:133:	arrayContains				0.0%
github.com/vinted/certificator/pkg/certificate/certificate.go:142:	VaultCertLocation			0.0%
github.com/vinted/certificator/pkg/certificate/certificate.go:146:	storeCertificateInVault			0.0%
github.com/vinted/certificator/pkg/certmetrics/metrics.go:87:		StartMetricsServer			0.0%
github.com/vinted/certificator/pkg/certmetrics/metrics.go:112:		newHandler				100.0%
github.com/vinted/certificator/pkg/certmetrics/metrics.go:136:		PushMetrics				0.0%
github.com/vinted/certificator/pkg/config/config.go:73:			LoadConfig				0.0%
github.com/vinted/certificator/pkg/haproxy/client.go:57:		Error					0.0%
github.com/vinted/certificator/pkg/haproxy/client.go:63:		IsHTTPStatus				100.0%
github.com/vinted/certificator/pkg/haproxy/client.go:68:		unexpectedStatusError			100.0%
github.com/vinted/certificator/pkg/haproxy/client.go:77:		NewClient				100.0%
github.com/vinted/certificator/pkg/haproxy/client.go:112:		NewClients				92.3%
github.com/vinted/certificator/pkg/haproxy/client.go:137:		Endpoint				100.0%
github.com/vinted/certificator/pkg/haproxy/client.go:142:		doRequest				88.9%
github.com/vinted/certificator/pkg/haproxy/client.go:160:		parseAPITime				66.7%
github.com/vinted/certificator/pkg/haproxy/client.go:215:		ListCertificates			100.0%
github.com/vinted/certificator/pkg/haproxy/client.go:229:		ListCertificateRefs			90.9%
github.com/vinted/certificator/pkg/haproxy/client.go:271:		GetCertificateDetail			68.4%
github.com/vinted/certificator/pkg/haproxy/client.go:315:		UpdateCertificate			80.0%
github.com/vinted/certificator/pkg/haproxy/client.go:349:		UpdateStorageCertificate		90.9%
github.com/vinted/certificator/pkg/haproxy/client.go:368:		CreateCertificate			78.9%
github.com/vinted/certificator/pkg/haproxy/client.go:401:		EnsureStorageCertificate		50.0%
github.com/vinted/certificator/pkg/haproxy/client.go:422:		DeleteCertificate			100.0%
github.com/vinted/certificator/pkg/haproxy/client.go:441:		ExtractDomainFromPath			100.0%
github.com/vinted/certificator/pkg/haproxy/client.go:461:		NormalizeDomainForVault			0.0%
github.com/vinted/certificator/pkg/haproxy/client.go:468:		StorageCertificateName			100.0%
github.com/vinted/certificator/pkg/haproxy/client.go:474:		IsExpiring				100.0%
github.com/vinted/certificator/pkg/haproxy/client.go:486:		NormalizeSerial				100.0%
github.com/vinted/certificator/pkg/haproxy/client.go:519:		Error					100.0%
github.com/vinted/certificator/pkg/haproxy/client.go:523:		Info					0.0%
github.com/vinted/certificator/pkg/haproxy/client.go:527:		Debug					100.0%
github.com/vinted/certificator/pkg/haproxy/client.go:531:		Warn					0.0%
github.com/vinted/certificator/pkg/haproxy/client.go:536:		toLogrusFields				85.7%
github.com/vinted/certificator/pkg/vault/vault.go:18:			NewVaultClient				0.0%
github.com/vinted/certificator/pkg/vault/vault.go:27:			TokenLookupSelf				0.0%
github.com/vinted/certificator/pkg/vault/vault.go:39:			KVWrite					0.0%
github.com/vinted/certificator/pkg/vault/vault.go:53:			KVDelete				0.0%
github.com/vinted/certificator/pkg/vault/vault.go:66:			KVRead					0.0%
github.com/vinted/certificator/pkg/vault/vault.go:87:			vaultFullPath				0.0%
total:									(statements)				52.3%

@ananthb
ananthb merged commit 3d34b03 into master Aug 12, 2026
1 check passed
@ananthb
ananthb deleted the certificatee-legacy-cert-cleanup branch August 12, 2026 14:20
ananthb added a commit that referenced this pull request Aug 13, 2026
…oop (#43)

v0.5.5 (#42) deleted a legacy duplicate's storage file every cycle, but
HAProxy's runtime cert listing never drops it without a reload, so
ListCertificateRefs kept rediscovering the same stale ref and the
ordinary per-ref sync recreated it via EnsureStorageCertificate's
404-fallback-to-create. Confirmed directly against a real Data Plane
API (shared_lb_fra_staging / haproxy-fra1): storage delete succeeds
and the file is gone from disk, but the runtime listing keeps
reporting the old content until an unrelated reload happens; a
runtime-level delete is not an option either, since HAProxy refuses to
remove a certificate still referenced by a bind (500 "in use").

Restructure processHAProxyEndpoint into three phases: fetch every
ref's live metadata first, classify legacy duplicates from that
complete picture, then sync everything that isn't a confirmed
duplicate. Confirmed duplicates are deleted once (never recreated,
since they no longer go through the normal per-ref sync at all) as
long as their keeper stays stable; if the keeper isn't stable yet, the
duplicate falls back to being synced normally so nothing goes
unrenewed while cleanup waits for a calmer cycle.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant