diff --git a/.github/workflows/ci-update-canonization.yml b/.github/workflows/ci-update-canonization.yml index e1e6f6de0..2061bca49 100644 --- a/.github/workflows/ci-update-canonization.yml +++ b/.github/workflows/ci-update-canonization.yml @@ -228,22 +228,47 @@ jobs: echo "G3-U2 PASS — current=$CURRENT target=$TARGET" # ------------------------------------------------------------------ - # G3-U4 — preflight enforcement: all five checks must appear in the - # plan's Preflight block. We don't require every check to pass — - # CI environments don't have nftband running — but every check - # MUST be evaluated and reported. + # G3-U4 — preflight enforcement: all seven checks must appear in the + # plan's Preflight block (PR-16 P-1..P-5 + PR-17 P-6..P-7). We don't + # require every check to pass — CI environments don't have nftband + # running — but every check MUST be evaluated and reported. # ------------------------------------------------------------------ - - name: G3-U4 — preflight reports all 5 checks + - name: G3-U4 — preflight reports all 7 checks shell: bash run: | set -Eeuo pipefail - for name in authority_nftban service_nftband_active artifact_version_file dependency_nft state_no_stale_in_progress; do + for name in \ + authority_nftban \ + service_nftband_active \ + artifact_version_file \ + dependency_nft \ + state_no_stale_in_progress \ + rebuild_recovery_available \ + install_origin_coherent; do grep -q "$name" /tmp/dryrun.out || { echo "::error::G3-U4 FAIL: preflight check '$name' missing from plan" exit 1 } done - echo "G3-U4 PASS — all 5 preflight checks reported" + echo "G3-U4 PASS — all 7 preflight checks reported" + + # ------------------------------------------------------------------ + # G3-U4-deepen (PR-17) — recovery planning metadata must be rendered + # so apply (PR-18) has explicit input about rollback reachability. + # ------------------------------------------------------------------ + - name: G3-U4-deepen — recovery plan metadata rendered + shell: bash + run: | + set -Eeuo pipefail + grep -q "Recovery plan" /tmp/dryrun.out || { + echo "::error::G3-U4-deepen FAIL: plan has no Recovery block" + exit 1 + } + grep -q "Mechanism" /tmp/dryrun.out || { + echo "::error::G3-U4-deepen FAIL: Recovery block missing Mechanism" + exit 1 + } + echo "G3-U4-deepen PASS — recovery planning metadata present" summary: name: Update Canonization summary diff --git a/cmd/nftban-installer/update_dryrun.go b/cmd/nftban-installer/update_dryrun.go index 460416cd4..a410c8247 100644 --- a/cmd/nftban-installer/update_dryrun.go +++ b/cmd/nftban-installer/update_dryrun.go @@ -69,25 +69,30 @@ func runUpdateDryRun(ctx context.Context, exec executor.Executor, sf *state.Stat } log.PhaseEnd("Detect") - // 2. Update-specific preflight (P-1 through P-5). + // 2. Install origin — declared flag wins; fall back to package-manager + // probe (PR-17) so package installs that don't re-pass --rpm/--deb + // still get a correct plan. + origin := detectInstallOrigin(cfg) + if origin == "" { + origin = update.DetectInstallOrigin(exec, log) + } + + // 3. Update-specific preflight (P-1..P-7 — PR-16 + PR-17). log.Phase("Preflight") - pre := update.Preflight(exec, log) + pre := update.Preflight(exec, log, origin) log.PhaseEnd("Preflight") - // 3. Version detection. sourceDir is empty in the package-install case; - // the update package treats that as non-fatal for PR-16 (package-install - // target detection lands in PR-17). - current, target, err := update.DetectVersions(exec, cfg.sourceDir, log) + // 4. Version detection — source tree wins; package manager is the + // fallback for package-install hosts (PR-17). + current, target, err := update.DetectVersions(exec, cfg.sourceDir, origin, log) if err != nil { log.Error("update dry-run: version detection failed: %v", err) return state.ExitFailed } - // 4. Install origin — drives which apply path PR-18 will use. - origin := detectInstallOrigin(cfg) - - // 5. Assemble + render plan. + // 5. Assemble plan + attach PR-17 recovery metadata (planning-only). plan := update.BuildPlan(pre, current, target, origin) + plan.AttachRecovery(update.BuildRecoveryPlan(exec)) plan.Render(os.Stdout) // 6. Write a copy of the plan JSON to the state dir for audit/history diff --git a/internal/installer/update/plan.go b/internal/installer/update/plan.go index 2271e53fe..d39439f36 100644 --- a/internal/installer/update/plan.go +++ b/internal/installer/update/plan.go @@ -77,6 +77,29 @@ type Plan struct { // this abstract ("reuse rebuild pipeline"); PR-18 fills in concrete // steps. Actions []string `json:"actions,omitempty"` + + // Recovery carries planning-only metadata about the rollback route that + // PR-18's apply step will rely on. PR-17 populates this but does NOT + // execute any recovery — that's PR-18 scope per INV-U-002. + Recovery *RecoveryPlan `json:"recovery,omitempty"` +} + +// RecoveryPlan is PR-17 metadata describing the rollback path apply will +// follow on failure. It is DESCRIPTIVE only; no state mutation in PR-17. +type RecoveryPlan struct { + // Available reports whether a rollback to the prior known-good state + // is possible without operator intervention. + Available bool `json:"available"` + + // Mechanism is the rollback mechanism that would be used — currently + // "rebuild" (validate-in-namespace + flush+load + v1.96 recovery), the + // only canonized recovery surface today. + Mechanism string `json:"mechanism"` + + // Notes carries human-oriented context that helps an operator reason + // about recovery without digging into code (e.g. "prior state file + // present and terminal", "no prior install_state — fresh recovery"). + Notes []string `json:"notes,omitempty"` } // PlanSchemaVersion is the current wire contract for Plan. Consumers should @@ -84,32 +107,48 @@ type Plan struct { const PlanSchemaVersion = "1.99.0" // DetectVersions reads the current installed version from the FHS VERSION -// file and the target version from the source-tree VERSION (for source -// installs) or from the package metadata path. +// file and the target version from: // -// For PR-16 we support source-install detection; package-install target -// detection is handled in PR-17. -func DetectVersions(exec executor.Executor, sourceDir string, log *logging.Logger) (current, target string, err error) { +// - /VERSION when sourceDir is non-empty (source installs) +// - the package manager (rpm -q / dpkg -s) when origin is "rpm"/"deb" +// and sourceDir is empty (PR-17 scope) +// +// Missing target is non-fatal — the plan carries a warning downstream. +// Missing current IS fatal — we must know where we are before planning +// any transition (INV-U-002 rollbackability). +func DetectVersions(exec executor.Executor, sourceDir, origin string, log *logging.Logger) (current, target string, err error) { current = readCurrentVersion(exec, log) - // Source install: target VERSION lives at /VERSION. + // Priority 1: source tree VERSION (takes precedence over package query + // so a caller can test with a source tree even on a package-install host). if sourceDir != "" { tPath := filepath.Join(sourceDir, "VERSION") if data, rErr := exec.ReadFile(tPath); rErr == nil { target = strings.TrimSpace(string(data)) + log.Debug("update: target version from source tree %s = %s", tPath, target) } else { log.Debug("update: source VERSION not readable at %s: %v", tPath, rErr) } } + // Priority 2 (PR-17): package manager query. Only consulted when source + // detection did not yield a result. We pass in `origin` rather than + // auto-detecting inside this function so the caller stays authoritative + // about the install-origin decision. + if target == "" && (origin == "rpm" || origin == "deb") { + if t, qErr := DetectPackageTarget(exec, origin, log); qErr != nil { + log.Warn("update: package target query failed: %v", qErr) + } else if t != "" { + target = t + log.Debug("update: target version from package manager (%s) = %s", origin, target) + } + } + if current == "" { return current, target, fmt.Errorf("update: cannot detect current version (no VERSION file)") } if target == "" { - // PR-16: target detection for package installs lands in PR-17. - // For now, missing target is non-fatal — the plan records it and - // the preflight flags it as a warning. - log.Info("update: target version not yet detected (package-install detection lands in PR-17)") + log.Info("update: target version not detected (no source tree + no package ownership)") } return current, target, nil } @@ -172,6 +211,20 @@ func (p *Plan) Render(w io.Writer) { fmt.Fprintln(w, "") } + if p.Recovery != nil { + fmt.Fprintln(w, " Recovery plan (metadata only — apply lands in PR-18):") + availMark := "available" + if !p.Recovery.Available { + availMark = "NOT available — operator intervention may be required" + } + fmt.Fprintf(w, " Rollback : %s\n", availMark) + fmt.Fprintf(w, " Mechanism : %s\n", displayString(p.Recovery.Mechanism)) + for _, n := range p.Recovery.Notes { + fmt.Fprintf(w, " · %s\n", n) + } + fmt.Fprintln(w, "") + } + if len(p.Actions) > 0 { fmt.Fprintln(w, " Actions (dry-run — no mutation):") for _, a := range p.Actions { @@ -209,11 +262,22 @@ func BuildPlan(pre *PreflightResult, current, target, origin string) *Plan { } if target == "" { p.Warnings = append(p.Warnings, - "target version not detected (package-install target detection lands in PR-17)") + "target version not detected (no source tree + no package ownership)") + } + if origin == "" { + p.Warnings = append(p.Warnings, + "install origin not detected — pass --source, --rpm, or --deb to disambiguate") } return p } +// AttachRecovery decorates the plan with the PR-17 recovery metadata. +// Separate from BuildPlan so callers without an exec handle (tests) can +// still build a plan without synthesizing recovery state. +func (p *Plan) AttachRecovery(r *RecoveryPlan) { + p.Recovery = r +} + // displayVersion returns a placeholder for empty versions so the rendered // table doesn't print blank cells that look like a bug. func displayVersion(v string) string { diff --git a/internal/installer/update/preflight.go b/internal/installer/update/preflight.go index 8b10a10e1..172e2ab16 100644 --- a/internal/installer/update/preflight.go +++ b/internal/installer/update/preflight.go @@ -61,7 +61,10 @@ type PreflightResult struct { // Called by phaseDetect when cfg.mode == "upgrade". // // The function is READ-ONLY. It must never mutate host state. -func Preflight(exec executor.Executor, log *logging.Logger) *PreflightResult { +// +// origin is the operator-declared install origin ("rpm", "deb", "source", +// or ""). PR-17 uses it for the new P-7 coherence check. +func Preflight(exec executor.Executor, log *logging.Logger, origin string) *PreflightResult { res := &PreflightResult{Passed: true} // P-1 authority — the existing authority.Detect would give us a full @@ -150,9 +153,116 @@ func Preflight(exec executor.Executor, log *logging.Logger) *PreflightResult { logCheck(log, c) } + // P-6 (PR-17) rebuild recovery available — the planning-only check + // that tells apply (PR-18) whether a clean rollback is reachable. + // We do NOT execute recovery here; we just report whether the + // prerequisites exist: terminal prior state + ip nftban table + nft + // binary. If any is missing, apply must treat rollback as + // operator-assisted rather than automatic (INV-U-002). + { + ok := rebuildRecoveryAvailable(exec) + c := PreflightCheck{ + Name: "rebuild_recovery_available", + Passed: ok, + Severity: "warning", + } + if !ok { + c.Detail = "rebuild recovery may require operator intervention on failure — review before apply" + } + res.Checks = append(res.Checks, c) + logCheck(log, c) + } + + // P-7 (PR-17) install origin coherent — the declared --rpm / --deb / + // --source flag must match the host's actual install origin. A + // mismatch (e.g. --rpm on a DEB host) is a hard warning: apply would + // route through the wrong delivery model. PR-17 detects this early. + { + declared := origin + detected := DetectInstallOrigin(exec, log) + coherent := declared == "" || detected == "" || declared == detected + c := PreflightCheck{ + Name: "install_origin_coherent", + Passed: coherent, + Severity: "warning", + } + if !coherent { + c.Detail = "declared origin (" + declared + ") does not match detected origin (" + detected + ")" + } + res.Checks = append(res.Checks, c) + logCheck(log, c) + } + return res } +// rebuildRecoveryAvailable reports whether the prerequisites for automatic +// rollback via rebuild recovery are present: +// +// - the prior install_state is terminal (COMMITTED / DEGRADED / FAILED_*) +// - ip nftban table exists (authority held) +// - nft binary is present (handled by P-4; we still check here so this +// predicate is self-contained for callers outside the full preflight) +// +// PR-17 scope: this is metadata only. PR-18 will wire it into the apply +// path via INV-U-002. +func rebuildRecoveryAvailable(exec executor.Executor) bool { + if !exec.NftTableExists("ip", "nftban") { + return false + } + if r := exec.Run("sh", "-c", "command -v nft >/dev/null 2>&1"); r.ExitCode != 0 { + return false + } + // Prior state must be readable OR absent (absent = fresh recovery is + // fine). If present but non-terminal (in-progress), recovery is not + // clean. + const p = "/var/lib/nftban/state/install_state" + if !exec.FileExists(p) { + return true + } + data, err := exec.ReadFile(p) + if err != nil { + return false + } + s := trim(string(data)) + if s == "" { + return true + } + // Same terminal-state list as isStaleInProgress but inverted. + switch s { + case "COMMITTED", "DEGRADED", + "FAILED_SSH_UNKNOWN", "FAILED_AUTHORITY_ABORT", "FAILED_RENDER", + "FAILED_REBUILD", "FAILED_NO_FIREWALL", "FAILED_TAKEOVER": + return true + } + return false +} + +// BuildRecoveryPlan derives the PR-17 recovery metadata from exec state. +// Planning-only: no mutation, no recovery execution. +func BuildRecoveryPlan(exec executor.Executor) *RecoveryPlan { + r := &RecoveryPlan{ + Mechanism: "rebuild", + } + r.Available = rebuildRecoveryAvailable(exec) + const p = "/var/lib/nftban/state/install_state" + if !exec.FileExists(p) { + r.Notes = append(r.Notes, + "no prior install_state — a fresh recovery is clean (no rollback target)") + } else { + data, err := exec.ReadFile(p) + if err == nil { + r.Notes = append(r.Notes, + "prior install_state = "+trim(string(data))) + } + } + if !r.Available { + r.Notes = append(r.Notes, + "preconditions for automatic rollback unmet — apply will require operator assist on failure") + } + return r +} + // isStaleInProgress returns true if install_state exists AND is not a // terminal state (COMMITTED / DEGRADED / FAILED_*). We treat an in-progress // marker as a caution. diff --git a/internal/installer/update/target.go b/internal/installer/update/target.go new file mode 100644 index 000000000..67baa2e26 --- /dev/null +++ b/internal/installer/update/target.go @@ -0,0 +1,176 @@ +// ============================================================================= +// NFTBan v1.99 PR-17 — Package-Install Target Version Detection +// ============================================================================= +// SPDX-License-Identifier: MPL-2.0 +// meta:name="installer-update-target" +// meta:type="lib" +// meta:owner="Antonios Voulvoulis " +// meta:created_date="2026-04-19" +// meta:description="Detect target version from rpm -q / dpkg -s (package installs)" +// meta:inventory.files="internal/installer/update/target.go" +// meta:inventory.binaries="" +// meta:inventory.env_vars="" +// meta:inventory.config_files="" +// meta:inventory.systemd_units="" +// meta:inventory.network="" +// meta:inventory.privileges="root" +// ============================================================================= +// +// PR-17 deepens G3-U4 (preflight) with: +// +// - DetectPackageTarget: query rpm -q / dpkg -s for the "staged" version +// of nftban-core. For a package upgrade, the staged version is the one +// on disk in the package DB; the running version is in VERSION. A +// non-empty diff tells us an upgrade is pending. +// +// - DetectInstallOrigin: probe rpm -q / dpkg -s to classify the host's +// install origin when the operator didn't pass --rpm / --deb / --source. +// Used by the update-dry-run orchestrator to render a sensible plan. +// +// Both functions are READ-ONLY. No mutation; no package manager transactions. +// +// Design principle — minimum stable discriminator set: +// +// This package intentionally classifies at the level of "install origin" +// (rpm / deb / source) only. It does NOT branch on distro point releases +// (Ubuntu 22.04.1 vs 22.04.5, AlmaLinux 9.3 vs 9.5, etc.) because no +// behavioural difference in the update trigger path changes with point +// releases — the package manager (rpm / dpkg) is the stable discriminator, +// not the OS version string. +// +// Full VersionID is still captured upstream in detect.DistroInfo for +// evidence and debugging, but it stays metadata — never branching input — +// unless a concrete behavioural difference is proven (different package +// semantics, service/unit path changes, polkit path changes, etc.). +// +// This is the same discipline used by internal/installer/detect/distro.go +// (only ID branches, VersionID is metadata) and by internal/installer/ +// payload/payload.go::isDebianFamily (family-level dispatch). +// +// ============================================================================= + +package update + +import ( + "strings" + + "github.com/itcmsgr/nftban/internal/installer/executor" + "github.com/itcmsgr/nftban/internal/installer/logging" +) + +// PackageNames is the canonical ordered list of package identifiers to probe. +// Modern packages ship as "nftban-core"; pre-v1.73 packages shipped as +// "nftban". We try both for ownership probes so legacy hosts still classify +// correctly. This mirrors cli/lib/nftban/cli/cmd_update_detection.sh:: +// _detect_install_type so the Go and shell paths report the same origin. +var PackageNames = []string{"nftban-core", "nftban"} + +// DetectInstallOrigin returns "rpm", "deb", or "source" when it can infer +// the host's install origin, or "" when it cannot. Probing order: +// +// 1. rpm -q for each name in PackageNames — if any exits 0, origin is "rpm" +// 2. dpkg -s for each name in PackageNames — if any exits 0, origin is "deb" +// 3. NFTBAN_SOURCE_DIR env var set — origin is "source" +// 4. otherwise — "" (unknown) +// +// The probing is ORDER-SENSITIVE by design: package managers coexist on +// some systems (e.g. alien, conversion tooling), so we prefer rpm if it +// reports ownership. This is the Go mirror of the shell +// _detect_install_type helper; both must stay in sync until PR-21 removes +// the shell path. +func DetectInstallOrigin(exec executor.Executor, log *logging.Logger) string { + for _, pkg := range PackageNames { + if hasPackage(exec, "rpm", "-q", pkg) { + log.Debug("update: install origin detected via rpm -q %s = rpm", pkg) + return "rpm" + } + } + for _, pkg := range PackageNames { + if hasPackage(exec, "dpkg", "-s", pkg) { + log.Debug("update: install origin detected via dpkg -s %s = deb", pkg) + return "deb" + } + } + if exec.Getenv("NFTBAN_SOURCE_DIR") != "" { + log.Debug("update: install origin inferred from NFTBAN_SOURCE_DIR = source") + return "source" + } + log.Debug("update: install origin not detected — no package manager ownership + no source dir") + return "" +} + +// hasPackage returns true when the package manager reports ownership +// (exit 0) for the given package. Empty output or non-zero exit = false. +func hasPackage(exec executor.Executor, name string, args ...string) bool { + r := exec.Run(name, args...) + if r.ExitCode != 0 { + return false + } + return strings.TrimSpace(r.Stdout) != "" +} + +// DetectPackageTarget queries the package database for the version of +// nftban-core that would be installed if an upgrade ran. For a real +// upgrade scenario this equals the version on disk in the package DB, +// which is ALSO the running version when the package is in sync. +// +// Returns ("", nil) when the package manager is present but the package +// is not owned. Returns ("", err) only on plumbing failures (unlikely — +// we explicitly tolerate non-zero exits as "not owned"). +// +// PR-17 scope: this detection is advisory only. Apply work (PR-18) will +// use the SAME query to drive the rebuild-pipeline input delta — per +// INV-U-001 there is no separate package-specific apply path. +func DetectPackageTarget(exec executor.Executor, origin string, log *logging.Logger) (string, error) { + for _, pkg := range PackageNames { + switch origin { + case "rpm": + if v := queryRPM(exec, pkg, log); v != "" { + return v, nil + } + case "deb": + if v := queryDEB(exec, pkg, log); v != "" { + return v, nil + } + default: + // Source install: target is the source tree's VERSION file and + // is handled in plan.DetectVersions directly. Unknown origin: + // no query — fall through. + return "", nil + } + } + return "", nil +} + +// queryRPM runs `rpm -q --queryformat '%{VERSION}' `. +// Returns the version string, or "" when not installed. +func queryRPM(exec executor.Executor, pkg string, log *logging.Logger) string { + r := exec.Run("rpm", "-q", "--queryformat", "%{VERSION}", pkg) + if r.ExitCode != 0 { + log.Debug("update: rpm -q %s exit=%d (package not installed)", pkg, r.ExitCode) + return "" + } + v := strings.TrimSpace(r.Stdout) + // rpm -q for a missing package sometimes prints the error to stdout AND + // still exits non-zero, so this is belt-and-suspenders. + if strings.Contains(v, "not installed") || strings.Contains(v, "is not installed") { + return "" + } + return v +} + +// queryDEB runs `dpkg -s ` and extracts the `Version:` field. +// Returns the version string, or "" when not installed. +func queryDEB(exec executor.Executor, pkg string, log *logging.Logger) string { + r := exec.Run("dpkg", "-s", pkg) + if r.ExitCode != 0 { + log.Debug("update: dpkg -s %s exit=%d (package not installed)", pkg, r.ExitCode) + return "" + } + for _, line := range strings.Split(r.Stdout, "\n") { + if strings.HasPrefix(line, "Version:") { + return strings.TrimSpace(strings.TrimPrefix(line, "Version:")) + } + } + return "" +} diff --git a/internal/installer/update/target_test.go b/internal/installer/update/target_test.go new file mode 100644 index 000000000..a142831a9 --- /dev/null +++ b/internal/installer/update/target_test.go @@ -0,0 +1,304 @@ +// ============================================================================= +// NFTBan v1.99 PR-17 — Target Detection Tests +// ============================================================================= +// SPDX-License-Identifier: MPL-2.0 +// meta:name="installer-update-target-test" +// meta:type="test" +// meta:owner="Antonios Voulvoulis " +// meta:created_date="2026-04-19" +// meta:description="Unit tests for package-install target + origin detection + recovery plan" +// meta:inventory.files="internal/installer/update/target_test.go" +// meta:inventory.binaries="" +// meta:inventory.env_vars="" +// meta:inventory.config_files="" +// meta:inventory.systemd_units="" +// meta:inventory.network="" +// meta:inventory.privileges="none" +// ============================================================================= +package update + +import ( + "strings" + "testing" + + "github.com/itcmsgr/nftban/internal/installer/executor" +) + +// DetectInstallOrigin ──────────────────────────────────────────────────────── + +func TestDetectInstallOrigin_RPM(t *testing.T) { + mock := executor.NewMockExecutor() + mock.RunResults["rpm:-q:nftban-core"] = executor.Result{ + ExitCode: 0, + Stdout: "nftban-core-1.98.2-1.el9.x86_64\n", + } + + got := DetectInstallOrigin(mock, newTestLogger()) + if got != "rpm" { + t.Errorf("DetectInstallOrigin = %q; want rpm", got) + } +} + +func TestDetectInstallOrigin_DEB(t *testing.T) { + mock := executor.NewMockExecutor() + // rpm not present — simulated as exit 127. + mock.RunResults["rpm:-q:nftban-core"] = executor.Result{ExitCode: 127} + mock.RunResults["dpkg:-s:nftban-core"] = executor.Result{ + ExitCode: 0, + Stdout: "Package: nftban-core\nVersion: 1.98.2\n", + } + + got := DetectInstallOrigin(mock, newTestLogger()) + if got != "deb" { + t.Errorf("DetectInstallOrigin = %q; want deb", got) + } +} + +func TestDetectInstallOrigin_Source(t *testing.T) { + mock := executor.NewMockExecutor() + mock.RunResults["rpm:-q:nftban-core"] = executor.Result{ExitCode: 127} + mock.RunResults["dpkg:-s:nftban-core"] = executor.Result{ExitCode: 127} + mock.Env["NFTBAN_SOURCE_DIR"] = "/opt/nftban-src" + + got := DetectInstallOrigin(mock, newTestLogger()) + if got != "source" { + t.Errorf("DetectInstallOrigin = %q; want source", got) + } +} + +func TestDetectInstallOrigin_Unknown(t *testing.T) { + mock := executor.NewMockExecutor() + // No package manager ownership, no source env. + mock.RunResults["rpm:-q:nftban-core"] = executor.Result{ExitCode: 127} + mock.RunResults["dpkg:-s:nftban-core"] = executor.Result{ExitCode: 127} + + got := DetectInstallOrigin(mock, newTestLogger()) + if got != "" { + t.Errorf("DetectInstallOrigin = %q; want empty", got) + } +} + +// DetectPackageTarget ─────────────────────────────────────────────────────── + +func TestDetectPackageTarget_RPM(t *testing.T) { + mock := executor.NewMockExecutor() + mock.RunResults["rpm:-q:--queryformat:%{VERSION}:nftban-core"] = executor.Result{ + ExitCode: 0, + Stdout: "1.98.2", + } + mock.RunResults["rpm:-q:--queryformat:%{VERSION}:nftban"] = executor.Result{ExitCode: 1} + + got, err := DetectPackageTarget(mock, "rpm", newTestLogger()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "1.98.2" { + t.Errorf("DetectPackageTarget = %q; want 1.98.2", got) + } +} + +func TestDetectPackageTarget_DEB(t *testing.T) { + mock := executor.NewMockExecutor() + mock.RunResults["dpkg:-s:nftban-core"] = executor.Result{ + ExitCode: 0, + Stdout: "Package: nftban-core\nStatus: install ok installed\nVersion: 1.98.2\n", + } + + got, err := DetectPackageTarget(mock, "deb", newTestLogger()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "1.98.2" { + t.Errorf("DetectPackageTarget = %q; want 1.98.2", got) + } +} + +func TestDetectPackageTarget_NotOwned_RPM(t *testing.T) { + mock := executor.NewMockExecutor() + mock.RunResults["rpm:-q:--queryformat:%{VERSION}:nftban-core"] = executor.Result{ + ExitCode: 1, + Stdout: "package nftban-core is not installed", + } + + got, _ := DetectPackageTarget(mock, "rpm", newTestLogger()) + if got != "" { + t.Errorf("not-installed rpm should return empty, got %q", got) + } +} + +func TestDetectPackageTarget_UnknownOrigin(t *testing.T) { + mock := executor.NewMockExecutor() + + got, err := DetectPackageTarget(mock, "", newTestLogger()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "" { + t.Errorf("unknown origin should return empty, got %q", got) + } +} + +// DetectVersions — PR-17 package detection integration ────────────────────── + +func TestDetectVersions_PackageDebFallback(t *testing.T) { + mock := executor.NewMockExecutor() + mock.Files["/usr/lib/nftban/VERSION"] = []byte("1.98.2\n") + // No source dir; deb origin; package manager reports target 1.99.0. + mock.RunResults["dpkg:-s:nftban-core"] = executor.Result{ + ExitCode: 0, + Stdout: "Package: nftban-core\nVersion: 1.99.0\n", + } + + current, target, err := DetectVersions(mock, "", "deb", newTestLogger()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if current != "1.98.2" { + t.Errorf("current = %q; want 1.98.2", current) + } + if target != "1.99.0" { + t.Errorf("target = %q; want 1.99.0 (from dpkg -s)", target) + } +} + +func TestDetectVersions_SourceOverridesPackage(t *testing.T) { + // Source dir takes precedence even when package origin is declared — + // the operator's explicit --source-dir wins. Prevents surprise apply + // against staged package when operator clearly wanted source tree. + mock := executor.NewMockExecutor() + mock.Files["/usr/lib/nftban/VERSION"] = []byte("1.98.2\n") + mock.Files["/tmp/src/VERSION"] = []byte("1.99.0\n") + mock.RunResults["rpm:-q:--queryformat:%{VERSION}:nftban-core"] = executor.Result{ + ExitCode: 0, + Stdout: "1.98.5", // different from source tree + } + + _, target, err := DetectVersions(mock, "/tmp/src", "rpm", newTestLogger()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if target != "1.99.0" { + t.Errorf("source-tree should win over package: target = %q; want 1.99.0", target) + } +} + +// Preflight — P-6 rebuild_recovery_available ──────────────────────────────── + +func TestPreflight_RecoveryUnavailable_IsWarning(t *testing.T) { + mock := executor.NewMockExecutor() + seedHappyPreflight(mock) + // Break recovery: no ip nftban table. + delete(mock.NftTables, "ip:nftban") + + res := Preflight(mock, newTestLogger(), "source") + + // P-1 authority_nftban is critical + fails. That's expected. + // P-6 rebuild_recovery_available is warning + fails. Also expected. + var p6Failed bool + for _, c := range res.Checks { + if c.Name == "rebuild_recovery_available" && !c.Passed { + p6Failed = true + if c.Severity != "warning" { + t.Errorf("P-6 severity = %q; want warning", c.Severity) + } + } + } + if !p6Failed { + t.Error("P-6 rebuild_recovery_available should report a failed warning check") + } +} + +// Preflight — P-7 install_origin_coherent ─────────────────────────────────── + +func TestPreflight_OriginCoherent_Match(t *testing.T) { + mock := executor.NewMockExecutor() + seedHappyPreflight(mock) + mock.RunResults["dpkg:-s:nftban-core"] = executor.Result{ + ExitCode: 0, + Stdout: "Version: 1.98.2\n", + } + mock.RunResults["rpm:-q:nftban-core"] = executor.Result{ExitCode: 127} + + res := Preflight(mock, newTestLogger(), "deb") + + for _, c := range res.Checks { + if c.Name == "install_origin_coherent" && !c.Passed { + t.Errorf("P-7 should pass when declared origin matches detected: %s", c.Detail) + } + } +} + +func TestPreflight_OriginCoherent_Mismatch_IsWarning(t *testing.T) { + mock := executor.NewMockExecutor() + seedHappyPreflight(mock) + // Host is deb but operator declared rpm. + mock.RunResults["rpm:-q:nftban-core"] = executor.Result{ExitCode: 127} + mock.RunResults["dpkg:-s:nftban-core"] = executor.Result{ + ExitCode: 0, + Stdout: "Version: 1.98.2\n", + } + + res := Preflight(mock, newTestLogger(), "rpm") + + var p7Failed bool + for _, c := range res.Checks { + if c.Name == "install_origin_coherent" && !c.Passed { + p7Failed = true + if c.Severity != "warning" { + t.Errorf("P-7 severity = %q; want warning", c.Severity) + } + if !strings.Contains(c.Detail, "rpm") || !strings.Contains(c.Detail, "deb") { + t.Errorf("P-7 detail should mention both declared and detected: %s", c.Detail) + } + } + } + if !p7Failed { + t.Error("P-7 install_origin_coherent should report a failed warning on mismatch") + } +} + +// BuildRecoveryPlan ───────────────────────────────────────────────────────── + +func TestBuildRecoveryPlan_HappyPath(t *testing.T) { + mock := executor.NewMockExecutor() + seedHappyPreflight(mock) + + r := BuildRecoveryPlan(mock) + if !r.Available { + t.Error("recovery should be Available when prerequisites met") + } + if r.Mechanism != "rebuild" { + t.Errorf("mechanism = %q; want rebuild", r.Mechanism) + } +} + +func TestBuildRecoveryPlan_NoState_NotesFreshRecovery(t *testing.T) { + mock := executor.NewMockExecutor() + seedHappyPreflight(mock) + delete(mock.Files, "/var/lib/nftban/state/install_state") + + r := BuildRecoveryPlan(mock) + if !r.Available { + t.Error("recovery with no prior state should be Available (fresh)") + } + var foundFreshNote bool + for _, n := range r.Notes { + if strings.Contains(n, "no prior install_state") { + foundFreshNote = true + } + } + if !foundFreshNote { + t.Error("notes should mention fresh-recovery case") + } +} + +func TestBuildRecoveryPlan_InProgressState_Unavailable(t *testing.T) { + mock := executor.NewMockExecutor() + seedHappyPreflight(mock) + mock.Files["/var/lib/nftban/state/install_state"] = []byte("PREPARE_COMPLETE\n") + + r := BuildRecoveryPlan(mock) + if r.Available { + t.Error("recovery should be Unavailable when prior state is non-terminal") + } +} diff --git a/internal/installer/update/update_test.go b/internal/installer/update/update_test.go index f6b0a584f..3f76ef874 100644 --- a/internal/installer/update/update_test.go +++ b/internal/installer/update/update_test.go @@ -46,7 +46,7 @@ func TestPreflight_AllPass(t *testing.T) { mock := executor.NewMockExecutor() seedHappyPreflight(mock) - res := Preflight(mock, newTestLogger()) + res := Preflight(mock, newTestLogger(), "") if !res.Passed { t.Errorf("expected preflight to pass, got failing checks:") for _, c := range res.Checks { @@ -55,9 +55,9 @@ func TestPreflight_AllPass(t *testing.T) { } } } - // All 5 checks should be present regardless of outcome. - if len(res.Checks) != 5 { - t.Errorf("expected 5 preflight checks, got %d", len(res.Checks)) + // All 7 checks (PR-16 P-1..P-5 + PR-17 P-6..P-7) should be present. + if len(res.Checks) != 7 { + t.Errorf("expected 7 preflight checks, got %d", len(res.Checks)) } } @@ -69,7 +69,7 @@ func TestPreflight_NoAuthority_Fails(t *testing.T) { seedHappyPreflight(mock) delete(mock.NftTables, "ip:nftban") - res := Preflight(mock, newTestLogger()) + res := Preflight(mock, newTestLogger(), "") if res.Passed { t.Error("preflight should fail when nftban does not own authority") } @@ -83,7 +83,7 @@ func TestPreflight_DaemonDown_Fails(t *testing.T) { seedHappyPreflight(mock) mock.Services["nftband.service"] = false - res := Preflight(mock, newTestLogger()) + res := Preflight(mock, newTestLogger(), "") if res.Passed { t.Error("preflight should fail when nftband is down") } @@ -97,7 +97,7 @@ func TestPreflight_MissingVERSION_Fails(t *testing.T) { seedHappyPreflight(mock) delete(mock.Files, "/usr/lib/nftban/VERSION") - res := Preflight(mock, newTestLogger()) + res := Preflight(mock, newTestLogger(), "") if res.Passed { t.Error("preflight should fail when VERSION is missing") } @@ -111,7 +111,7 @@ func TestPreflight_MissingNft_Fails(t *testing.T) { seedHappyPreflight(mock) mock.RunResults["sh:-c:command -v nft >/dev/null 2>&1"] = executor.Result{ExitCode: 127} - res := Preflight(mock, newTestLogger()) + res := Preflight(mock, newTestLogger(), "") if res.Passed { t.Error("preflight should fail when nft is missing") } @@ -127,7 +127,7 @@ func TestPreflight_StaleInProgress_IsWarning(t *testing.T) { seedHappyPreflight(mock) mock.Files["/var/lib/nftban/state/install_state"] = []byte("PREPARE_COMPLETE\n") - res := Preflight(mock, newTestLogger()) + res := Preflight(mock, newTestLogger(), "") if !res.Passed { t.Error("preflight should still pass when only a warning-severity check fails") } @@ -152,7 +152,7 @@ func TestDetectVersions_HappyPath(t *testing.T) { mock.Files["/usr/lib/nftban/VERSION"] = []byte("1.98.2\n") mock.Files["/tmp/srcdir/VERSION"] = []byte("1.99.0\n") - current, target, err := DetectVersions(mock, "/tmp/srcdir", newTestLogger()) + current, target, err := DetectVersions(mock, "/tmp/srcdir", "", newTestLogger()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -168,7 +168,7 @@ func TestDetectVersions_MissingCurrent_Errors(t *testing.T) { mock := executor.NewMockExecutor() // Deliberately no /usr/lib/nftban/VERSION seeded. - _, _, err := DetectVersions(mock, "", newTestLogger()) + _, _, err := DetectVersions(mock, "", "", newTestLogger()) if err == nil { t.Error("expected error when VERSION file is missing") } @@ -178,7 +178,7 @@ func TestDetectVersions_MissingTarget_IsNonFatal(t *testing.T) { mock := executor.NewMockExecutor() mock.Files["/usr/lib/nftban/VERSION"] = []byte("1.98.2\n") - current, target, err := DetectVersions(mock, "", newTestLogger()) + current, target, err := DetectVersions(mock, "", "", newTestLogger()) if err != nil { t.Fatalf("missing target should be non-fatal for PR-16, got: %v", err) }