Refactor frontline database layer and add SQL query documentation (#5288)

* refactor: copy new mysql package

* fix: generate into new mysql package

* refactor: move frontline queries

* optimize frontline database

* fix: remove wrong index

* chore: clean up indices

some were also uncommented for no reason, I have checked the db

* docs: make sql comments more helpful and less verbose

* Update dev/k8s/manifests/frontline.yaml

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Update svc/frontline/config.go

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

Author: chronark

dev/k8s/manifests/frontline.yaml

@@ -12,13 +12,12 @@ data:
     http_port = 7443
     apex_domain = "unkey.local"
     ctrl_addr = "http://ctrl-api:7091"
+    database_url = "unkey:password@tcp(mysql:3306)/unkey?parseTime=true&interpolateParams=true"
 
     [tls]
     cert_file = "/certs/unkey.local.crt"
     key_file = "/certs/unkey.local.key"
 
-    [database]
-    primary = "unkey:password@tcp(mysql:3306)/unkey?parseTime=true&interpolateParams=true"
 
     [gossip]
     lan_port = 7946

docs/engineering/contributing/quality/documentation.mdx

@@ -25,11 +25,13 @@ Before submitting documentation, verify each item.
 - [ ] Package has a `doc.go` if it has non-trivial behavior
 - [ ] Non-obvious behavior is documented (edge cases, nil handling, concurrency)
 - [ ] The why is explained for design choices that are not self-evident
+- [ ] Every named SQL query has a doc comment block
 
 **Quality**
 - [ ] Doc comments start with the symbol name
 - [ ] Uses prose, not bullet lists, unless items are parallel
 - [ ] Depth matches complexity
+- [ ] SQL comments add non-obvious context instead of restating obvious clauses
 - [ ] Cross-references use bracket syntax: `[TypeName]`, `[FuncName]`
 - [ ] No stale documentation from copy-paste or refactoring
 
@@ -38,6 +40,7 @@ Before submitting documentation, verify each item.
 - [ ] For value plus error returns, you checked what value returns on failure
 - [ ] For unmarshal operations, you verified whether partial values return
 - [ ] Examples compile and run
+- [ ] SQL comment examples match real query behavior (ordering, fallback, joins)
 
 ## Writing style
 
@@ -151,6 +154,45 @@ func (r *RateLimiter) Allow(ctx context.Context, identifier string, cost int) (b
 
 **Context**: Document context behavior only if it is non-standard.
 
+## SQL query documentation
+
+Named SQL queries are part of the public contract between application code and the database. Treat query comments like API documentation.
+
+For SQL query docs, explain why this query exists, how it resolves non-obvious behavior, and what guarantees callers can rely on. Do not just restate the `SELECT` clause.
+
+Keep SQL comments concise. In most cases, two to five lines are enough. If a comment is longer than the query, each sentence must carry non-obvious information such as fallback guarantees, deterministic ordering, intentional join behavior, or performance tradeoffs.
+
+Avoid duplicate explanations. If the SQL already makes behavior obvious, for example `AND s.health = 'healthy'`, do not repeat that in prose unless you are documenting a non-obvious guarantee that depends on it.
+
+For selection queries with fallback logic, document deterministic behavior explicitly. If exact matching must win over wildcard matching, explain how SQL enforces it, for example with `ORDER BY` and not candidate list order.
+
+For query docs that are not obvious from a quick read, include a small concrete example with inputs and the expected returned row. This is required for logic that depends on ordering, fallback, or tie-breaking.
+
+Document performance-sensitive choices when they are intentional, for example using `LIMIT 1` to avoid transferring large payload rows that are not selected.
+
+```sql
+-- name: FindBestCertificateByCandidates :one
+-- FindBestCertificateByCandidates returns one certificate row for the provided
+-- hostnames, preferring an exact hostname over wildcard matches.
+-- MySQL does not preserve IN-list order, so exact-first behavior is enforced by
+-- ORDER BY against exact_hostname, not by candidate position.
+--
+-- Example: with candidates ['api.example.com', '*.example.com'] and
+-- exact_hostname 'api.example.com', this query returns 'api.example.com' when
+-- both rows exist. If only '*.example.com' exists, it returns the wildcard row.
+--
+-- LIMIT 1 avoids returning non-selected certificate and key payload rows.
+SELECT
+  hostname,
+  workspace_id,
+  certificate,
+  encrypted_private_key
+FROM certificates
+WHERE hostname IN (sqlc.slice('hostnames'))
+ORDER BY hostname = sqlc.arg(exact_hostname) DESC
+LIMIT 1;
+```
+
 ## What not to document
 
 Do not document implementation details in doc comments. Those belong inside the function. Do not explain that context is used for cancellation or mention O(1) performance unless it is surprising.

pkg/db/schema.sql

@@ -539,7 +539,9 @@ CREATE TABLE `deployment_topology` (
 	`desired_status` enum('stopped','running') NOT NULL,
 	`created_at` bigint NOT NULL,
 	`updated_at` bigint,
-	CONSTRAINT `deployment_topology_pk` PRIMARY KEY(`pk`)
+	CONSTRAINT `deployment_topology_pk` PRIMARY KEY(`pk`),
+	CONSTRAINT `unique_region_per_deployment` UNIQUE(`deployment_id`,`region_id`),
+	CONSTRAINT `unique_version_per_region` UNIQUE(`region_id`,`version`)
 );
 
 CREATE TABLE `acme_users` (
@@ -603,7 +605,7 @@ CREATE TABLE `sentinels` (
 	`environment_id` varchar(255) NOT NULL,
 	`k8s_name` varchar(64) NOT NULL,
 	`k8s_address` varchar(255) NOT NULL,
-	`region_id` varchar(255) NOT NULL DEFAULT 'TODO',
+	`region_id` varchar(255) NOT NULL,
 	`image` varchar(255) NOT NULL,
 	`desired_state` enum('running','standby','archived') NOT NULL DEFAULT 'running',
 	`health` enum('unknown','paused','healthy','unhealthy') NOT NULL DEFAULT 'unknown',
@@ -617,7 +619,9 @@ CREATE TABLE `sentinels` (
 	CONSTRAINT `sentinels_pk` PRIMARY KEY(`pk`),
 	CONSTRAINT `sentinels_id_unique` UNIQUE(`id`),
 	CONSTRAINT `sentinels_k8s_name_unique` UNIQUE(`k8s_name`),
-	CONSTRAINT `sentinels_k8s_address_unique` UNIQUE(`k8s_address`)
+	CONSTRAINT `sentinels_k8s_address_unique` UNIQUE(`k8s_address`),
+	CONSTRAINT `one_env_per_region` UNIQUE(`environment_id`,`region_id`),
+	CONSTRAINT `unique_version_per_region` UNIQUE(`region_id`,`version`)
 );
 
 CREATE TABLE `instances` (
@@ -708,7 +712,9 @@ CREATE TABLE `cilium_network_policies` (
 	`created_at` bigint NOT NULL,
 	`updated_at` bigint,
 	CONSTRAINT `cilium_network_policies_pk` PRIMARY KEY(`pk`),
-	CONSTRAINT `cilium_network_policies_id_unique` UNIQUE(`id`)
+	CONSTRAINT `cilium_network_policies_id_unique` UNIQUE(`id`),
+	CONSTRAINT `one_deployment_per_region` UNIQUE(`deployment_id`,`region_id`,`k8s_name`),
+	CONSTRAINT `unique_version_per_region` UNIQUE(`region_id`,`version`)
 );
 
 CREATE TABLE `clusters` (
@@ -778,11 +784,12 @@ CREATE INDEX `project_idx` ON `custom_domains` (`project_id`);
 CREATE INDEX `verification_status_idx` ON `custom_domains` (`verification_status`);
 CREATE INDEX `workspace_idx` ON `acme_challenges` (`workspace_id`);
 CREATE INDEX `status_idx` ON `acme_challenges` (`status`);
-CREATE INDEX `idx_environment_id` ON `sentinels` (`environment_id`);
+CREATE INDEX `idx_environment_health_region_routing` ON `sentinels` (`environment_id`,`region_id`,`health`);
 CREATE INDEX `idx_deployment_id` ON `instances` (`deployment_id`);
 CREATE INDEX `idx_region` ON `instances` (`region_id`);
 CREATE INDEX `environment_id_idx` ON `frontline_routes` (`environment_id`);
 CREATE INDEX `deployment_id_idx` ON `frontline_routes` (`deployment_id`);
+CREATE INDEX `fqdn_environment_deployment_idx` ON `frontline_routes` (`fully_qualified_domain_name`,`environment_id`,`deployment_id`);
 CREATE INDEX `installation_id_idx` ON `github_repo_connections` (`installation_id`);
 CREATE INDEX `workspace_idx` ON `horizontal_autoscaling_policies` (`workspace_id`);
 

pkg/mysql/database.go

@@ -149,11 +149,11 @@ func (d *database) RO() *Replica {
 // This should be called when the application is shutting down.
 func (d *database) Close() error {
 	// Close the write replica connection
-	writeCloseErr := d.writeReplica.db.Close()
+	writeCloseErr := d.writeReplica.Close()
 
 	// Only close the read replica if it's a separate connection
 	if d.readReplica != nil {
-		readCloseErr := d.readReplica.db.Close()
+		readCloseErr := d.readReplica.Close()
 		if readCloseErr != nil {
 			return fault.Wrap(readCloseErr)
 		}

pkg/mysql/replica.go

@@ -174,3 +174,7 @@ func (r *Replica) Begin(ctx context.Context) (DBTx, error) {
 	// Wrap the transaction with tracing
 	return WrapTxWithContext(tx, r.mode+"_tx", ctx), nil
 }
+
+func (r *Replica) Close() error {
+	return r.db.Close()
+}

pkg/mysql/schema.sql

@@ -539,7 +539,9 @@ CREATE TABLE `deployment_topology` (
 	`desired_status` enum('stopped','running') NOT NULL,
 	`created_at` bigint NOT NULL,
 	`updated_at` bigint,
-	CONSTRAINT `deployment_topology_pk` PRIMARY KEY(`pk`)
+	CONSTRAINT `deployment_topology_pk` PRIMARY KEY(`pk`),
+	CONSTRAINT `unique_region_per_deployment` UNIQUE(`deployment_id`,`region_id`),
+	CONSTRAINT `unique_version_per_region` UNIQUE(`region_id`,`version`)
 );
 
 CREATE TABLE `acme_users` (
@@ -603,7 +605,7 @@ CREATE TABLE `sentinels` (
 	`environment_id` varchar(255) NOT NULL,
 	`k8s_name` varchar(64) NOT NULL,
 	`k8s_address` varchar(255) NOT NULL,
-	`region_id` varchar(255) NOT NULL DEFAULT 'TODO',
+	`region_id` varchar(255) NOT NULL,
 	`image` varchar(255) NOT NULL,
 	`desired_state` enum('running','standby','archived') NOT NULL DEFAULT 'running',
 	`health` enum('unknown','paused','healthy','unhealthy') NOT NULL DEFAULT 'unknown',
@@ -617,7 +619,9 @@ CREATE TABLE `sentinels` (
 	CONSTRAINT `sentinels_pk` PRIMARY KEY(`pk`),
 	CONSTRAINT `sentinels_id_unique` UNIQUE(`id`),
 	CONSTRAINT `sentinels_k8s_name_unique` UNIQUE(`k8s_name`),
-	CONSTRAINT `sentinels_k8s_address_unique` UNIQUE(`k8s_address`)
+	CONSTRAINT `sentinels_k8s_address_unique` UNIQUE(`k8s_address`),
+	CONSTRAINT `one_env_per_region` UNIQUE(`environment_id`,`region_id`),
+	CONSTRAINT `unique_version_per_region` UNIQUE(`region_id`,`version`)
 );
 
 CREATE TABLE `instances` (
@@ -708,7 +712,9 @@ CREATE TABLE `cilium_network_policies` (
 	`created_at` bigint NOT NULL,
 	`updated_at` bigint,
 	CONSTRAINT `cilium_network_policies_pk` PRIMARY KEY(`pk`),
-	CONSTRAINT `cilium_network_policies_id_unique` UNIQUE(`id`)
+	CONSTRAINT `cilium_network_policies_id_unique` UNIQUE(`id`),
+	CONSTRAINT `one_deployment_per_region` UNIQUE(`deployment_id`,`region_id`),
+	CONSTRAINT `unique_version_per_region` UNIQUE(`region_id`,`version`)
 );
 
 CREATE TABLE `clusters` (
@@ -778,11 +784,12 @@ CREATE INDEX `project_idx` ON `custom_domains` (`project_id`);
 CREATE INDEX `verification_status_idx` ON `custom_domains` (`verification_status`);
 CREATE INDEX `workspace_idx` ON `acme_challenges` (`workspace_id`);
 CREATE INDEX `status_idx` ON `acme_challenges` (`status`);
-CREATE INDEX `idx_environment_id` ON `sentinels` (`environment_id`);
+CREATE INDEX `idx_environment_health_region_routing` ON `sentinels` (`environment_id`,`region_id`,`health`);
 CREATE INDEX `idx_deployment_id` ON `instances` (`deployment_id`);
 CREATE INDEX `idx_region` ON `instances` (`region_id`);
 CREATE INDEX `environment_id_idx` ON `frontline_routes` (`environment_id`);
 CREATE INDEX `deployment_id_idx` ON `frontline_routes` (`deployment_id`);
+CREATE INDEX `fqdn_environment_deployment_idx` ON `frontline_routes` (`fully_qualified_domain_name`,`environment_id`,`deployment_id`);
 CREATE INDEX `installation_id_idx` ON `github_repo_connections` (`installation_id`);
 CREATE INDEX `workspace_idx` ON `horizontal_autoscaling_policies` (`workspace_id`);
 

svc/frontline/BUILD.bazel

@@ -18,7 +18,6 @@ go_library(
         "//pkg/clock",
         "//pkg/cluster",
         "//pkg/config",
-        "//pkg/db",
         "//pkg/logger",
         "//pkg/otel",
         "//pkg/prometheus",
@@ -28,6 +27,7 @@ go_library(
         "//pkg/tls",
         "//pkg/version",
         "//pkg/zen",
+        "//svc/frontline/internal/db",
         "//svc/frontline/internal/errorpage",
         "//svc/frontline/routes",
         "//svc/frontline/services/caches",

svc/frontline/config.go

@@ -58,8 +58,9 @@ type Config struct {
 	// See [config.TLS].
 	TLS *config.TLS `toml:"tls"`
 
-	// Database configures MySQL connections. See [config.DatabaseConfig].
-	Database config.DatabaseConfig `toml:"database"`
+	// DatabaseURL is the connection string for the MySQL database.
+	// It should be a globally load balanced endpoint and only requires read access.
+	DatabaseURL string `toml:"database_url" config:"required"`
 
 	Observability config.Observability `toml:"observability"`
 

svc/frontline/internal/db/BUILD.bazel

@@ -0,0 +1,18 @@
+load("@rules_go//go:def.bzl", "go_library")
+
+go_library(
+    name = "db",
+    srcs = [
+        "certificate_find_by_hostnames.sql_generated.go",
+        "database.go",
+        "doc.go",
+        "generate.go",
+        "ingress_route_find_by_fqdn.sql_generated.go",
+        "models_generated.go",
+        "querier_generated.go",
+        "sentinel_find_by_environment_id.sql_generated.go",
+    ],
+    importpath = "github.com/unkeyed/unkey/svc/frontline/internal/db",
+    visibility = ["//visibility:public"],
+    deps = ["//pkg/mysql"],
+)