feat(config): warn on non-canonical target connect host (#396) - #400
Merged
Conversation
Root cause of #396 (reproduced end-to-end with two produced ZIPs): the Analyzer keys database identity on the composite (cluster_key, database_name) where cluster_key = lower(trim(host)):port, with the host echoed verbatim from the export's target_identity. Signals faithfully emits the CONFIGURED host, so collecting the same database under two different host strings (its DNS endpoint from one Signals instance and a loopback/tunnel address or a resolved IP from another) yields two distinct cluster_keys: the Analyzer holds the second as an unknown identity and its snapshots are silently held unprocessed. Signals cannot know the canonical host a database was registered under elsewhere, so it cannot unify the two — but it can stop the failure being SILENT. Add a startup WARNING when an enabled target's connect host is a loopback (localhost / 127.0.0.0/8 / ::1) or a bare IP literal: a non-canonical identity that will split against the DNS endpoint. It logs the target name + reason class only, never the host value or credentials (R078 no-leak discipline). NonCanonicalHostReason is unit-tested across loopback names, IPv4/IPv6 loopback, private + public IP literals, and DNS hostnames. closes #396
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause (reproduced end-to-end — two produced ZIPs; full write-up on #396)
The Analyzer keys database identity on the composite (cluster_key, database_name) where
cluster_key = lower(trim(host)):port, with the host echoed verbatim from the export'starget_identity. Signals faithfully emits the configured host (a produced ZIP shows{"host":"postgres","dbname":"postgres"}), so the same database collected under two different host strings — its DNS endpoint from one Signals instance vs a loopback/tunnel address or a resolved IP from another — yields two distinct cluster_keys. The Analyzer recognizes the first and holds the second as an unknown identity, silently leaving those snapshots unprocessed.The filename-prefix "hash" in the issue is a random
instance_id(changes only on a Signals DB/volume reset), not the cluster identity — so the two prefixes were two Signals instances, and the split was driven by a non-canonical connect host in one instance's config, not any Signals identity-derivation defect. Signals core already does exactly what the issue asks ("always the configured connect host, never a resolved IP").Fix
Signals cannot know which host string a database was registered under elsewhere, so it cannot unify them — but it can stop the failure being silent. This adds a startup WARNING when an enabled target's connect host is a loopback (
localhost/127.0.0.0/8/::1) or a bare IP literal — the non-canonical identities that split against a DNS endpoint. It logs the target name + reason class only, never the host value or credentials (R078 no-leak discipline). This would have flagged the live incident at boot.The operational rule (documented on #396): one Signals feed per target, always the canonical endpoint host — never a parallel local/tunnel Signals run emitting a different host for the same DB.
Verification
NonCanonicalHostReasonunit-tested across loopback names, IPv4/IPv6 loopback, private + public IP literals, and DNS hostnames.gofmt/go vet/go testgreen;golangci-lint0 issues; docs-drift + de-arq guards pass.closes #396