Skip to content

feat(preflight): diagnose an unreachable registry before installing - #113

Merged
bsherman merged 3 commits into
mainfrom
benjamin/feat/preflight-registry-diagnostics
Oct 7, 2026
Merged

bsherman merged 3 commits into
mainfrom
benjamin/feat/preflight-registry-diagnostics

Conversation

@bsherman

@bsherman bsherman commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Closes #105.

Summary

  • After preflight-image fails to resolve an image or verify its signature with a failure that positively looks like connectivity (DNS, dial, timeout), probe the registry's HTTPS /v2/ endpoint (honoring proxy env). If there is no route to a server, fail with the new stable code registry_unreachable, naming the registry and keeping the original skopeo/cosign error. If the attempt failed on a machine with no routable address, the code is network_unreachable instead.
  • The probe is supplemental evidence, never a gate. Ambiguous outcomes keep their original code: TLS trust/alert/protocol failures, proxy 407/403, registry responses (manifest unknown, unauthorized, ...), cosign verdicts, and errors with no connectivity marker. Cached images still install offline; cancellation is never a network verdict. Both new codes take precedence over image_verification_failed.
  • Messages don't claim the machine is offline unless the interface check supports it (deviates from the literal wording in the issue). The interface check runs after the attempt, only to word a failure.
  • Injected via pipeline.Env.RegistryProbe; nil disables probing, so existing fake-runner tests never touch the network. install and the TUI wire the real probe (tested).
  • Docs: progress-protocol.md (two new codes), recipe-schema.md (cosign_pub_key row), architecture.md (Preflight). No ADR: narrowly scoped diagnostics, no policy change.

Known limits

  • registries.conf mirrors and plain-HTTP registries aren't probed; those cases keep the original code.
  • The wizard's early warning is out of scope (follow-up).

Test plan

  • make check
  • Unit tests: classification boundary (signed/unsigned, tag/digest, cached, local transports), registry-level and cosign-verdict skips, cancellation, bounded context, probe against 401 / untrusted cert / TLS alert / plain HTTP / proxy 502 and 407 / closed port, host parsing, interface check, normal and dry-run terminal codes
  • Nested-VM E2E not run

When preflight-image cannot resolve an image, or cannot verify its
signature, the failure said nothing about whether the registry could be
reached. Probe the registry's /v2/ endpoint (through the environment's
proxy settings) after such a failure and report network_unreachable when
there is no route to a server, keeping the original error attached.

The probe is evidence, not a gate: TLS trust and plain-HTTP failures count
as a server answering so they keep their original code, caller
cancellation is never reported as a network failure, and the message does
not claim the machine is offline, since one endpoint failing cannot show
that. A nil probe disables it.
@codecov

codecov Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.09091% with 21 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/bootcimg/probe.go 81.91% 13 Missing and 4 partials ⚠️
internal/bootcimg/bootcimg.go 80.00% 1 Missing and 1 partial ⚠️
internal/bootcimg/verify.go 83.33% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@bketelsen bketelsen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed with Claude Code. The overall shape is solid: the probe is supplemental, cancellation doesn't count as a network failure, and cached installs still work offline. Most comments are about the probe overriding evidence the original error already carries, and about classification gaps that produce a false network_unreachable. The PR's own tests pass locally, including under -race.

Comment thread internal/bootcimg/verify.go Outdated
Comment thread internal/bootcimg/verify.go Outdated
Comment thread internal/bootcimg/probe.go Outdated
Comment thread internal/bootcimg/probe.go Outdated
Comment thread internal/bootcimg/probe.go
Comment thread internal/progress/progress.go
Comment thread internal/steps/preflight.go
Comment thread internal/bootcimg/verify.go Outdated
Comment thread cmd/firn/tui.go
Review found the registry probe could turn unrelated failures into a
reachability verdict. Classify positively instead: only DNS/dial failures,
timeouts, and proxy 502/503/504 count, so a proxy 407, TLS alerts, registry
responses, cosign verdicts and unmarked errors keep their original code.

Split the code: registry_unreachable names the endpoint and says the network
or registry may be responsible; network_unreachable is used only when the
attempt failed on a machine with no routable address. The interface check now
runs after the attempt, so it can word a failure but never block a route that
works. Also fix the probed host for transport-prefixed, host:port and Docker
Hub refs, and wire-check the probe in install and the TUI.
@bsherman
bsherman added this pull request to the merge queue Oct 7, 2026
Merged via the queue into main with commit db31980 Oct 7, 2026
5 checks passed
@bsherman
bsherman deleted the benjamin/feat/preflight-registry-diagnostics branch October 7, 2026 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add pre-install network check

2 participants