* fix(analyze/secret): guard nil deref when spec omits fail: outcome (#2053)
When a Preflight spec defines only warn: and/or pass: outcomes and the
target Secret is missing, analyzeSecret dereferenced a nil failOutcome and
panicked. Reproduction from replicatedhq/troubleshoot#2053:
outcomes:
- warn:
when: "notFound"
message: "secret missing (warn)"
- pass:
message: "secret found"
This change:
- Collects fail, warn, and warn-when-notFound outcomes up front.
- Routes a missing secret (or missing key) through a single resolver:
prefer warn(when=notFound), fall back to fail, then any warn,
then synthesize a benign warn result. Never panics.
- Adds table-driven tests covering the three new shapes.
Same defect shape as #263 (imagePullSecret). The sibling analyzers
configmap.go and image.go have the same pattern but are out of scope
for this PR.
Refs: replicatedhq/troubleshoot#2053
* refactor(analyze/secret): drop warn handling per analyzer contract
Per review feedback (banjoh): the secret analyzer only supports fail
(not found) and pass (found) outcomes — `warn:` and `when:` are not
part of the analyzer's contract (https://troubleshoot.sh/docs/analyze/secrets).
Drop the warn / notFoundWarn branches added in the previous commit and
collapse the nil-fail fallback onto IsFail with a default message. The
core fix — guarding the nil deref at the previous secret.go:72 — stays
in place.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(analyze/secret): mirror image_pull_secret pattern; return nil when spec has neither outcome
Per banjoh review: collect failOutcome and passOutcome with non-nil
checks; when the spec contains neither, return nil and let the framework
surface the missing-outcome error rather than fabricating a result.
Structure now mirrors pkg/analyze/image_pull_secret.go: default to
IsFail with fail-outcome message (if set), flip to IsPass with
pass-outcome message when the secret/key check succeeds, fill default
messages at the end only when none were configured. Analyze() now
forwards a nil result through cleanly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(analyze/secret): assign outcome messages inside their own branch
Cursor bugbot caught: when the spec defined only a fail outcome and the
secret was found, the result showed IsPass=true with the fail outcome's
message. Pre-assigning the fail message at the top before flipping
IsPass meant the stale message leaked through whenever no pass outcome
was configured.
Move both message assignments into their respective branches so a pass
result never carries a fail message. Default messages still fill in
when no outcome is set on the active branch. Added a regression test
covering fail-only spec + secret found.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(analyze/secret): address Greptile P1s — combined outcome + empty configured messages
Two fixes flagged by review on #2054:
1. Combined pass outcome dropped: the outcome loop used 'else if' for pass, so a
single outcome object containing both fail and pass silently dropped the pass.
Capture fail and pass independently.
2. Empty configured messages overwritten: the default-message fallback fired
whenever result.Message was empty, clobbering a configured outcome that
intentionally has an empty message (e.g. URI-only). Track whether the matched
branch had a configured outcome and only fall back to a default when none was
supplied, preserving the configured message and URI verbatim.
Adds tests: combined fail+pass captured on both found/not-found paths, and
URI-only pass/fail outcomes preserved without default-message overwrite.
* fix(analyze/secret): drop fabricated default messages; error on missing outcome
Per maintainer review: analyzers do not fabricate default messages — an empty
outcome message is intentional (e.g. a URI-only outcome), so remove the default
message fallback and preserve the configured outcome verbatim. When a matched
branch has no configured outcome, the message stays empty.
Also address the missing-outcome case: when a spec defines neither a pass nor a
fail outcome, analyzeSecret returned (nil, nil), which the Analyze wrapper
swallowed into an empty result slice — the user saw neither a result nor a
config error. Return an explicit error so the framework surfaces the
misconfiguration.
Tests updated: the no-fail-outcome and only-fail-outcome cases now assert an
empty message rather than a fabricated one, and the neither-outcome case asserts
an error.
* fix(analyze/secret): default message only when no outcome is configured for the matched branch
Distinguish an absent matching outcome from an intentionally empty configured
message. A configured outcome with an empty message (e.g. a URI-only outcome) is
still preserved verbatim. But when the matched branch has no configured outcome at
all — a pass-only spec that took the fail path, or a fail-only spec that passed —
fall back to a default diagnostic instead of emitting an empty message.
Addresses the greptile P1 (endorsed by banjoh): dropping the default entirely
conflated the two cases and left users with a pass/fail result and no context.
Tests restore the default-message expectations for the no-configured-outcome
branches; URI-only (configured-empty) and neither-outcome (error) cases unchanged.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cluster-resources): stop emitting duplicate unredacted YAML copy
storeCustomResource was writing both a JSON and a YAML file for every
custom resource. The built-in redactors are authored for JSON, so the
YAML copy was left unredacted. JSON is valid YAML, so analyzers that
expect YAML can still parse the JSON file.
Also convert the built-in kurl installer redactors from YAML-specific
paths to scoped JSON regex redactors so they continue to apply after the
YAML copy is removed.
* fix(cluster-resources): add YAML symlinks and cover cluster-scoped kurl installer
- Add a .yaml symlink for every custom-resource JSON file so existing
analyzers that look for the old YAML copy keep working. The symlink
points at the JSON file, so redaction of the JSON also redacts the YAML
view.
- Fix the built-in kurl installer redactor to match both the cluster-scoped
file (installers.cluster.kurl.sh.json) and the namespaced file pattern
(installers.cluster.kurl.sh/*.json). The previous selector only matched
the namespaced form.
---------
Co-authored-by: ElasticClaw Factory <factory@replicated.com>
* fix(preflight): add tracing span and align error handling for host collector redaction
Follow-up to #2101.
Adds an OpenTelemetry span around host collector redaction and returns the
unredacted collectResult on redaction failure, matching the behavior of
remote host collectors and in-cluster support-bundle collectors.
* fix(preflight): assign collected data before redaction so errors preserve output
Add a countDistinct(<labelKey>) aggregate to the nodeResources analyzer
"when" expression language. It counts distinct values of a node label
across the filtered nodes and returns an int the existing comparison
operators evaluate.
Enables the AIR-238 3-AZ preflight: warn when Keeper-eligible nodes do
not span 3 availability zones, e.g.
"countDistinct(topology.kubernetes.io/zone) < 3".
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Xav Paice <xavpaice@users.noreply.github.com>
CollectHostWithContext was the only collection path (in-cluster support
bundle, remote/SSH host collectors) that never ran collected data through
the redaction engine. A `run` collector's captured environment in
particular can carry credentials verbatim -- e.g. HTTPS_PROXY with
embedded Basic Auth -- straight into the bundle's <collectorName>-info.json
with no redaction applied.
Wire it through collect.RedactResult the same way CollectRemoteWithContext
and pkg/supportbundle/collect.go already do, so the built-in default
redactors apply to local host preflight output too.
Fixes#2100
Updates top-level go.mod/go.sum with latest patch-level dependency
releases. Also refreshes examples/sdk/helm-template go.mod/go.sum
while preserving the local replace directive for troubleshoot.
Validated with:
- make build
- make test
- go build ./... in examples/sdk/helm-template
Co-authored-by: Replicated Developer <dev@replicated.com>
* chore(deps): update Go dependencies for top-level and helm-template example
* ci: re-trigger preflight e2e after timeout
* ci: add compile-preflight dependency for validate-supportbundle-e2e-go
---------
Co-authored-by: ElasticClaw <elasticclaw@replicated.com>
Updates indirect deps pulled in via the top-level go.mod (notably Google
Cloud, AWS, Sentry, and genproto modules) and refreshes the corresponding
helm-template example sums. Verified with make build and make test.
Co-authored-by: ElasticClaw <elasticclaw@openclaw.ai>
- github.com/longhorn/go-iscsi-helper: replaced by github.com/longhorn/go-common-libs
and migrated pkg/longhorn/util/iscsi.go to the new namespace executor API
- helm.sh/helm/v3: v3.21.2 -> v3.21.3
- oras.land/oras-go/v2: v2.6.1 -> v2.6.2
- google.golang.org/api: v0.287.1 -> v0.288.0
- golang.org/x/tools: v0.47.0 -> v0.48.0
- github.com/GoogleCloudPlatform/opentelemetry-operations-go/*: v1.33.0/v0.57.0 -> v1.34.0/v0.58.0
- examples/sdk/helm-template: helm.sh/helm/v3 v3.21.2 -> v3.21.3
Fixes required by the update environment:
- Makefile: use $(shell go env GOPATH)/bin for controller-gen/client-gen so
generate works when the tools are not on PATH
- pkg/collect/host_kernel_configs.go: only use /proc/config.gz when the
requested kernel release matches the running kernel, making the collector
robust to hosts that expose a generic /proc/config.gz
Co-authored-by: ElasticClaw Bot <elasticclaw@openclaw.ai>
* Add per-collector subcommands to the collect binary
Add `collect http|postgres|mysql|mssql|redis` subcommands that run a single
collector and print its native result JSON to stdout, so a collector can run
inside a Pod via the troubleshoot image (e.g. as a runPod collector).
Each has its own flags mapping to the collector's spec fields and its own help.
The Kubernetes client (used only for TLS-from-Secret) is built to match the
production preflight/support-bundle path (QPS/Burst/UserAgent).
* Add clickhouse subcommand to the collect binary
Add `collect clickhouse`, matching the other per-collector subcommands, so the
clickhouse collector can run inside a Pod via the troubleshoot image (e.g. as a
runPod collector). Same flags as the other database collectors (--uri + --tls-*).
* Make global flags work on the collect subcommands
Register --debug and the Kubernetes flags (--kubeconfig, --context, etc.) as
persistent flags, and move the shared setup to PersistentPreRun/PersistentPostRun,
so the per-collector subcommands inherit the global flags and run the same
logger/profiling setup as `collect [url]`. Previously these were local flags with
setup in PreRun, so subcommands rejected --debug/--kubeconfig and ignored the setup.
Replace the hardcoded MAX_CONCURRENT_REDACTORS = 10 ceiling in
pkg/collect/redact.go with a runtime-resolved value driven by the
TROUBLESHOOT_MAX_CONCURRENT_REDACTORS env var. The default (10) is
unchanged, so behavior is identical unless an operator opts in.
- DefaultMaxConcurrentRedactors exported as the default
- MaxConcurrentRedactorsEnvVar exported as the env var name
- maxConcurrentRedactors() helper parses the env, logs on invalid input,
and falls back to the default on missing/empty/non-numeric/<=0 values
- Table-driven tests in pkg/collect/redact_test.go cover unset, empty,
positive override, default-equal, zero, negative, non-numeric, and
whitespace-padded inputs
Unblocks Pixee's standalone support-bundle pipeline, which hits the
10-concurrent ceiling on large bundles.
Refs: sc-138321, replicated-collab/pixee-replicated#131
