From 51b58fef213d6587aeb068be5cbd502a6937da2f Mon Sep 17 00:00:00 2001 From: mrhid6 Date: Tue, 15 Sep 2026 07:47:29 +0000 Subject: [PATCH] feat: updates.Apply with security scope, window deadline, busy guard and captured output --- internal/sync/sync.go | 2 +- internal/updates/apply_test.go | 18 ++++ internal/updates/updates.go | 54 ++++++++++- internal/updates/updates_linux.go | 150 ++++++++++++++++++++++++++---- internal/updates/updates_other.go | 15 ++- 5 files changed, 214 insertions(+), 25 deletions(-) create mode 100644 internal/updates/apply_test.go diff --git a/internal/sync/sync.go b/internal/sync/sync.go index 1e49c56..e58b02a 100644 --- a/internal/sync/sync.go +++ b/internal/sync/sync.go @@ -482,7 +482,7 @@ func runInventory(ctx context.Context, cfg *config.Config) { func handleApplyUpdates(cfg *config.Config, cmd *pb.ServerCommand) { log.Printf("applying OS updates (cmd=%s)…", cmd.CommandId) - if err := updates.ApplyAll(); err != nil { + if _, err := updates.Apply(updates.ApplyOptions{}); err != nil { log.Printf("OS upgrade failed (cmd=%s): %v", cmd.CommandId, err) return } diff --git a/internal/updates/apply_test.go b/internal/updates/apply_test.go new file mode 100644 index 0000000..dd51ede --- /dev/null +++ b/internal/updates/apply_test.go @@ -0,0 +1,18 @@ +package updates + +import ( + "errors" + "testing" + "time" +) + +// Two package managers running at once corrupt each other's locks. The second +// caller must be told, not queued. +func TestApplyRefusesWhileBusy(t *testing.T) { + applyMu.Lock() + defer applyMu.Unlock() + _, err := Apply(ApplyOptions{Deadline: time.Now().Add(time.Minute)}) + if !errors.Is(err, ErrBusy) { + t.Fatalf("err = %v, want ErrBusy", err) + } +} diff --git a/internal/updates/updates.go b/internal/updates/updates.go index 293da52..99d3378 100644 --- a/internal/updates/updates.go +++ b/internal/updates/updates.go @@ -1,5 +1,11 @@ package updates +import ( + "errors" + "sync" + "time" +) + // PackageUpdate is one pending update. On Linux it is a package with a version // on each side. On Windows CurrentVersion is empty and NewVersion carries the // KB article ID: a Windows update is not a version bump of a named package, @@ -14,11 +20,49 @@ type PackageUpdate struct { // CheckAvailable lists pending OS updates. func CheckAvailable() ([]PackageUpdate, error) { return checkAvailable() } -// ApplyAll installs every pending update. It never reboots: a control plane -// silently restarting a production server is unrecoverable from the UI, so the -// reboot stays a decision a person or a workflow makes. RebootRequired reports -// when one is owed. -func ApplyAll() error { return applyAll() } +// ApplyOptions selects what an Apply run installs and when it must stop. +type ApplyOptions struct { + // SecurityOnly installs security fixes only. A host with no security + // metadata reports Unsupported and installs nothing: it never falls back + // to installing everything. + SecurityOnly bool + // Deadline is when the upgrade must be finished, normally the end of the + // maintenance window. Zero means defaultApplyCap from now. + Deadline time.Time +} + +// Result is what one Apply run did. Output is the tail of the package +// manager's combined output, for the operator to read when something failed. +type Result struct { + Output string + Unsupported bool + Reason string // why Unsupported, in words for the run page +} + +// ErrBusy means another Apply is already running on this host. +var ErrBusy = errors.New("an update run is already in progress on this host") + +const defaultApplyCap = 2 * time.Hour + +var applyMu sync.Mutex + +// Apply installs pending updates. It never reboots: ScheduleReboot is a +// separate decision taken by the caller, and only when the command asked. +func Apply(opts ApplyOptions) (Result, error) { + if !applyMu.TryLock() { + return Result{}, ErrBusy + } + defer applyMu.Unlock() + deadline := opts.Deadline + if deadline.IsZero() { + deadline = time.Now().Add(defaultApplyCap) + } + return apply(opts.SecurityOnly, deadline) +} + +// ScheduleReboot restarts the host after a short grace period, so a result +// sent just before it has time to leave. +func ScheduleReboot() error { return scheduleReboot() } // RebootRequired reports whether this host is waiting on a restart. func RebootRequired() bool { return rebootRequired() } diff --git a/internal/updates/updates_linux.go b/internal/updates/updates_linux.go index 4c16402..2491a37 100644 --- a/internal/updates/updates_linux.go +++ b/internal/updates/updates_linux.go @@ -4,8 +4,12 @@ import ( "bufio" "bytes" "context" + "errors" + "fmt" + "io" "os" "os/exec" + "path/filepath" "strings" "time" ) @@ -44,29 +48,141 @@ func checkAvailable() ([]PackageUpdate, error) { } -func applyAll() error { - switch detectPM() { - case "apt": +// aptRefreshTimeout bounds the index refresh alone. The upgrade itself runs +// until the caller's deadline: one shared five-minute limit used to kill large +// upgrades partway through. +const aptRefreshTimeout = 5 * time.Minute - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) - defer cancel() - if err := exec.CommandContext(ctx, "apt-get", "update", "-qq").Run(); err != nil { - return err +func run(ctx context.Context, out io.Writer, env []string, name string, args ...string) error { + fmt.Fprintf(out, "$ %s %s\n", name, strings.Join(args, " ")) + cmd := exec.CommandContext(ctx, name, args...) + cmd.Stdout, cmd.Stderr = out, out + cmd.Env = append(os.Environ(), env...) + return cmd.Run() +} + +func apply(securityOnly bool, deadline time.Time) (Result, error) { + out := newTailBuffer(outputTailMax) + ctx, cancel := context.WithDeadline(context.Background(), deadline) + defer cancel() + + var err error + switch pm := detectPM(); pm { + case "apt": + var res Result + res, err = applyApt(ctx, out, securityOnly) + if res.Unsupported { + res.Output = out.String() + return res, nil } - return exec.CommandContext(ctx, "apt-get", "upgrade", "-y").Run() - case "dnf": - return exec.Command("dnf", "upgrade", "-y").Run() - case "yum": - return exec.Command("yum", "upgrade", "-y").Run() - case "pacman": - return exec.Command("pacman", "-Syu", "--noconfirm").Run() + case "dnf", "yum": + args := []string{"upgrade", "-y"} + if securityOnly { + args = append(args, "--security") + } + err = run(ctx, out, nil, pm, args...) case "zypper": - return exec.Command("zypper", "update", "-y").Run() + if securityOnly { + err = run(ctx, out, nil, "zypper", "--non-interactive", "patch", "--category", "security") + } else { + err = run(ctx, out, nil, "zypper", "--non-interactive", "update") + } + // 102 and 103 mean "installed, and a reboot or restart is now needed". + // That is success; RebootRequired reports the rest. + var ee *exec.ExitError + if errors.As(err, &ee) && (ee.ExitCode() == 102 || ee.ExitCode() == 103) { + err = nil + } + case "pacman": + if securityOnly { + return Result{Unsupported: true, Reason: "pacman publishes no security metadata"}, nil + } + err = run(ctx, out, nil, "pacman", "-Syu", "--noconfirm") case "apk": - return exec.Command("apk", "upgrade").Run() + if securityOnly { + return Result{Unsupported: true, Reason: "apk publishes no security metadata"}, nil + } + if err = run(ctx, out, nil, "apk", "update"); err == nil { + err = run(ctx, out, nil, "apk", "upgrade") + } default: - return nil + return Result{Unsupported: true, Reason: "no supported package manager found"}, nil } + if ctx.Err() == context.DeadlineExceeded { + err = fmt.Errorf("stopped at the end of the maintenance window: %w", err) + } + return Result{Output: out.String()}, err +} + +func applyApt(ctx context.Context, out io.Writer, securityOnly bool) (Result, error) { + env := []string{"DEBIAN_FRONTEND=noninteractive"} + var srcOpts []string + if securityOnly { + dir, res, err := writeSecuritySourceParts() + if err != nil || res.Unsupported { + return res, err + } + defer os.RemoveAll(dir) + srcOpts = []string{ + "-o", "Dir::Etc::SourceList=/dev/null", + "-o", "Dir::Etc::SourceParts=" + dir, + // Without this, an update against the reduced source set deletes + // every other list file and the next normal apt call sees nothing. + "-o", "APT::Get::List-Cleanup=0", + } + } + rctx, rcancel := context.WithTimeout(ctx, aptRefreshTimeout) + defer rcancel() + if err := run(rctx, out, env, "apt-get", append([]string{"update", "-q"}, srcOpts...)...); err != nil { + return Result{}, fmt.Errorf("apt-get update: %w", err) + } + args := []string{"upgrade", "-y", "-q", + "-o", "Dpkg::Options::=--force-confdef", + "-o", "Dpkg::Options::=--force-confold"} + return Result{}, run(ctx, out, env, "apt-get", append(args, srcOpts...)...) +} + +// writeSecuritySourceParts writes the security-only sources to a temporary +// directory for Dir::Etc::SourceParts. The caller removes the directory. +func writeSecuritySourceParts() (string, Result, error) { + files := map[string]string{} + paths := []string{"/etc/apt/sources.list"} + for _, pat := range []string{"/etc/apt/sources.list.d/*.list", "/etc/apt/sources.list.d/*.sources"} { + m, _ := filepath.Glob(pat) + paths = append(paths, m...) + } + for _, p := range paths { + if b, err := os.ReadFile(p); err == nil { + files[p] = string(b) + } + } + list, d822, ok := securitySources(files) + if !ok { + return "", Result{Unsupported: true, Reason: "no security suites found in apt sources"}, nil + } + dir, err := os.MkdirTemp("", "vantage-apt-security-") + if err != nil { + return "", Result{}, err + } + if list != "" { + if err := os.WriteFile(filepath.Join(dir, "security.list"), []byte(list), 0o644); err != nil { + os.RemoveAll(dir) + return "", Result{}, err + } + } + if d822 != "" { + if err := os.WriteFile(filepath.Join(dir, "security.sources"), []byte(d822), 0o644); err != nil { + os.RemoveAll(dir) + return "", Result{}, err + } + } + return dir, Result{}, nil +} + +// scheduleReboot gives the host one minute, so the PatchResult announcing the +// reboot is on the wire before the network goes down. +func scheduleReboot() error { + return exec.Command("shutdown", "-r", "+1", "Vantage patch policy").Run() } // rebootRequired reads what the distributions themselves record. Debian and diff --git a/internal/updates/updates_other.go b/internal/updates/updates_other.go index f97615b..48ead03 100644 --- a/internal/updates/updates_other.go +++ b/internal/updates/updates_other.go @@ -4,6 +4,17 @@ // without it this file compiles on Linux too and collides with updates_linux.go. package updates +import ( + "errors" + "time" +) + func checkAvailable() ([]PackageUpdate, error) { return nil, nil } -func applyAll() error { return nil } -func rebootRequired() bool { return false } + +func apply(securityOnly bool, deadline time.Time) (Result, error) { + return Result{Unsupported: true, Reason: "OS updates are not supported on this platform"}, nil +} + +func scheduleReboot() error { return errors.New("reboot is not supported on this platform") } + +func rebootRequired() bool { return false }