From c7b213497711a9254c8000711a30e2be6a84c16d Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Fri, 25 Sep 2026 18:27:27 +0700 Subject: [PATCH] feat(cli): keep serving DNS through Control D API maintenance A hard maintenance window took DNS down on endpoints that already had a working configuration: the bootstrap preflight read the maintenance answer as a failed fetch and exited, turning a temporary API outage into an endpoint outage. Startup now falls back to the configuration on disk when a content- and identity-bound marker records that the API produced it. Without one, ctrld stops with API_MAINTENANCE (exit 37). Maintenance never self-uninstalls or counts as a permanent rejection, a fallback run retries every 5-7 minutes until a recovered configuration is applied, and deactivation is refused with 503 while the PIN cannot be verified. Master adaptation: pending-recovery and PIN-known handling live in applyFetchedResolverConfig; logging uses *ctrld.Logger; the reload fetch seam is fetchResolverConfigFn; the control-server and self-uninstall probes use the existing fetchResolverConfig seam with loggerCtx. --- cmd/cli/api_maintenance.go | 251 ++++++++++++ cmd/cli/api_maintenance_review_test.go | 544 +++++++++++++++++++++++++ cmd/cli/api_maintenance_test.go | 376 +++++++++++++++++ cmd/cli/cli.go | 79 +++- cmd/cli/control_server.go | 14 +- cmd/cli/dns_proxy.go | 4 +- cmd/cli/prog.go | 83 +++- cmd/cli/provision_result.go | 4 + cmd/cli/provision_result_test.go | 4 +- docs/provisioning-failure-codes.md | 1 + 10 files changed, 1339 insertions(+), 21 deletions(-) create mode 100644 cmd/cli/api_maintenance.go create mode 100644 cmd/cli/api_maintenance_review_test.go create mode 100644 cmd/cli/api_maintenance_test.go diff --git a/cmd/cli/api_maintenance.go b/cmd/cli/api_maintenance.go new file mode 100644 index 0000000..e9640eb --- /dev/null +++ b/cmd/cli/api_maintenance.go @@ -0,0 +1,251 @@ +package cli + +import ( + "crypto/sha256" + "encoding/hex" + "math/rand" + "os" + "strings" + "time" + + "github.com/go-playground/validator/v10" + + "github.com/Control-D-Inc/ctrld" + "github.com/Control-D-Inc/ctrld/internal/controld" +) + +// configReadFromDisk records that startup read a configuration file this host +// already had, instead of generating a default one because none existed. +// +// The distinction only matters while the API is in maintenance. A generated +// default carries the public freedns upstreams, so falling back to it would +// answer queries outside the Control D profile the operator provisioned — +// quietly serving a policy nobody chose, which is worse than not starting. +// Only a file the host already had can stand in for the configuration the API +// would have returned. +var configReadFromDisk bool + +// maintenanceNoConfigDetail is what ctrld reports when maintenance stops a +// start that has nothing to fall back on. It names the operation that cannot +// happen rather than the fetch that failed, because "could not fetch resolver +// config" reads as a network fault and sends the reader after connectivity +// that is not broken. +const maintenanceNoConfigDetail = "cannot provision during Control D API maintenance: " + + "this host has no previously working configuration to fall back on. Retry once maintenance ends." + +// managedConfigMarkerFile records that the configuration file next to it was +// written from an API answer, and for which identity. +// +// File presence cannot carry this on its own. A start that has no +// configuration writes the seeded default before the API is ever reached, so a +// provisioning that failed leaves a file behind that the next start reads +// successfully. That file holds the public freedns upstreams, so trusting it +// would run the endpoint outside the Control D profile it was provisioned for +// — the exact outcome the fallback exists to avoid. +const managedConfigMarkerFile = ".managed_config" + +// managedConfigMarkerPath is a var so tests can place the marker in a temp dir. +var managedConfigMarkerPath = func() string { return absHomeDir(managedConfigMarkerFile) } + +// managedConfigMarker fingerprints both the identity and the configuration file +// the marker is recorded for. +// +// The identity alone cannot carry it. A marker that names only the identity +// outlives the file it vouched for: delete or move that file and the next start +// regenerates the seeded default, which the stale marker then certifies. The +// same happens when --config points somewhere else under the same home. Binding +// to the contents means any file the API did not produce fails the check, +// without having to detect how it got there. +// +// The raw identity is kept off disk: it is redacted from retained logs, and a +// marker only ever has to answer whether this is the same identity. +func managedConfigMarker(uid, path string) (string, error) { + data, err := os.ReadFile(path) + if err != nil { + return "", err + } + identity := sha256.Sum256([]byte(uid)) + contents := sha256.Sum256(data) + return hex.EncodeToString(identity[:]) + ":" + hex.EncodeToString(contents[:]), nil +} + +// recordManagedConfig notes that the configuration on disk came from the API +// for uid. The caller must have persisted that configuration first, or the +// marker would vouch for a file that does not match it. +func recordManagedConfig(uid string) error { + if uid == "" { + return nil + } + marker, err := managedConfigMarker(uid, defaultConfigFile) + if err != nil { + return err + } + return os.WriteFile(managedConfigMarkerPath(), []byte(marker), 0600) +} + +// managedConfigRecordedFor reports whether the configuration file in use was +// written from an API answer for uid. +func managedConfigRecordedFor(uid string) bool { + if uid == "" { + return false + } + recorded, err := os.ReadFile(managedConfigMarkerPath()) + if err != nil { + return false + } + want, err := managedConfigMarker(uid, defaultConfigFile) + if err != nil { + return false + } + return strings.TrimSpace(string(recorded)) == want +} + +// lastKnownConfigUsable reports whether cfg can carry this start by itself. +// +// It is deliberately quiet: this is a probe, and the caller reports either +// outcome. Startup validates the configuration again further on and reports +// its own errors there, so a failure here must not also print them. +func lastKnownConfigUsable(cfg *ctrld.Config) bool { + if !configReadFromDisk || cfg == nil { + return false + } + // A file this host already had is necessary but not sufficient: it must + // also be one the API produced, for the identity being asked for now. + if !managedConfigRecordedFor(cdUID) { + return false + } + return ctrld.ValidateConfig(validator.New(), cfg) == nil +} + +// useLastKnownConfigDuringMaintenance reports whether startup may continue on +// the configuration already on disk because the API is in maintenance. +// +// Maintenance says nothing about this host's configuration — the same request +// succeeds once the window ends — so stopping DNS for its duration turns a +// temporary API outage into an endpoint outage. Continuing is only honest when +// the configuration on disk is one the API itself produced on an earlier run: +// configReadFromDisk establishes that this host already had the file, and the +// managed-config marker that the API produced it for this identity. The marker +// is the load-bearing half - a file alone can be a generated default. +// +// The resolver config stays nil: this run has none, and the reload path +// already treats a nil one as "no previous response to compare against", so +// the first successful reload after maintenance adopts the fresh answer. +func useLastKnownConfigDuringMaintenance(err error, cfg *ctrld.Config) bool { + if !controld.IsMaintenance(err) { + return false + } + logger := mainLog.Load().With().Str("mode", "cd") + if !lastKnownConfigUsable(cfg) { + journal(logger.Error()).Str("reason", "api_maintenance"). + Msg("Control D API is in maintenance and no previously working configuration is on disk") + return false + } + journal(logger.Warn()).Str("reason", "api_maintenance"). + Msg("Control D API is in maintenance; starting with the last known working configuration on disk") + return true +} + +// logResolverConfigFetchFailure reports a resolver-config fetch that failed on +// a path which keeps serving DNS afterwards: the periodic reload and the +// self-uninstall probe. +// +// Maintenance gets its own line, and it is journaled. "could not fetch +// resolver config" reads as a fault on this host and sends the reader after +// connectivity that is not broken, which is how the reported incident was +// first triaged. It also leaves a later "why is my configuration stale" +// question unanswerable once the debug stream has rotated. The reload runs +// hourly by default, so one retained line per window is bounded. +func logResolverConfigFetchFailure(logger *ctrld.Logger, err error) { + if controld.IsMaintenance(err) { + journal(logger.Warn()).Err(err).Str("reason", "api_maintenance"). + Msg("Control D API is in maintenance; keeping the current configuration until it returns") + return + } + logger.Warn().Err(err).Msg("Could not fetch resolver config") +} + +const ( + // maintenanceReloadInterval is how soon a run that started on the + // configuration on disk asks the API again. It is deliberately far below + // refetch_time: such a run holds no answer from the API at all, so its + // exclude list, Internal Domains, deactivation pin and self-upgrade check + // are all still those of the previous run until one arrives. + maintenanceReloadInterval = 5 * time.Minute + // maintenanceReloadJitter spreads those retries across the hosts that + // started during the same window. + // + // The hourly ticker needs no jitter because each host starts it at its own + // boot time, which spreads the fleet on its own. A maintenance window + // removes that spread: every host that falls back does so within the same + // few minutes, so a fixed retry would put them all in lockstep and land as + // one burst on the API exactly as it comes back. + maintenanceReloadJitter = 2 * time.Minute +) + +// maintenanceReloadDelay returns the wait before the next retry of a run that +// started on the configuration on disk. It is a var so a test can drive the +// reload loop without waiting out a real interval. +var maintenanceReloadDelay = func() time.Duration { + return maintenanceReloadInterval + time.Duration(rand.Int63n(int64(maintenanceReloadJitter))) +} + +// maintenanceConfigApplied records that a run which started on the +// configuration on disk has handed an answer from the API to the reload path. +// +// It is deliberately not called when the fetch succeeds, nor when the response +// is handed to the reload. The reload receiver reads the configuration and +// fetches again, and either step can fail; until it does not, the endpoint is +// still serving what it started with. Clearing the flag earlier would stop the +// faster retry and report a recovery that had not happened. +func (p *prog) maintenanceConfigApplied(logger *ctrld.Logger) { + if p.startedInAPIMaintenance.CompareAndSwap(true, false) { + journal(logger.Info()).Str("reason", "api_maintenance"). + Msg("Control D API answered again; this run applied the configuration it returned") + } +} + +// persistManagedConfig writes cfg and, on a run the API manages, records that +// the file it just wrote came from there. +// +// The two belong together. The marker is bound to the file's contents, and +// every API-driven change rewrites that file, so a write that does not refresh +// the marker leaves the host ineligible for the very fallback its own +// configuration is meant to provide - including recovery from a maintenance +// start. A failed write certifies nothing, and a run with no resolver UID +// writes nothing the API produced: generated defaults and NextDNS +// configurations must never be certified. +func persistManagedConfig(cfg *ctrld.Config, logger *ctrld.Logger) { + if err := writeConfigFile(cfg); err != nil { + logger.Error().Err(err).Msg("Could not write new config") + return + } + if cdUID == "" { + return + } + if err := recordManagedConfig(cdUID); err != nil { + logger.Warn().Err(err).Msg("could not record that this configuration came from the API") + } +} + +// recordManagedConfigForStart refreshes the marker once a start has finished +// writing its configuration. +// +// fetchedThisRun is whether the API answered this run. wasManagedBeforeWrite is +// whether the file already carried a marker for this identity before anything +// this start rewrote it. Either is enough: a maintenance fallback fetches +// nothing, yet still rewrites the file through the listener and intercept-mode +// updates, and the marker is bound to the file's contents - so without the +// second the next start in the same window loses the fallback this one used. +// Neither means the file is a generated default or otherwise unmanaged, which +// must never be certified. +func recordManagedConfigForStart(fetchedThisRun, wasManagedBeforeWrite bool) { + if !fetchedThisRun && !wasManagedBeforeWrite { + return + } + if err := recordManagedConfig(cdUID); err != nil { + // Not fatal: this run is fine. It only costs the next start its + // fallback if the API is in maintenance then. + mainLog.Load().Warn().Err(err).Msg("could not record that this configuration came from the API") + } +} diff --git a/cmd/cli/api_maintenance_review_test.go b/cmd/cli/api_maintenance_review_test.go new file mode 100644 index 0000000..f3b1959 --- /dev/null +++ b/cmd/cli/api_maintenance_review_test.go @@ -0,0 +1,544 @@ +package cli + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/spf13/viper" + + "github.com/Control-D-Inc/ctrld" + "github.com/Control-D-Inc/ctrld/internal/controld" +) + +// TestMaintenanceFallbackRejectsGeneratedDefaultAcrossStarts covers the gap a +// boolean set by hand cannot: a first start with no configuration writes the +// seeded default before the API is ever reached, so a provisioning that failed +// leaves a file the next start reads successfully. Without the managed-config +// marker that file is accepted as a "last known configuration" and the +// endpoint runs on the public freedns upstreams instead of its profile. +func TestMaintenanceFallbackRejectsGeneratedDefaultAcrossStarts(t *testing.T) { + dir := t.TempDir() + oldV, oldFile, oldPath := v, defaultConfigFile, configPath + oldRead, oldUID, oldMarker := configReadFromDisk, cdUID, managedConfigMarkerPath + t.Cleanup(func() { + v, defaultConfigFile, configPath = oldV, oldFile, oldPath + configReadFromDisk, cdUID, managedConfigMarkerPath = oldRead, oldUID, oldMarker + }) + configPath = "" + cdUID = "provisioned-uid" + managedConfigMarkerPath = func() string { return filepath.Join(dir, managedConfigMarkerFile) } + cfgPath := filepath.Join(dir, "ctrld.toml") + + // One process start, using the real config reader and default generation. + start := func(t *testing.T) (*ctrld.Config, bool) { + t.Helper() + v = viper.NewWithOptions(viper.KeyDelimiter("::")) + ctrld.InitConfig(v, "ctrld") + ctrld.SetConfigNameWithPath(v, "ctrld", dir) + defaultConfigFile = cfgPath + configReadFromDisk = false + // writeDefaultConfig is false: the file exists by now, and default + // generation is covered above. + readConfigFile(false, false) + started := &ctrld.Config{} + if err := v.Unmarshal(started); err != nil { + t.Fatalf("unmarshal: %v", err) + } + return started, lastKnownConfigUsable(started) + } + + // The failed first start: no configuration existed, so ctrld generated the + // seeded default and wrote it. Reproduced by writing exactly what + // readConfigFile writes there, minus its listener probe, which binds + // privileged ports and cannot run in a test. What matters for the guard is + // the file's content and that no read set configReadFromDisk. + v = viper.NewWithOptions(viper.KeyDelimiter("::")) + ctrld.InitConfig(v, "ctrld") + defaultConfigFile = cfgPath + configReadFromDisk = false + generated := &ctrld.Config{} + if err := v.Unmarshal(generated); err != nil { + t.Fatalf("unmarshal defaults: %v", err) + } + if err := writeConfigFile(generated); err != nil { + t.Fatalf("write generated default: %v", err) + } + if configReadFromDisk { + t.Fatal("generating a default must not count as reading one") + } + if lastKnownConfigUsable(generated) { + t.Fatal("the first start accepted a configuration it had just generated") + } + // Prove the generated default is the thing that must never be trusted. + var endpoints []string + for _, uc := range generated.Upstream { + endpoints = append(endpoints, uc.Endpoint) + } + if !strings.Contains(strings.Join(endpoints, " "), "freedns.controld.com") { + t.Fatalf("expected the seeded freedns default, got %v", endpoints) + } + + // Second start: the leftover file now reads back through the real reader, + // but no API answer ever produced it. + second, ok := start(t) + if !configReadFromDisk { + t.Fatal("the leftover file was not read, so the boundary is not being exercised") + } + if ok { + t.Fatal("a generated default was accepted as a last known configuration on a later start") + } + + // A genuine provisioning records the marker; the fallback then applies. + if err := recordManagedConfig(cdUID); err != nil { + t.Fatalf("recordManagedConfig: %v", err) + } + if _, ok := start(t); !ok { + t.Fatal("a configuration the API produced was rejected") + } + + // The marker belongs to one identity and must not carry to another. + cdUID = "a-different-uid" + if _, ok := start(t); ok { + t.Fatal("a marker for another identity authorized this one") + } + _ = second +} + +// TestMaintenanceRecoveryDeliversReload covers what recovery has to mean. The +// first answer after a maintenance start is the only one that differs from the +// nil configuration the run holds; if it is compared away, every later answer +// equals the stored one and the excludes and Internal Domains of the previous +// run stay in place for good. +func TestMaintenanceRecoveryDeliversReload(t *testing.T) { + oldFetch, oldDelay, oldUID := fetchResolverConfigFn, maintenanceReloadDelay, cdUID + t.Cleanup(func() { + fetchResolverConfigFn, maintenanceReloadDelay, cdUID = oldFetch, oldDelay, oldUID + }) + cdUID = "test-uid" + maintenanceReloadDelay = func() time.Duration { return time.Millisecond } + + var calls atomic.Int64 + fetchResolverConfigFn = func(context.Context, *controld.ResolverConfigRequest, bool) (*controld.ResolverConfig, error) { + calls.Add(1) + // A plain answer: no custom config, and excludes the run has never seen. + return &controld.ResolverConfig{Exclude: []string{"excluded.example"}}, nil + } + + p := &prog{ + cfg: &ctrld.Config{Service: ctrld.ServiceConfig{RefetchTime: ptrTo(3600)}}, + stopCh: make(chan struct{}), + runAbortCh: make(chan struct{}), + apiForceReloadCh: make(chan struct{}), + apiReloadCh: make(chan *ctrld.Config), + } + p.logger.Store(discardLogger()) + p.startedInAPIMaintenance.Store(true) + + done := make(chan struct{}) + go func() { defer close(done); p.apiConfigReload() }() + t.Cleanup(func() { + close(p.stopCh) + <-done + }) + + select { + case <-p.apiReloadCh: + // Stand in for the reload receiver applying it. + p.maintenanceConfigApplied(discardLogger()) + case <-time.After(10 * time.Second): + t.Fatal("recovery never delivered a reload, so the endpoint kept the configuration it started with") + } + + deadline := time.Now().Add(5 * time.Second) + for p.startedInAPIMaintenance.Load() { + if time.Now().After(deadline) { + t.Fatal("the run stayed in maintenance after applying the configuration") + } + time.Sleep(time.Millisecond) + } + if calls.Load() == 0 { + t.Fatal("no fetch happened") + } +} + +// TestDeactivationPinUnknownDuringMaintenance covers the PIN state a +// maintenance start leaves behind. cdDeactivationPin's default means "the API +// says there is none"; a run that never asked must not present that as +// permission to deactivate. +func TestDeactivationPinUnknownDuringMaintenance(t *testing.T) { + oldUID, oldPin, oldKnown := cdUID, cdDeactivationPin.Load(), deactivationPinKnown.Load() + t.Cleanup(func() { + cdUID = oldUID + cdDeactivationPin.Store(oldPin) + deactivationPinKnown.Store(oldKnown) + deactivationLockedUntil.Store(0) + }) + cdUID = "test-uid" + deactivationLockedUntil.Store(0) + + // The handler re-fetches before it checks the PIN. Without this the test + // reaches the production API with a made-up uid, and the statuses below + // would depend on that call failing rather than on the maintenance answer + // the case is about. + oldFetch := fetchResolverConfig + t.Cleanup(func() { fetchResolverConfig = oldFetch }) + var fetches atomic.Int64 + fetchResolverConfig = func(context.Context, *controld.ResolverConfigRequest, bool) (*controld.ResolverConfig, error) { + fetches.Add(1) + return nil, maintenanceAPIErr(http.StatusServiceUnavailable) + } + + post := func(t *testing.T) int { + t.Helper() + p := &prog{cs: &controlServer{mux: http.NewServeMux()}, pinCodeValidCh: make(chan struct{}, 1)} + p.logger.Store(discardLogger()) + p.registerControlServerHandler() + body, _ := json.Marshal(&deactivationRequest{Pin: defaultDeactivationPin}) + req := httptest.NewRequest(http.MethodPost, deactivationPath, strings.NewReader(string(body))) + w := httptest.NewRecorder() + p.cs.mux.ServeHTTP(w, req) + return w.Code + } + + t.Run("a run that never asked refuses", func(t *testing.T) { + cdDeactivationPin.Store(defaultDeactivationPin) + deactivationPinKnown.Store(false) + if got := post(t); got != http.StatusServiceUnavailable { + t.Fatalf("status = %d, want %d: an unknown PIN authorized deactivation", got, http.StatusServiceUnavailable) + } + }) + + t.Run("a confirmed no-PIN configuration still allows it", func(t *testing.T) { + cdDeactivationPin.Store(defaultDeactivationPin) + deactivationPinKnown.Store(true) + if got := post(t); got != http.StatusOK { + t.Fatalf("status = %d, want 200: a device with no PIN can no longer be deactivated", got) + } + }) + + t.Run("an answering API settles the question", func(t *testing.T) { + // The re-fetch is what makes the PIN knowable again mid-run, so the + // stub has to be able to answer, not only fail. + fetchResolverConfig = func(context.Context, *controld.ResolverConfigRequest, bool) (*controld.ResolverConfig, error) { + return &controld.ResolverConfig{}, nil + } + cdDeactivationPin.Store(defaultDeactivationPin) + deactivationPinKnown.Store(false) + if got := post(t); got != http.StatusOK { + t.Fatalf("status = %d, want 200: an API answer did not settle the PIN", got) + } + if !deactivationPinKnown.Load() { + t.Fatal("an API answer left the PIN unknown") + } + }) + + if fetches.Load() == 0 { + t.Fatal("the handler never reached the fetch seam, so the stub proved nothing") + } +} + +// The client must not report an unverifiable PIN as a wrong one, and must +// still refuse the operation. +func TestUnverifiableDeactivationPinIsARefusal(t *testing.T) { + if !isCheckDeactivationPinErr(errUnverifiableDeactivationPin) { + t.Fatal("an unverifiable PIN did not stop stop/uninstall") + } +} + +// TestManagedMarkerDoesNotOutliveItsConfig covers the provenance residual: a +// marker naming only the identity survives the file it vouched for, so a +// previously managed host whose config goes missing regenerates the seeded +// default and the stale marker certifies it. +func TestManagedMarkerDoesNotOutliveItsConfig(t *testing.T) { + dir := t.TempDir() + oldV, oldFile, oldPath := v, defaultConfigFile, configPath + oldRead, oldUID, oldMarker := configReadFromDisk, cdUID, managedConfigMarkerPath + t.Cleanup(func() { + v, defaultConfigFile, configPath = oldV, oldFile, oldPath + configReadFromDisk, cdUID, managedConfigMarkerPath = oldRead, oldUID, oldMarker + }) + configPath = "" + cdUID = "provisioned-uid" + managedConfigMarkerPath = func() string { return filepath.Join(dir, managedConfigMarkerFile) } + cfgPath := filepath.Join(dir, "ctrld.toml") + + start := func(t *testing.T) bool { + t.Helper() + v = viper.NewWithOptions(viper.KeyDelimiter("::")) + ctrld.InitConfig(v, "ctrld") + ctrld.SetConfigNameWithPath(v, "ctrld", dir) + defaultConfigFile = cfgPath + configReadFromDisk = false + readConfigFile(false, false) + started := &ctrld.Config{} + if err := v.Unmarshal(started); err != nil { + t.Fatalf("unmarshal: %v", err) + } + return lastKnownConfigUsable(started) + } + + // A managed configuration and its marker, written the way a successful run + // writes them. + managed := provisionedConfig() + defaultConfigFile = cfgPath + if err := writeConfigFile(managed); err != nil { + t.Fatalf("write managed config: %v", err) + } + if err := recordManagedConfig(cdUID); err != nil { + t.Fatalf("recordManagedConfig: %v", err) + } + if !start(t) { + t.Fatal("positive control: a managed configuration was rejected") + } + + // The file goes missing and the next start regenerates the seeded default, + // while the marker for the same identity stays behind. + if err := os.Remove(cfgPath); err != nil { + t.Fatal(err) + } + v = viper.NewWithOptions(viper.KeyDelimiter("::")) + ctrld.InitConfig(v, "ctrld") + defaultConfigFile = cfgPath + regenerated := &ctrld.Config{} + if err := v.Unmarshal(regenerated); err != nil { + t.Fatalf("unmarshal defaults: %v", err) + } + if err := writeConfigFile(regenerated); err != nil { + t.Fatalf("write regenerated default: %v", err) + } + var endpoints []string + for _, uc := range regenerated.Upstream { + endpoints = append(endpoints, uc.Endpoint) + } + if !strings.Contains(strings.Join(endpoints, " "), "freedns.controld.com") { + t.Fatalf("expected the seeded freedns default, got %v", endpoints) + } + + if start(t) { + t.Fatal("a stale marker certified a regenerated default") + } +} + +// TestMaintenanceRecoveryOutlivesAFailedApply covers the other residual: the +// reload receiver fetches again and can fail. Acknowledging recovery when the +// trigger is sent would clear the flag while the endpoint still serves what it +// started with, and every later identical answer would then compare equal to +// the already-stored response and never be applied again. +func TestMaintenanceRecoveryOutlivesAFailedApply(t *testing.T) { + oldFetch, oldDelay, oldUID := fetchResolverConfigFn, maintenanceReloadDelay, cdUID + t.Cleanup(func() { + fetchResolverConfigFn, maintenanceReloadDelay, cdUID = oldFetch, oldDelay, oldUID + }) + cdUID = "test-uid" + maintenanceReloadDelay = func() time.Duration { return time.Millisecond } + + // The same answer every time, as a recovered API would give. + fetchResolverConfigFn = func(context.Context, *controld.ResolverConfigRequest, bool) (*controld.ResolverConfig, error) { + return &controld.ResolverConfig{Exclude: []string{"excluded.example"}}, nil + } + + p := &prog{ + cfg: &ctrld.Config{Service: ctrld.ServiceConfig{RefetchTime: ptrTo(3600)}}, + stopCh: make(chan struct{}), + runAbortCh: make(chan struct{}), + apiForceReloadCh: make(chan struct{}), + apiReloadCh: make(chan *ctrld.Config), + } + p.logger.Store(discardLogger()) + p.startedInAPIMaintenance.Store(true) + + // A receiver that takes the trigger and then fails to apply, the way the + // real one does when its own fetch cannot reach the API. + var delivered atomic.Int64 + go func() { + for { + select { + case <-p.apiReloadCh: + delivered.Add(1) + case <-p.stopCh: + return + } + } + }() + + done := make(chan struct{}) + go func() { defer close(done); p.apiConfigReload() }() + t.Cleanup(func() { + close(p.stopCh) + <-done + }) + + // A failed apply must leave the run pending and keep redelivering, even + // though every answer after the first is identical. + deadline := time.Now().Add(10 * time.Second) + for delivered.Load() < 3 { + if time.Now().After(deadline) { + t.Fatalf("redelivery stopped after %d triggers; an identical answer was compared away", delivered.Load()) + } + time.Sleep(time.Millisecond) + } + if !p.startedInAPIMaintenance.Load() { + t.Fatal("recovery was acknowledged without the configuration being applied") + } +} + +// TestManagedReloadKeepsFallbackEligibility covers the provenance residual on +// the reload path: the marker is bound to the file's contents, and every +// API-driven change rewrites that file. A reload that persists a new managed +// configuration without refreshing the marker leaves the host unable to fall +// back on the configuration it just successfully applied. +func TestManagedReloadKeepsFallbackEligibility(t *testing.T) { + dir := t.TempDir() + oldV, oldFile, oldPath := v, defaultConfigFile, configPath + oldRead, oldUID, oldMarker := configReadFromDisk, cdUID, managedConfigMarkerPath + t.Cleanup(func() { + v, defaultConfigFile, configPath = oldV, oldFile, oldPath + configReadFromDisk, cdUID, managedConfigMarkerPath = oldRead, oldUID, oldMarker + }) + configPath = "" + cdUID = "provisioned-uid" + managedConfigMarkerPath = func() string { return filepath.Join(dir, managedConfigMarkerFile) } + cfgPath := filepath.Join(dir, "ctrld.toml") + + // A restart that reads whatever is on disk now, as a maintenance start does. + restart := func(t *testing.T) bool { + t.Helper() + v = viper.NewWithOptions(viper.KeyDelimiter("::")) + ctrld.InitConfig(v, "ctrld") + ctrld.SetConfigNameWithPath(v, "ctrld", dir) + defaultConfigFile = cfgPath + configReadFromDisk = false + readConfigFile(false, false) + started := &ctrld.Config{} + if err := v.Unmarshal(started); err != nil { + t.Fatalf("unmarshal: %v", err) + } + return lastKnownConfigUsable(started) + } + + // Managed startup. + defaultConfigFile = cfgPath + if err := writeConfigFile(provisionedConfig()); err != nil { + t.Fatal(err) + } + if err := recordManagedConfig(cdUID); err != nil { + t.Fatal(err) + } + if !restart(t) { + t.Fatal("a managed startup was not eligible to fall back") + } + + // An API-driven reload persists a changed managed configuration, through + // the same call the reload receiver makes. + changed := provisionedConfig() + changed.Upstream["0"].Endpoint = "https://dns.controld.com/changed5678" + logger := mainLog.Load() + defaultConfigFile = cfgPath + persistManagedConfig(changed, logger) + + onDisk, err := os.ReadFile(cfgPath) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(onDisk), "changed5678") { + t.Fatal("the reload did not persist the changed configuration") + } + if !restart(t) { + t.Fatal("a successfully applied managed configuration was not eligible to fall back") + } + + // The guards must survive: a write with no resolver UID certifies nothing. + cdUID = "" + stillChanged := provisionedConfig() + stillChanged.Upstream["0"].Endpoint = "https://dns.controld.com/unmanaged9999" + persistManagedConfig(stillChanged, logger) + cdUID = "provisioned-uid" + if restart(t) { + t.Fatal("a write outside managed mode refreshed the marker") + } +} + +// TestFallbackStartRewriteKeepsEligibility covers the startup half of the +// content-bound marker. A maintenance fallback fetches nothing, so it records +// no new marker, but it can still rewrite the file through the listener or +// intercept-mode updates. If the marker is left on the pre-write bytes, the +// next start in the same window is denied the fallback this one just used. +func TestFallbackStartRewriteKeepsEligibility(t *testing.T) { + dir := t.TempDir() + oldV, oldFile, oldPath := v, defaultConfigFile, configPath + oldRead, oldUID, oldMarker := configReadFromDisk, cdUID, managedConfigMarkerPath + t.Cleanup(func() { + v, defaultConfigFile, configPath = oldV, oldFile, oldPath + configReadFromDisk, cdUID, managedConfigMarkerPath = oldRead, oldUID, oldMarker + }) + configPath = "" + cdUID = "provisioned-uid" + managedConfigMarkerPath = func() string { return filepath.Join(dir, managedConfigMarkerFile) } + cfgPath := filepath.Join(dir, "ctrld.toml") + + restart := func(t *testing.T) bool { + t.Helper() + v = viper.NewWithOptions(viper.KeyDelimiter("::")) + ctrld.InitConfig(v, "ctrld") + ctrld.SetConfigNameWithPath(v, "ctrld", dir) + defaultConfigFile = cfgPath + configReadFromDisk = false + readConfigFile(false, false) + started := &ctrld.Config{} + if err := v.Unmarshal(started); err != nil { + t.Fatalf("unmarshal: %v", err) + } + return lastKnownConfigUsable(started) + } + + // An earlier successful run left a managed configuration and its marker. + defaultConfigFile = cfgPath + if err := writeConfigFile(provisionedConfig()); err != nil { + t.Fatal(err) + } + if err := recordManagedConfig(cdUID); err != nil { + t.Fatal(err) + } + if !restart(t) { + t.Fatal("a managed configuration was not eligible to fall back") + } + + // A maintenance start: no fetch happened, so nothing new is recorded, but + // the listener/intercept updates still rewrite the file. This mirrors the + // startup ordering in run(): remember provenance, rewrite, then re-record. + managedConfigFetched := false + wasManagedConfig := managedConfigRecordedFor(cdUID) + if !wasManagedConfig { + t.Fatal("the pre-write file was not recognized as managed") + } + rewritten := provisionedConfig() + rewritten.Listener["0"].Port = 5354 // e.g. the macOS intercept port fallback + if err := writeConfigFile(rewritten); err != nil { + t.Fatal(err) + } + recordManagedConfigForStart(managedConfigFetched, wasManagedConfig) + + if !restart(t) { + t.Fatal("a rewrite during a maintenance fallback denied the next start its fallback") + } + + // The guard must survive: a rewrite with no prior managed provenance and no + // fetch still certifies nothing. + if err := os.Remove(managedConfigMarkerPath()); err != nil { + t.Fatal(err) + } + if managedConfigRecordedFor(cdUID) { + t.Fatal("the marker survived its own removal") + } + if restart(t) { + t.Fatal("a file with no managed provenance was accepted") + } +} diff --git a/cmd/cli/api_maintenance_test.go b/cmd/cli/api_maintenance_test.go new file mode 100644 index 0000000..bb84e2c --- /dev/null +++ b/cmd/cli/api_maintenance_test.go @@ -0,0 +1,376 @@ +package cli + +import ( + "context" + "net/http" + "path/filepath" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/Control-D-Inc/ctrld" + "github.com/Control-D-Inc/ctrld/internal/controld" +) + +// maintenanceAPIErr builds the API's hard maintenance answer: the dedicated +// code with the maintenance message. +func maintenanceAPIErr(status int) error { + e := &controld.ErrorResponse{StatusCode: status} + e.ErrorField.Code = controld.MaintenanceCode + e.ErrorField.Message = controld.MaintenanceMessage + " Please try again later." + return e +} + +// legacyMaintenanceAPIErr is the answer of a deployment that predates the +// dedicated code: the maintenance message alone. +func legacyMaintenanceAPIErr(status int) error { + e := &controld.ErrorResponse{StatusCode: status} + e.ErrorField.Message = controld.MaintenanceMessage + " Please try again later." + return e +} + +// deviceInvalidErr models the API refusing a device that no longer exists. +func deviceInvalidErr() error { + e := &controld.ErrorResponse{StatusCode: http.StatusNotFound} + e.ErrorField.Code = controld.InvalidConfigCode + e.ErrorField.Message = "device does not exist" + return e +} + +// provisionedConfig models the configuration an earlier successful run wrote. +func provisionedConfig() *ctrld.Config { + return &ctrld.Config{ + Listener: map[string]*ctrld.ListenerConfig{ + "0": {IP: "127.0.0.1", Port: 53}, + }, + Network: map[string]*ctrld.NetworkConfig{ + "0": {Name: "Network 0", Cidrs: []string{"0.0.0.0/0"}}, + }, + Upstream: map[string]*ctrld.UpstreamConfig{ + "0": {Name: "Control D", Type: ctrld.ResolverTypeDOH, Endpoint: "https://dns.controld.com/abcd1234", Timeout: 5000}, + }, + } +} + +// withConfigOnDisk sets up what startup would have left behind: whether a +// configuration file was read, and whether a previous run recorded that the +// API produced it. Both are required for a fallback, and the marker lives in a +// temp dir so a real one on this host cannot influence the result. +func withConfigOnDisk(t *testing.T, read, managed bool) { + t.Helper() + oldRead, oldMarker, oldUID := configReadFromDisk, managedConfigMarkerPath, cdUID + t.Cleanup(func() { + configReadFromDisk, managedConfigMarkerPath, cdUID = oldRead, oldMarker, oldUID + }) + dir := t.TempDir() + managedConfigMarkerPath = func() string { return filepath.Join(dir, managedConfigMarkerFile) } + oldFile := defaultConfigFile + t.Cleanup(func() { defaultConfigFile = oldFile }) + defaultConfigFile = filepath.Join(dir, "ctrld.toml") + cdUID = "test-uid" + configReadFromDisk = read + if managed { + // The marker is bound to the file it vouches for, so one has to exist. + if err := writeConfigFile(provisionedConfig()); err != nil { + t.Fatalf("write config: %v", err) + } + if err := recordManagedConfig(cdUID); err != nil { + t.Fatalf("recordManagedConfig: %v", err) + } + } +} + +// Test_apiFailureCodeClassifiesMaintenance covers the classification the whole +// fallback rests on. Maintenance must not be reported as unreachability, which +// is what the incident showed (API_UNREACHABLE after a successful connection), +// and must never reach the device-invalid path that self-uninstalls. +func Test_apiFailureCodeClassifiesMaintenance(t *testing.T) { + for _, answer := range []func(int) error{maintenanceAPIErr, legacyMaintenanceAPIErr} { + for _, status := range []int{http.StatusServiceUnavailable, http.StatusBadGateway, http.StatusBadRequest, http.StatusNotFound} { + code, ok := apiFailureCode(answer(status)) + if !ok || code != provisionCodeAPIMaintenance { + t.Fatalf("status %d: code = %q ok = %v, want API_MAINTENANCE", status, code, ok) + } + } + } +} + +// Test_maintenanceIsNotAPermanentRejection covers the status trap: a +// maintenance answer carrying a client-error status would otherwise be read as +// "this request will be refused again" and stop the service for good. +func Test_maintenanceIsNotAPermanentRejection(t *testing.T) { + for _, answer := range []func(int) error{maintenanceAPIErr, legacyMaintenanceAPIErr} { + for _, status := range []int{http.StatusBadRequest, http.StatusNotFound, http.StatusForbidden} { + if _, permanent := permanentAPIRejection(answer(status)); permanent { + t.Fatalf("status %d classified as a permanent rejection", status) + } + } + } +} + +func Test_useLastKnownConfigDuringMaintenance(t *testing.T) { + t.Run("a previously working config carries the start", func(t *testing.T) { + withConfigOnDisk(t, true, true) + if !useLastKnownConfigDuringMaintenance(maintenanceAPIErr(http.StatusServiceUnavailable), provisionedConfig()) { + t.Fatal("maintenance did not fall back to the config on disk") + } + }) + + t.Run("a generated default is not a fallback", func(t *testing.T) { + // A fresh install: no file existed, so startup wrote the freedns + // default. Serving it would answer outside the operator's profile. + withConfigOnDisk(t, false, false) + if useLastKnownConfigDuringMaintenance(maintenanceAPIErr(http.StatusServiceUnavailable), provisionedConfig()) { + t.Fatal("a generated default was accepted as a last known configuration") + } + }) + + t.Run("an unusable config on disk is not a fallback", func(t *testing.T) { + withConfigOnDisk(t, true, true) + cfg := provisionedConfig() + cfg.Upstream = nil + if useLastKnownConfigDuringMaintenance(maintenanceAPIErr(http.StatusServiceUnavailable), cfg) { + t.Fatal("an invalid config was accepted as a last known configuration") + } + }) + + t.Run("a file with no record of an API answer is not a fallback", func(t *testing.T) { + withConfigOnDisk(t, true, false) + if useLastKnownConfigDuringMaintenance(maintenanceAPIErr(http.StatusServiceUnavailable), provisionedConfig()) { + t.Fatal("a configuration no API answer produced was accepted") + } + }) + + t.Run("other failures never take this path", func(t *testing.T) { + withConfigOnDisk(t, true, true) + for _, err := range []error{retryableNetworkErr(), deviceInvalidErr()} { + if useLastKnownConfigDuringMaintenance(err, provisionedConfig()) { + t.Fatalf("%v was treated as maintenance", err) + } + } + }) +} + +// Test_maintenanceWithoutFallbackFails covers the other half of the request: +// when there is nothing to fall back on, ctrld still stops, but says what +// cannot be done rather than reporting a fetch or connectivity failure. +func Test_maintenanceWithoutFallbackFails(t *testing.T) { + exitCode, notified := stubProvisionGlobals(t) + uninstalled := false + uninstallInvalidCdUIDFn = func(*prog, *ctrld.Logger, bool) bool { uninstalled = true; return true } + + handleAPIPreflightFailure(&prog{}, maintenanceAPIErr(http.StatusServiceUnavailable), func() { *notified = true }) + + if want := provisionExitCodeForCode[provisionCodeAPIMaintenance]; *exitCode != want { + t.Errorf("exit = %d, want %d", *exitCode, want) + } + if uninstalled { + t.Error("maintenance reached the self-uninstall path") + } + if !*notified { + t.Error("notify not called") + } + r, err := readProvisionResult() + if err != nil { + t.Fatal(err) + } + if r.Code != string(provisionCodeAPIMaintenance) { + t.Errorf("code = %q, want API_MAINTENANCE", r.Code) + } + if r.Message != maintenanceNoConfigDetail { + t.Errorf("message = %q, want the maintenance guidance", r.Message) + } +} + +// maintenanceWithDeviceCode is the answer that makes the guard load-bearing: +// maintenance carried alongside the device-invalid code. Without the guard in +// selfUninstallCheck, this uninstalls a working install over a temporary API +// outage. +func maintenanceWithDeviceCode() error { + e := &controld.ErrorResponse{StatusCode: http.StatusServiceUnavailable} + e.ErrorField.Code = controld.InvalidConfigCode + e.ErrorField.Message = controld.MaintenanceMessage + " Please try again later." + return e +} + +func stubSelfUninstall(t *testing.T) *bool { + t.Helper() + old := selfUninstallFn + t.Cleanup(func() { selfUninstallFn = old }) + called := false + selfUninstallFn = func(*prog, *ctrld.Logger) { called = true } + return &called +} + +func newSelfUninstallTestProg() *prog { + return &prog{dnsWatcherStopCh: make(chan struct{})} +} + +func Test_selfUninstallCheckSkipsMaintenance(t *testing.T) { + t.Run("maintenance never uninstalls", func(t *testing.T) { + called := stubSelfUninstall(t) + p := newSelfUninstallTestProg() + selfUninstallCheck(maintenanceWithDeviceCode(), p, discardLogger()) + if *called { + t.Fatal("maintenance triggered a self-uninstall") + } + select { + case <-p.dnsWatcherStopCh: + t.Fatal("maintenance stopped the DNS watchers") + default: + } + }) + + t.Run("a real device rejection still uninstalls", func(t *testing.T) { + called := stubSelfUninstall(t) + selfUninstallCheck(deviceInvalidErr(), newSelfUninstallTestProg(), discardLogger()) + if !*called { + t.Fatal("a deleted device no longer self-uninstalls") + } + }) + + t.Run("an unrelated failure does nothing", func(t *testing.T) { + called := stubSelfUninstall(t) + selfUninstallCheck(retryableNetworkErr(), newSelfUninstallTestProg(), discardLogger()) + if *called { + t.Fatal("a network failure triggered a self-uninstall") + } + }) +} + +// Test_logResolverConfigFetchFailure covers what the reload paths say. The +// incident was first triaged as a connectivity fault because the only line was +// "could not fetch resolver config" after a successful connection. +func Test_logResolverConfigFetchFailure(t *testing.T) { + t.Run("maintenance is named and retained", func(t *testing.T) { + logs := captureDebugMainLog(t) + logger := mainLog.Load().With().Str("mode", "api-reload") + logResolverConfigFetchFailure(logger, maintenanceAPIErr(http.StatusServiceUnavailable)) + + events := jsonLogEvents(t, logs, "Control D API is in maintenance; keeping the current configuration until it returns") + if len(events) != 1 { + t.Fatalf("maintenance events: got %d, want 1", len(events)) + } + if events[0]["reason"] != "api_maintenance" { + t.Errorf("reason = %v, want api_maintenance", events[0]["reason"]) + } + if events[0]["journal"] != true { + t.Error("the maintenance line is not retained in the journal") + } + }) + + t.Run("other failures keep the existing line", func(t *testing.T) { + logs := captureDebugMainLog(t) + logger := mainLog.Load().With().Str("mode", "api-reload") + logResolverConfigFetchFailure(logger, retryableNetworkErr()) + + if got := len(jsonLogEvents(t, logs, "Could not fetch resolver config")); got != 1 { + t.Fatalf("generic fetch-failure events: got %d, want 1", got) + } + }) +} + +// Test_maintenanceReloadDelay covers the two properties the retry depends on: +// it is far below refetch_time so a degraded run recovers promptly, and it is +// spread so hosts that fell back together do not return as one burst. +func Test_maintenanceReloadDelay(t *testing.T) { + const refetchDefault = time.Hour + seen := make(map[time.Duration]bool) + for range 200 { + d := maintenanceReloadDelay() + if d < maintenanceReloadInterval || d >= maintenanceReloadInterval+maintenanceReloadJitter { + t.Fatalf("delay %s outside [%s, %s)", d, maintenanceReloadInterval, maintenanceReloadInterval+maintenanceReloadJitter) + } + if d >= refetchDefault { + t.Fatalf("delay %s is no faster than the normal refetch interval", d) + } + seen[d] = true + } + if len(seen) < 2 { + t.Fatal("every delay was identical; the fleet would retry in lockstep") + } +} + +// Test_maintenanceRetryRecoversBeforeRefetchInterval drives the real reload +// loop: a run that started on the configuration on disk must ask the API again +// without waiting out refetch_time, and must stop asking once it gets an +// answer. +func Test_maintenanceRetryRecoversBeforeRefetchInterval(t *testing.T) { + oldFetch, oldDelay, oldUID := fetchResolverConfigFn, maintenanceReloadDelay, cdUID + t.Cleanup(func() { + fetchResolverConfigFn, maintenanceReloadDelay, cdUID = oldFetch, oldDelay, oldUID + }) + cdUID = "test-uid" + maintenanceReloadDelay = func() time.Duration { return time.Millisecond } + + var calls atomic.Int64 + recovered := make(chan struct{}) + var once sync.Once + fetchResolverConfigFn = func(context.Context, *controld.ResolverConfigRequest, bool) (*controld.ResolverConfig, error) { + // Stay in maintenance for the first few retries, then answer. + if calls.Add(1) < 3 { + return nil, maintenanceAPIErr(http.StatusServiceUnavailable) + } + once.Do(func() { close(recovered) }) + return &controld.ResolverConfig{}, nil + } + + p := &prog{ + cfg: &ctrld.Config{ + // An hour: the test must pass because of the maintenance retry, + // never because the normal ticker happened to fire. + Service: ctrld.ServiceConfig{RefetchTime: ptrTo(3600)}, + }, + stopCh: make(chan struct{}), + runAbortCh: make(chan struct{}), + apiForceReloadCh: make(chan struct{}), + apiReloadCh: make(chan *ctrld.Config), + } + p.logger.Store(discardLogger()) + p.startedInAPIMaintenance.Store(true) + + // Recovery now means the configuration reached the reload, so something + // has to receive it. + go func() { + for { + select { + case <-p.apiReloadCh: + // Stand in for the reload receiver applying it. + p.maintenanceConfigApplied(discardLogger()) + case <-p.stopCh: + return + } + } + }() + + done := make(chan struct{}) + go func() { defer close(done); p.apiConfigReload() }() + t.Cleanup(func() { + close(p.stopCh) + <-done + }) + + select { + case <-recovered: + case <-time.After(10 * time.Second): + t.Fatal("the maintenance retry never reached the API") + } + + // The flag clears on the successful fetch, which is what stops the retry. + deadline := time.Now().Add(5 * time.Second) + for p.startedInAPIMaintenance.Load() { + if time.Now().After(deadline) { + t.Fatal("the run stayed in maintenance after the API answered") + } + time.Sleep(time.Millisecond) + } + + settled := calls.Load() + time.Sleep(50 * time.Millisecond) + if extra := calls.Load() - settled; extra > 1 { + t.Fatalf("the faster retry kept running after recovery: %d more fetches", extra) + } +} + +func ptrTo[T any](v T) *T { return &v } diff --git a/cmd/cli/cli.go b/cmd/cli/cli.go index 13887ed..2842f05 100644 --- a/cmd/cli/cli.go +++ b/cmd/cli/cli.go @@ -390,6 +390,9 @@ func run(appCallback *AppCallback, stopCh chan struct{}) { p.cs = cs oldLogPath := cfg.Service.LogPath + // Whether this run got its configuration from the API, which is what the + // managed-config marker may claim once the configuration is on disk. + managedConfigFetched := false if uid := cdUIDFromProvToken(); uid != "" { cdUID = uid p.initLoggingAfterProvisioning() @@ -422,16 +425,34 @@ func run(appCallback *AppCallback, stopCh chan struct{}) { return } - handleAPIPreflightFailure(p, pf.err, p.notifyExitToLogServer) - return + // Maintenance is temporary and says nothing about this host's + // configuration, so a start that already has a working one carries + // on with it rather than taking DNS down for the window. + if !useLastKnownConfigDuringMaintenance(pf.err, &cfg) { + handleAPIPreflightFailure(p, pf.err, p.notifyExitToLogServer) + return + } + // This run holds no answer from the API, so the reload loop asks + // again well before refetch_time would. + p.startedInAPIMaintenance.Store(true) default: p.mu.Lock() p.rc = pf.rc p.mu.Unlock() + managedConfigFetched = true } } updated := updateListenerConfig(&cfg, p.notifyExitToLogServer) + // Whether the file on disk, before anything below rewrites it, is already + // one the API produced for this identity. + wasManagedConfig := managedConfigRecordedFor(cdUID) + // The marker must never vouch for a configuration that was not written. + // Without this, a first managed run whose listener needed no change would + // record the marker over the seeded default still on disk. + if managedConfigFetched && !wasManagedConfig { + updated = true + } // Bootstrap and listener binding both succeeded, so an earlier run's // recorded failure no longer describes this install. @@ -475,6 +496,7 @@ func run(appCallback *AppCallback, stopCh chan struct{}) { } p.Info().Msg("Writing config file to: " + defaultConfigFile) } + recordManagedConfigForStart(managedConfigFetched, wasManagedConfig) if newLogPath := cfg.Service.LogPath; newLogPath != "" && oldLogPath != newLogPath { p.switchLogPath(oldLogPath, newLogPath) @@ -587,6 +609,9 @@ func readConfigFile(writeDefaultConfig, notice bool) bool { // If err == nil, there's a config supplied via `--config`, no default config written. err := v.ReadInConfig() if err == nil { + // Only a file the host already had can stand in for the API answer + // during maintenance; the default written below cannot. + configReadFromDisk = true if notice { mainLog.Load().Notice().Msg("Reading config: " + v.ConfigFileUsed()) } @@ -740,6 +765,15 @@ func processNoConfigFlags(noConfigStart bool) { // If cdDeactivationPin equals to this default, it means the pin code is not set from Control D API. const defaultDeactivationPin = -1 +// deactivationPinKnown reports whether this run ever had an answer from the API +// about the deactivation PIN. +// +// cdDeactivationPin cannot carry this: its default means "the API said there is +// no PIN", and a run that started on the configuration on disk during +// maintenance never asked. Without the distinction such a run would present an +// unknown PIN as no PIN and hand out deactivation to anyone who asked. +var deactivationPinKnown atomic.Bool + // cdDeactivationPin is used in cd mode to decide whether stop and uninstall commands can be run. var cdDeactivationPin atomic.Int64 @@ -813,6 +847,11 @@ func permanentAPIRejection(err error) (*controld.ErrorResponse, bool) { if !errors.As(err, &uer) { return nil, false } + // Maintenance is the API being away, not the API refusing this request, so + // it stays retryable even when it arrives with a client-error status. + if controld.IsMaintenance(err) { + return nil, false + } switch uer.StatusCode { case http.StatusRequestTimeout, http.StatusTooManyRequests: return nil, false @@ -832,6 +871,11 @@ func apiFailureCode(err error) (provisionFailureCode, bool) { if err == nil { return "", false } + // Before the device check: maintenance must never reach the self-uninstall + // path, because it says nothing about whether this device still exists. + if controld.IsMaintenance(err) { + return provisionCodeAPIMaintenance, true + } var uer *controld.ErrorResponse if errors.As(err, &uer) && uer.ErrorField.Code == controld.InvalidConfigCode { return provisionCodeAPIDeviceInvalid, true @@ -924,6 +968,12 @@ func handleAPIPreflightFailure(p *prog, err error, notify func()) { } cdLogger := logger.With().Str("mode", "cd") code, _ := apiFailureCode(err) + if code == provisionCodeAPIMaintenance { + // The caller already established there is nothing to fall back on. + cdLogger.Error().Err(err).Msg("Failed to fetch resolver config, the Control D API is in maintenance") + failProvision(newProvisionResult(code, maintenanceNoConfigDetail, nil, provisionSecrets()...), notify) + return + } var uer *controld.ErrorResponse if errors.As(err, &uer) && uer.ErrorField.Code == controld.InvalidConfigCode { r := newProvisionResult(code, apiRejectionSummary(uer.StatusCode), nil, provisionSecrets()...) @@ -1046,10 +1096,21 @@ func processCDFlags(ctx context.Context, cfg *ctrld.Config) (*controld.ResolverC if isMobile() { return nil, err } - logger.Warn().Err(err).Msg("Could not fetch resolver config") + if controld.IsMaintenance(err) { + // The caller reports this: only it knows whether a fallback is + // available. Saying "Could not fetch resolver config" here puts a + // connectivity-shaped failure at the top of the bootstrap log, + // which is how the incident behind this was first mis-triaged. + logger.Debug().Err(err).Msg("Resolver config fetch answered with maintenance") + } else { + logger.Warn().Err(err).Msg("Could not fetch resolver config") + } return nil, err } + // The answer settles the PIN either way: a response without one means the + // API says there is none, which is not the same as never having asked. + deactivationPinKnown.Store(true) if resolverConfig.DeactivationPin != nil { logger.Debug().Msg("Saving deactivation pin") cdDeactivationPin.Store(*resolverConfig.DeactivationPin) @@ -2430,6 +2491,12 @@ var errRequiredDeactivationPin = errors.New("deactivation pin is required to sto // errTooManyDeactivationPin represents an error indicating excessive deactivation PIN request attempts. var errTooManyDeactivationPin = errors.New("too many request attempts") +// errUnverifiableDeactivationPin reports that this run never had an answer from +// the API about the PIN, so it can neither check one nor establish that none is +// required. Refusing is the only safe answer: the alternative hands a +// PIN-protected device to whoever asks for the duration of an API outage. +var errUnverifiableDeactivationPin = errors.New("cannot verify the deactivation pin while the Control D API is unavailable, try again once it returns") + // checkDeactivationPin validates if the deactivation pin matches one in ControlD config. func checkDeactivationPin(s service.Service, stopCh chan struct{}) error { mainLog.Load().Debug().Msg("Checking deactivation pin") @@ -2466,6 +2533,9 @@ func checkDeactivationPin(s service.Service, stopCh chan struct{}) error { return nil // valid pin case http.StatusNotFound: return nil // the server is running older version of ctrld + case http.StatusServiceUnavailable: + mainLog.Load().Error().Msg(errUnverifiableDeactivationPin.Error()) + return errUnverifiableDeactivationPin } } mainLog.Load().Error().Err(err).Msg(errInvalidDeactivationPin.Error()) @@ -2476,7 +2546,8 @@ func checkDeactivationPin(s service.Service, stopCh chan struct{}) error { func isCheckDeactivationPinErr(err error) bool { return errors.Is(err, errInvalidDeactivationPin) || errors.Is(err, errRequiredDeactivationPin) || - errors.Is(err, errTooManyDeactivationPin) + errors.Is(err, errTooManyDeactivationPin) || + errors.Is(err, errUnverifiableDeactivationPin) } // ensureUninstall ensures that s.Uninstall will remove ctrld service from system completely. diff --git a/cmd/cli/control_server.go b/cmd/cli/control_server.go index e75e409..253df5c 100644 --- a/cmd/cli/control_server.go +++ b/cmd/cli/control_server.go @@ -241,7 +241,8 @@ func (p *prog) registerControlServerHandler() { Version: appVersion, Metadata: ctrld.SystemMetadataRuntime(context.Background()), } - if rc, err := controld.FetchResolverConfig(loggerCtx, rcReq, cdDev); rc != nil { + if rc, err := fetchResolverConfig(loggerCtx, rcReq, cdDev); rc != nil { + deactivationPinKnown.Store(true) if rc.DeactivationPin != nil { cdDeactivationPin.Store(*rc.DeactivationPin) } else { @@ -261,8 +262,17 @@ func (p *prog) registerControlServerHandler() { p.Warn().Err(err).Msg("Could not re-fetch deactivation pin code") } - // If pin code not set, allowing deactivation. + // If pin code not set, allowing deactivation - but only when the API + // actually said so. A run that started on the configuration on disk + // during maintenance never asked, and the re-fetch above leaves the + // default untouched while the API stays away, so an unknown PIN would + // otherwise read as no PIN and authorize the deactivation it protects. if !deactivationPinSet() { + if !deactivationPinKnown.Load() { + p.Warn().Msg("Cannot verify deactivation pin: no answer from the Control D API this run") + w.WriteHeader(http.StatusServiceUnavailable) + return + } w.WriteHeader(http.StatusOK) return } diff --git a/cmd/cli/dns_proxy.go b/cmd/cli/dns_proxy.go index 73f6f99..ca75103 100644 --- a/cmd/cli/dns_proxy.go +++ b/cmd/cli/dns_proxy.go @@ -1645,12 +1645,12 @@ func (p *prog) doSelfUninstall(pr *proxyResponse) { Version: appVersion, Metadata: ctrld.SystemMetadata(loggerCtx), } - _, err := controld.FetchResolverConfig(loggerCtx, req, cdDev) + _, err := fetchResolverConfig(loggerCtx, req, cdDev) logger.Debug().Msg("Maximum number of refused queries reached, checking device status") selfUninstallCheck(err, p, logger) if err != nil { - logger.Warn().Err(err).Msg("Could not fetch resolver config") + logResolverConfigFetchFailure(logger, err) } // Cool-of period to prevent abusing the API. go p.selfUninstallCoolOfPeriod() diff --git a/cmd/cli/prog.go b/cmd/cli/prog.go index fc937f7..e2d57d0 100644 --- a/cmd/cli/prog.go +++ b/cmd/cli/prog.go @@ -164,10 +164,17 @@ type prog struct { lastNAT64Mu sync.Mutex lastNAT64Logged string - selfUninstallMu sync.Mutex - refusedQueryCount int - canSelfUninstall atomic.Bool - checkingSelfUninstall bool + selfUninstallMu sync.Mutex + refusedQueryCount int + canSelfUninstall atomic.Bool + + // startedInAPIMaintenance records that this run began on the configuration + // already on disk because the API was in maintenance, so it holds no answer + // from the API at all. The reload loop retries faster while it is set, and + // clears it once a recovered configuration has been applied - not when a + // fetch returns, because the reload receiver fetches again and can fail. + startedInAPIMaintenance atomic.Bool + checkingSelfUninstall bool loopMu sync.Mutex loop map[string]bool @@ -585,14 +592,16 @@ func (p *prog) runWait() { } addExtraSplitDnsRule(newCfg) - if err := writeConfigFile(newCfg); err != nil { - p.Error().Err(err).Msg("Could not write new config") - } + persistManagedConfig(newCfg, p.logger.Load()) // This needs to be done here, otherwise, the DNS handler may observe an invalid // upstream config because its initialization function have not been called yet. p.Debug().Msg("Setup upstream with new config") p.setupUpstream(newCfg) + // The configuration is in effect now. Every failure above continues the + // loop without applying anything, so a run that started in maintenance + // stays pending and keeps retrying until one gets this far. + p.maintenanceConfigApplied(p.logger.Load()) p.mu.Lock() oldUpstreams := p.cfg.Upstream @@ -762,7 +771,7 @@ func (p *prog) apiConfigReload() { } selfUninstallCheck(err, p, logger) if err != nil { - logger.Warn().Err(err).Msg("Could not fetch resolver config") + logResolverConfigFetchFailure(logger, err) return } @@ -773,12 +782,37 @@ func (p *prog) apiConfigReload() { lastUpdated = p.applyFetchedResolverConfig(loggerCtx, logger, resolverConfig, forced, lastUpdated) } + // A run that started in maintenance holds no answer from the API, so it + // waits out the whole refetch interval before it has one. Retry faster + // until it does, without touching refetch_time: that interval governs + // every endpoint, and shortening it fleet-wide would multiply load on an + // API that is by definition already struggling. + var maintenanceRetry *time.Timer + var maintenanceRetryC <-chan time.Time + if p.startedInAPIMaintenance.Load() { + maintenanceRetry = time.NewTimer(maintenanceReloadDelay()) + defer maintenanceRetry.Stop() + maintenanceRetryC = maintenanceRetry.C + logger.Debug().Msg("Started on the configuration on disk, retrying the API faster until it answers") + } for { select { case <-p.apiForceReloadCh: doReloadApiConfig(true, logger.With().Bool("forced", true)) case <-ticker.C: doReloadApiConfig(false, logger) + case <-maintenanceRetryC: + // The normal ticker or a forced reload may have recovered first. + if !p.startedInAPIMaintenance.Load() { + maintenanceRetryC = nil + continue + } + doReloadApiConfig(false, logger.With().Bool("maintenance_retry", true)) + if p.startedInAPIMaintenance.Load() { + maintenanceRetry.Reset(maintenanceReloadDelay()) + continue + } + maintenanceRetryC = nil case <-ctx.Done(): return } @@ -806,6 +840,9 @@ func (p *prog) applyFetchedResolverConfig( if loggerCtx.Err() != nil { return lastUpdated } + // The answer settles the PIN either way: a response without one means the + // API says there is none, which is not the same as never having asked. + deactivationPinKnown.Store(true) if resolverConfig.DeactivationPin != nil { newDeactivationPin := *resolverConfig.DeactivationPin curDeactivationPin := cdDeactivationPin.Load() @@ -835,11 +872,22 @@ func (p *prog) applyFetchedResolverConfig( noExcludeListChanged := true // Regenerate Internal Domains on add, removal, domain, mode or resolver change. noInternalDomainsChanged := true - if rc != nil { + // A run that started on the configuration on disk has applied no answer + // from the API yet, so nothing here is known to be current and the + // response has to reach the apply path instead of being compared + // against a configuration that was never applied. This holds until the + // apply lands, not until a fetch returns: the receiver fetches again + // and can fail, and p.rc is already replaced above, so comparing would + // make every later response equal the stored one and leave the excludes + // and Internal Domains of the previous run in place for good. + pendingRecovery := p.startedInAPIMaintenance.Load() + if rc != nil && !pendingRecovery { slices.Sort(rc.Exclude) slices.Sort(resolverConfig.Exclude) noExcludeListChanged = slices.Equal(rc.Exclude, resolverConfig.Exclude) noInternalDomainsChanged = internalDomainsEqual(rc.SplitDNS, resolverConfig.SplitDNS) + } else { + noExcludeListChanged, noInternalDomainsChanged = false, false } if noCustomConfig && noExcludeListChanged && noInternalDomainsChanged { return lastUpdated @@ -860,7 +908,7 @@ func (p *prog) applyFetchedResolverConfig( return lastUpdated } - if resolverConfig.Ctrld.CustomLastUpdate > lastUpdated || forced { + if resolverConfig.Ctrld.CustomLastUpdate > lastUpdated || forced || pendingRecovery { lastUpdated = time.Now().Unix() cfg := &ctrld.Config{} var cfgErr error @@ -2389,13 +2437,26 @@ func (p *prog) dnsChanged(iface *net.Interface, nameservers []string) bool { } // selfUninstallCheck checks if the error dues to controld.InvalidConfigCode, perform self-uninstall then. +// selfUninstallFn is the uninstall itself, indirected so a test can prove what +// does and does not reach it. The real one ends in os.Exit, which a test cannot +// survive to make an assertion about. +var selfUninstallFn = selfUninstall + func selfUninstallCheck(uninstallErr error, p *prog, logger *ctrld.Logger) { + // Maintenance is the API being away; it establishes nothing about whether + // this device still exists. Guarded here rather than at each caller because + // this is the one place that acts, and acting would uninstall a working + // install over a temporary outage. + if controld.IsMaintenance(uninstallErr) { + logger.Debug().Msg("API is in maintenance, skipping device status check") + return + } var uer *controld.ErrorResponse if errors.As(uninstallErr, &uer) && uer.ErrorField.Code == controld.InvalidConfigCode { p.stopDnsWatchers() // Perform self-uninstall now. - selfUninstall(p, logger) + selfUninstallFn(p, logger) } } diff --git a/cmd/cli/provision_result.go b/cmd/cli/provision_result.go index c123476..19940e1 100644 --- a/cmd/cli/provision_result.go +++ b/cmd/cli/provision_result.go @@ -38,6 +38,7 @@ const ( provisionCodeAPIUnreachable provisionFailureCode = "API_UNREACHABLE" provisionCodeAPIRejected provisionFailureCode = "API_REJECTED" provisionCodeAPIDeviceInvalid provisionFailureCode = "API_DEVICE_INVALID" + provisionCodeAPIMaintenance provisionFailureCode = "API_MAINTENANCE" provisionCodeTokenInvalid provisionFailureCode = "TOKEN_INVALID" provisionCodeTokenExpired provisionFailureCode = "TOKEN_EXPIRED" provisionCodeTokenLimitReached provisionFailureCode = "TOKEN_LIMIT_REACHED" @@ -58,6 +59,7 @@ var allProvisionFailureCodes = []provisionFailureCode{ provisionCodeAPIUnreachable, provisionCodeAPIRejected, provisionCodeAPIDeviceInvalid, + provisionCodeAPIMaintenance, provisionCodeTokenInvalid, provisionCodeTokenExpired, provisionCodeTokenLimitReached, @@ -78,6 +80,7 @@ var provisionStageForCode = map[provisionFailureCode]provisionStage{ provisionCodeAPIUnreachable: provisionStageBootstrap, provisionCodeAPIRejected: provisionStageBootstrap, provisionCodeAPIDeviceInvalid: provisionStageBootstrap, + provisionCodeAPIMaintenance: provisionStageBootstrap, provisionCodeTokenInvalid: provisionStageBootstrap, provisionCodeTokenExpired: provisionStageBootstrap, provisionCodeTokenLimitReached: provisionStageBootstrap, @@ -102,6 +105,7 @@ var provisionExitCodeForCode = map[provisionFailureCode]int{ provisionCodeAPIUnreachable: 30, provisionCodeAPIRejected: 31, provisionCodeAPIDeviceInvalid: 32, + provisionCodeAPIMaintenance: 37, provisionCodeTokenInvalid: 33, provisionCodeTokenExpired: 34, provisionCodeTokenLimitReached: 35, diff --git a/cmd/cli/provision_result_test.go b/cmd/cli/provision_result_test.go index e4720ab..b3afdd1 100644 --- a/cmd/cli/provision_result_test.go +++ b/cmd/cli/provision_result_test.go @@ -68,8 +68,8 @@ func TestProvisionCodesMapToOneStageAndInRangeExit(t *testing.T) { } seenExits[exit] = code } - if len(allProvisionFailureCodes) != 17 { - t.Errorf("expected 17 codes, got %d", len(allProvisionFailureCodes)) + if len(allProvisionFailureCodes) != 18 { + t.Errorf("expected 18 codes, got %d", len(allProvisionFailureCodes)) } } diff --git a/docs/provisioning-failure-codes.md b/docs/provisioning-failure-codes.md index e1f3ea6..bff84b4 100644 --- a/docs/provisioning-failure-codes.md +++ b/docs/provisioning-failure-codes.md @@ -42,6 +42,7 @@ The **Next action / evidence** column lists what to look at next. | `API_UNREACHABLE` | bootstrap | 30 | The Control D API could not be reached or answered with a retryable error (network failure, proxy interference, 5xx, timeout) and retries ran out. The service manager may retry the service later. | Make sure that the device has network access to `api.controld.com` (DNS, proxy, firewall, captive portal). `ctrld-client diag`'s `api_reachability` block already ran this same probe. Ask whether other TLS traffic works. | | `API_REJECTED` | bootstrap | 31 | The API answered and permanently rejected the configuration (4xx other than 408/429): bad or revoked token, malformed request. ctrld exits without burning service-manager restarts because retrying cannot change the answer. This is also the fallback for the four `TOKEN_*` codes below. Those codes need the API response to carry `error.metadata.reason`. An API deployment that does not yet populate that field reports every provisioning-code rejection as `API_REJECTED` instead. | Verify the provision token / org configuration in the Control D dashboard. Re-push after fixing credentials. Evidence: HTTP status in the result file `message`. | | `API_DEVICE_INVALID` | bootstrap | 32 | The API reports the device/resolver no longer exists (error code 40402). When the daemon's own bootstrap preflight discovers this, it self-uninstalls, because the identity is gone server-side. The direct `--cd ` install path (`ctrld-client start --cd`) can also hit this code before the service exists. On that path there is nothing to uninstall yet. | Confirm the device was deleted or re-provisioned in the dashboard; re-provision with a current token. No local evidence needed beyond the code. | +| `API_MAINTENANCE` | bootstrap | 37 | The API answered that it is in hard maintenance (HTTP 503, `error.code` 50302, or on older deployments the message `Maintenance is in progress.`), and this host had no previously working configuration on disk to fall back on. A host that does have one keeps serving DNS from it and never reaches this code. Maintenance is temporary and says nothing about the configuration, so ctrld does not self-uninstall and does not treat it as a permanent rejection. | Wait for the maintenance window to end and re-run. Nothing to fix on the device: the same request succeeds once the API returns. A device that was already provisioned keeps resolving throughout. | | `TOKEN_INVALID` | bootstrap | 33 | The API rejected the `--cd-org` provisioning code with reason `token_invalid`: the code is not recognized. This code needs the deployed API to send `error.metadata.reason: "token_invalid"`. See `API_REJECTED` above for the fallback when it does not. | Make sure that the code is correct. Re-enter it exactly as given. | | `TOKEN_EXPIRED` | bootstrap | 34 | The API rejected the `--cd-org` provisioning code with reason `token_expired`. This has the same API-reason dependency as `TOKEN_INVALID`. | Get a new provisioning code from your administrator. | | `TOKEN_LIMIT_REACHED` | bootstrap | 35 | The API rejected the `--cd-org` provisioning code with reason `token_limit_reached`: it reached its device limit. This has the same API-reason dependency as `TOKEN_INVALID`. | Free up a device slot or use a different provisioning code. |