Support an explicit endpoint key to decouple identity from name

[sleeyax]

Summary

The endpoint key is derived from group and name and is used as the endpoint's identity in storage and API routes, so renaming an endpoint or moving it between groups orphans its history.

Added an optional key field to Endpoint and ExternalEndpoint that, when set, is used as the key instead (sanitized, without a group prefix), keeping identity stable across name and group changes.

The explicit key is threaded through ExternalEndpoint.ToEndpoint() so pushed results are stored under it, and the memory and SQL stores now report the actual key on Status rather than re-deriving it from group and name.

Fixes #1146.

Checklist

• Tested and/or added tests to validate that the changes work as intended, if applicable.
• Updated documentation in README.md, if applicable.

[wahajahmed010]

Solid feature — opt-in key field, threaded through both stores, with a proper DB migration for the legacy UNIQUE(name, group) constraint. Tests cover collision detection, propagation via ToEndpoint, sanitization, and the SQL rebuild is idempotent.

One real concern, two nits:

1. SQLite migration FK pragma scope (real issue). In dropLegacyEndpointNameGroupUniqueConstraint, the ordering is:

defer conn.Close()  // line A
defer conn.ExecContext(`PRAGMA foreign_keys=ON`)  // line B

Go's defer is LIFO, so conn.Close() (A) runs before the FK re-enable (B). With a closed connection, the re-enable will fail and the error is discarded (_, _ = ...). On a long-lived, pooled connection this could leave foreign_keys disabled across the pool. Fix: either re-enable FK inside the tx (PRAGMA works in a transaction on SQLite ≥ 3.6.19) or swap the defers so close happens last.

2. key.Sanitize export rename. sanitize → Sanitize is fine in-tree, but if this package is consumed by external plugins (or if anything in internal/ tests the private symbol) the rename could ripple. Worth a quick grep -r 'key\.sanitize' ./... before merge.

3. README claims _ cannot appear in an explicit key. Sanitization does replace _ with -, so the claim holds — but no test asserts ExplicitKey: "foo_bar" produces "foo-bar". A 2-line test in TestEndpoint_Key would lock that invariant.

[sleeyax]

Thanks for the review! Went through all three points — one produced a change, the other two are already covered.

1. SQLite migration FK pragma scope — I think the LIFO reasoning is inverted here. The defers are registered in this order:

defer conn.Close()                                                          // registered 1st
if _, err = conn.ExecContext(ctx, `PRAGMA foreign_keys=OFF`); err != nil { ... }
defer func() { _, _ = conn.ExecContext(ctx, `PRAGMA foreign_keys=ON`) }()   // registered 2nd

LIFO means the last registered defer runs first, so on return the order is: re-enable foreign_keys → conn.Close(). The pragma is executed on a live connection, which is the ordering the review asks for. That was deliberate: conn.Close() is registered immediately after acquiring the connection so it can't be leaked on any of the early-return paths, and the pragma restore is registered after the PRAGMA foreign_keys=OFF succeeds so it only runs if there's something to restore. No change made.

2. key.Sanitize export rename — ran grep -rn "key\.sanitize" --include="*.go" ., no hits. The only references are the two new key.Sanitize call sites in config/endpoint/endpoint.go and config/endpoint/external_endpoint.go. The package lives under config/, not internal/, so it was already importable by external consumers — the rename widens the exported API rather than breaking it, and there's no sanitize symbol left for anything to reference. No change made.

3. Missing underscore test — good catch, fixed in f05a103. The existing explicit-key-is-sanitized case used "My Custom.Key", which never exercised the _ → - substitution the README claim rests on:

{
	// Guarantees an explicit key can never collide with a derived group/name key, which always contains a "_" separator.
	name:     "explicit-key-underscore-is-replaced",
	endpoint: Endpoint{Group: "g", Name: "n", ExplicitKey: "foo_bar"},
	expected: "foo-bar",
},

[wahajahmed010]

Solid change and well motivated — the rename-orphans-history problem is real, and threading an explicit key through both Endpoint and ExternalEndpoint is the right shape. The schema-migration guard (dropLegacyEndpointNameGroupUniqueConstraint only runs when the legacy constraint is still present) keeps the upgrade idempotent, and preserving endpoint_id across the SQLite rebuild protects the FK chains. Tests are thorough.

A few concerns worth addressing before merge:

