diff --git a/README.md b/README.md index 98ff8ec..3762885 100644 --- a/README.md +++ b/README.md @@ -75,6 +75,33 @@ Thank you so much! ## Recognitions -## Repo Activity +## Release Testing & Feedback +A special thanks to the community members who help by testing releases and reporting issues. 💙 + + + + + + +
+ + @NukeThemTillTheyGlow + +
+ @NukeThemTillTheyGlow +
+ release testing + 		+ + @marc6901 + +
+ @marc6901 +
+ release testing +
+ +
+## Repo Activity ![Alt](https://repobeats.axiom.co/api/embed/d9565d6d1ed8222a5da5fedf25c18a9c8beab382.svg "Repobeats analytics image") \ No newline at end of file diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 57ad64f..33deac6 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -907,7 +907,7 @@ BACKUP_PVE_FIREWALL=true # PVE firewall configuration BACKUP_VZDUMP_CONFIG=true # /etc/vzdump.conf # Access control lists -BACKUP_PVE_ACL=true # User permissions +BACKUP_PVE_ACL=true # Access control (users/roles/groups/ACL; realms when configured) # Scheduled jobs BACKUP_PVE_JOBS=true # Backup jobs configuration diff --git a/docs/RESTORE_GUIDE.md b/docs/RESTORE_GUIDE.md index 7e0bcd9..75d0c85 100644 --- a/docs/RESTORE_GUIDE.md +++ b/docs/RESTORE_GUIDE.md @@ -86,7 +86,7 @@ Restore operations are organized into **20–22 categories** (PBS = 20, PVE = 22 Each category is handled in one of three ways: - **Normal**: extracted directly to `/` (system paths) after safety backup -- **Staged**: extracted to `/tmp/proxsave/restore-stage-*` and then applied in a controlled way (file copy/validation or `pvesh`) +- **Staged**: extracted to `/tmp/proxsave/restore-stage-*` and then applied in a controlled way (file copy/validation or `pvesh`); when staged files are written to system paths, ProxSave applies them **atomically** and enforces the final permissions/ownership (i.e. not left to `umask`) - **Export-only**: extracted to an export directory for manual review (never written to system paths) ### PVE-Specific Categories (11 categories) @@ -98,7 +98,7 @@ Each category is handled in one of three ways: | `storage_pve` | PVE Storage Configuration | **Staged** storage definitions (applied via API) + VZDump config | `./etc/pve/storage.cfg`
`./etc/pve/datacenter.cfg`
`./etc/vzdump.conf` | | `pve_jobs` | PVE Backup Jobs | **Staged** scheduled backup jobs (applied via API) | `./etc/pve/jobs.cfg`
`./etc/pve/vzdump.cron` | | `pve_notifications` | PVE Notifications | **Staged** notification targets and matchers (applied via API) | `./etc/pve/notifications.cfg`
`./etc/pve/priv/notifications.cfg` | -| `pve_access_control` | PVE Access Control | **Staged** access control + secrets restored 1:1 via pmxcfs file apply (root@pam safety rail) | `./etc/pve/user.cfg`
`./etc/pve/domains.cfg`
`./etc/pve/priv/shadow.cfg`
`./etc/pve/priv/token.cfg`
`./etc/pve/priv/tfa.cfg` | +| `pve_access_control` | PVE Access Control | **Staged** access control + secrets restored 1:1 via pmxcfs file apply (root@pam safety rail) | `./etc/pve/user.cfg`
`./etc/pve/domains.cfg` (when present)
`./etc/pve/priv/shadow.cfg`
`./etc/pve/priv/token.cfg`
`./etc/pve/priv/tfa.cfg` | | `pve_firewall` | PVE Firewall | **Staged** firewall rules and node host firewall (pmxcfs file apply + rollback timer) | `./etc/pve/firewall/`
`./etc/pve/nodes/*/host.fw` | | `pve_ha` | PVE High Availability (HA) | **Staged** HA resources/groups/rules (pmxcfs file apply + rollback timer) | `./etc/pve/ha/resources.cfg`
`./etc/pve/ha/groups.cfg`
`./etc/pve/ha/rules.cfg` | | `pve_sdn` | PVE SDN | **Staged** SDN definitions (pmxcfs file apply; definitions only) | `./etc/pve/sdn/`
`./etc/pve/sdn.cfg` | @@ -145,8 +145,9 @@ Not all categories are available in every backup. The restore workflow: ### PVE Access Control 1:1 (pve_access_control) On standalone PVE restores (non-cluster backups), ProxSave restores `pve_access_control` **1:1** by applying staged files directly to the pmxcfs mount (`/etc/pve`): -- `/etc/pve/user.cfg` + `/etc/pve/domains.cfg` -- `/etc/pve/priv/{shadow,token,tfa}.cfg` +- `/etc/pve/user.cfg` (includes users/roles/groups/ACL) +- `/etc/pve/domains.cfg` (realms, when present) +- `/etc/pve/priv/{shadow,token,tfa}.cfg` (when present) **Root safety rail**: - `root@pam` is preserved from the fresh install (not imported from the backup). @@ -2668,11 +2669,10 @@ tar -xzf /path/to/decrypted.tar.gz ./specific/file/path **Q: Does restore preserve file permissions and ownership?** -A: Yes, completely: -- **Ownership**: UID/GID preserved -- **Permissions**: Mode bits preserved -- **Timestamps**: mtime and atime preserved -- **ctime**: Cannot be set (kernel-managed) +A: Yes: +- **Extraction**: ProxSave preserves UID/GID, mode bits and timestamps (mtime/atime) for extracted entries. +- **Staged categories**: files are extracted under `/tmp/proxsave/restore-stage-*` and then applied to system paths using atomic replace; ProxSave explicitly applies mode bits (not left to `umask`) and preserves/derives ownership/group to match expected system defaults (important on PBS, where `proxmox-backup-proxy` runs as `backup`). +- **ctime**: Cannot be set (kernel-managed). --- diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 91cdbdf..90a15d4 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -623,6 +623,32 @@ MIN_DISK_SPACE_PRIMARY_GB=5 # Lower threshold - If it says **DISARMED/CLEARED**, reconnect using the **post-apply IP** (new config remains active). - Check the rollback log path printed in the footer for details. +#### PBS UI/API fails after restore: `proxmox-backup-proxy` permission denied + +**Symptoms**: +- PBS web UI login fails or API requests return authentication errors +- `systemctl status proxmox-backup-proxy` shows a restart loop +- Journal contains errors like: + - `unable to read "/etc/proxmox-backup/user.cfg" - Permission denied (os error 13)` + - `unable to read "/etc/proxmox-backup/authkey.pub" - Permission denied (os error 13)` + - `configuration directory '/etc/proxmox-backup' permission problem` + +**Cause**: +- PBS services (notably `proxmox-backup-proxy`) run as user `backup` and require specific ownership/permissions under `/etc/proxmox-backup`. +- If staged restore (or manual file copy) rewrites these files with the wrong owner/group/mode, the services cannot read them and may refuse to start. + +**What to do**: +1. Ensure you're running the latest ProxSave build and rerun restore for the staged PBS categories you selected (e.g. `pbs_access_control`, `pbs_jobs`, `pbs_remotes`, `pbs_host`, `pbs_notifications`, `datastore_pbs`). ProxSave applies staged files atomically and enforces final permissions/ownership (not left to `umask`). +2. If PBS is already broken and you need a quick recovery: + - Identify the blocking path component with `namei -l /etc/proxmox-backup/user.cfg`. + - Restore package defaults (recommended): reinstall `proxmox-backup-server` and restart services. Example: + ```bash + apt-get update + apt-get install --reinstall proxmox-backup-server + systemctl restart proxmox-backup proxmox-backup-proxy + ``` + - Or fix ownership/permissions to match a clean install of your PBS version (verify `/etc/proxmox-backup` and the files referenced in the journal are readable by user `backup`). + #### Error during network preflight: `addr_add_dry_run() got an unexpected keyword argument 'nodad'` **Symptoms**: diff --git a/internal/backup/collector_pve.go b/internal/backup/collector_pve.go index ae0109b..756356a 100644 --- a/internal/backup/collector_pve.go +++ b/internal/backup/collector_pve.go @@ -164,6 +164,9 @@ func (c *Collector) populatePVEManifest() { disableHint string log bool countNotFound bool + // suppressNotFoundLog suppresses any log output when a file is not found. + // This is useful for optional files that may not exist on a default installation. + suppressNotFoundLog bool } logEntry := func(opts manifestLogOpts, entry ManifestEntry) { @@ -186,6 +189,9 @@ func (c *Collector) populatePVEManifest() { case StatusSkipped: c.logger.Info(" %s: skipped (excluded)", opts.description) case StatusNotFound: + if opts.suppressNotFoundLog { + return + } if strings.TrimSpace(opts.disableHint) != "" { c.logger.Warning(" %s: not configured. If unused, set %s=false to disable.", opts.description, opts.disableHint) } else { @@ -268,10 +274,13 @@ func (c *Collector) populatePVEManifest() { countNotFound: true, }) record(filepath.Join(pveConfigPath, "domains.cfg"), c.config.BackupPVEACL, manifestLogOpts{ - description: "Domain configuration", - disableHint: "BACKUP_PVE_ACL", - log: true, - countNotFound: true, + description: "Domain configuration", + disableHint: "BACKUP_PVE_ACL", + log: true, + // domains.cfg may not exist on a default standalone PVE install (built-in realms only). + // Don't warn or count as "missing" in that case. + countNotFound: false, + suppressNotFoundLog: true, }) // Scheduled jobs. diff --git a/internal/config/templates/backup.env b/internal/config/templates/backup.env index f579cff..7a7cd71 100644 --- a/internal/config/templates/backup.env +++ b/internal/config/templates/backup.env @@ -276,7 +276,7 @@ METRICS_PATH=${BASE_DIR}/metrics BACKUP_CLUSTER_CONFIG=true BACKUP_PVE_FIREWALL=true BACKUP_VZDUMP_CONFIG=true -BACKUP_PVE_ACL=true +BACKUP_PVE_ACL=true # Access control (users/roles/groups/ACL; realms when configured) BACKUP_PVE_JOBS=true BACKUP_PVE_SCHEDULES=true BACKUP_PVE_REPLICATION=true diff --git a/internal/orchestrator/.backup.lock b/internal/orchestrator/.backup.lock deleted file mode 100644 index e38741a..0000000 --- a/internal/orchestrator/.backup.lock +++ /dev/null @@ -1,3 +0,0 @@ -pid=29780 -host=pve -time=2026-02-09T14:30:21+01:00 diff --git a/internal/orchestrator/fs_atomic.go b/internal/orchestrator/fs_atomic.go new file mode 100644 index 0000000..d74a5b4 --- /dev/null +++ b/internal/orchestrator/fs_atomic.go @@ -0,0 +1,176 @@ +package orchestrator + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "syscall" +) + +type uidGid struct { + uid int + gid int + ok bool +} + +func uidGidFromFileInfo(info os.FileInfo) uidGid { + if info == nil { + return uidGid{} + } + st, ok := info.Sys().(*syscall.Stat_t) + if !ok || st == nil { + return uidGid{} + } + return uidGid{uid: int(st.Uid), gid: int(st.Gid), ok: true} +} + +func modeBits(mode os.FileMode) os.FileMode { + return mode & 0o7777 +} + +func findNearestExistingDirMeta(dir string) (uidGid, os.FileMode) { + dir = filepath.Clean(strings.TrimSpace(dir)) + if dir == "" || dir == "." { + return uidGid{}, 0o755 + } + + candidate := dir + for { + info, err := restoreFS.Stat(candidate) + if err == nil && info != nil && info.IsDir() { + inheritMode := modeBits(info.Mode()) + if inheritMode == 0 { + inheritMode = 0o755 + } + return uidGidFromFileInfo(info), inheritMode + } + + parent := filepath.Dir(candidate) + if parent == candidate || parent == "." || parent == "" { + break + } + candidate = parent + } + + return uidGid{}, 0o755 +} + +func ensureDirExistsWithInheritedMeta(dir string) error { + dir = filepath.Clean(strings.TrimSpace(dir)) + if dir == "" || dir == "." || dir == string(os.PathSeparator) { + return nil + } + + if info, err := restoreFS.Stat(dir); err == nil { + if info != nil && info.IsDir() { + return nil + } + return fmt.Errorf("path exists but is not a directory: %s", dir) + } else if err != nil && !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("stat %s: %w", dir, err) + } + + owner, perm := findNearestExistingDirMeta(filepath.Dir(dir)) + if err := restoreFS.MkdirAll(dir, perm); err != nil { + return fmt.Errorf("mkdir %s: %w", dir, err) + } + + f, err := restoreFS.Open(dir) + if err != nil { + return fmt.Errorf("open dir %s: %w", dir, err) + } + defer f.Close() + + if os.Geteuid() == 0 && owner.ok { + if err := f.Chown(owner.uid, owner.gid); err != nil { + return fmt.Errorf("chown dir %s: %w", dir, err) + } + } + if err := f.Chmod(perm); err != nil { + return fmt.Errorf("chmod dir %s: %w", dir, err) + } + return nil +} + +func desiredOwnershipForAtomicWrite(destPath string) uidGid { + destPath = filepath.Clean(strings.TrimSpace(destPath)) + if destPath == "" || destPath == "." { + return uidGid{} + } + + if info, err := restoreFS.Stat(destPath); err == nil && info != nil && !info.IsDir() { + return uidGidFromFileInfo(info) + } + + parent := filepath.Dir(destPath) + if info, err := restoreFS.Stat(parent); err == nil && info != nil && info.IsDir() { + parentOwner := uidGidFromFileInfo(info) + if parentOwner.ok { + return uidGid{uid: 0, gid: parentOwner.gid, ok: true} + } + } + + return uidGid{} +} + +func writeFileAtomic(path string, data []byte, perm os.FileMode) error { + path = filepath.Clean(strings.TrimSpace(path)) + if path == "" || path == "." { + return fmt.Errorf("invalid path") + } + perm = modeBits(perm) + if perm == 0 { + perm = 0o644 + } + + dir := filepath.Dir(path) + if err := ensureDirExistsWithInheritedMeta(dir); err != nil { + return err + } + + owner := desiredOwnershipForAtomicWrite(path) + + tmpPath := fmt.Sprintf("%s.proxsave.tmp.%d", path, nowRestore().UnixNano()) + f, err := restoreFS.OpenFile(tmpPath, os.O_CREATE|os.O_WRONLY|os.O_EXCL|os.O_TRUNC, 0o600) + if err != nil { + return err + } + + writeErr := func() error { + if len(data) == 0 { + return nil + } + _, err := f.Write(data) + return err + }() + if writeErr == nil { + if os.Geteuid() == 0 && owner.ok { + if err := f.Chown(owner.uid, owner.gid); err != nil { + writeErr = err + } + } + if writeErr == nil { + if err := f.Chmod(perm); err != nil { + writeErr = err + } + } + } + + closeErr := f.Close() + if writeErr != nil { + _ = restoreFS.Remove(tmpPath) + return writeErr + } + if closeErr != nil { + _ = restoreFS.Remove(tmpPath) + return closeErr + } + + if err := restoreFS.Rename(tmpPath, path); err != nil { + _ = restoreFS.Remove(tmpPath) + return err + } + return nil +} diff --git a/internal/orchestrator/fs_atomic_test.go b/internal/orchestrator/fs_atomic_test.go new file mode 100644 index 0000000..0385501 --- /dev/null +++ b/internal/orchestrator/fs_atomic_test.go @@ -0,0 +1,129 @@ +package orchestrator + +import ( + "os" + "syscall" + "testing" + "time" +) + +func TestWriteFileAtomic_EnforcesModeDespiteUmask(t *testing.T) { + fakeFS := NewFakeFS() + origFS := restoreFS + origTime := restoreTime + t.Cleanup(func() { + restoreFS = origFS + restoreTime = origTime + _ = os.RemoveAll(fakeFS.Root) + }) + + restoreFS = fakeFS + restoreTime = &FakeTime{Current: time.Unix(10, 0)} + + oldUmask := syscall.Umask(0o077) + defer syscall.Umask(oldUmask) + + destPath := "/etc/proxmox-backup/user.cfg" + if err := writeFileAtomic(destPath, []byte("test\n"), 0o640); err != nil { + t.Fatalf("writeFileAtomic: %v", err) + } + + info, err := fakeFS.Stat(destPath) + if err != nil { + t.Fatalf("stat: %v", err) + } + if got := info.Mode().Perm(); got != 0o640 { + t.Fatalf("mode=%#o, want %#o", got, 0o640) + } +} + +func TestWriteFileAtomic_PreservesOwnershipFromExistingDest(t *testing.T) { + if os.Geteuid() != 0 { + t.Skip("requires root") + } + + fakeFS := NewFakeFS() + origFS := restoreFS + origTime := restoreTime + t.Cleanup(func() { + restoreFS = origFS + restoreTime = origTime + _ = os.RemoveAll(fakeFS.Root) + }) + + restoreFS = fakeFS + restoreTime = &FakeTime{Current: time.Unix(11, 0)} + + destPath := "/etc/proxmox-backup/user.cfg" + if err := fakeFS.WriteFile(destPath, []byte("old\n"), 0o640); err != nil { + t.Fatalf("seed dest: %v", err) + } + if err := os.Chown(fakeFS.onDisk(destPath), 123, 456); err != nil { + t.Fatalf("chown seed dest: %v", err) + } + if err := os.Chmod(fakeFS.onDisk(destPath), 0o640); err != nil { + t.Fatalf("chmod seed dest: %v", err) + } + + if err := writeFileAtomic(destPath, []byte("new\n"), 0o640); err != nil { + t.Fatalf("writeFileAtomic: %v", err) + } + + info, err := fakeFS.Stat(destPath) + if err != nil { + t.Fatalf("stat: %v", err) + } + owner := uidGidFromFileInfo(info) + if !owner.ok { + t.Fatalf("uid/gid not available from fileinfo") + } + if owner.uid != 123 || owner.gid != 456 { + t.Fatalf("uid/gid=%d:%d, want %d:%d", owner.uid, owner.gid, 123, 456) + } +} + +func TestWriteFileAtomic_InheritsGroupFromParentWhenDestMissing(t *testing.T) { + if os.Geteuid() != 0 { + t.Skip("requires root") + } + + fakeFS := NewFakeFS() + origFS := restoreFS + origTime := restoreTime + t.Cleanup(func() { + restoreFS = origFS + restoreTime = origTime + _ = os.RemoveAll(fakeFS.Root) + }) + + restoreFS = fakeFS + restoreTime = &FakeTime{Current: time.Unix(12, 0)} + + parentDir := "/etc/proxmox-backup" + if err := fakeFS.MkdirAll(parentDir, 0o700); err != nil { + t.Fatalf("seed parent dir: %v", err) + } + if err := os.Chown(fakeFS.onDisk(parentDir), 777, 888); err != nil { + t.Fatalf("chown seed parent dir: %v", err) + } + if err := os.Chmod(fakeFS.onDisk(parentDir), 0o700); err != nil { + t.Fatalf("chmod seed parent dir: %v", err) + } + + destPath := "/etc/proxmox-backup/remote.cfg" + if err := writeFileAtomic(destPath, []byte("remote: test\n"), 0o640); err != nil { + t.Fatalf("writeFileAtomic: %v", err) + } + + info, err := fakeFS.Stat(destPath) + if err != nil { + t.Fatalf("stat: %v", err) + } + owner := uidGidFromFileInfo(info) + if !owner.ok { + t.Fatalf("uid/gid not available from fileinfo") + } + if owner.uid != 0 || owner.gid != 888 { + t.Fatalf("uid/gid=%d:%d, want %d:%d", owner.uid, owner.gid, 0, 888) + } +} diff --git a/internal/orchestrator/network_staged_apply.go b/internal/orchestrator/network_staged_apply.go index c4bc2f7..3412d15 100644 --- a/internal/orchestrator/network_staged_apply.go +++ b/internal/orchestrator/network_staged_apply.go @@ -70,8 +70,8 @@ func copyDirOverlay(srcDir, destDir string) ([]string, error) { return nil, nil } - if err := restoreFS.MkdirAll(destDir, 0o755); err != nil { - return nil, fmt.Errorf("mkdir %s: %w", destDir, err) + if err := ensureDirExistsWithInheritedMeta(destDir); err != nil { + return nil, fmt.Errorf("ensure %s: %w", destDir, err) } var applied []string @@ -132,17 +132,12 @@ func copyFileOverlay(src, dest string) (bool, error) { return false, fmt.Errorf("read %s: %w", src, err) } - if err := restoreFS.MkdirAll(filepath.Dir(dest), 0o755); err != nil { - return false, fmt.Errorf("mkdir %s: %w", filepath.Dir(dest), err) - } - mode := os.FileMode(0o644) if info != nil { mode = info.Mode().Perm() } - if err := restoreFS.WriteFile(dest, data, mode); err != nil { + if err := writeFileAtomic(dest, data, mode); err != nil { return false, fmt.Errorf("write %s: %w", dest, err) } return true, nil } - diff --git a/internal/orchestrator/orchestrator_test.go b/internal/orchestrator/orchestrator_test.go index 68910c5..2b5be72 100644 --- a/internal/orchestrator/orchestrator_test.go +++ b/internal/orchestrator/orchestrator_test.go @@ -413,9 +413,24 @@ func TestOrchestrator_RunPreBackupChecks(t *testing.T) { orch := New(logger, false) - checkerCfg := &checks.CheckerConfig{} + backupDir := t.TempDir() + logDir := t.TempDir() + lockDir := filepath.Join(backupDir, "lock") + checkerCfg := &checks.CheckerConfig{ + BackupPath: backupDir, + LogPath: logDir, + LockDirPath: lockDir, + LockFilePath: filepath.Join(lockDir, ".backup.lock"), + MinDiskPrimaryGB: 0, + SafetyFactor: 1.0, + MaxLockAge: time.Hour, + } + if err := checkerCfg.Validate(); err != nil { + t.Fatalf("checker config validation failed: %v", err) + } checker := checks.NewChecker(logger, checkerCfg) orch.SetChecker(checker) + t.Cleanup(func() { _ = orch.ReleaseBackupLock() }) ctx := context.Background() err := orch.RunPreBackupChecks(ctx) @@ -432,9 +447,24 @@ func TestOrchestrator_RunPreBackupChecks_ContextCancellation(t *testing.T) { orch := New(logger, false) - checkerCfg := &checks.CheckerConfig{} + backupDir := t.TempDir() + logDir := t.TempDir() + lockDir := filepath.Join(backupDir, "lock") + checkerCfg := &checks.CheckerConfig{ + BackupPath: backupDir, + LogPath: logDir, + LockDirPath: lockDir, + LockFilePath: filepath.Join(lockDir, ".backup.lock"), + MinDiskPrimaryGB: 0, + SafetyFactor: 1.0, + MaxLockAge: time.Hour, + } + if err := checkerCfg.Validate(); err != nil { + t.Fatalf("checker config validation failed: %v", err) + } checker := checks.NewChecker(logger, checkerCfg) orch.SetChecker(checker) + t.Cleanup(func() { _ = orch.ReleaseBackupLock() }) ctx, cancel := context.WithCancel(context.Background()) cancel() // Cancel immediately diff --git a/internal/orchestrator/pbs_staged_apply.go b/internal/orchestrator/pbs_staged_apply.go index 57124f2..3111daa 100644 --- a/internal/orchestrator/pbs_staged_apply.go +++ b/internal/orchestrator/pbs_staged_apply.go @@ -205,10 +205,7 @@ func applyPBSDatastoreCfgFromStage(ctx context.Context, logger *logging.Logger, } destPath := "/etc/proxmox-backup/datastore.cfg" - if err := restoreFS.MkdirAll(filepath.Dir(destPath), 0o755); err != nil { - return fmt.Errorf("ensure %s: %w", filepath.Dir(destPath), err) - } - if err := restoreFS.WriteFile(destPath, []byte(out.String()), 0o640); err != nil { + if err := writeFileAtomic(destPath, []byte(out.String()), 0o640); err != nil { return fmt.Errorf("write %s: %w", destPath, err) } @@ -380,10 +377,7 @@ func applyPBSConfigFileFromStage(ctx context.Context, logger *logging.Logger, st return nil } - if err := restoreFS.MkdirAll(filepath.Dir(destPath), 0o755); err != nil { - return fmt.Errorf("ensure %s: %w", filepath.Dir(destPath), err) - } - if err := restoreFS.WriteFile(destPath, []byte(trimmed+"\n"), 0o640); err != nil { + if err := writeFileAtomic(destPath, []byte(trimmed+"\n"), 0o640); err != nil { return fmt.Errorf("write %s: %w", destPath, err) } diff --git a/internal/orchestrator/pve_staged_apply.go b/internal/orchestrator/pve_staged_apply.go index 3af6892..a85d816 100644 --- a/internal/orchestrator/pve_staged_apply.go +++ b/internal/orchestrator/pve_staged_apply.go @@ -102,10 +102,7 @@ func applyPVEVzdumpConfFromStage(logger *logging.Logger, stageRoot string) error return removeIfExists(destPath) } - if err := restoreFS.MkdirAll(filepath.Dir(destPath), 0o755); err != nil { - return fmt.Errorf("ensure %s: %w", filepath.Dir(destPath), err) - } - if err := restoreFS.WriteFile(destPath, []byte(trimmed+"\n"), 0o644); err != nil { + if err := writeFileAtomic(destPath, []byte(trimmed+"\n"), 0o644); err != nil { return fmt.Errorf("write %s: %w", destPath, err) } @@ -443,10 +440,10 @@ func pveStorageMountGuardCandidatesFromSections(sections []proxmoxNotificationSe } type pveStorageMountGuardItem struct { - GuardTarget string - StorageID string - StorageType string - IsNetwork bool + GuardTarget string + StorageID string + StorageType string + IsNetwork bool RequiresFstab bool } @@ -462,10 +459,10 @@ func pveStorageMountGuardItems(candidates []pveStorageMountGuardCandidate, mount switch storageType { case "nfs", "cifs", "cephfs", "glusterfs": out = append(out, pveStorageMountGuardItem{ - GuardTarget: filepath.Join("/mnt/pve", storageID), - StorageID: storageID, - StorageType: storageType, - IsNetwork: true, + GuardTarget: filepath.Join("/mnt/pve", storageID), + StorageID: storageID, + StorageType: storageType, + IsNetwork: true, RequiresFstab: false, }) @@ -490,10 +487,10 @@ func pveStorageMountGuardItems(candidates []pveStorageMountGuardCandidate, mount continue } out = append(out, pveStorageMountGuardItem{ - GuardTarget: target, - StorageID: storageID, - StorageType: storageType, - IsNetwork: false, + GuardTarget: target, + StorageID: storageID, + StorageType: storageType, + IsNetwork: false, RequiresFstab: true, }) } diff --git a/internal/orchestrator/restore_access_control.go b/internal/orchestrator/restore_access_control.go index 5556e21..efdda05 100644 --- a/internal/orchestrator/restore_access_control.go +++ b/internal/orchestrator/restore_access_control.go @@ -838,42 +838,6 @@ func readProxmoxConfigSectionsOptional(path string) ([]proxmoxNotificationSectio return parseProxmoxNotificationSections(raw) } -func writeFileAtomic(path string, data []byte, perm os.FileMode) error { - dir := filepath.Dir(path) - if err := restoreFS.MkdirAll(dir, 0o755); err != nil { - return err - } - - tmpPath := fmt.Sprintf("%s.proxsave.tmp.%d", path, nowRestore().UnixNano()) - f, err := restoreFS.OpenFile(tmpPath, os.O_CREATE|os.O_WRONLY|os.O_EXCL|os.O_TRUNC, perm) - if err != nil { - return err - } - - writeErr := func() error { - if len(data) == 0 { - return nil - } - _, err := f.Write(data) - return err - }() - closeErr := f.Close() - if writeErr != nil { - _ = restoreFS.Remove(tmpPath) - return writeErr - } - if closeErr != nil { - _ = restoreFS.Remove(tmpPath) - return closeErr - } - - if err := restoreFS.Rename(tmpPath, path); err != nil { - _ = restoreFS.Remove(tmpPath) - return err - } - return nil -} - func renderProxmoxConfig(sections []proxmoxNotificationSection) string { if len(sections) == 0 { return "" diff --git a/internal/orchestrator/restore_firewall.go b/internal/orchestrator/restore_firewall.go index 91b7f4d..9e655c1 100644 --- a/internal/orchestrator/restore_firewall.go +++ b/internal/orchestrator/restore_firewall.go @@ -613,8 +613,8 @@ func syncDirExact(srcDir, destDir string) ([]string, error) { return nil, nil } - if err := restoreFS.MkdirAll(destDir, 0o755); err != nil { - return nil, fmt.Errorf("mkdir %s: %w", destDir, err) + if err := ensureDirExistsWithInheritedMeta(destDir); err != nil { + return nil, fmt.Errorf("ensure %s: %w", destDir, err) } stageFiles := make(map[string]struct{}) @@ -661,8 +661,8 @@ func syncDirExact(srcDir, destDir string) ([]string, error) { if err != nil { return fmt.Errorf("readlink %s: %w", src, err) } - if err := restoreFS.MkdirAll(filepath.Dir(dest), 0o755); err != nil { - return fmt.Errorf("mkdir %s: %w", filepath.Dir(dest), err) + if err := ensureDirExistsWithInheritedMeta(filepath.Dir(dest)); err != nil { + return fmt.Errorf("ensure %s: %w", filepath.Dir(dest), err) } _ = restoreFS.Remove(dest) if err := restoreFS.Symlink(target, dest); err != nil { @@ -674,8 +674,8 @@ func syncDirExact(srcDir, destDir string) ([]string, error) { if info.IsDir() { stageDirs[rel] = struct{}{} - if err := restoreFS.MkdirAll(dest, 0o755); err != nil { - return fmt.Errorf("mkdir %s: %w", dest, err) + if err := ensureDirExistsWithInheritedMeta(dest); err != nil { + return fmt.Errorf("ensure %s: %w", dest, err) } if err := walkStage(src); err != nil { return err diff --git a/internal/orchestrator/restore_ha.go b/internal/orchestrator/restore_ha.go index a6062a2..5341941 100644 --- a/internal/orchestrator/restore_ha.go +++ b/internal/orchestrator/restore_ha.go @@ -296,8 +296,8 @@ func applyPVEHAFromStage(logger *logging.Logger, stageRoot string) (applied []st stageHA := filepath.Join(stageRoot, "etc", "pve", "ha") destHA := "/etc/pve/ha" - if err := restoreFS.MkdirAll(destHA, 0o755); err != nil { - return nil, fmt.Errorf("mkdir %s: %w", destHA, err) + if err := ensureDirExistsWithInheritedMeta(destHA); err != nil { + return nil, fmt.Errorf("ensure %s: %w", destHA, err) } // Only prune config files if the stage actually contains HA config. @@ -488,4 +488,3 @@ func buildHARollbackScript(markerPath, backupPath, logPath string) string { } return strings.Join(lines, "\n") + "\n" } - diff --git a/internal/orchestrator/restore_notifications.go b/internal/orchestrator/restore_notifications.go index bcc8d88..6181c09 100644 --- a/internal/orchestrator/restore_notifications.go +++ b/internal/orchestrator/restore_notifications.go @@ -326,10 +326,7 @@ func applyConfigFileFromStage(logger *logging.Logger, stageRoot, relPath, destPa return nil } - if err := restoreFS.MkdirAll(filepath.Dir(destPath), 0o755); err != nil { - return fmt.Errorf("ensure %s: %w", filepath.Dir(destPath), err) - } - if err := restoreFS.WriteFile(destPath, []byte(trimmed+"\n"), perm); err != nil { + if err := writeFileAtomic(destPath, []byte(trimmed+"\n"), perm); err != nil { return fmt.Errorf("write %s: %w", destPath, err) } logging.DebugStep(logger, "notifications staged apply file", "Applied %s -> %s", relPath, destPath)