feat: config files

[chronark]

Summary

This PR looks large, but the main change is in pkg/config. The rest is just about applying those changes to each service.

Introduces pkg/config, a new configuration package that replaces environment-variable-based configuration with file-based config (TOML, YAML, JSON).

What this does

• Struct-tag-driven validation: config:"required,default=8080,min=1,max=65535"
• Environment variable expansion: ${VAR} in config files
• Validator interface: Custom cross-field validation via Validate() method
• Collects all errors: Reports every validation failure at once instead of failing fast

---

For dev/k8s/manifests I found a very neat way to configure them via ConfigMap, but for docker-compose it's not as great, cause it requries an extra file. but we were talking about getting rid of compose anyways

This passes all tests and you can develop with it locally

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
dashboard	Ready	Preview, Comment	Feb 18, 2026 4:50pm

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
engineering	Ignored	Preview	Feb 18, 2026 4:50pm

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Centralizes runtime configuration into TOML files and a new pkg/config loader. Replaces many CLI flags and environment-variable wiring with file-based loading, adds typed/tagged config structs and validation, updates service startup to load configs and inject runtime fields, and adjusts builds, tests, dev manifests, and Vault master-key model.

Changes

Cohort / File(s)	Summary
Config package pkg/config/BUILD.bazel, pkg/config/common.go, pkg/config/config.go, pkg/config/config_test.go, pkg/config/doc.go, pkg/config/tags.go	New package providing generic Load[T]/LoadBytes, environment expansion, TOML unmarshalling, tag-driven defaults/validation, Validator interface, tests and docs.
Entrypoints & CLI cmd/api/main.go, cmd/ctrl/api.go, cmd/ctrl/worker.go, cmd/frontline/main.go, cmd/krane/main.go, cmd/preflight/main.go, cmd/sentinel/main.go, cmd/vault/main.go, cmd/*/BUILD.bazel	Replaces per-flag CLIs with --config/--config-data; actions call config.Load[...], set runtime fields (e.g., Clock, IDs), and delegate to service Run. BUILD files updated to depend on //pkg/config.
Service config types svc/*/config.go, e.g. svc/api/config.go, svc/ctrl/api/config.go, svc/ctrl/worker/config.go, svc/frontline/config.go, svc/krane/config.go, svc/preflight/config.go, svc/sentinel/config.go, svc/vault/config.go	Major restructure: nested, TOML-tagged config structs (Observability, Database, Vault, Gossip, ClickHouse, Control, TLSFiles, etc.), pointer-receiver Validate() methods, runtime-only fields, and many field renames/relocations.
Runtime wiring svc/*/run.go, svc/*/*_test.go, svc/api/*, svc/ctrl/*, svc/krane/*, svc/frontline/*, svc/preflight/*, svc/sentinel/*, svc/vault/*	Updated runtime code and tests to use new nested config paths, nil-checks for optional subsystems, new validation/defaulting behavior, and to obtain runtime fields (Clock, IDs) after loading.
Vault master-key model svc/vault/internal/*, svc/vault/integration/*, svc/vault/testutil/*	Replaces MasterKeys []string with MasterKey string and optional PreviousMasterKey *string; updates parsing, load logic, service internals, and tests accordingly.
Krane sentinel embedding svc/krane/internal/sentinel/apply.go, svc/krane/internal/sentinel/BUILD.bazel	Embed configs into sentinel containers as a TOML blob via UNKEY_CONFIG_DATA instead of many UNKEY_* env vars; adds toml dependency and //pkg/config to build.
Build / dependency updates MODULE.bazel, go.mod, pkg/*/BUILD.bazel, svc/*/BUILD.bazel, cmd/*/BUILD.bazel, svc/krane/internal/sentinel/BUILD.bazel	Adds BurntSushi/toml and gopkg.in/yaml.v3, introduces //pkg/config to many targets, removes several legacy deps (uid/tls/assert) where replaced by config usage.
Dev orchestration & manifests dev/config/*.toml, dev/docker-compose.yaml, dev/k8s/manifests/*.yaml	Adds per-service TOML dev configs, converts compose/k8s to mount/pass --config and ConfigMaps instead of many env vars; updates Deployments/Services to mount /etc/unkey/unkey.toml.
Docs & examples pkg/config/doc.go, web/.../config.mdx, svc/*/doc.go	Adds package docs for config, updates service docs and examples to show TOML-based config.Load usage and property-style config reference.
Tests & harnesses svc/api/*_test.go, svc/*/integration/*, svc/*/harness.go	Tests and harnesses updated to construct/use new nested config types (Database, Observability, ClickHouse, TLS, Control, etc.) and to load/validate via pkg/config where applicable.