1. explicit-key-cannot-collide-with-derived-key test looks wrong. The test case asserts shouldError: false for:

   - name: api
     group: backend
   - name: unrelated
     key: backend_api

   But the validation in ValidateEndpointsConfig uses ep.Key() for both — endpoint 1's derived key is backend_api, and endpoint 2's Sanitize("backend_api") is also backend_api. So the duplicate check should fire. Either the test is wrong (and should be shouldError: true) or the validation is wrong (and a sanitized-explicit key that matches another endpoint's derived key should be allowed). Whichever the intent, the current state is inconsistent — I bet this is the kind of thing that produces a confusing "I added a key and my config stopped loading" issue in the field.

2. Sanitization asymmetry. The derived key uses Sanitize(group) + "_" + Sanitize(name), but the explicit key is just Sanitize(key) — no group prefix. That's the right call for stability (the whole point of the feature), but it means an explicit key "backend_api" is now indistinguishable from a derived key from another endpoint, which loops back to concern #1. Worth a sentence in the docs calling out that explicit keys share the same namespace as derived keys and need to be globally unique across your config.

3. SQLite migration on every startup that detects the legacy constraint. The check is cheap (a SELECT sql FROM sqlite_master), but the rebuild is not — PRAGMA foreign_keys=OFF, a 4-statement transaction, all on a single connection. The defer re-enables foreign_keys, which is good, but there's a small window where another writer hitting the same store could see inconsistent state. Probably fine in practice (this only runs once per database), but worth either (a) gating the rebuild behind a config-file flag the first time only, or (b) at minimum printing a one-time logr.Infof so operators have a breadcrumb when their Gatus startup suddenly takes longer.

4. Postgres migration is silent. _, _ = s.db.Exec(...) for ALTER TABLE endpoints DROP CONSTRAINT ... swallows errors. If a user is on an older Postgres where the constraint name is different (or absent because they have a custom schema), the migration quietly does nothing — which is fine for correctness, but a logr.Debugf("legacy constraint not present, skipping") when the ALTER returns no rows would be helpful for support.

5. Minor: status.(*endpoint.Status).Key = endpointKey in memory.go and endpointStatus.Key = key in sql.go are untyped. A small helper like setStatusKey(status, key) would make the two stores symmetric and easier to keep aligned.

Approving the feature; the collision test in #1 is the must-fix.

[sleeyax]

Thanks for the thorough review @wahajahmed010! Here's how each point was addressed:

1. explicit-key-cannot-collide-with-derived-key test looks wrong.

The test is actually correct, and the collision it guards against cannot happen: Sanitize replaces _ with -, so the explicit key backend_api sanitizes to backend-api, while endpoint 1's derived key is backend_api (the _ separator is appended after sanitizing group and name). Since derived keys always contain a literal _ and sanitized explicit keys never can, the two namespaces are disjoint by construction — hence shouldError: false. This is also asserted directly by TestEndpoint_Key (explicit-key-with-underscores-is-sanitized) and called out in the README's "Endpoint key" notes. Since the confusion is understandable, bc6f1e0 adds a comment to the test case spelling this out.

2. Sanitization asymmetry — worth a sentence in the docs calling out that explicit keys share the same namespace as derived keys and need to be globally unique across your config.

Already covered in the README's "Endpoint key" section, which states that the key must be unique across all endpoints and external endpoints, and explains the sanitization guarantee above (an explicit key can never reproduce a derived group/name key).

3. SQLite migration on every startup that detects the legacy constraint — worth at minimum printing a one-time logr.Infof so operators have a breadcrumb.

Done in f65464b: a logr.Infof now announces the one-time endpoints table rebuild before it starts. I skipped the config-flag gating alternative — the rebuild runs at most once per database and gating it behind a flag felt like more machinery than the situation warrants.

4. Postgres migration is silent.

Done in 858e5a3: the schema migration now checks pg_constraint for the legacy constraint and logs the outcome — Infof when dropping it, Debugf when it's absent (nothing to drop), and Errorf if the check or the drop fails — instead of swallowing the ALTER result.

5. Minor: a small helper like setStatusKey(status, key) would make the two stores symmetric and easier to keep aligned.

Done in 418dd05, with a slight twist: rather than a cross-package setter, endpoint.NewStatusWithKey(key, group, name) constructs the status with the right key up front, and NewStatus delegates to it with the derived key. Both stores now build the status in one step with no post-hoc assignment.

[TwiN]

Why?

From my perspective, this is a cosmetic change that has no real technical benefits

relevant: #1146 (comment)

[sleeyax]

@TwiN please see all other replies on that issue you linked. People including myself have real usecases for this.