Repository navigation
Conversation
extract_pr_marker matched "^<!-- release-tag: ... -->$" line by line, and "$" does not match before "\r". GitHub stores a PR body edited through the API or the web form with CRLF line endings, so once the v0.45.0 PR body was edited (gh pr edit, to add the release summary) the marker was no longer found: on the merge post-merge-release logged "No release marker found", synced dev and published nothing, and the v0.45.0 tag was created by hand on the squash commit. The body is now normalised to LF before the match, in the one helper that post-merge-release, release-guard, release-notes-guard and release-intake all use. The marker still has to be a line of its own. TestExtractReleaseMarkerAcceptsCRLFBodies runs the real helper and fails on the previous script; on the stored PR #343 body (21 CRs) the old script finds no marker and the new one returns v0.45.0. post-merge-release sources this script from the merged commit, so the fix takes effect with the next release merge.
PBS writes the data of a DNS plugin in acme/plugins.cfg as base64url
without padding: "CF_Token=ab?\n" is stored as "Q0ZfVG9rZW49YWI_Cg"
(measured on pve-test, PBS 4.2.8). pbsDecodeStoredBase64 tried only the
standard alphabet, so a value carrying '-' or '_' failed to decode and
was handed to PBS as it was staged:
- "acme plugin add" got the base64url text itself in its data file and
stored it as the plugin's credentials, and the block said "applied";
- "acme plugin set", which takes standard base64 only, refused it
("Invalid symbol 95, offset 15") and the plugin was not applied.
v0.44.0 copied plugins.cfg as it was and restored both cases.
pbsDecodeStoredBase64 now also decodes the URL-safe alphabet, and the
update of an object with a data file flag gets the decoded data encoded
again in standard base64 (updateData). Measured on pve-test and VM 201
(PBS 4.2.8) with a probe that runs applyPBSAcmePluginsCfgViaAPI against
the real proxmox-backup-manager: before, one plugin stored the
base64url text as its data and the other kept its old data, "not
applied"; after, both store "CF_Token=ab?\n" and the block is applied.
…--key=value argument Since v0.44.1 the restore decodes a remote's stored password and passes it plain after --password. proxmox-backup-manager reads a separate argument that begins with '-' as an option, so a password like -S3cret made both create and update fail with - 'password': missing parameter value. - 'S3cret': missing parameter value. The remote was not applied and the refusal, password included, reached the INFO rejection line of the run log and the DEBUG lines. Before v0.44.1 the base64 form was passed, which never begins with '-'. Any other value that begins with '-' (a comment, an S3 key, a job or notification setting) was already refused the same way. proxmoxManagerFlag now passes such a value as one "--key=value" argument, which PBS accepts, for every flag built from a config section (section configs, datastores, notification endpoints and matchers). redactCLIArgs redacts a secret in that form as "--flag=<redacted>". Measured on pve-test (PBS 4.2.5 running) and VM 201 (PBS 4.2.8) with a probe that runs applyPBSRemoteCfgViaAPI against the real proxmox-backup-manager: before, a remote with password -S3cret and comment -dash was not applied and "S3cret" appeared in 6 lines of the output; after, it is created, remote.cfg stores the password as LVMzY3JldA== (-S3cret) and the comment as -dash, and the secret appears in no line, including the refusal of a second remote PBS rejects for its fingerprint.
… the backup group v0.44.1 started writing tape-encryption-key-config.json from the backup, with mode 0600 like the plain keys file. PBS writes the two differently: the keys 0600 root:root, the key configuration 0640 root:backup (measured on pve-test and VM 201, PBS 4.2.8, right after creating a key). proxmox-backup-proxy runs as user backup and reads the key configuration to list the keys and to label an encrypted media set, so after a restore that file was unreadable for it while the block said "applied". The key configuration block now writes 0640. writeFileAtomic keeps the root:backup owner of the file, or takes the group of /etc/proxmox-backup for a new one. Measured on pve-test and VM 201 with a probe that runs the block on the host's own key configuration: before, the file was left 0600 root:backup, the backup user got "Permission denied" and GET /config/tape-encryption-keys answered HTTP 400; after, 0640 root:backup, the backup user reads it and the API answers HTTP 200.
…s backups
v0.44.1 retries a refused "datastore create" with --reuse-datastore
true, which PBS 4.2.8 needs to configure a datastore over the data it
finds at its path. The retry fired only on the EEXIST refusal, which
PBS gives for a chunk store without backups. A datastore with backups
has vm/, ct/, host/ or ns/ next to .chunks, and PBS refuses its create
earlier with "datastore path not empty" (measured on pve-test), so the
retry never ran:
- on a reinstalled PBS, restoring a datastore whose disk still holds its
backups ended "not applied";
- in Clean mode, the rollback of a move whose new path was refused left
the datastore not configured, its jobs and permissions already
removed with it.
In Clean mode v0.44.0 configured both through the datastore.cfg file
fallback, which the datastore block no longer reaches for a datastore
PBS refuses.
pbsChunkStoreExistsError now also matches "datastore path not empty".
Over a path that holds other files and no chunk store, PBS refuses the
reuse too ("unable to open existing chunk store path") and changes
nothing there (measured), and the create's refusal stays the one
reported.
Measured with a probe that runs applyPBSDatastoreCfgViaAPI against the
real proxmox-backup-manager, on a datastore with a host/ directory next
to its chunk store:
- Merge, datastore not configured (pve-test and VM 201): before, "not
applied" with "datastore path not empty"; after, applied over the
existing data.
- Clean, configured at /srv/pxsds, backup at a refused path (pve-test):
before, "not configured, the data in /srv/pxsds is kept"; after, the
rollback reuses /srv/pxsds and the datastore stays configured there.
…--key=value argument
v0.44.1 restores node.cfg with one "node update" carrying every setting.
pbsNodeUpdateFlags built each flag as two arguments, so a value that
begins with '-' was read by proxmox-backup-manager as an option and the
whole update was refused. PBS stores the node description (Notes) as
"#" lines, so Notes that open with a markdown bullet ("- ...") were
enough: in Merge mode none of the backup's node settings were applied,
in Clean mode they came from the file fallback with a warning.
The node flags now go through proxmoxManagerFlag, as the other PBS
flags do since 28523d0.
Measured on pve-test and VM 201 (PBS 4.2.8) with a probe that runs
applyPBSNodeCfgViaAPI against the real proxmox-backup-manager with
email-from and a description "- primary PBS\nsecond line": before, the
update was refused and node.cfg left unchanged; after, node.cfg holds
"#- primary PBS", "#second line" and the email-from.
…half-way v0.44.1 creates the 65536 hash directories of .chunks when it recreates a datastore through the datastore.cfg file fallback, then .lock. When that stopped half-way (disk or inodes exhausted, the run killed), the directories already made stayed and .lock was missing. The next restore saw entries in .chunks, warned about the missing .lock and said "Datastore X is already initialized, its directory is left as it is", so the missing directories were never created and every backup or garbage collection writing into one of them failed. PBS writes .lock when it creates a chunk store, and ProxSave writes it last: entries in .chunks without .lock are such a creation. The recreation now completes it, creating the missing hash directories and .lock and removing nothing, and says Datastore X initialization is incomplete, its missing directories are created in place of "already initialized" (wording approved by the maintainer). A chunk store with .lock is left as it is, as before. TestCreatePBSDatastoreStructureSkipsExistingData now gives its datastore the .lock PBS writes, so it keeps pinning that case. Measured on pve-test and VM 201 with a probe that runs RecreateDirectoriesFromConfig over a chunk store holding 32768 of its 65536 hash directories and no .lock: before, "already initialized" and still 32768 directories, no .lock; after, the new line, 65536 directories and .lock, all backup:backup.
…ackup of a datastore restore v0.44.1 has the datastore.cfg file fallback remove a datastore's jobs and permissions with it, as PBS does: it rewrites prune.cfg, verification.cfg, sync.cfg, tape-job.cfg and acl.cfg. The safety backup holds only the paths of the selected categories, and datastore_pbs holds none of those five, so with datastore_pbs selected without the jobs, tape and access control categories (a custom selection, or the STORAGE mode for tape-job.cfg and acl.cfg) the removed jobs and permissions were in no safety backup, while the restore says the safety backup holds the current versions of the files it overwrites. safetyBackupPaths adds the five files to the safety backup whenever datastore_pbs is selected. The category is unchanged, so nothing more is restored from the backup. The same files also cover what PBS's own "datastore remove" deletes on the API path. Measured on pve-test and VM 201 with a probe that builds the safety backup of a restore selecting datastore_pbs over the host: on VM 201, which has all six files, before it held datastore.cfg only, after it holds datastore.cfg, prune.cfg, verification.cfg, sync.cfg, tape-job.cfg and acl.cfg; on pve-test, which has acl.cfg among them, acl.cfg is added.
…s logs remaining v0.44.1 counts as a destination's logs only the run logs whose host this machine answers to. An archive adopted by server identity keeps the host name it was written under (pve.home.arpa while the host now answers only to pve), and deleteAssociatedLog deletes its log with it, so after rotating such archives the deleted logs were not among the counted ones and "logs remaining" came out too low: 0 with 2 logs of this server still on disk. Only the retention DEBUG summary and RetentionSummary.LogsRemaining carry the count. countOwnRunLogs now also counts a log whose host is the host of one of the archives the retention pass works on. The local and secondary pass hands it the archives it looked at, the cloud pass the same through deleteBatched, which takes that list in place of its length. Measured on pve-test and VM 201 with the repro test of the audit run as a probe on the host's filesystem (a server whose archives and logs are named pve.home.arpa, the host answering to pve, one log under each name left after rotation): before, LogsRemaining=0 with 2 logs on disk; after, LogsRemaining=2. The cloud pass is covered by the shared counter only, not run against a remote.
…d out v0.44.1 keeps a backup's sidecars when its archive cannot be removed, so the backup stays listed and the next pass retries it. A remove that timed out (FS_IO_TIMEOUT on a stalled mount) is not a refusal: safefs stops waiting but the unlink can still complete, and then the archive was gone with its .sha256, .manifest.json, .metadata and .metadata.sha256 left behind for good, since nothing lists or removes a sidecar whose archive is gone. v0.44.0 removed the sidecars in that case. The local and secondary retention now keep the sidecars only when a data remove was refused; after a timeout they are removed too, as in v0.44.0 (choice of the maintainer). The backup is still reported as not deleted and keeps its log; a log left without its archive goes with the orphan-log pass. The cloud delete is unchanged: its deadline covers the whole delete, so after a timeout the sidecar deletes would fail as they did in v0.44.0. Measured on pve-test and VM 201 with the repro test of the audit run as a probe on the host's filesystem (FS_IO_TIMEOUT=1, the expired archive's unlink completing 1.5 s late): before, the archive was gone and its four sidecars left; after, none left. Both print "Not deleted: ...: timed out after 1s".
…BS reopens it with When ProxSave recreates a datastore's chunk store (the datastore.cfg file fallback), it left the datastore directory 0750 and .lock 0600. PBS creates them 0755 and 0644 and reopens an existing chunk store only with exactly those modes, .chunks and its hash directories 0750, all backup:backup: "datastore create --reuse-datastore" over the ProxSave layout failed with "unable to open existing chunk store path ... - permissions or owner not correct" (measured on pve-test, PBS 4.2.8, changing one mode at a time on a chunk store PBS created). Garbage collection of the datastore configured through datastore.cfg ran, but a later restore through the PBS API, which reuses the chunk store, could not configure it. Same in v0.44.0. setPBSChunkStoreModes gives the datastore directory, .chunks and .lock those modes at the end of the recreation, over a new path and over the scaffold an earlier version left. The chmods go through an os.Root of the datastore directory, so a symlink put in place of .chunks or .lock (the directory belongs to the backup user) cannot lead them outside it. A failed chmod is reported with the existing datastore permission lines and does not stop the recreation. Measured on pve-test and VM 201 with a probe that runs RecreateDirectoriesFromConfig on a new path, then "datastore create --reuse-datastore true" and a garbage collection: before, 0750 and .lock 0600, the create refused; after, 0755 and .lock 0644, the create and the garbage collection TASK OK.
…es it applyPVEAccessControlFromStage read user.cfg, priv/token.cfg, priv/shadow.cfg and priv/tfa.cfg with the section-config parser, but only domains.cfg has that format. PVE writes user.cfg as colon lines (user:..., token:..., group:..., pool:..., role:..., acl:...), token.cfg as "<userid>!<tokenid> <secret>", shadow.cfg as "<name>:<hash>:" and tfa.cfg as JSON (measured with pveum on PVE 9.1.9). On a real host the apply: - wrote token.cfg and tfa.cfg empty and shadow.cfg unreadable, so every API token, @pve password and TFA was lost cluster-wide; - never found root@pam in user.cfg, kept the backup's root@pam line and appended a section-style "user: root@pam" block that PVE reads as root@pam disabled; - dropped this host's root@pam tokens. The tests passed because their fixtures used the section format for these files. The merge now reads each file in its own format (restore_access_control_pve.go): user.cfg is restored 1:1 except root@pam, whose user line, tokens and the ACL entries of those tokens are this host's (the ACL rule chosen by the maintainer), and root@pam keeps Administrator on "/"; it is written the way PVE writes it, users each followed by their tokens, then groups, pools, roles and ACLs, as PVE drops an ACL member defined after it. token.cfg keeps the backup's secrets and this host's root@pam ones, tfa.cfg the backup's entries and this host's root@pam entry, shadow.cfg (pve realm only, never root@pam) is restored as it is. Everything is merged before the first write. The section-format helpers only this path used are removed, and the tests use the measured formats. Measured on pve-test (PVE 9.1.9) and VM 201 (PVE 9.2.2): a backup with an @pve user (password, token, TFA, group, pool, role, ACL) and a root@pam token, restored over a host with a root@pam token of its own. Before: root@pam disabled, token.cfg and tfa.cfg empty, the @pve password refused, every token answering HTTP 401. After: root@pam enabled with Administrator on "/", the @pve user logs in with its password, keeps its TOTP and its token answers 200, the host's root token answers 200 with its ACL, the backup's root token is gone.
The tar stream reached the external compressor (xz, zstd, pigz, bzip2, lzma) through an io.Pipe set as cmd.Stdin. os/exec copies that into the child's stdin from a goroutine of its own, which stops on EPIPE when the compressor exits but never closes the io.Pipe, so the tar writer blocked in its next write forever and CreateArchive never returned: a compressor that hit a full disk or crashed, or one a cancel killed while the tar was still being written, hung the run. In cron mode, with no watchdog, the run lock then stayed held by a live process and every later run was skipped as already in progress. The tar is now written straight into the compressor's stdin pipe (cmd.StdinPipe), started after the compressor, so its next write fails with EPIPE and the writer returns. The error is the compressor's exit, or the cancellation when that is what stopped it. xz and zstd go through pipeTarThroughCommand like the other compressors; the drain that unblocked the io.Pipe after a failed start is no longer needed. Measured on pve-test and VM 201 with a probe that archives 32 MiB of random data with the real zstd, xz and bzip2: with the output on a full 2 MiB tmpfs, before, all three were still blocked after 30 s; after, they return in 14 ms to 2.2 s with the compressor's exit status. With a cancel after 1 s, before, xz and bzip2 were still blocked after 30 s; after, both return "context canceled" within 10 ms of the cancel.
… a symlink copySymlinkFile stores a symlink in the staging tree with its target as it is, absolute targets included (/opt/app -> /data/app). A later copy whose destination lay below it (CUSTOM_BACKUP_PATHS listing /opt/app and then /opt/app/config.yml, or a default tree holding such a link with a custom path beneath it) followed the link into the live host: the destination directory got the symlink's owner and mode 0777, and the destination file, the very file being read, was truncated before the copy, so the live file ended empty with its mtime restored and the run reported nothing. prepareCopyDestination and safeCopyDir now refuse a destination below a directory the staging tree holds as a symlink leading out of it: the link is resolved with EvalSymlinks, so a relative link pointing at a staged link with an absolute target is caught too; a link that resolves inside the staging tree is followed as before. The copy fails with the existing custom-path WARNING, the reason worded as approved by the maintainer: "it is under /opt/app, a symlink in the backup". Measured on pve-test and VM 201 with a probe that collects CUSTOM_BACKUP_PATHS <dir>/applink and <dir>/applink/config.yml, applink an absolute symlink to <dir>/data/app: before, the live directory went 0750 to 0777 and config.yml was emptied, with no warning; after, both are untouched and the WARNING names the file. A full backup with the dev binary on pve-test exits as the installed binary does, with the same warnings: the default collection hits no such link.
…ed file
uniqueCandidates gave the exclusion patterns, for a path inside the
staging tree, its physical path (/tmp/proxsave/<run>/etc/pve/x) and
that path relative to "/" as well as the host path it stands for. The
collector checks every copy destination and the archiver every staged
file, so a pattern aimed at the host's /tmp ('/tmp/**', '**/tmp/**',
'tmp/**') or '**/proxsave/**' matched everything staged: the run
collected nothing ("skipped (excluded)" on every item) and wrote an
empty archive that verified.
A path inside the staging tree is now judged by its host path and its
name only. Patterns name host paths, so nothing an operator can write
loses its meaning: TestPVECoreDBCaptureAndExclusions excluded the
config.db destination by its physical staging path, which changes on
every run; it now names the host path that destination stands for.
Measured on pve-test with full backups using BACKUP_EXCLUDE_PATTERNS
"/tmp/**": the installed binary reported every PVE configuration item
"skipped (excluded)", the dev binary collects them. On VM 201, which has
no ProxSave install, a probe collecting /etc/hostname into a staging
tree under /tmp/proxsave with that pattern: before, not staged; after,
staged.
"Verifying directory structure" checked BACKUP_PATH, LOG_PATH, SECONDARY_LOG_PATH and LOCK_PATH with a plain os.Stat and os.MkdirAll. On a dead NFS/CIFS hard mount that stat blocks in the kernel, and it runs before the backup lock and before the storage initialization, whose bounded checks would have disabled the secondary and kept the backup going: the run hung there. Every earlier touch of these paths (the security preflight, disableSecondarySharingLocalDirectories) is already bounded. The checks now go through safefs with the FS_IO_TIMEOUT bound. A directory that does not answer is reported in place of its check/cross line, wording approved by the maintainer: ✗ Secondary log directory not checked: /mnt/nas/log did not answer within 30s (dead/stale mount?) and nothing else is tried on it; the run continues. Measured on pve-test and VM 201 with pmxcfs stopped (SIGSTOP) and the check run on a directory under /etc/pve with a 3 s bound: before, still blocked after 20 s; after, the line above after 3 s.
The PBS notifications restore through the API passed some endpoint
settings as positional arguments: smtp and sendmail recipients, the
webhook url, the gotify server and token ("notification endpoint
<type> create <name> <value>..."). proxmox-backup-manager takes the
name as its only positional argument (PBS 4.2.8 usage, measured on
pve-test) and refused every such command, echoing the extra values;
the apply stopped at the first endpoint, so in Merge mode no endpoint
or matcher of the backup was restored.
Every setting now goes as a flag, as the verify/repair step already
did. The positional extraction also required a recipient for sendmail,
which PBS does not; it is replaced by the settings "create" requires
per type (smtp from-address and server, gotify server and token,
webhook method and url), with the existing "missing ...; skipping"
warning. The tests used a "recipients" key PBS does not write and
pinned the positional argv; they now use "mailto" and the flag form.
Measured on pve-test and VM 201 with a probe that applies a staged
notifications.cfg and notifications-priv.cfg (smtp with password,
gotify with token, webhook, sendmail, a matcher) in Merge mode: before,
"smtp create pxssmtp x@y.z ..." failed and nothing was created; after,
all four endpoints and the matcher exist, and neither secret appears in
the output.
The collector bounds its storage probes on PVE and in the PBS PXAR bricks so a dead mount cannot wedge the run, but three probes were not: "df -h" in the storage mounts brick, which statfs's every mount, and, in the PBS datastore inventory, a plain os.Stat of each datastore path, five more for its markers, and "findmnt -T" / "df -T" on it. On a dead NFS/CIFS hard mount (or any FUSE mount that stops answering) they block in the kernel, and the inventory runs before the PXAR bricks that would have skipped the datastore with a timeout warning. They now run under FS_IO_TIMEOUT: the stats through safefs, the commands with a context bounded by it. A datastore path whose stat times out is recorded as not reachable, with a DEBUG line. Measured on pve-test and VM 201 with pmxcfs stopped (SIGSTOP), so the /etc/pve FUSE mount stops answering, and a probe running the storage mounts brick with a 3 s bound: before, still blocked after 20 s; after, it returns after 3 s.
collectCustomPaths and collectUserHomes look at each entry with lstat, so a CUSTOM_BACKUP_PATHS entry or a /home entry that is a symlink to a directory (/opt/app -> /data/app, a relocated /home/alice) went through the file copy, which stores the link only: none of the content the operator named reached the archive, and the manifest said collected. The link is still stored, and the directory it points at is now collected too, at its own path, so a restore recreates both as they are on the host (the choice of the maintainer). A target that is not a directory, is gone, lies in the staging tree or contains it (a link to "/"), or whose link is excluded by the patterns is left out; homes keep the .ssh exclusion of BACKUP_SSH_KEYS=false. Measured on pve-test and VM 201 with a probe collecting CUSTOM_BACKUP_PATHS <dir>/applink, a symlink to <dir>/data/app: before, the staging tree held the link only; after, the link and data/app/config.yml.
The cron line ProxSave writes, on install and when it reverts from the daemon to cron, was always "<schedule> /usr/local/bin/proxsave --backup". The daemon unit it replaces runs with --config <path>, and no environment variable names the config, so with a config outside BASE_DIR/configs/backup.env the nightly cron run read the default file (usually absent) instead of the operator's. migrateLegacyCronEntries now takes the config path and, when it is not the default, the line ends with "--config <path>" (quoted for the shell when needed); with the default config the line is unchanged (the choice of the maintainer). The install passes the config it wrote, the revert the one the daemon was pinned to. Measured on pve-test and VM 201 with a probe that writes the real crontab for a config at /root/pxs-custom.env: before, the line had no --config; after, "... --backup --config /root/pxs-custom.env". The crontab was restored after each run.
0.46.0 so far brings fixes only, so its entry is the single generic line. The release-notes guard holds prereleases to their final version's entry, so pr-v0.46.0-beta1 needs it.
The install wizard writes backup.env before the steps that follow it
(AGE recipient, healthchecks, Telegram, scheduler). An install aborted
after the write ("Installation aborted", exit 0) left the new file in
place with the previous schedule still running: an ENCRYPT_ARCHIVE=true
with no recipient made every later run exit 2, a SCHEDULER_MODE=daemon
with no daemon installed made every run warn and exit 1.
runInstallMode now takes a snapshot of backup.env before the install
and, when the install is aborted, puts it back as it was, or removes it
when the install created it (the choice of the maintainer). The banner
is unchanged; the restore is logged at DEBUG.
Measured with a probe on the real configuration path, writing a new
backup.env and restoring it as an aborted install does: on pve-test the
existing backup.env came back byte-identical with its mode; on VM 201,
which had none, the file written by the install was removed. A full
interactive --install was not driven.
Both install wizards offered SECONDARY_LOG_PATH as optional ("optional
- press Enter to skip", "optional: leave empty to skip"), but with
SECONDARY_ENABLED=true and the path empty every run logged two
WARNINGs and exited 1, so the daemon pinged /1 and the Healthchecks
check went down every night over backups that succeeded.
The wizards now require it like SECONDARY_PATH (the choice of the
maintainer), with the texts approved by the maintainer: the CLI asks
"Secondary log path (SECONDARY_LOG_PATH): " and answers an empty value
with the existing "SECONDARY_LOG_PATH is required when
SECONDARY_ENABLED=true"; the TUI field reads "SECONDARY_LOG_PATH:
filesystem path for the logs of the redundant copies." Hosts already
configured with the path empty keep warning until the operator sets it.
The two install transcripts change on that prompt only; three prompt
tests that fed an empty log path now give one, one checks the refusal.
Measured on pve-test and VM 201 with a probe driving the CLI prompt with
an empty log path then a valid one: the refusal is printed and the
prompt asks again.
…r is confirmed In HEALTHCHECK_MODE=self the documentation says NOTIFY_ON filters the other channels only when the operator's own monitor receives: both checks configured and the daemon transmitting. Decide applied the filter whenever the section was initialized, and the section was initialized with only an alive check and with any fresh heartbeat, even one that failed (TxUnreachable: the monitor is down or the check was deleted) or never had a URL (TxNotProvisioned). The run that knew the monitor was down was filtered too, its "monitor unreachable" line coming after Email, Telegram, Gotify and Webhook had been silenced. A self-mode section with no backup check, an unreadable status, or a last heartbeat in those states is now SectionSelfUnconfirmed: NOTIFY_ON is not applied and every channel sends, Healthchecks stays enabled and still pings (the choice of the maintainer). Only DEBUG lines are added. Measured with a probe running the decision on pve-test and VM 201 with a self-mode config holding only an alive check: section self_unconfirmed, effective NOTIFY_ON always.
…t committed buildNetworkApplyNotCommittedError read the IP the rollback restores from <rollback work dir>/before.txt. Since v0.40.0 the UI flow arms the rollback with the work dir /tmp/proxsave and writes the before snapshot into the diagnostics directory, so nothing was there and the IP was always "unknown": the not-committed message lost "Expected after rollback" and "Reconnect using:", the rollback IP WARNING was dropped, and the footer never printed the pre-apply IP. A management interface on a VLAN was missed even with the right file, since iproute2 prints it as name@parent (vmbr0.10@vmbr0). The handle now carries the before snapshot the flow wrote and reads it first (the work dir's before.txt stays the fallback), and the snapshot parser matches the name before "@". Measured on pve-test and VM 201 with a probe that takes the real before snapshot and builds the not-committed error for vmbr0: before, the original IP was "unknown"; after, 10.1.3.30/26 and 192.168.178.103/24, as "ip -br addr" shows them.
… plan
planNICNameRepair marks a mapping a conflict when its old name exists
on this host and safe otherwise. In a chain {ens20->eno1, eno1->eth0}
the first link was safe and the second a conflict, so the repair
without conflicts (the staged install always, and the default answer of
the conflict prompt) renamed ens20 onto eno1 while eno1 was still the
other NIC's name: the interfaces file ended with two stanzas and two
bridge-ports/bond-slaves references under one name, reported as
applied, and a later "Apply conflicts" could no longer separate them.
A safe mapping whose new name is the old name of a conflict now moves
to the conflicts too (repeated for longer chains), so the chain is
applied whole, in one pass, or not at all. TestPlanAndApplyNICNameRepair
pinned the collapsed output of exactly this chain; it now expects both
links among the conflicts and the file untouched without them.
Not exercised on a host with renamed NICs (renaming the test hosts'
interfaces would cut them off); the package tests, this case included,
pass when run on pve-test and VM 201.
…tes it
readPVEPoolsFromExportUserCfg parsed /etc/pve/user.cfg with the
section-config parser, but PVE writes it as colon lines
(pool:<poolid>:<comment>:<vmids>:<storages>:, measured on PVE 9.1.9).
A real pool line became a pool whose ID was the whole line
("dev:Dev pool:100,101:local,backup_ext:"), which "pveum pool add"
refuses, so the SAFE cluster apply never restored a pool. The test
passed because its fixture used the section format.
The pools are now read with the user.cfg line parser of the access
control restore (restore_access_control_pve.go), and the test uses the
measured format.
Measured on pve-test and VM 201 with a probe that reads an exported
user.cfg holding "pool:pxspool:probe pool:::" and applies the pool
definitions with the real pveum: before, applied=0 failed=1; after,
applied=1 and pxspool exists. The pool was removed and user.cfg put
back after each run.
…issing The datastore block sent every datastore this PBS does not list to "datastore create <name> <path>" with no check of the path. When the disk or ZFS pool a datastore lives on was not mounted, PBS built an empty chunk store on the root filesystem, the block said "applied", and the later directory recreation called it "already initialized". The file fallback and the directory recreation already guard this case; the fstab mount guard does not cover GUI Directory datastores (systemd .mount units) or ZFS pools. create now checks the path with the same guard as the file fallback (on the root filesystem, at a location that looks like a mount, no datastore data there) and leaves the datastore not applied, with the event line approved by the maintainer: Datastore ds1 is at /mnt/datastore/ds1, on the root filesystem: its disk is not mounted The package tests, whose datastore paths sit on the build host's root filesystem, replace the guard; one test exercises it. Measured on pve-test and VM 201 with a probe restoring a datastore at /mnt/pxsnomount (nothing mounted there): before, "applied" and a chunk store on the root filesystem; after, the line above, "not applied", and nothing written.
…store update
The datastore block built the update flags from every setting of the
staged datastore except its path, so an existing S3 datastore (backend)
or removable datastore (backing-device) was updated with flags "datastore
update" does not take: PBS refused the whole update ("schema does not
allow additional properties", measured on pve-test and VM 201, PBS
4.2.8), none of its other settings (gc-schedule, keep-*, notify,
tuning, maintenance-mode) were restored, and the run ended with
warnings. The update that follows a refused create had the same flags.
The update now leaves those two settings out, in both flag forms; the
create still passes them.
Measured on pve-test and VM 201 against a scratch datastore: "datastore
update" with --backing-device or --backend is refused, the same update
without them succeeds and sets the comment. No S3 or removable
datastore was available to restore end to end.
The sync job block read the current jobs with "sync-job list", which returns pull jobs only (--sync-direction defaults to pull, PBS 4.2.8 usage). In Clean mode a push job this PBS has and the backup does not was never seen: not removed, not logged, and the block said applied while the job kept pushing this PBS's backups after a 1:1 restore. The sync job list now asks for --sync-direction all. The tests that pin the list command, and the sync characterization golden on that line only, follow. Measured on pve-test and VM 201 with a probe that restores, in Clean mode, the host's own sync.cfg after a push job was added: before, the push job survived silently; after, "Sync job pxspush is configured on this PBS but not present in the backup" and "PBS sync job pxspush: removed". The scratch datastore, remote and job were removed after.
…t back installConfigSnapshot.restore returned nothing: a directory it could not open was skipped silently, and a remove, rewrite or chmod that failed was logged at DEBUG. runInstallMode then exited 0 as for a clean abort, with backup.env left as the install wrote it (an ENCRYPT_ARCHIVE=true with no recipient, a daemon scheduler with no daemon). restore now returns what it could not put back. finishAbortedInstall shows it after the "Installation aborted" banner and exits 2 (configuration error); a restore that succeeds still exits 0. Approved line: "<path> not restored as it was before the install: <cause>". TestAbortedInstallReportsARestoreThatFails (backup.env replaced by a non-empty directory, so the rename over it fails as root too) on pve-test and VM 201: before (HEAD's abort branch, restore then exit 0, rebuilt through an overlay), exit 0 and no ERROR; after, exit 2 and the ERROR line. CodeRabbit PR #345 thread 4388376953 (finding 8).
safeCopyFile and safeCopyDir checked their source against the staging tree as text. A CUSTOM_BACKUP_PATHS entry whose symlinks resolve into the staging tree (/alias/run/etc with /alias pointing at the staging tree's parent) matched neither the lexical nor the resolved staging root, and below the staging root the inode check (ce3435d) does not match either: the archive's own files were copied into the archive again. sourceWithinStagingDir also compares the source with its symlinks resolved (one EvalSymlinks per source, custom paths only); the walk below it keeps the path and inode checks. No visible change. TestCustomPathResolvingIntoTheStagingTreeIsSkipped on pve-test and VM 201: before, the directory and the file resolving into the staging tree were copied into it; after, both skipped and the entry outside it still collected. CodeRabbit PR #345 thread 4392035106 (N1).
…s not write currentPBSRootTokenACLLines skipped every host acl.cfg line that is not an acl:<propagate>:<path>:<userlist>:<roles> entry. A host file with other lines (a section-format block) then gave no root@pam token entries to keep, and the restore's acl.cfg was written without them: the host's retained tokens lost their permissions. Such a line now stops the PBS access control restore before anything is written. Approved text, in the form of the other host-read errors: "read current /etc/proxmox-backup/acl.cfg: line <n> is not an acl: entry". Measured on pve-test: PBS writes acl:1:/system:root@pam!t:Audit (proxmox-backup-manager acl update); the real acl.cfg on pve-test (empty) and VM 201 (one acl: line) pass the check. TestPBSAccessControlStopsOnAHostACLNotInLineFormat on pve-test and VM 201: before, no error and acl.cfg rewritten; after, the error and the host files untouched. CodeRabbit PR #345 thread 4392035117 (N2).
9c84c95 resolved a custom path source in a separate sourceWithinStagingDir and left isWithinStagingDir comparing text only. isWithinStagingDir now compares the path as written and with its symlinks resolved, so every source check goes through it (safeCopyFile, safeCopyDir, the symlinked-directory check); sourceWithinStagingDir is gone. The copy walk checks each entry with isWithinStagingDirAsText and the inode check, as before: no EvalSymlinks per walked file. TestCustomPathResolvingIntoTheStagingTreeIsSkipped now has the reviewer's exact case too, /alias/run with /alias pointing at the staging tree's parent. On pve-test and VM 201 against 082e0f6 (before 9c84c95): /alias/run, /alias/run/etc and the file under it were copied into the staging tree; now all three skipped. CodeRabbit PR #345 thread 4392035106 (N1).
applyPBSACLFromStage found a staged acl.cfg with a line that is not an acl: entry only after user.cfg, domains.cfg and token.cfg had been written: WARNING "staged acl.cfg has unknown format; skipping apply", the ACL left out, and the restored users without their permissions. applyPBSAccessControlFromStage now checks the staged acl.cfg line by line (firstNonPBSACLLine) with the other staged files, before the first write, and stops with the approved text, in the form of the existing staged-file errors: "parse staged acl.cfg: line <n> is not an acl: entry". applyPBSACLFromStage no longer has an unknown-format branch; this host's acl.cfg goes through the same check (currentPBSRootTokenACLLines, b1dd0d9). TestPBSAccessControlStopsOnAStagedACLNotInLineFormatBeforeAnyWrite on pve-test and VM 201: before, no error and the host files rewritten; after, the error and the host files untouched. CodeRabbit PR #345 thread 4392035117 (N2).
…l.cfg is empty applyPBSACLFromStage removed this host's acl.cfg when the staged one was empty, and ignored hostRootTokenACL. PBS leaves acl.cfg at 0 bytes when no ACL is set (measured on pve-test), so restoring a backup of such a host onto one whose root@pam tokens have ACL entries left those retained tokens authenticating and denied everywhere. An empty staged acl.cfg now goes through the same write as one with entries: this host's root@pam token entries and root@pam's Admin on "/". applyPBSACLFromStage no longer takes a logger; the line moves to its caller, same position, approved text: "PBS access control: staged acl.cfg is empty; only this host's root@pam entries are kept" (was "...; removing <path>"). TestPBSAccessControlKeepsHostRootTokenACLWhenTheStagedACLIsEmpty on pve-test and VM 201: before, acl.cfg removed; after, "acl:1:/datastore/ds1:root@pam!host:DatastoreBackup" and "acl:1:/:root@pam:Admin". CodeRabbit PR #345 thread 4392035117 (N2).
…he TUI The CLI category menu counted the keys of its selection map, and a category turned off stays in the map as false. Turning one category on and off, or selecting all and then turning each off, and pressing "c" passed the check: the restore ran with zero categories and ended with "Restore completed successfully." and exit 0. The TUI refuses the same selection (WithMinSelected(1)). "c" now builds the selection first and refuses it when it is empty, with the TUI's text, approved: "Select at least 1 item(s); 0 selected" (was "Warning: No categories selected. Please select at least one category."); the menu is shown again. Piped stdin that ends right after such a "c" now exits 130 (aborted), as every other CLI menu at the end of stdin (accepted). The other CLI vs TUI menu differences are noted in diagnostics, not changed. TestCategoryMenuRefusesAnEmptySelectionLikeTheTUI on pve-test and VM 201: before, "1 1 c" and "a 1 2 c" returned 0 categories with no error; after, refused with the TUI text and the menu shown again. Audit finding o-core-1 (low).
shellQuote quoted a value only when it held whitespace or one of
"'\$&;|<>. The rollback scripts (network, firewall, HA, PVE access
control) take LOG, MARKER and BACKUP from BASE_DIR, which comes from
the executable's path: a BASE_DIR with ( ) or a backtick made every
rollback script a syntax error in /bin/sh ('Syntax error: "("
unexpected', exit 2), so the rollback never ran and wrote no log.
shellQuote now single-quotes every value, an embedded ' written as
'\''. The scripts get the same strings; no visible text changes.
docs/SECURITY.md: the sentences describing the gap replaced with the
approved text.
TestRollbackScriptsKeepEveryBaseDirCharacter (each of the 4 scripts
with 5 BASE_DIRs: plain, ( ), backtick, ', glob/#/~/{}; /bin/sh -n
and the values the shell reads) on pve-test and VM 201: before,
"(" unexpected; after, every script parses and every value round-trips.
Audit finding o-network-5 (low).
…rage EvaluateKeyShow compared the storage.cfg encryption-key value with the fingerprint "key show" reports. pve-storage writes the fingerprint, or a bare 1 when the key JSON has none (keys from before proxmox-backup-client 1.0.3) and for every encrypted PBS storage added before pve-storage 3cc2eb73 (2020-11) and never re-keyed. PVE uses the 1 only as a yes/no marker, so the comparison refused the storage's own key: "Encryption key: does not match storage", the PBS destination not initialized, and every run's PBS step failed. The exact value 1 now skips the comparison, like an absent value, with a DEBUG line in checkKey; a passphrase is still reported, and any other value is still compared. No visible text added. TestEvaluateKeyShowTakesPVEsBareMarker on pve-test and VM 201: before, "does not match storage" for a key with and without a fingerprint; after, no fact. Audit finding platform-1 (low).
The namespace inventory never found a real namespace. It first ran
"proxmox-backup-manager datastore namespace list", a command PBS does
not have, then looked for vm/ct/host/namespace directly under the
datastore, while PBS keeps a namespace at <ds>/ns/<name> and a child
one at <ds>/ns/<name>/ns/<child>. Every <ds>_namespaces.json in the
archive held the root namespace only.
The inventory now walks <ds>/ns recursively, up to the 7 levels PBS
allows, recording each namespace with its parent and path; the
nonexistent command and the top-level check are gone, and the
collector uses the same walk for configured and override datastores.
Only a DEBUG line changes ("... from the datastore directory <path>").
Measured on pve-test (PBS 4.2): namespaces other, r2, r2/child and
r2/child/deep live at ns/other, ns/r2, ns/r2/ns/child and
ns/r2/ns/child/ns/deep. On that real datastore the old walk returned
the root only and the new one returns exactly the API's list
("", other, r2, r2/child, r2/child/deep).
TestDiscoverNamespacesReadsThePBSNamespaceTree and
TestDiscoverNamespacesStopsAtPBSMaxDepth on pve-test and VM 201:
before FAIL, after PASS. The invented <ds>/<ns>/vm fixtures and the
CLI tests were removed.
Audit finding platform-2 (low).
The offline-storage mount guard always targeted /mnt/pve/<id> for nfs, cifs, cephfs and glusterfs, and read storage.cfg's path only for dir storages. PVE mounts a network storage at its path, which it writes on every create: /mnt/pve/<id> by default, any path when one is given. A storage at another path was "guarded" on a directory nothing mounts, with "PVE mount guard: ... applying guard bind mount on /mnt/pve/<id>" and a restore ending with warnings, while its real mountpoint stayed unguarded. The guard now targets the storage's path, and /mnt/pve/<id> only when the stanza has none; a path outside /mnt, /media and /run/media is skipped by the existing check (DEBUG). No new text. Docs RESTORE_TECHNICAL.md and RESTORE_GUIDE.md reworded as approved. Measured on pve-test: pvesm add nfs with --path /mnt/probenas writes "path /mnt/probenas", without --path "path /mnt/pve/probedef". TestPVEMountGuardTargetsTheNetworkStoragePath on pve-test and VM 201: before /mnt/pve/nas for a storage at /mnt/nas; after /mnt/nas, and /mnt/pve/<id> for a stanza without a path. Audit finding o-pbs-pve-10 (low).
gofmt rewrites '' inside a doc comment as a typographic quote, so the comment added in 8309188 ("an embedded ' written as '\''") was not gofmt-clean. Reworded without the sequence. Comment only.
maybeApplyPBSConfigsFromStage applied pbs_remotes before pbs_jobs. In
a Clean restore the remote pass removes stale remotes, and PBS refuses
to remove a remote a sync job still uses: a remote used only by a job
that the jobs pass was about to remove stayed ("PBS remote rX: not
removed", "PBS remote configuration: partially applied", the restore
ending with warnings). The file fallback had the same order problem,
deciding which remotes to keep from the sync.cfg not yet rewritten.
The jobs block now runs before the remotes block, in Clean and Merge
alike. Measured on pve-test (PBS 4.2.8): sync-job create and update,
pull and push, accept a remote that does not exist yet. No new text;
the job blocks print before "Restoring PBS remotes...", and the
combined DEBUG outcome line lists them first (test updated).
TestPBSJobsAreAppliedBeforeRemotes on pve-test and VM 201: before,
order remote, sync, verification, prune; after sync, verification,
prune, remote, in both modes.
Audit finding o-pbs-pve-7 (low).
confirmRestorePlan handed the plan screen the normal, staged and export categories, and the screen drops a category's paths from "Files/directories that will be restored:" only when ExportOnly is set. Categories moved to the export directory for this run (pve_cluster in SAFE mode, another product's categories on a single-product host) keep ExportOnly false, so their paths, such as "/var/lib/pve-cluster/", were listed as restored under the OVERWRITTEN warning although they go to the export directory. The export categories are now handed to the plan as copies with ExportOnly set; the plan's own categories keep the flag as it is (applyClusterSafety moves pve_cluster back when SAFE is turned off). "Categories to restore:" is unchanged. No new text. TestRestorePlanDoesNotListExportedCategoryPaths on pve-test and VM 201: before, "/var/lib/pve-cluster/" listed as restored; after, only the restored category's paths. Audit finding o-restore-7 (low).
… flags
A SAFE cluster restore that recreates resource mappings ("Apply now")
decoded only id, comment and map from the export, and the create call
sent --id, --comment and --map. The export has no comment: it carries
description, and for pci mdev and live-migration-capable (measured on
pve-test, PVE 9.1.9, the flags as numbers). A recreated mapping lost
all three while "Applied pci mapping <id>" was printed; without
live-migration-capable a mapped VM can no longer live-migrate.
The export's description is sent with --description, and for pci
--mdev 1 and --live-migration-capable 1 when set. A mapping that
already exists keeps its own description and flags (only the map
entries are merged); the dead comment handling is removed. On a PVE
older than 8.3 pvesh refuses --live-migration-capable and the mapping
is reported by the existing failure lines. No new text.
Real pvesh on pve-test (PVE 9.1.9) and VM 201 (PVE 9.2.2), mapping
recreated from an export with description, mdev and
live-migration-capable: before, the live mapping had id and map only;
after, all three fields. TestSafeClusterApplyRecreatesMappingDescription
AndPCIFlags uses the measured export.
Audit finding o-pbs-pve-8 (low).
The decrypt workflow writes one file in the destination, the bundle: the archive, its .metadata and .sha256 stay in the temp work directory and go into the bundle. It still asked about an existing "decrypted archive" at <dest>/<archive>, a path it never writes, and Overwrite deleted that file. A different path for the bundle in a directory that did not exist failed with "move decrypted bundle: open <path>: no such file or directory". The archive prompt (CLI "Decrypted Archive <path> already exists.", TUI "Existing file") and the temp rename it fed are removed; the bundle prompt stays. The bundle's directory is created (0755, DEBUG line); a failure reads "create bundle directory: <err>" (approved). The tail of runDecryptWorkflowWithUI is now writeDecryptedBundle, unchanged otherwise. TestDecryptBundleAsksOnlyAboutTheBundleAndCreatesItsDirectory on pve-test and VM 201: before (HEAD's tail rebuilt through an overlay), the archive was asked about and the move failed with ENOENT; after, only the existing bundle is asked about, the bundle lands in the new directory and the file named like the archive is untouched. Audit finding o-core-2 (low).
…rupted
A Ctrl+C or SIGTERM in the seconds of the network preflight
("ifup -n -a") cancelled the restore's ctx; the preflight recorded the
cancel as a failure and the automatic rollback ran its script under
that same ctx, which exec does not start. The backup's network files
stayed in /etc, unvalidated, until the next reboot, and the log read
"Network restore aborted: staged configuration failed validation ...
and rollback failed: rollback script failed: context canceled".
rollbackNetworkFilesNow runs the script under context.WithoutCancel,
which covers its 5 callers; no time limit is added (a second Ctrl+C
still ends the process). An interrupted check is reported as such,
approved texts:
"Network restore interrupted before validation finished (ifup -n -a).
Rolled back /etc/network/*, /etc/hosts, /etc/hostname,
/etc/resolv.conf to the pre-restore state (rollback=<path>)." and
"Network apply interrupted before validation finished (ifup -n -a).
Rolled back the managed network configuration (...) to the pre-restore
state (rollback=<path>).", instead of "Network preflight: FAILED"
(DEBUG then); the restore still aborts on the cancel. Nothing is
written to /etc once the restore is cancelled.
TestNetworkRollbackRunsAfterTheRestoreIsCancelled on pve-test and VM
201: before, the rollback script got the cancelled ctx; after, a live
one.
Audit finding o-network-3 (low).
…s left down A restore that stopped the PBS services (ssl, maintenance_pbs, pbs_notifications) started them again, and ran the notifications verify/repair, only in a deferred cleanup, after the verdict, the service advice, the ZFS check and the reboot line had been printed. A start or verify failure landed below "Restore completed successfully.", and the advice said "PBS services - stopped and started again during this restore" even when a unit had not started. As for PVE, restartStoppedPBSServices runs once, right before the verdict, so its warnings are counted there; the deferred cleanup only does work on an error or an abort. startPBSServices now returns a pbsServicesStartError naming the units left down (its message is unchanged), and the advice names them, approved text: " PBS services - stopped for this restore and not running: <units> (start failed)" (WARNING). No exit code change. TestPBSRestartAdviceNamesTheUnitsLeftDown (systemctl unavailable) on pve-test and VM 201: before, the advice said the services were started again; after, it names proxmox-backup and proxmox-backup-proxy. Audit finding o-restore-5 (low).
…h an error Dual PVE+PBS host, cluster backup, SAFE, both access control categories. maybeApplyAccessControlFromStage logged WARNING "PVE access control: cluster backup detected; skipping 1:1 access control apply in SAFE mode" with no condition, also after the operator applied the PVE one with "Apply 1:1 (expert)" and kept it. And when the PVE apply ended with an error (not committed after Rollback or the timeout, rollback not armed, apply failed), maybeApplyAccessControl WithUI returned before the PBS one ran, with nothing saying so. The SAFE warning is removed: the PVE step reports its own outcome. After a PVE error the PBS access control is applied anyway and the PVE error is returned; a PBS failure is logged as "Access control staged apply: <err>", which the runner would otherwise hide behind the PVE not-committed lines. An abort or an inconsistent state stops it, and so does a PVE rollback armed on the full safety backup, which would revert the PBS files; then, approved text: "PBS access control: not applied; the armed rollback restores the full safety backup". TestDualSafeAccessControlAppliesPBSAfterAPVEError on pve-test and VM 201: before, the PBS apply was not attempted after the PVE error; after, it is, and the SAFE warning is gone. TestPBSAccessControlNotAppliedUnderAFullSafetyBackupRollback (after only, it tests the new branch) passes on both. Audit finding o-restore-6 (low).
golangci-lint v2.15 (modernize slicescontains) flagged the loop in TestSafeClusterApplyRecreatesMappingDescriptionAndPCIFlags (7caaaa7). No behaviour change.
…eation
A removable datastore (PBS 3.3+) has backing-device and a relative
path, which PBS mounts at /mnt/datastore/<name> on its own device.
parsePBSDatastoreEntries kept only name and path, so recreation
refused it ('unsafe datastore path "usb1" from datastore.cfg: path
"usb1" is not absolute') and the restore ended with warnings; the file
fallback checked the relative path against the working directory.
Entries now record backing-device and close at the next header (PBS
writes backing-device before path, measured on pve-test, PBS 4.2).
Recreation leaves a removable datastore alone: DEBUG when "blkid -U
<uuid>" finds its device, otherwise the approved WARNING "Datastore
<name>: removable, device not connected". The file fallback accepts a
block with backing-device without the path checks.
Measured on pve-test with a loop device: datastore.cfg "backing-device
<uuid>" then "path usb9", mounted at /mnt/datastore/usb9; blkid -U
exits 2 once the device is gone (cleaned up after).
TestRecreationLeavesRemovableDatastoresToPBS on pve-test and VM 201:
before FAIL (refused as unsafe), after PASS with the WARNING only when
the device is absent.
Audit finding o-pbs-pve-9 (low).
The Clean 1:1 notifications restore listed the live matchers and
endpoints without --output-format=json, so the text table failed to
parse and every Clean apply fell back to the file ("PBS notifications
API apply failed; falling back to file-based apply: parse matcher
list: invalid character ..."), and its removals used "remove", a verb
PBS does not have ("delete"). The removal code had never run.
With the list read as JSON and "delete", the decided behaviour:
1. Built-in objects (origin builtin or modified-builtin: default-
matcher, mail-to-root) are never removed; PBS only resets them.
2. Order: stale matchers removed, endpoints upserted, matchers
upserted, stale endpoints removed last (PBS refuses to delete an
endpoint a matcher still references).
3. A kind this PBS cannot list (PBS 3.2 has no webhook list) has its
removals skipped instead of aborting the restore, approved WARNING
"PBS notifications: Clean 1:1 did not remove stale <kind>
endpoint(s): this PBS could not list them (<cause>)", and "... stale
matcher(s): ..." for matchers.
4. An empty staged notifications.cfg removes nothing in Clean,
approved WARNING "PBS notifications: Clean 1:1 removed nothing: the
notifications.cfg in the backup is empty".
The "matcher/endpoint remove ... failed" lines now read "delete".
Real proxmox-backup-manager on pve-test (PBS 4.2.5 running) and VM 201
(4.2.8): live stale-m (matcher) using stale-ep (sendmail), backup with
keep-m/keep-ep. Before: "parse matcher list: invalid character",
nothing removed. After: matcher:stale-m and sendmail:stale-ep removed,
keep-m/keep-ep created, built-ins kept. Both hosts put back as they
were. Tests updated to the decided behaviour.
Audit finding o-pbs-pve-2 (finder medium, verifier low).
…t its id When a SAFE cluster restore recreates a resource mapping that already exists, the create fails and the live mapping is read with "pvesh get /cluster/mapping/<type>/<id>" to merge its map entries. That object has no "id" (measured on pve-test, PVE 9.1.9), so parsePVEClusterMappingObject reported it unreadable and the merge never happened: "Failed to apply pci mapping <id>: create pci mapping \"<id>\" failed and the existing mapping could not be read (get error: <nil>, parse error: <nil>)". The id asked for stands in when the object has none. No text change. Real pvesh on pve-test (PVE 9.1.9) and VM 201 (PVE 9.2.2), a mapping already present with one entry and the export with another: before, the error above and the live mapping unchanged; after, the live mapping holds both entries. TestParsePVEClusterMappingObjectWithoutID uses the measured object. Found while fixing audit finding o-pbs-pve-8; maintainer decision to fix now (2026-10-28).
…fig directory takeInstallConfigSnapshot records backup.env as absent when its directory does not exist yet, and the wizard creates that directory only when it writes the file. An install aborted before then made restore call os.OpenRoot on the missing directory and return the not-exist error, so finishAbortedInstall (ef076c8) printed "<path> not restored as it was before the install: ..." and exited 2 although there was nothing to put back. Absent before and directory absent now is nothing to restore (DEBUG); any other error keeps the ERROR and exit 2. No visible text changes. TestAbortedInstallWithoutAConfigDirectoryRestoresNothing on pve-test: before, "restore: open ...: no such file or directory"; after, nil and exit 0. VM 201 no longer exists on pve1; maintainer decision to test on pve-test only for now. CodeRabbit PR #345 thread 4401016266.
…once per source The comment added in be1a1e9 said isWithinStagingDir "is called once per source", and that commit's message said there was no EvalSymlinks per walked file. Both were wrong: the copy walk hands every file to safeCopyFile, which calls isWithinStagingDir, so under CUSTOM_BACKUP_PATHS each copied file costs one EvalSymlinks. That resolution is what skips a symlink file outside the staging tree whose target is inside it, so it stays (maintainer decision); the comment now says so. Comment only. CodeRabbit PR #345 thread 4401016306.
|
Sorry @tis24dev, your pull request is larger than the review limit of 150,000 diff characters |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Important Review skippedToo many files! This PR contains 154 files, which is 4 over the limit of 150. To get a review, reduce the PR to 150 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.
 |
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated release PR for
v0.46.0.Summary by CodeRabbit