Sequence Diagram(s)

sequenceDiagram
participant CLI as Entrypoint (binary)
participant FS as Config file / ConfigMap
participant Loader as pkg/config.Load[T]
participant Service as Service.Run(ctx, cfg)
participant Runtime as Runtime deps (Clock, uid)

CLI->>FS: read/unmount /etc/unkey/unkey.toml (or --config-data)
CLI->>Loader: call Load[T](path or bytes)
Loader->>FS: read bytes / expand env vars
Loader->>Loader: TOML unmarshal, apply defaults, validate
Loader-->>CLI: return typed cfg (T)
CLI->>Runtime: inject runtime fields (Clock, InstanceID, FrontlineID)
CLI->>Service: call Run(ctx, cfg)
Service->>Service: initialize subsystems using cfg (Vault, DB, Observability, Gossip)
Service-->>CLI: start server / listen

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description provides a comprehensive summary of the changes, explaining the main feature (pkg/config), key capabilities (validation, env expansion, Validator interface), and scope. However, it does not follow the required template structure with sections like 'Type of change', 'How should this be tested', or the checklist.	Fill out all required template sections including 'Type of change', 'How should this be tested', and complete the checklist items to meet repository standards.
Docstring Coverage	⚠️ Warning	Docstring coverage is 38.10% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'feat: config files' is clear and concise, accurately summarizing the main feature addition (config file system) in this large changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch chronark/config-package

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)

svc/ctrl/worker/run.go (1)

125-174: ⚠️ Potential issue | 🔴 Critical

Guard nil cfg.GitHub before deref.
Nil allowed above; later deref panics.

🔧 Minimal fix

 var ghClient githubclient.GitHubClient = githubclient.NewNoop()
+allowUnauth := false
 if cfg.GitHub != nil {
 	client, ghErr := githubclient.NewClient(githubclient.ClientConfig{
 		AppID:         cfg.GitHub.AppID,
 		PrivateKeyPEM: cfg.GitHub.PrivateKeyPEM,
 		WebhookSecret: "",
 	})
 	if ghErr != nil {
 		return fmt.Errorf("failed to create GitHub client: %w", ghErr)
 	}
 	ghClient = client
 	logger.Info("GitHub client initialized")
+	allowUnauth = cfg.GitHub.AllowUnauthenticatedDeployments
 } else {
 	logger.Info("GitHub client disabled (credentials not configured)")
 }
@@
-		AllowUnauthenticatedDeployments: cfg.GitHub.AllowUnauthenticatedDeployments,
+		AllowUnauthenticatedDeployments: allowUnauth,
 	}),

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@svc/ctrl/worker/run.go` around lines 125 - 174, The code dereferences
cfg.GitHub.AllowUnauthenticatedDeployments without guarding for nil (cfg.GitHub
can be nil earlier), causing a panic; fix by computing a safe boolean (e.g.
allowUnauthenticated := false; if cfg.GitHub != nil { allowUnauthenticated =
cfg.GitHub.AllowUnauthenticatedDeployments }) and pass that into
deploy.Config.AllowUnauthenticatedDeployments instead of directly using
cfg.GitHub.AllowUnauthenticatedDeployments; update the call site around
hydrav1.NewDeployServiceServer -> deploy.New -> deploy.Config to use this
guarded value.

svc/api/cancel_test.go (2)

19-21: ⚠️ Potential issue | 🟡 Minor

Wrap scenario in t.Run.
As per coding guidelines, **/*_test.go: Organize tests using t.Run() for scenarios.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@svc/api/cancel_test.go` around lines 19 - 21, The TestContextCancellation
test should be converted to use a subtest via t.Run: wrap the existing test
logic inside a t.Run("context cancellation" or similar) closure so the scenario
is executed as a named subtest; update the TestContextCancellation function to
call t.Run and move the current test body into the provided func(t *testing.T)
block, keeping assertions and setup unchanged.

