From 1312315c9d240e6c3047fc5eb665ad8821826e87 Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 4 Jun 2026 15:48:19 -0700 Subject: [PATCH 1/6] Add exhaustive switch vetting and enforce it across the tree Wire the nishanths/exhaustive analyzer in as a `go vet` vettool (new `make vet` target + go.mod tool directive) and make every enum switch satisfy it. Rather than blanket-suppress with `//exhaustive:ignore`, each switch now lists all enum members explicitly. Where the missing members should behave identically to the existing `default`, they're grouped into a case that `fallthrough`s into the default so runtime behavior is unchanged while a future enum addition forces a deliberate decision. The ignore directive is kept only on switches over large external/stdlib enums where enumeration is impractical, with the rationale inline: - syscall.Errno (~130 members) in httpingress - tea.KeyType (~90 members) in prompt/deploy_ui - reflect.Kind (~27 members) in entity reader and specs_match_test CI: run `make vet` as part of the lint job in test.yml. --- .github/workflows/test.yml | 3 +++ Makefile | 5 ++++- appconfig/configerror.go | 6 ++++++ cli/commands/deploy_ui.go | 1 + components/coordinate/advertise.go | 3 +++ components/diskio/disk_mount_controller.go | 6 ++++++ components/diskio/disk_volume_controller.go | 6 ++++++ components/ipalloc/ipalloc.go | 3 +++ controllers/deployment/launcher.go | 6 ++++++ controllers/deployment/specs_match_test.go | 1 + controllers/disk/disk_controller.go | 6 ++++++ controllers/disk/disk_lease_controller.go | 3 +++ controllers/integration/chaos_test.go | 6 ++++++ controllers/sandbox/saga_controller.go | 3 +++ controllers/sandbox/sandbox.go | 10 +++++++++- controllers/sandbox/volume.go | 3 +++ controllers/sandboxpool/manager_test.go | 4 ++++ controllers/service/gc.go | 3 ++- go.mod | 3 +++ go.sum | 2 ++ lsvd/disk.go | 2 ++ lsvd/logger/logger.go | 3 +++ lsvd/torture.go | 2 ++ observability/status.go | 2 ++ pkg/entity/attr.go | 15 +++++++++++++-- pkg/entity/reader.go | 1 + pkg/oidcauth/composite.go | 4 +++- pkg/saga/executor.go | 2 ++ pkg/saga/memory_storage.go | 2 ++ pkg/slogfmt/logger.go | 3 +++ pkg/stackbuild/node.go | 3 +++ pkg/stackbuild/python.go | 3 +++ pkg/ui/named_value.go | 3 +++ pkg/ui/prompt.go | 1 + servers/exec_proxy/exec_proxy.go | 4 +++- servers/httpingress/httpingress.go | 1 + 36 files changed, 127 insertions(+), 7 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 98b818588..bd0f2f28b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -72,6 +72,9 @@ jobs: with: version: v2.6.1 + - name: go vet (exhaustive switch checking) + run: make vet + - name: Check go.mod tidiness run: | # Run go mod tidy and check if there are any changes diff --git a/Makefile b/Makefile index 8271267c3..96fd5e307 100644 --- a/Makefile +++ b/Makefile @@ -218,6 +218,9 @@ lint-fix: ## Run golangci-lint with auto-fix lint-pr: ## Run golangci-lint on changes from main golangci-lint run --new-from-rev main ./... +vet: ## Run go vet with the exhaustive analyzer as a vettool + go vet -vettool="$$(go tool -n exhaustive)" ./... + docs-lint: ## Lint docs (no JS toolchain needed) @bash hack/docs-lint.sh @@ -239,7 +242,7 @@ generate-check: bin/miren ## Verify go generate is up to date fi @echo "✓ go generate is up to date" -.PHONY: lint lint-fix lint-pr docs-lint generate-check +.PHONY: lint lint-fix lint-pr vet docs-lint generate-check # # Release Packaging diff --git a/appconfig/configerror.go b/appconfig/configerror.go index 67fb7c3c0..08dc6ebf0 100644 --- a/appconfig/configerror.go +++ b/appconfig/configerror.go @@ -270,6 +270,12 @@ func walkAST(p *tomlast.Parser, parts []string, depth int) int { shape := p.Shape(keyNode.Raw) return shape.Start.Line } + + case tomlast.Invalid, tomlast.Comment, tomlast.Key, tomlast.Array, + tomlast.InlineTable, tomlast.String, tomlast.Bool, tomlast.Float, + tomlast.Integer, tomlast.LocalDate, tomlast.LocalTime, + tomlast.LocalDateTime, tomlast.DateTime: + // Not top-level expressions; keep scanning. } } diff --git a/cli/commands/deploy_ui.go b/cli/commands/deploy_ui.go index 06c63eaf5..8d88ab903 100644 --- a/cli/commands/deploy_ui.go +++ b/cli/commands/deploy_ui.go @@ -232,6 +232,7 @@ func (m *deployInfo) Update(msg tea.Msg) (tea.Model, tea.Cmd) { switch msg := msg.(type) { case tea.KeyMsg: + //exhaustive:ignore tea.KeyType has ~90 members; default handles the rest switch msg.Type { case tea.KeyCtrlC: m.interrupted = true diff --git a/components/coordinate/advertise.go b/components/coordinate/advertise.go index 70d5b40b6..154d24934 100644 --- a/components/coordinate/advertise.go +++ b/components/coordinate/advertise.go @@ -295,6 +295,9 @@ func ComputeAdvertise(in AdvertiseInput) ([]AdvertiseCandidate, []string) { case netcheckUnreachable: cand.Included = false cand.Reason = "address family proven unreachable by netcheck" + case netcheckNotRun: + // No netcheck result yet; keep the candidate. + fallthrough default: cand.Included = true cand.Reason = "no netcheck override" diff --git a/components/diskio/disk_mount_controller.go b/components/diskio/disk_mount_controller.go index d72290882..c6d6895bf 100644 --- a/components/diskio/disk_mount_controller.go +++ b/components/diskio/disk_mount_controller.go @@ -169,6 +169,9 @@ func (c *DiskMountController) reconcileMountMounted(ctx context.Context, mount * case storage_v1alpha.DM_DETACHED: c.log.Info("mount detached but desired mounted, recovering", "entity_id", entityId) return c.attachAndMount(ctx, mount) + case storage_v1alpha.DM_UNMOUNTING, storage_v1alpha.DM_DETACHING: + // Tearing down while desired state is mounted; unexpected. + fallthrough default: c.log.Warn("unexpected actual state for mounted", "actual_state", mount.ActualState) return nil @@ -187,6 +190,9 @@ func (c *DiskMountController) reconcileMountUnmounted(ctx context.Context, mount return nil case storage_v1alpha.DM_UNMOUNTING, storage_v1alpha.DM_DETACHING: return nil + case storage_v1alpha.DM_PENDING, storage_v1alpha.DM_ATTACHING, storage_v1alpha.DM_ATTACHED, storage_v1alpha.DM_MOUNTING, storage_v1alpha.DM_MOUNTED, storage_v1alpha.DM_ERROR: + // Still attached/mounted; tear it down to reach the unmounted state. + fallthrough default: return c.unmountAndDetach(ctx, mount) } diff --git a/components/diskio/disk_volume_controller.go b/components/diskio/disk_volume_controller.go index 2d4700826..5d4ff9d3e 100644 --- a/components/diskio/disk_volume_controller.go +++ b/components/diskio/disk_volume_controller.go @@ -369,6 +369,9 @@ func (c *DiskVolumeController) reconcileVolumePresent(ctx context.Context, volum case storage_v1alpha.DV_ERROR: c.log.Info("volume in error state, attempting recreation", "entity_id", entityId) return c.createVolume(ctx, volume) + case storage_v1alpha.DV_DELETING, storage_v1alpha.DV_DELETED: + // Volume is being torn down while desired state is present; unexpected. + fallthrough default: c.log.Warn("unexpected actual state for present volume", "actual_state", volume.ActualState) return nil @@ -394,6 +397,9 @@ func (c *DiskVolumeController) reconcileVolumeAbsent(ctx context.Context, volume return nil case storage_v1alpha.DV_DELETING: return nil + case storage_v1alpha.DV_PENDING, storage_v1alpha.DV_CREATING, storage_v1alpha.DV_READY, storage_v1alpha.DV_ERROR: + // Volume still exists; delete it to reach the absent state. + fallthrough default: return c.deleteVolume(ctx, volume) } diff --git a/components/ipalloc/ipalloc.go b/components/ipalloc/ipalloc.go index c174970f5..0b070e601 100644 --- a/components/ipalloc/ipalloc.go +++ b/components/ipalloc/ipalloc.go @@ -169,6 +169,9 @@ func (a *Allocator) Watch(ctx context.Context, eac *entityserver_v1alpha.EntityA switch op.OperationType() { case entityserver_v1alpha.EntityOperationCreate, entityserver_v1alpha.EntityOperationUpdate: // fine + case entityserver_v1alpha.EntityOperationDelete: + // Nothing to assign on delete. + fallthrough default: return nil } diff --git a/controllers/deployment/launcher.go b/controllers/deployment/launcher.go index addc6fde1..6f8420446 100644 --- a/controllers/deployment/launcher.go +++ b/controllers/deployment/launcher.go @@ -1350,6 +1350,9 @@ func (l *Launcher) ensureServiceForPorts(ctx context.Context, app *core_v1alpha. switch p.Protocol { case core_v1alpha.ConfigSpecServicesPortsUDP: np.Protocol = network_v1alpha.UDP + case core_v1alpha.ConfigSpecServicesPortsTCP: + // TCP is also the default for an unspecified protocol. + fallthrough default: np.Protocol = network_v1alpha.TCP } @@ -1693,6 +1696,9 @@ func (l *Launcher) hasActiveSandboxForPool(ctx context.Context, poolID entity.Id switch sb.Status { case compute_v1alpha.RUNNING, compute_v1alpha.PENDING, compute_v1alpha.NOT_READY: // Active — may still hold disk resources + case compute_v1alpha.STOPPED, compute_v1alpha.DEAD: + // Terminal — no longer holds resources. + fallthrough default: continue } diff --git a/controllers/deployment/specs_match_test.go b/controllers/deployment/specs_match_test.go index 22f04fa0a..8cf11a501 100644 --- a/controllers/deployment/specs_match_test.go +++ b/controllers/deployment/specs_match_test.go @@ -17,6 +17,7 @@ import ( func structFingerprint(t reflect.Type) string { var walk func(reflect.Type) string walk = func(t reflect.Type) string { + //exhaustive:ignore reflect.Kind has ~27 members; default handles the rest switch t.Kind() { case reflect.Slice, reflect.Array, reflect.Pointer: return t.Kind().String() + "<" + walk(t.Elem()) + ">" diff --git a/controllers/disk/disk_controller.go b/controllers/disk/disk_controller.go index 2f8fd2303..ba6c72426 100644 --- a/controllers/disk/disk_controller.go +++ b/controllers/disk/disk_controller.go @@ -224,6 +224,9 @@ func (d *DiskController) handleProvisioning(ctx context.Context, disk *storage_v "error", existingVolume.ErrorMessage) return nil + case storage_v1alpha.DV_PENDING, storage_v1alpha.DV_CREATING, storage_v1alpha.DV_DELETING, storage_v1alpha.DV_DELETED: + // Not yet ready; wait for the volume to settle. + fallthrough default: d.Log.Debug("disk_volume still provisioning", "disk", disk.ID, @@ -445,6 +448,9 @@ func diskModeToVolumeMode(mode storage_v1alpha.DiskMode) storage_v1alpha.DiskVol switch mode { case storage_v1alpha.ACCELERATOR: return storage_v1alpha.VM_ACCELERATOR + case storage_v1alpha.UNIVERSAL: + // Universal is also the default for an unspecified mode. + fallthrough default: return storage_v1alpha.VM_UNIVERSAL } diff --git a/controllers/disk/disk_lease_controller.go b/controllers/disk/disk_lease_controller.go index 83002f69e..7c103aa20 100644 --- a/controllers/disk/disk_lease_controller.go +++ b/controllers/disk/disk_lease_controller.go @@ -420,6 +420,9 @@ func (d *DiskLeaseController) handlePendingLease(ctx context.Context, lease *sto } // Fall through to create a new mount entity + case storage_v1alpha.DM_PENDING, storage_v1alpha.DM_ATTACHING, storage_v1alpha.DM_ATTACHED, storage_v1alpha.DM_MOUNTING, storage_v1alpha.DM_UNMOUNTING, storage_v1alpha.DM_DETACHING: + // Mount lifecycle still in progress; wait for it to settle. + fallthrough default: d.Log.Debug("disk_mount still in progress", "lease", leaseId, diff --git a/controllers/integration/chaos_test.go b/controllers/integration/chaos_test.go index 907174ad7..6cc5b354c 100644 --- a/controllers/integration/chaos_test.go +++ b/controllers/integration/chaos_test.go @@ -84,6 +84,9 @@ func (r *chaosReport) collectEntityStats(t *testing.T, ctx context.Context, h *T r.provisionedDisks++ case storage.ERROR: r.errorDisks++ + case storage.PROVISIONING, storage.ATTACHED, storage.DETACHED, storage.DELETING, storage.RESTORING: + // Counted together as "other". + fallthrough default: r.otherDisks++ } @@ -108,6 +111,9 @@ func (r *chaosReport) collectEntityStats(t *testing.T, ctx context.Context, h *T r.mountedMounts++ case storage.DM_DETACHED: r.detachedMounts++ + case storage.DM_PENDING, storage.DM_ATTACHING, storage.DM_ATTACHED, storage.DM_MOUNTING, storage.DM_UNMOUNTING, storage.DM_DETACHING, storage.DM_ERROR: + // Counted together as "other". + fallthrough default: r.otherMounts++ } diff --git a/controllers/sandbox/saga_controller.go b/controllers/sandbox/saga_controller.go index dc456cdf6..d4dfd5fe9 100644 --- a/controllers/sandbox/saga_controller.go +++ b/controllers/sandbox/saga_controller.go @@ -133,6 +133,9 @@ func (s *SagaSandboxController) Create(ctx context.Context, co *compute.Sandbox, } return s.createSandboxViaSaga(ctx, co) + case compute.NOT_READY: + // Transient boot state; nothing to reconcile until it resolves. + fallthrough default: s.log.Warn("ignoring sandbox status", "status", co.Status) return nil diff --git a/controllers/sandbox/sandbox.go b/controllers/sandbox/sandbox.go index b5259f48d..aac4678bb 100644 --- a/controllers/sandbox/sandbox.go +++ b/controllers/sandbox/sandbox.go @@ -211,6 +211,8 @@ func (c *SandboxController) SetPortStatus(id string, port observability.BoundPor ports.Ports = slices.DeleteFunc(ports.Ports, func(p observability.BoundPort) bool { return p == port }) + case observability.PortStatusActive: + // Liveness signal only; does not change the bound set. } c.portCond.Broadcast() @@ -689,8 +691,11 @@ func (c *SandboxController) isContainerHealthy(ctx context.Context, containerID // We don't expect paused sandboxes in normal operation c.Log.Debug("task in paused/pausing state, marking unhealthy", "id", containerID, "status", status.Status) return false + case containerd.Unknown: + // Unknown status is unhealthy. + fallthrough default: - // Unknown or any other status is unhealthy + // Any other status is unhealthy c.Log.Debug("task in unknown/unhealthy state", "id", containerID, "status", status.Status) return false } @@ -919,6 +924,9 @@ func (c *SandboxController) Create(ctx context.Context, co *compute.Sandbox, met } return c.createSandbox(ctx, co, meta, false) + case compute.NOT_READY: + // Transient boot state; nothing to reconcile until it resolves. + fallthrough default: c.Log.Warn("ignoring sandbox status", "status", co.Status) return nil diff --git a/controllers/sandbox/volume.go b/controllers/sandbox/volume.go index b202a4a6c..90e73e915 100644 --- a/controllers/sandbox/volume.go +++ b/controllers/sandbox/volume.go @@ -447,6 +447,9 @@ func (c *SandboxController) waitForLeaseBound(ctx context.Context, leaseID entit // Still pending, continue waiting continue + case storage.RELEASED: + // Lease released out from under us while waiting to bind. + fallthrough default: return "", fmt.Errorf("unexpected disk lease status: %s", lease.Status) } diff --git a/controllers/sandboxpool/manager_test.go b/controllers/sandboxpool/manager_test.go index fd75f4f72..ba3c2f030 100644 --- a/controllers/sandboxpool/manager_test.go +++ b/controllers/sandboxpool/manager_test.go @@ -129,6 +129,8 @@ func TestManagerScaleUpPartial(t *testing.T) { running++ case compute_v1alpha.PENDING: pending++ + case compute_v1alpha.NOT_READY, compute_v1alpha.STOPPED, compute_v1alpha.DEAD: + // Not counted in this assertion. } } @@ -564,6 +566,8 @@ func TestManagerScaleDownFixedModeProactive(t *testing.T) { runningCount++ case compute_v1alpha.STOPPED: stoppedCount++ + case compute_v1alpha.PENDING, compute_v1alpha.NOT_READY, compute_v1alpha.DEAD: + // Not counted in this assertion. } } diff --git a/controllers/service/gc.go b/controllers/service/gc.go index 9a3e60b7d..2479191af 100644 --- a/controllers/service/gc.go +++ b/controllers/service/gc.go @@ -308,8 +308,9 @@ func (s *ServiceController) applyGC(ctx context.Context, target *targetState, ac if !target.chains.Contains(chain) { orphanLeaves = append(orphanLeaves, chain) } + case chainKindStatic, chainKindUnknown: + // Leave alone. } - // chainKindStatic and chainKindUnknown: leave alone. } orphanParentCount := len(orphanNodePorts) + len(orphanServices) diff --git a/go.mod b/go.mod index cd03d8c61..3b4704fd0 100644 --- a/go.mod +++ b/go.mod @@ -148,6 +148,7 @@ require ( github.com/kaptinlin/go-i18n v0.1.4 // indirect github.com/kaptinlin/jsonschema v0.4.6 // indirect github.com/magefile/mage v1.17.0 // indirect + github.com/nishanths/exhaustive v0.12.0 // indirect github.com/petar-dambovaliev/aho-corasick v0.0.0-20250424160509-463d218d4745 // indirect github.com/shibumi/go-pathspec v1.3.0 // indirect github.com/tidwall/gjson v1.18.0 // indirect @@ -430,3 +431,5 @@ require ( tags.cncf.io/container-device-interface v0.8.0 // indirect tags.cncf.io/container-device-interface/specs-go v0.8.0 // indirect ) + +tool github.com/nishanths/exhaustive/cmd/exhaustive diff --git a/go.sum b/go.sum index ce273e566..b2462dba8 100644 --- a/go.sum +++ b/go.sum @@ -1004,6 +1004,8 @@ github.com/nats-io/nuid v1.0.1/go.mod h1:19wcPz3Ph3q0Jbyiqsd0kePYG7A95tJPxeL+1OS github.com/ncruces/go-strftime v1.0.0 h1:HMFp8mLCTPp341M/ZnA4qaf7ZlsbTc+miZjCLOFAw7w= github.com/ncruces/go-strftime v1.0.0/go.mod h1:Fwc5htZGVVkseilnfgOVb9mKy6w1naJmn9CehxcKcls= github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= +github.com/nishanths/exhaustive v0.12.0 h1:vIY9sALmw6T/yxiASewa4TQcFsVYZQQRUQJhKRf3Swg= +github.com/nishanths/exhaustive v0.12.0/go.mod h1:mEZ95wPIZW+x8kC4TgC+9YCUgiST7ecevsVDTgc2obs= github.com/nrdcg/auroradns v1.1.0 h1:KekGh8kmf2MNwqZVVYo/fw/ZONt8QMEmbMFOeljteWo= github.com/nrdcg/auroradns v1.1.0/go.mod h1:O7tViUZbAcnykVnrGkXzIJTHoQCHcgalgAe6X1mzHfk= github.com/nrdcg/bunny-go v0.1.0 h1:GAHTRpHaG/TxfLZlqoJ8OJFzw8rI74+jOTkzxWh0uHA= diff --git a/lsvd/disk.go b/lsvd/disk.go index 11e9d28e1..80dd4986f 100644 --- a/lsvd/disk.go +++ b/lsvd/disk.go @@ -687,6 +687,8 @@ func (d *Disk) checkFlush(ctx context.Context) error { } switch reason { + case FlushNo: + // Already handled by the early return above; unreachable here. case FlushTime: d.log.Info("flushing segment due to maximum segment lifetime", "age", time.Since(d.curOC.builder.openedAt), diff --git a/lsvd/logger/logger.go b/lsvd/logger/logger.go index 4507e385a..55b1b6ae0 100644 --- a/lsvd/logger/logger.go +++ b/lsvd/logger/logger.go @@ -718,6 +718,9 @@ func appendTextValue(s *handleState, v slog.Value) error { return nil } s.appendString(fmt.Sprintf("%+v", v.Any())) + case slog.KindBool, slog.KindDuration, slog.KindFloat64, slog.KindInt64, slog.KindUint64, slog.KindGroup, slog.KindLogValuer: + // Defer to the generic value formatter. + fallthrough default: *s.buf = appendValue(v, *s.buf) } diff --git a/lsvd/torture.go b/lsvd/torture.go index 293a57435..f2f12add0 100644 --- a/lsvd/torture.go +++ b/lsvd/torture.go @@ -371,6 +371,8 @@ func (g *TortureGenerator) Next() TortureOperation { case TortureOpZero: op.Extent = g.nextExtent(true) g.lastWrite = op.Extent + case TortureOpSync, TortureOpCloseReopen: + // Whole-disk operations; no extent or data to populate. } return op diff --git a/observability/status.go b/observability/status.go index 0e68c6373..79816e04f 100644 --- a/observability/status.go +++ b/observability/status.go @@ -72,6 +72,8 @@ func (s *StatusMonitor) SetPortStatus(entity string, port BoundPort, status Port es.boundPorts[port] = struct{}{} case PortStatusUnbound: delete(es.boundPorts, port) + case PortStatusActive: + // Liveness signal only; does not change the bound set. } } diff --git a/pkg/entity/attr.go b/pkg/entity/attr.go index 44abf4369..810cd096e 100644 --- a/pkg/entity/attr.go +++ b/pkg/entity/attr.go @@ -243,6 +243,9 @@ func (v *Value) setFromTuple(x valueDecodeTuple, decoder unmarshaler) error { } v.any = label + case KindAny, KindBytes: + // No dedicated decoding; use the generic any unmarshal below. + fallthrough default: err := decoder.Unmarshal(x.Value, &v.any) if err != nil { @@ -696,6 +699,9 @@ func (v Value) String() string { case KindInt64, KindUint64, KindFloat64, KindBool, KindString: var buf []byte return string(v.append(buf)) + case KindAny, KindDuration, KindTime, KindId, KindKeyword, KindArray, KindComponent, KindLabel, KindBytes: + // Prefix these kinds with their short name to disambiguate. + fallthrough default: var buf []byte return v.Kind().ShortString() + ": " + string(v.append(buf)) @@ -863,9 +869,11 @@ func (v Value) Clone() Value { } } return Value{any: &EntityComponent{attrs: clonedAttrs}} + case KindAny, KindBool, KindDuration, KindFloat64, KindInt64, KindString, KindTime, KindUint64, KindId, KindKeyword, KindLabel: + // For these types the value is either stored in num or is an + // immutable type, so a shallow copy is sufficient. + fallthrough default: - // For all other types (strings, primitives, time, id, keyword, label), - // the value is either stored in num or is an immutable type, so shallow copy is fine return v } } @@ -1039,6 +1047,9 @@ func (v Value) sum(w io.Writer) { v.sum(w) w.Write([]byte{','}) } + case KindLabel, KindBytes: + // Use the generic text representation. + fallthrough default: w.Write(v.append(nil)) } diff --git a/pkg/entity/reader.go b/pkg/entity/reader.go index 869a9ed0e..da80f78bd 100644 --- a/pkg/entity/reader.go +++ b/pkg/entity/reader.go @@ -57,6 +57,7 @@ func readInfo(e AttrGetter, val any) error { continue } + //exhaustive:ignore reflect.Kind has ~27 members; default handles the rest switch fieldVal.Kind() { case reflect.String: fieldVal.SetString(val.String()) diff --git a/pkg/oidcauth/composite.go b/pkg/oidcauth/composite.go index 7555865e3..51f150170 100644 --- a/pkg/oidcauth/composite.go +++ b/pkg/oidcauth/composite.go @@ -102,8 +102,10 @@ func (c *CompositeAuthorizer) Authorize(ctx context.Context, identity *rpc.Ident // OIDC callers are restricted to the oidc-deploy role return authorizeOIDC(resource, action) + case rpc.AuthMethodJWT, rpc.AuthMethodAnonymous, rpc.AuthMethodToken: + // Delegate to primary (cloud RBAC). + fallthrough default: - // JWT and other methods → delegate to primary (cloud RBAC) if c.primary != nil { return c.primary.Authorize(ctx, identity, resource, action) } diff --git a/pkg/saga/executor.go b/pkg/saga/executor.go index 91f619161..be9f3d400 100644 --- a/pkg/saga/executor.go +++ b/pkg/saga/executor.go @@ -500,6 +500,8 @@ func (e *Executor) Recover(ctx context.Context) error { case StatusUndoing: // Resume undo recoverErrors = append(recoverErrors, e.runUndo(ctx, def, exec)) + case StatusCompleted, StatusFailed: + // Terminal states; nothing to recover. } } diff --git a/pkg/saga/memory_storage.go b/pkg/saga/memory_storage.go index 06b9399a6..5f6066e2d 100644 --- a/pkg/saga/memory_storage.go +++ b/pkg/saga/memory_storage.go @@ -61,6 +61,8 @@ func (m *MemoryStorage) ListIncomplete(ctx context.Context) ([]*Execution, error switch exec.Status { case StatusPending, StatusRunning, StatusUndoing: result = append(result, exec) + case StatusCompleted, StatusFailed: + // Terminal states are complete; skip them. } } return result, nil diff --git a/pkg/slogfmt/logger.go b/pkg/slogfmt/logger.go index dc9bcde8f..c052be458 100644 --- a/pkg/slogfmt/logger.go +++ b/pkg/slogfmt/logger.go @@ -727,6 +727,9 @@ func appendTextValue(s *handleState, v slog.Value) error { return nil } s.appendString(fmt.Sprintf("%+v", v.Any())) + case slog.KindBool, slog.KindDuration, slog.KindFloat64, slog.KindInt64, slog.KindUint64, slog.KindGroup, slog.KindLogValuer: + // Defer to the generic value formatter. + fallthrough default: *s.buf = appendValue(v, *s.buf) } diff --git a/pkg/stackbuild/node.go b/pkg/stackbuild/node.go index 030b4af0c..57f2da463 100644 --- a/pkg/stackbuild/node.go +++ b/pkg/stackbuild/node.go @@ -185,6 +185,9 @@ func (s *NodeStack) GenerateLLB(dir string, opts BuildOptions) (*llb.State, erro llb.AddMount("/usr/local/share/.cache/yarn", yarnCache, llb.AsPersistentCacheDir("yarn", llb.CacheMountShared)), llb.WithCustomName("[phase] Installing Node.js dependencies with yarn"), ).Root() + case nodePkgNpm: + // npm is also the default when no package manager is detected. + fallthrough default: // Create cache mounts npmCache := llb.Scratch().File( diff --git a/pkg/stackbuild/python.go b/pkg/stackbuild/python.go index a1f1a4d66..bc5831f68 100644 --- a/pkg/stackbuild/python.go +++ b/pkg/stackbuild/python.go @@ -367,6 +367,9 @@ func (s *PythonStack) Entrypoint() string { return "pipenv run" case pythonPkgUv: return "uv run" + case pythonPkgPip: + // pip has no run wrapper. + fallthrough default: return "" } diff --git a/pkg/ui/named_value.go b/pkg/ui/named_value.go index a6d02d17e..b8d95f61f 100644 --- a/pkg/ui/named_value.go +++ b/pkg/ui/named_value.go @@ -160,6 +160,9 @@ func (n *NamedValueList) styleValue(item NamedValue) string { return n.styles.BoolValue.Render(item.Value) case ValueTypeNull: return n.styles.NullValue.Render(item.Value) + case ValueTypeOther: + // Other is also the fallback for any unspecified type. + fallthrough default: return n.styles.OtherValue.Render(item.Value) } diff --git a/pkg/ui/prompt.go b/pkg/ui/prompt.go index 1167d5a11..5da39ceb7 100644 --- a/pkg/ui/prompt.go +++ b/pkg/ui/prompt.go @@ -125,6 +125,7 @@ func (m textInputModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { switch msg := msg.(type) { case tea.KeyMsg: + //exhaustive:ignore tea.KeyType has ~90 members; default handles the rest switch msg.Type { case tea.KeyEnter: m.value = m.textInput.Value() diff --git a/servers/exec_proxy/exec_proxy.go b/servers/exec_proxy/exec_proxy.go index a49b70ef1..fc5768987 100644 --- a/servers/exec_proxy/exec_proxy.go +++ b/servers/exec_proxy/exec_proxy.go @@ -260,8 +260,10 @@ func (s *Server) waitForSandboxRunning(ctx context.Context, sbID entity.Id) (*en case compute_v1alpha.DEAD, compute_v1alpha.STOPPED: resultCh <- result{err: fmt.Errorf("sandbox failed to start, status: %s", sb.Status)} return true + case compute_v1alpha.PENDING, compute_v1alpha.NOT_READY: + // Not a terminal state; keep waiting. + fallthrough default: - // PENDING or NOT_READY - keep waiting s.Log.Debug("sandbox not ready yet", "id", sbID, "status", sb.Status) return false } diff --git a/servers/httpingress/httpingress.go b/servers/httpingress/httpingress.go index 03504089a..6fddb16ca 100644 --- a/servers/httpingress/httpingress.go +++ b/servers/httpingress/httpingress.go @@ -965,6 +965,7 @@ func isProxyConnectionError(err error) bool { var syscallErr *os.SyscallError if errors.As(opErr.Err, &syscallErr) { if errno, ok := syscallErr.Err.(syscall.Errno); ok { + //exhaustive:ignore syscall.Errno has ~130 members; default handles the rest switch errno { case syscall.ECONNREFUSED: // connection refused return true From e0e156462214ff1ce19e6bacd7642d0bbbdf96a5 Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 4 Jun 2026 16:15:06 -0700 Subject: [PATCH 2/6] Run exhaustive through golangci-lint instead of a separate go vet golangci-lint bundles the exhaustive analyzer, so enable it there rather than maintaining a parallel `go vet` vettool. This drops the dedicated plumbing: - remove the `make vet` target - remove the `go vet (exhaustive)` step from the lint CI job - remove the nishanths/exhaustive tool directive from go.mod/go.sum The linter is configured with default-signifies-exhaustive: false to match the prior vettool behavior, and still honors `//exhaustive:ignore`. Tradeoff: golangci-lint's path exclusions are global, so the already lint-exempt paths (lsvd/, disk/, dataset/, pkg/cel, pkg/containerdx, *.gen.go) are no longer exhaustive-checked. The switches in those paths remain exhaustive; they're just not re-enforced. --- .github/workflows/test.yml | 3 --- .golangci.yml | 9 +++++++++ Makefile | 5 +---- go.mod | 5 +---- go.sum | 2 -- 5 files changed, 11 insertions(+), 13 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index bd0f2f28b..98b818588 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -72,9 +72,6 @@ jobs: with: version: v2.6.1 - - name: go vet (exhaustive switch checking) - run: make vet - - name: Check go.mod tidiness run: | # Run go mod tidy and check if there are any changes diff --git a/.golangci.yml b/.golangci.yml index b8e444a95..29142df85 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,10 +1,19 @@ version: "2" linters: + enable: + - exhaustive + disable: - errcheck default: standard + settings: + exhaustive: + # A bare `default:` does not by itself make a switch exhaustive; every + # enum member must be listed (use a `//exhaustive:ignore` directive to + # opt a specific switch out). + default-signifies-exhaustive: false exclusions: paths: - "dataset/" diff --git a/Makefile b/Makefile index 96fd5e307..8271267c3 100644 --- a/Makefile +++ b/Makefile @@ -218,9 +218,6 @@ lint-fix: ## Run golangci-lint with auto-fix lint-pr: ## Run golangci-lint on changes from main golangci-lint run --new-from-rev main ./... -vet: ## Run go vet with the exhaustive analyzer as a vettool - go vet -vettool="$$(go tool -n exhaustive)" ./... - docs-lint: ## Lint docs (no JS toolchain needed) @bash hack/docs-lint.sh @@ -242,7 +239,7 @@ generate-check: bin/miren ## Verify go generate is up to date fi @echo "✓ go generate is up to date" -.PHONY: lint lint-fix lint-pr vet docs-lint generate-check +.PHONY: lint lint-fix lint-pr docs-lint generate-check # # Release Packaging diff --git a/go.mod b/go.mod index 3b4704fd0..1bd78637a 100644 --- a/go.mod +++ b/go.mod @@ -90,6 +90,7 @@ require ( github.com/sirupsen/logrus v1.9.3 github.com/spf13/pflag v1.0.9 github.com/stretchr/testify v1.11.1 + github.com/tidwall/gjson v1.18.0 github.com/tonistiigi/fsutil v0.0.0-20250113203817-b14e27f4135a github.com/tonistiigi/go-csvvalue v0.0.0-20240710180619-ddb21b71c0b4 github.com/tonistiigi/units v0.0.0-20180711220420-6950e57a87ea @@ -148,10 +149,8 @@ require ( github.com/kaptinlin/go-i18n v0.1.4 // indirect github.com/kaptinlin/jsonschema v0.4.6 // indirect github.com/magefile/mage v1.17.0 // indirect - github.com/nishanths/exhaustive v0.12.0 // indirect github.com/petar-dambovaliev/aho-corasick v0.0.0-20250424160509-463d218d4745 // indirect github.com/shibumi/go-pathspec v1.3.0 // indirect - github.com/tidwall/gjson v1.18.0 // indirect github.com/tidwall/match v1.1.1 // indirect github.com/tidwall/pretty v1.2.1 // indirect github.com/valllabh/ocsf-schema-golang v1.0.3 // indirect @@ -431,5 +430,3 @@ require ( tags.cncf.io/container-device-interface v0.8.0 // indirect tags.cncf.io/container-device-interface/specs-go v0.8.0 // indirect ) - -tool github.com/nishanths/exhaustive/cmd/exhaustive diff --git a/go.sum b/go.sum index b2462dba8..ce273e566 100644 --- a/go.sum +++ b/go.sum @@ -1004,8 +1004,6 @@ github.com/nats-io/nuid v1.0.1/go.mod h1:19wcPz3Ph3q0Jbyiqsd0kePYG7A95tJPxeL+1OS github.com/ncruces/go-strftime v1.0.0 h1:HMFp8mLCTPp341M/ZnA4qaf7ZlsbTc+miZjCLOFAw7w= github.com/ncruces/go-strftime v1.0.0/go.mod h1:Fwc5htZGVVkseilnfgOVb9mKy6w1naJmn9CehxcKcls= github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= -github.com/nishanths/exhaustive v0.12.0 h1:vIY9sALmw6T/yxiASewa4TQcFsVYZQQRUQJhKRf3Swg= -github.com/nishanths/exhaustive v0.12.0/go.mod h1:mEZ95wPIZW+x8kC4TgC+9YCUgiST7ecevsVDTgc2obs= github.com/nrdcg/auroradns v1.1.0 h1:KekGh8kmf2MNwqZVVYo/fw/ZONt8QMEmbMFOeljteWo= github.com/nrdcg/auroradns v1.1.0/go.mod h1:O7tViUZbAcnykVnrGkXzIJTHoQCHcgalgAe6X1mzHfk= github.com/nrdcg/bunny-go v0.1.0 h1:GAHTRpHaG/TxfLZlqoJ8OJFzw8rI74+jOTkzxWh0uHA= From ef8cfde3f5885f5b1169422b9706aeb5b6d9efcc Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 4 Jun 2026 17:09:16 -0700 Subject: [PATCH 3/6] Make switches exhaustive in code merged from main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merging the latest main surfaced enum switches that the now-enabled exhaustive linter flags: - controllers/sandbox/log.go: gjson.Type — handle Null/JSON (skipped) - pkg/controller/controller.go: EntityOperation — Progress/Compacted route to the existing no-op default - pkg/entity/indexwatch/watcher.go: EntityOperation in two switches — Progress/Compacted handled explicitly; Create/Update/Delete fall through to eventFromOp - components/ipalloc/ipalloc.go: indexwatch.EventType — handle EventDeleted (resolved during the merge) EntityOperation gained Progress and Compacted members on main, which is exactly the kind of addition this linter is meant to catch. --- controllers/sandbox/log.go | 3 ++- pkg/controller/controller.go | 3 +++ pkg/entity/indexwatch/watcher.go | 5 +++++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/controllers/sandbox/log.go b/controllers/sandbox/log.go index 45ce1849c..497cadcd0 100644 --- a/controllers/sandbox/log.go +++ b/controllers/sandbox/log.go @@ -164,8 +164,9 @@ func (s *SandboxLogs) scanJSON(line string) (string, observability.LogStream, bo // Raw preserves the original numeric literal (large integers // included) and renders bools as "true"/"false". s.extra[k] = value.Raw + case gjson.Null, gjson.JSON: + // Null and nested objects/arrays are skipped. } - // gjson.Null and nested objects/arrays (gjson.JSON) are skipped. } return true }) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index b7fd5f9aa..bd44e1640 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -175,6 +175,9 @@ func (c *ReconcileController) Start(top context.Context) error { eventType = EventUpdated case entityserver_v1alpha.EntityOperationDelete: eventType = EventDeleted + case entityserver_v1alpha.EntityOperationProgress, entityserver_v1alpha.EntityOperationCompacted: + // Not entity mutations; nothing to dispatch. + fallthrough default: return nil } diff --git a/pkg/entity/indexwatch/watcher.go b/pkg/entity/indexwatch/watcher.go index 62c3a190c..464568146 100644 --- a/pkg/entity/indexwatch/watcher.go +++ b/pkg/entity/indexwatch/watcher.go @@ -357,6 +357,8 @@ func (w *Watcher) watch(ctx context.Context) (resnapshot bool, err error) { w.cursor = op.Revision() } return nil + case entityserver_v1alpha.EntityOperationCreate, entityserver_v1alpha.EntityOperationUpdate, entityserver_v1alpha.EntityOperationDelete: + // Entity mutations; converted to events by eventFromOp below. } ev, ok := w.eventFromOp(op) @@ -395,6 +397,9 @@ func (w *Watcher) eventFromOp(op *entityserver_v1alpha.EntityOp) (Event, bool) { typ = EventUpdated case entityserver_v1alpha.EntityOperationDelete: typ = EventDeleted + case entityserver_v1alpha.EntityOperationProgress, entityserver_v1alpha.EntityOperationCompacted: + // Not entity mutations; no deliverable event. + fallthrough default: return Event{}, false } From 79c370e36b7d87ea1a9903dc1656dc0586f2210b Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 4 Jun 2026 17:28:17 -0700 Subject: [PATCH 4/6] Guard cert PEM nil-check with explicit return in registration test Enabling the exhaustive analyzer in golangci-lint perturbs staticcheck's analyzer scheduling enough that it intermittently stops recognizing t.Fatal as terminating, raising a false-positive SA5011 (possible nil deref) on the guarded block.Bytes access. Add an explicit return after t.Fatal so the non-nil invariant holds structurally, independent of whether staticcheck models t.Fatal's control flow. No behavior change. --- servers/runner/registration_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/servers/runner/registration_test.go b/servers/runner/registration_test.go index 1e6c0b614..d6ae3a5e8 100644 --- a/servers/runner/registration_test.go +++ b/servers/runner/registration_test.go @@ -168,6 +168,7 @@ func TestJoinCreatesNodeEntity(t *testing.T) { block, _ := pem.Decode(joinResult.CertPem()) if block == nil { t.Fatal("failed to decode cert PEM") + return } cert, err := x509.ParseCertificate(block.Bytes) if err != nil { From 02df475ea8c8d2804c95ff6d7298137ee46dd793 Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 4 Jun 2026 17:37:01 -0700 Subject: [PATCH 5/6] Update frozen sandbox controller hashes sandbox.go and volume.go gained explicit enum cases for the exhaustive linter (and sandbox.go also picked up changes from main). The parallel saga controller (saga_controller.go) carries the matching SandboxStatus case, so the two code paths stay in sync. Refresh the frozen hashes. --- controllers/sandbox/sandbox_frozen_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/sandbox/sandbox_frozen_test.go b/controllers/sandbox/sandbox_frozen_test.go index ef9782c85..3e7f99633 100644 --- a/controllers/sandbox/sandbox_frozen_test.go +++ b/controllers/sandbox/sandbox_frozen_test.go @@ -24,8 +24,8 @@ import ( // sha256sum controllers/sandbox/sandbox.go controllers/sandbox/volume.go controllers/sandbox/firewall.go func TestSandboxControllerFrozen(t *testing.T) { frozen := map[string]string{ - "sandbox.go": "65e398f232a887de37250abd8da5f27bf66e9ace6625331145d4e788a08f1707", - "volume.go": "292dbc050cd94901ab704a23605f5537c944787c9e06077a3fc004f40e9c0b6c", + "sandbox.go": "dffa128ea38a60fd98cd46f3f465159d096b7cd3397d9f164c6e5a3f4d57e8b3", + "volume.go": "b4697764d48a90adc04ce47968ccef11ceba50da8d19c889906c5c3a539065b3", "firewall.go": "648cb5d91091d5eb7400152b19695a8045585feae59c5dd36c12d663a27bb91f", } From cad330476f296689bc2358ee6ba933a53ad293d0 Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 4 Jun 2026 17:57:18 -0700 Subject: [PATCH 6/6] Release IP reservations when a service is deleted The ipalloc allocation table is append-only: IPs were recorded on snapshot/assign but never removed. A deleted service therefore held its addresses forever, slowly exhausting the subnet. Handle indexwatch.EventDeleted by pruning every allocation owned by the deleted service id, returning those IPs to the pool. Add a unit test covering the round-trip (release frees only the target service's IPs and they can be re-allocated). Fixes swt-e255. --- components/ipalloc/ipalloc.go | 20 ++++++++++++- components/ipalloc/ipalloc_test.go | 46 ++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/components/ipalloc/ipalloc.go b/components/ipalloc/ipalloc.go index 7858b5f00..5866a0586 100644 --- a/components/ipalloc/ipalloc.go +++ b/components/ipalloc/ipalloc.go @@ -187,12 +187,30 @@ func (a *Allocator) Watch(ctx context.Context, eac *entityserver_v1alpha.EntityA a.log.Error("failed to assign service", "error", err, "service", ev.Id) } case indexwatch.EventDeleted: - // Service removed; nothing to allocate. + // Service removed; return its reserved IPs to the pool. + a.releaseServiceAllocations(ev.Id) } } } } +// releaseServiceAllocations returns every IP reserved for the given service to +// the pool. The allocation table is otherwise append-only, so without this a +// deleted service would hold its addresses forever and slowly exhaust the +// subnet. +func (a *Allocator) releaseServiceAllocations(id entity.Id) { + owner := id.String() + + a.mu.Lock() + defer a.mu.Unlock() + + for addr, holder := range a.allocations { + if holder == owner { + delete(a.allocations, addr) + } + } +} + type service struct { network_v1alpha.Service *entity.Entity diff --git a/components/ipalloc/ipalloc_test.go b/components/ipalloc/ipalloc_test.go index 8a58a25d1..85a30bab5 100644 --- a/components/ipalloc/ipalloc_test.go +++ b/components/ipalloc/ipalloc_test.go @@ -1,10 +1,14 @@ package ipalloc import ( + "context" + "io" + "log/slog" "net/netip" "testing" "github.com/stretchr/testify/require" + "miren.dev/runtime/pkg/entity" ) func TestAllocator_random(t *testing.T) { @@ -48,3 +52,45 @@ func TestAllocator_random(t *testing.T) { r.Greater(len(seen), 900) }) } + +func TestAllocator_releaseServiceAllocations(t *testing.T) { + r := require.New(t) + + log := slog.New(slog.NewTextHandler(io.Discard, nil)) + a := NewAllocator(log, []netip.Prefix{ + netip.MustParsePrefix("10.10.0.0/16"), + }) + + svcA := entity.Id("svc/a") + svcB := entity.Id("svc/b") + + ipsA, err := a.Allocate(context.Background(), svcA) + r.NoError(err) + r.NotEmpty(ipsA) + + ipsB, err := a.Allocate(context.Background(), svcB) + r.NoError(err) + r.NotEmpty(ipsB) + + // Both services hold their reservations. + r.Len(a.allocations, len(ipsA)+len(ipsB)) + + // Releasing A returns only A's addresses to the pool. + a.releaseServiceAllocations(svcA) + + for _, ip := range ipsA { + _, ok := a.allocations[ip] + r.Falsef(ok, "expected %s to be released", ip) + } + for _, ip := range ipsB { + holder, ok := a.allocations[ip] + r.Truef(ok, "expected %s to remain reserved", ip) + r.Equal(svcB.String(), holder) + } + r.Len(a.allocations, len(ipsB)) + + // A's freed addresses can be handed out again. + ipsA2, err := a.Allocate(context.Background(), svcA) + r.NoError(err) + r.NotEmpty(ipsA2) +}