diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 13432eb..ea32546 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -108,15 +108,15 @@ jobs: # were errcheck-catchable; gofmt -s drift was a recurring # nit). Going back to the action when #6 has a strategy. - # Tool versions are unpinned (@latest) because older tagged - # releases pulled stale golang.org/x/tools that fail to build - # against current Go ("invalid array length -delta * delta"). - # Pinning is preferable for reproducibility but waiting on the - # upstream tools to ship newer tags. Track in #6. + # Tool versions are pinned for reproducibility (was @latest, which + # drifted — see #127). The current tags build cleanly against this + # module's Go floor; the old "stale x/tools breaks the build" worry + # is gone now that these tools ship newer tags. Keep these in lockstep + # with the *_VERSION vars in the Makefile. Bump deliberately. - name: errcheck run: | - go install github.com/kisielk/errcheck@latest + go install github.com/kisielk/errcheck@v1.20.0 errcheck ./... - name: gofmt -s @@ -131,14 +131,28 @@ jobs: - name: ineffassign run: | - go install github.com/gordonklaus/ineffassign@latest + go install github.com/gordonklaus/ineffassign@v0.2.0 ineffassign ./... - name: misspell run: | - go install github.com/client9/misspell/cmd/misspell@latest + go install github.com/client9/misspell/cmd/misspell@v0.3.4 misspell -error . + # deadcode: reachability scan from the CLI entrypoint (~5s). ADVISORY + # (continue-on-error) for now — it reports unreachable funcs but does not + # fail the job. The module still carries pre-existing dead-ish funcs that + # are unsafe to delete blindly: Stringer methods (Status.String, + # JobOutcome.String) reached only via fmt reflection that static analysis + # can't see, plus test-only parity harnesses (ReadLabelValues, + # inferColumnType — di#349). Flip to blocking once that backlog is + # cleared. Tracked in #6 / #127. + - name: deadcode (advisory) + continue-on-error: true + run: | + go install golang.org/x/tools/cmd/deadcode@v0.48.0 + deadcode ./cmd/tracebloc + build: timeout-minutes: 20 name: Build (${{ matrix.os }}/${{ matrix.arch }}) diff --git a/Makefile b/Makefile index bcf7328..c16509e 100644 --- a/Makefile +++ b/Makefile @@ -12,10 +12,17 @@ GO ?= go GOLANGCI_LINT ?= golangci-lint PKGS := ./... +# Pinned lint/analysis tool versions (reproducibility — no more @latest drift). +# Keep these in lockstep with .github/workflows/build.yml. Bump deliberately. +ERRCHECK_VERSION ?= v1.20.0 +INEFFASSIGN_VERSION ?= v0.2.0 +MISSPELL_VERSION ?= v0.3.4 +DEADCODE_VERSION ?= v0.48.0 + # ---- top-level targets ------------------------------------------- .PHONY: ci -ci: vet test lint fmt-check schema-check +ci: vet test lint fmt-check schema-check deadcode @echo "==> ci: all green" .PHONY: build @@ -55,9 +62,21 @@ test-integration: # available for a richer local pass. .PHONY: lint lint: - $(GO) run github.com/kisielk/errcheck@latest ./... - $(GO) run github.com/gordonklaus/ineffassign@latest ./... - $(GO) run github.com/client9/misspell/cmd/misspell@latest -error . + $(GO) run github.com/kisielk/errcheck@$(ERRCHECK_VERSION) ./... + $(GO) run github.com/gordonklaus/ineffassign@$(INEFFASSIGN_VERSION) ./... + $(GO) run github.com/client9/misspell/cmd/misspell@$(MISSPELL_VERSION) -error . + +# deadcode: reachability scan from the CLI entrypoint (~5s). ADVISORY for now +# (non-blocking) — it prints unreachable funcs but never fails the build. The +# module still carries pre-existing dead-ish funcs that are unsafe to delete +# blindly: Stringer methods (Status.String, JobOutcome.String) reached only via +# fmt reflection that static analysis can't see, plus test-only parity harnesses +# (ReadLabelValues, inferColumnType — di#349). Flip to blocking once that +# backlog is cleared. Tracked in tracebloc/cli#6 / #127. +.PHONY: deadcode +deadcode: + @echo "==> deadcode (advisory): unreachable funcs from ./cmd/tracebloc" + @$(GO) run golang.org/x/tools/cmd/deadcode@$(DEADCODE_VERSION) ./cmd/tracebloc || true .PHONY: lint-full lint-full: diff --git a/internal/cli/client.go b/internal/cli/client.go index 516bb95..dfff8de 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -86,9 +86,8 @@ func newClientCreateCmd() *cobra.Command { "client name (default: $TRACEBLOC_CLIENT_NAME, else auto-generated -NN; shown on your dashboard + carbon reports)") cmd.Flags().StringVar(&location, "location", os.Getenv("TRACEBLOC_CLIENT_LOCATION"), "optional location zone for carbon reporting, e.g. DE (default: $TRACEBLOC_CLIENT_LOCATION; omitted if unset)") - cmd.Flags().StringVar(&kubeconfigPath, "kubeconfig", "", - "path to the kubeconfig for the target cluster (default: $KUBECONFIG, then ~/.kube/config) — read to anchor the client to this cluster") - cmd.Flags().StringVar(&contextOverride, "context", "", + addKubeconfigFlags(cmd, &kubeconfigPath, &contextOverride, + "path to the kubeconfig for the target cluster (default: $KUBECONFIG, then ~/.kube/config) — read to anchor the client to this cluster", "kubeconfig context for the target cluster (default: current-context)") cmd.Flags().StringVar(&credentialFile, "credential-file", "", "write the machine credential to this path (mode 0600, sourceable env) instead of printing it — for the installer to feed the chart (never shown on the terminal)") diff --git a/internal/cli/cluster.go b/internal/cli/cluster.go index a316fa0..65b41a2 100644 --- a/internal/cli/cluster.go +++ b/internal/cli/cluster.go @@ -101,11 +101,8 @@ Exit codes: }, } - cmd.Flags().StringVar(&kubeconfigPath, "kubeconfig", "", - "path to the kubeconfig file (default: $KUBECONFIG, then ~/.kube/config)") - cmd.Flags().StringVar(&contextOverride, "context", "", - "name of the kubeconfig context to use (default: kubeconfig's current-context)") - cmd.Flags().StringVarP(&nsOverride, "namespace", "n", "", + addKubeconfigFlags(cmd, &kubeconfigPath, &contextOverride, kubeconfigFlagUsage, contextFlagUsage) + addNamespaceFlag(cmd, &nsOverride, "namespace where your tracebloc client is installed (default: the context's namespace, or 'default')") cmd.Flags().Int64Var(&tokenExpiry, "token-expiry-seconds", 600, "requested SA token expiration in seconds (default 600 = 10 min; ignored for static-secret fallback)") diff --git a/internal/cli/data.go b/internal/cli/data.go index f857aba..a0c6f87 100644 --- a/internal/cli/data.go +++ b/internal/cli/data.go @@ -278,12 +278,8 @@ Exit codes: }, } - cmd.Flags().StringVar(&kubeconfigPath, "kubeconfig", "", - "path to the kubeconfig file (default: $KUBECONFIG, then ~/.kube/config)") - cmd.Flags().StringVar(&contextOverride, "context", "", - "name of the kubeconfig context to use (default: kubeconfig's current-context)") - cmd.Flags().StringVarP(&nsOverride, "namespace", "n", "", - "namespace where your tracebloc client is installed") + addKubeconfigFlags(cmd, &kubeconfigPath, &contextOverride, kubeconfigFlagUsage, contextFlagUsage) + addNamespaceFlag(cmd, &nsOverride, namespaceFlagUsage) // Required spec flags. We DON'T mark them required-at-cobra-level // because cobra's "required flag" error message is terse and diff --git a/internal/cli/data_delete.go b/internal/cli/data_delete.go index 5be9f37..8a71e53 100644 --- a/internal/cli/data_delete.go +++ b/internal/cli/data_delete.go @@ -80,12 +80,8 @@ Exit codes: }, } - cmd.Flags().StringVar(&kubeconfigPath, "kubeconfig", "", - "path to the kubeconfig file (default: $KUBECONFIG, then ~/.kube/config)") - cmd.Flags().StringVar(&contextOverride, "context", "", - "name of the kubeconfig context to use (default: kubeconfig's current-context)") - cmd.Flags().StringVarP(&nsOverride, "namespace", "n", "", - "namespace where your tracebloc client is installed") + addKubeconfigFlags(cmd, &kubeconfigPath, &contextOverride, kubeconfigFlagUsage, contextFlagUsage) + addNamespaceFlag(cmd, &nsOverride, namespaceFlagUsage) cmd.Flags().BoolVar(&dryRun, "dry-run", false, "show what would be deleted without deleting anything") cmd.Flags().BoolVarP(&yes, "yes", "y", false, diff --git a/internal/cli/data_list.go b/internal/cli/data_list.go index f76f342..ac5a325 100644 --- a/internal/cli/data_list.go +++ b/internal/cli/data_list.go @@ -76,12 +76,8 @@ Exit codes: }, } - cmd.Flags().StringVar(&kubeconfigPath, "kubeconfig", "", - "path to the kubeconfig file (default: $KUBECONFIG, then ~/.kube/config)") - cmd.Flags().StringVar(&contextOverride, "context", "", - "name of the kubeconfig context to use (default: kubeconfig's current-context)") - cmd.Flags().StringVarP(&nsOverride, "namespace", "n", "", - "namespace where your tracebloc client is installed") + addKubeconfigFlags(cmd, &kubeconfigPath, &contextOverride, kubeconfigFlagUsage, contextFlagUsage) + addNamespaceFlag(cmd, &nsOverride, namespaceFlagUsage) cmd.Flags().BoolVar(&outputJSON, "output-json", false, "emit the dataset list as JSON on stdout (human output → stderr)") diff --git a/internal/cli/delete.go b/internal/cli/delete.go index 754b7b1..d6cc100 100644 --- a/internal/cli/delete.go +++ b/internal/cli/delete.go @@ -91,11 +91,10 @@ and are erased. Not undoable.`, "uninstall the software but keep ~/.tracebloc (local config + on-host datasets)") cmd.Flags().BoolVar(&o.force, "force", false, "offboard even if tracebloc still reports this client online") - cmd.Flags().StringVar(&o.kubeconfigPath, "kubeconfig", "", - "path to the kubeconfig for the target cluster (default: $KUBECONFIG, then ~/.kube/config)") - cmd.Flags().StringVar(&o.contextOverride, "context", "", + addKubeconfigFlags(cmd, &o.kubeconfigPath, &o.contextOverride, + "path to the kubeconfig for the target cluster (default: $KUBECONFIG, then ~/.kube/config)", "kubeconfig context for the target cluster (default: current-context)") - cmd.Flags().StringVarP(&o.namespace, "namespace", "n", "", + addNamespaceFlag(cmd, &o.namespace, "namespace of this machine's tracebloc release (default: the active client's namespace)") return cmd } diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go index 422fe01..9b73221 100644 --- a/internal/cli/doctor.go +++ b/internal/cli/doctor.go @@ -61,11 +61,8 @@ Exit codes: }, } - cmd.Flags().StringVar(&kubeconfigPath, "kubeconfig", "", - "path to the kubeconfig file (default: $KUBECONFIG, then ~/.kube/config)") - cmd.Flags().StringVar(&contextOverride, "context", "", - "name of the kubeconfig context to use (default: kubeconfig's current-context)") - cmd.Flags().StringVarP(&nsOverride, "namespace", "n", "", + addKubeconfigFlags(cmd, &kubeconfigPath, &contextOverride, kubeconfigFlagUsage, contextFlagUsage) + addNamespaceFlag(cmd, &nsOverride, "namespace where your tracebloc client is installed (default: the context's namespace, or 'default')") return cmd diff --git a/internal/cli/flags.go b/internal/cli/flags.go new file mode 100644 index 0000000..0ecdc75 --- /dev/null +++ b/internal/cli/flags.go @@ -0,0 +1,31 @@ +package cli + +import "github.com/spf13/cobra" + +// Shared usage strings for the cluster-targeting flags. The --kubeconfig and +// --context phrasing is shared by the cluster-info / data* / doctor commands +// (the client-anchor and offboard-delete commands word theirs differently and +// pass their own literals); namespaceFlagUsage is used by the commands that +// take the separate --namespace/-n flag via addNamespaceFlag. +const ( + kubeconfigFlagUsage = "path to the kubeconfig file (default: $KUBECONFIG, then ~/.kube/config)" + contextFlagUsage = "name of the kubeconfig context to use (default: kubeconfig's current-context)" + namespaceFlagUsage = "namespace where your tracebloc client is installed" +) + +// addKubeconfigFlags registers the shared --kubeconfig/--context pair on cmd, +// binding them to the caller's variables. The name, default ("") and (lack of) +// shorthand match kubectl conventions; only the usage text is parametrized so +// each command can keep its own phrasing while the flag mechanics live in one +// place. +func addKubeconfigFlags(cmd *cobra.Command, kubeconfig, context *string, kubeconfigUsage, contextUsage string) { + cmd.Flags().StringVar(kubeconfig, "kubeconfig", "", kubeconfigUsage) + cmd.Flags().StringVar(context, "context", "", contextUsage) +} + +// addNamespaceFlag registers the shared -n/--namespace flag. It is intentionally +// separate from addKubeconfigFlags so commands that take a kubeconfig but no +// namespace (e.g. `client create`) don't grow a spurious --namespace flag. +func addNamespaceFlag(cmd *cobra.Command, namespace *string, usage string) { + cmd.Flags().StringVarP(namespace, "namespace", "n", "", usage) +} diff --git a/internal/config/config.go b/internal/config/config.go index 54a855c..b4dc08d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -235,9 +235,11 @@ func (c *Config) Save() error { return nil } -// Clear removes the config file (full sign-out + reset, all envs). A missing -// file is not an error. -func Clear() error { +// clearAll removes the config file (full sign-out + reset, all envs). A missing +// file is not an error. Unexported: no production caller (logout uses Save); +// retained only for the same-package test that pins the remove-and-tolerate- +// missing behavior. (`clear` is a Go builtin, hence clearAll.) +func clearAll() error { path, err := Path() if err != nil { return err diff --git a/internal/config/config_test.go b/internal/config/config_test.go index e49934a..afc6c7d 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -58,14 +58,14 @@ func TestLoadMissingIsEmpty(t *testing.T) { func TestClear(t *testing.T) { t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) _ = (&Config{CurrentEnv: "prod", Profiles: map[string]*Profile{"prod": {Token: "x"}}}).Save() - if err := Clear(); err != nil { + if err := clearAll(); err != nil { t.Fatal(err) } if c, _ := Load(); c.SignedIn() { - t.Error("after Clear, should not be signed in") + t.Error("after clearAll, should not be signed in") } - if err := Clear(); err != nil { - t.Errorf("Clear on a missing file should be nil, got %v", err) + if err := clearAll(); err != nil { + t.Errorf("clearAll on a missing file should be nil, got %v", err) } } diff --git a/internal/push/category.go b/internal/push/category.go index 08cbad3..bbf2011 100644 --- a/internal/push/category.go +++ b/internal/push/category.go @@ -298,8 +298,9 @@ func SupportedCategoryIDs() []string { return ids } -// AllCategoryIDs returns every recognized category id, in registry order. -func AllCategoryIDs() []string { +// allCategoryIDs returns every recognized category id, in registry order. +// Unexported: only the same-package registry test consumes it. +func allCategoryIDs() []string { ids := make([]string, 0, len(categoryRegistry)) for _, c := range categoryRegistry { ids = append(ids, c.ID) diff --git a/internal/push/category_registry_test.go b/internal/push/category_registry_test.go index 91428bd..40975ad 100644 --- a/internal/push/category_registry_test.go +++ b/internal/push/category_registry_test.go @@ -23,8 +23,8 @@ func TestRegistryKnownCategories(t *testing.T) { "time_series_forecasting", "time_series_classification", "time_to_event_prediction", } - if got := AllCategoryIDs(); !equalSet(got, want) { - t.Fatalf("AllCategoryIDs() = %v, want set %v", got, want) + if got := allCategoryIDs(); !equalSet(got, want) { + t.Fatalf("allCategoryIDs() = %v, want set %v", got, want) } for _, id := range want { if !IsKnown(id) { @@ -159,7 +159,7 @@ func TestRegistryWithinSchema(t *testing.T) { for _, id := range schemaCategoryEnum(t) { inSchema[id] = true } - for _, id := range AllCategoryIDs() { + for _, id := range allCategoryIDs() { if !inSchema[id] && !registryAliases[id] { t.Errorf("registry category %q is not in the ingest.v1 schema enum and not a declared "+ "alias — add it to the schema (data-ingestors) if it's real, or declare it in "+ diff --git a/internal/submit/client.go b/internal/submit/client.go index 972d91b..e1eebab 100644 --- a/internal/submit/client.go +++ b/internal/submit/client.go @@ -185,10 +185,11 @@ func (e *SubmitError) Error() string { e.Endpoint, e.StatusCode, strings.TrimSpace(e.Body)) } -// IsSubmitError reports whether err is a *SubmitError. Convenience +// isSubmitError reports whether err is a *SubmitError. Convenience // for the orchestrator's exit-code mapping; errors.As would also -// work but this reads cleaner at the branch site. -func IsSubmitError(err error) bool { +// work but this reads cleaner at the branch site. Unexported: only +// same-package tests reference it today. +func isSubmitError(err error) bool { var se *SubmitError return errors.As(err, &se) } diff --git a/internal/submit/client_test.go b/internal/submit/client_test.go index 7519c14..5ae2bbd 100644 --- a/internal/submit/client_test.go +++ b/internal/submit/client_test.go @@ -104,7 +104,7 @@ func TestHTTPSubmitter_4xxSurfacesBody(t *testing.T) { if err == nil { t.Fatal("Submit returned nil on 4xx") } - if !IsSubmitError(err) { + if !isSubmitError(err) { t.Errorf("err is not *SubmitError: %T", err) } for _, want := range []string{"HTTP 422", "missing required field 'intent'"} { @@ -160,7 +160,7 @@ func TestHTTPSubmitter_5xxNotAuthError(t *testing.T) { if IsAuthError(err) { t.Errorf("IsAuthError(500) = true, want false") } - if !IsSubmitError(err) { + if !isSubmitError(err) { t.Errorf("err is not *SubmitError: %T", err) } } diff --git a/internal/submit/submit_test.go b/internal/submit/submit_test.go index 0eeadf6..d94c44b 100644 --- a/internal/submit/submit_test.go +++ b/internal/submit/submit_test.go @@ -117,7 +117,7 @@ func TestRun_SubmitErrorPropagates(t *testing.T) { if err == nil { t.Fatal("Run returned nil on submit error") } - if !IsSubmitError(err) { + if !isSubmitError(err) { t.Errorf("err is not *SubmitError: %T", err) } }