---

11-17: ⚠️ Potential issue | 🟠 Major

Set Clock in test config.
Nil clock risks panic if Run expects Clock set.

🩹 Minimal fix

 	"github.com/stretchr/testify/require"
+	"github.com/unkeyed/unkey/pkg/clock"
 	sharedconfig "github.com/unkeyed/unkey/pkg/config"
@@
-		Clock:      nil, // Will use real clock
+		Clock:      clock.New(),

Also applies to: 40-41

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@svc/api/cancel_test.go` around lines 11 - 17, The test constructs a config
without a Clock which can cause a panic when api.Run (or other svc/api
functions) expects cfg.Clock; update the test setup in cancel_test.go to
initialize and assign a Clock on the config used by the test (e.g., create a
mock/real clock instance and set cfg.Clock or use the package helper like
config.WithClock if available) for both places in the file (around the current
import area usage and the later test at lines 40-41) so the Run call receives a
non-nil Clock.

svc/sentinel/run.go (1)

123-136: ⚠️ Potential issue | 🟡 Minor

Validate or zero WAN fields explicitly for sentinels.

Per architecture requirements, sentinels operate intra-region only and must not use WAN gossip. Lines 124 and 131 resolve and pass WANSeeds and WANPort to cluster.New() whenever gossip is enabled. While WAN activation requires bridge election and non-empty seeds, sentinel deployments should not accept these fields at all. Add validation to reject non-empty WANSeeds/WANPort in sentinel context, or explicitly zero them before passing to cluster.New().

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@svc/sentinel/run.go` around lines 123 - 136, The sentinel runpath is passing
WAN configuration into cluster.New (wanSeeds, cfg.Gossip.WANPort) even though
sentinels must not use WAN gossip; update the logic around lanSeeds/wanSeeds and
the cluster.New(...) call in run.go to either (1) validate and return an error
if cfg.Gossip.WANSeeds is non-empty or cfg.Gossip.WANPort is set when running as
a sentinel (e.g., using cfg.SentinelID/Region to detect sentinel mode), or (2)
explicitly zero out WAN fields before calling cluster.New (set WANSeeds to
nil/empty and WANBindPort/WANPort to 0) so WAN is disabled for sentinels; adjust
the creation call to pass the sanitized/validated WANSeeds and WANBindPort and
ensure any error path reports the invalid config rather than allowing WAN gossip
to be used.

🧹 Nitpick comments (1)

svc/sentinel/config.go (1)

61-79: Redundant region validation in Validate() method.

The Region field already has config:"required,oneof=local.dev|us-east-1.aws|..." tag on line 42, which performs the same validation. The manual check in Validate() duplicates this logic and creates a maintenance burden if regions change.

Consider removing the manual validation:

♻️ Proposed simplification

 func (c *Config) Validate() error {
-	validRegions := []string{
-		"local.dev",
-		"us-east-1.aws",
-		"us-east-2.aws",
-		"us-west-1.aws",
-		"us-west-2.aws",
-		"eu-central-1.aws",
-	}
-
-	if !slices.Contains(validRegions, c.Region) {
-		return fmt.Errorf("invalid region: %s, must be one of %v", c.Region, validRegions)
-	}
-
 	return nil
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@svc/sentinel/config.go` around lines 61 - 79, The manual region check in
Config.Validate duplicates the struct tag validation on the Region field; remove
the redundant slices.Contains block from the Validate() method (the code that
defines validRegions and the if !slices.Contains(...) return) so that validation
relies on the existing config tag (oneof on Region), leaving Validate() to only
contain any truly cross-field checks or simply return nil; update or run tests
to ensure no other callers rely on the removed manual error message.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@svc/api/cancel_test.go`:
- Around line 11-13: The import block contains the same module imported twice
("github.com/unkeyed/unkey/pkg/config" and aliased as sharedconfig) causing a
build error; remove the duplicate so the package is imported only once and use a
single identifier consistently (either config or sharedconfig) throughout the
file (e.g., update references to config.New or sharedconfig.New accordingly),
and apply the same removal of duplicate imports in the other affected import
blocks around the same file (lines noted in review).

---

Outside diff comments:
In `@svc/api/cancel_test.go`:
- Around line 19-21: The TestContextCancellation test should be converted to use
a subtest via t.Run: wrap the existing test logic inside a t.Run("context
cancellation" or similar) closure so the scenario is executed as a named
subtest; update the TestContextCancellation function to call t.Run and move the
current test body into the provided func(t *testing.T) block, keeping assertions
and setup unchanged.
- Around line 11-17: The test constructs a config without a Clock which can
cause a panic when api.Run (or other svc/api functions) expects cfg.Clock;
update the test setup in cancel_test.go to initialize and assign a Clock on the
config used by the test (e.g., create a mock/real clock instance and set
cfg.Clock or use the package helper like config.WithClock if available) for both
places in the file (around the current import area usage and the later test at
lines 40-41) so the Run call receives a non-nil Clock.

In `@svc/ctrl/worker/run.go`:
- Around line 125-174: The code dereferences
cfg.GitHub.AllowUnauthenticatedDeployments without guarding for nil (cfg.GitHub
can be nil earlier), causing a panic; fix by computing a safe boolean (e.g.
allowUnauthenticated := false; if cfg.GitHub != nil { allowUnauthenticated =
cfg.GitHub.AllowUnauthenticatedDeployments }) and pass that into
deploy.Config.AllowUnauthenticatedDeployments instead of directly using
cfg.GitHub.AllowUnauthenticatedDeployments; update the call site around
hydrav1.NewDeployServiceServer -> deploy.New -> deploy.Config to use this
guarded value.

In `@svc/sentinel/run.go`:
- Around line 123-136: The sentinel runpath is passing WAN configuration into
cluster.New (wanSeeds, cfg.Gossip.WANPort) even though sentinels must not use
WAN gossip; update the logic around lanSeeds/wanSeeds and the cluster.New(...)
call in run.go to either (1) validate and return an error if cfg.Gossip.WANSeeds
is non-empty or cfg.Gossip.WANPort is set when running as a sentinel (e.g.,
using cfg.SentinelID/Region to detect sentinel mode), or (2) explicitly zero out
WAN fields before calling cluster.New (set WANSeeds to nil/empty and
WANBindPort/WANPort to 0) so WAN is disabled for sentinels; adjust the creation
call to pass the sanitized/validated WANSeeds and WANBindPort and ensure any
error path reports the invalid config rather than allowing WAN gossip to be
used.

---

Duplicate comments:
In `@dev/config/ctrl-worker.toml`:
- Around line 32-34: The ClickHouse admin credentials are hardcoded under the
[clickhouse] keys (url and admin_url); replace the embedded secret by loading
the admin connection string from an environment variable (e.g.,
CLICKHOUSE_ADMIN_URL) or use a non-secret dev placeholder and fetch the real
value in the configuration/loader code, update code paths that read admin_url to
prefer the env-provided value, and remove the plaintext password from the
committed toml so secrets are not stored in repo.

In `@pkg/config/config.go`:
- Around line 8-11: The BUILD for this package is missing the logger dependency
referenced in pkg/config/config.go (the import
"github.com/unkeyed/unkey/pkg/logger"); update the package's BUILD rule to add
the //pkg/logger target as a dependency so the import resolves during build,
ensuring any targets that depend on pkg/config include that dependency in their
deps.

---

Nitpick comments:
In `@svc/sentinel/config.go`:
- Around line 61-79: The manual region check in Config.Validate duplicates the
struct tag validation on the Region field; remove the redundant slices.Contains
block from the Validate() method (the code that defines validRegions and the if
!slices.Contains(...) return) so that validation relies on the existing config
tag (oneof on Region), leaving Validate() to only contain any truly cross-field
checks or simply return nil; update or run tests to ensure no other callers rely
on the removed manual error message.

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

svc/krane/run.go (1)

88-95: ⚠️ Potential issue | 🟠 Major

Duplicate r.Recover() and r.DeferCtx(shutdownGrafana) calls.

Both are registered twice, which will cause shutdownGrafana to be called twice during shutdown (potentially causing nil pointer dereference if it's nil) and unnecessary recover overhead.

Proposed fix

 	r := runner.New()
 	defer r.Recover()

 	r.DeferCtx(shutdownGrafana)

-	defer r.Recover()
-
-	r.DeferCtx(shutdownGrafana)
-
 	cluster := controlplane.NewClient(controlplane.ClientConfig{

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@svc/krane/run.go` around lines 88 - 95, The code registers r.Recover() and
r.DeferCtx(shutdownGrafana) twice; remove the duplicate calls so each is
registered exactly once. Keep a single defer r.Recover() immediately after
creating the runner via runner.New(), and keep one r.DeferCtx(shutdownGrafana)
(placed where cleanup registrations occur), deleting the repeated lines that
call r.Recover() and r.DeferCtx(shutdownGrafana) a second time.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/config/common.go`:
- Around line 14-18: The MetricsConfig struct's PrometheusPort field is missing
a TOML struct tag so it won't be populated from TOML files; update the
PrometheusPort field in MetricsConfig to include an appropriate toml tag (e.g.
toml:"prometheus_port") so the TOML decoder maps the key to PrometheusPort
(reference the MetricsConfig type and the PrometheusPort field when making the
change).

In `@svc/krane/run.go`:
- Around line 199-201: The log call in the anonymous goroutine (r.Go -> func(ctx
context.Context) error) passes a "tls" key with no value to logger.Info,
producing malformed structured logs; update the logger.Info invocation in that
function (the logger.Info call) to include a boolean or value for the "tls" key
(for example use tls != nil or a tlsEnabled variable or the actual tlsConfig) so
the key has a corresponding value alongside "addr".

In `@svc/vault/internal/vault/service.go`:
- Around line 105-116: The parseMasterKey function currently returns a
KeyEncryptionKey even if its identifier is empty, which can collapse map keys;
after proto.Unmarshal in parseMasterKey, validate the key ID (e.g., check kek.Id
or the appropriate identifier field on vaultv1.KeyEncryptionKey) and return an
error like "empty master key id" if it's empty, so callers never receive a
KeyEncryptionKey with a blank ID; update parseMasterKey to perform this check
and return a wrapped fmt.Errorf on failure.
- Around line 94-100: When adding the previous master key into decryptionKeys,
guard against duplicate IDs so we don't overwrite the current key: after parsing
with parseMasterKey and before assigning decryptionKeys[oldKek.GetId()] =
oldKek, check whether that id already exists in decryptionKeys (e.g., if _,
exists := decryptionKeys[oldKek.GetId()]; exists) and return an error indicating
a duplicate key ID instead of overwriting; reference parseMasterKey,
previousMasterKey, decryptionKeys and oldKek.GetId() in your change.

---

Outside diff comments:
In `@svc/krane/run.go`:
- Around line 88-95: The code registers r.Recover() and
r.DeferCtx(shutdownGrafana) twice; remove the duplicate calls so each is
registered exactly once. Keep a single defer r.Recover() immediately after
creating the runner via runner.New(), and keep one r.DeferCtx(shutdownGrafana)
(placed where cleanup registrations occur), deleting the repeated lines that
call r.Recover() and r.DeferCtx(shutdownGrafana) a second time.

---

Duplicate comments:
In `@pkg/config/common.go`:
- Around line 93-95: The TOML tag on the SecretKey field in pkg/config/common.go
validates base64 string length incorrectly (min=32 permits 24-byte
keys/AES-192); update the struct tag for SecretKey to require the 44-char base64
string produced by openssl rand -base64 32 (i.e. change min=32 to min=44) so the
SecretKey field enforces AES-256-length keys while leaving the rest of the tag
intact.

[coderabbitai]

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@svc/vault/internal/vault/service.go`:
- Around line 103-114: In parseMasterKey, after proto.Unmarshal populates kek,
validate that the KeyEncryptionKey contains a non-empty key identifier (e.g.,
check kek.GetKeyId() or kek.KeyId) and return a descriptive error (e.g., "master
key missing key id") if it's empty; this ensures empty/invalid keys are rejected
before returning kek.
- Around line 92-98: The code blindly inserts oldKek into decryptionKeys using
oldKek.GetId(), which can overwrite an existing entry; update the block in the
function that handles previousMasterKey (around previousMasterKey,
parseMasterKey, oldKek.GetId(), decryptionKeys) to first check whether
decryptionKeys already contains that id and handle it (e.g., return an error
indicating a duplicate key ID or skip the insert) instead of unconditionally
assigning to decryptionKeys[oldKek.GetId()]; ensure the chosen behavior is
consistent with how other keys are added in this function.