Append leader hostname to backup filename#2389
Conversation
There was a problem hiding this comment.
Pull request overview
Adds leader-hostname-aware backup download filenames so backups from multiple CapRover swarms are easier to distinguish after download.
Changes:
- Introduces
BackupManager.buildBackupFileName(nodes, uniqueId)to generate a sanitized, hostname-aware backup filename. - Uses the new filename builder when moving the generated backup tar into the downloads directory.
- Adds unit tests covering leader selection, fallback behavior, and hostname sanitization.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/user/system/BackupManager.ts |
Adds filename builder and switches backup tar rename step to use hostname-aware filename. |
tests/buildBackupFileName.test.ts |
Adds focused Jest coverage for the filename builder behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static buildBackupFileName( | ||
| nodes: ServerDockerInfo[] | undefined, | ||
| uniqueId: string | ||
| ): string { | ||
| const PREFIX = 'caprover-backup' | ||
| const EXT = '.tar' | ||
|
|
||
| let leader: ServerDockerInfo | undefined | ||
| if (nodes && nodes.length) { | ||
| leader = nodes.find(n => n && n.isLeader) | ||
| if (!leader) { | ||
| leader = nodes[0] | ||
| } | ||
| } | ||
|
|
||
| const rawHostname = | ||
| leader && leader.hostname ? leader.hostname : '' | ||
| const sanitized = rawHostname.replace(/[^A-Za-z0-9._-]/g, '_') | ||
|
|
||
| if (sanitized) { | ||
| return `${PREFIX}-${sanitized}-${uniqueId}${EXT}` | ||
| } | ||
| return `${PREFIX}-${uniqueId}${EXT}` | ||
| } |
There was a problem hiding this comment.
buildBackupFileName() changes the default download filename from the previous <uuid>.tar (see the removed uuid.v4() + '.tar' logic) to caprover-backup-<uuid>.tar even when no hostname is available. This is a user-visible/API behavior change and also conflicts with the PR description’s claim that the no-hostname case preserves the existing shape. If backward compatibility is required, consider keeping the old <uuid>.tar naming for the no-hostname path (and/or using Express’s res.download(filePath, suggestedName) to present a friendly filename without changing the internal stored filename).
Backup tar files currently use a filename that only encodes a
timestamp and the leader's IP, e.g.
caprover-backup-2026_04_10-19_30_00-1744312200000-ip-1_2_3_4.tar
When you run several CapRover instances behind a NAT or with
similar IPs, that filename is not enough to tell the backups apart
after downloading them.
This appends the swarm leader's hostname to the existing filename:
caprover-backup-...-ip-1_2_3_4-host-captain-prod.tar
The hostname is sanitized to a portable charset ([A-Za-z0-9._-]) so
it is safe in filesystems and HTTP Content-Disposition headers, and
the segment is omitted entirely when the leader has no hostname, so
existing single-node setups see no change beyond an additional
suffix when one is available.
The sanitization step is exposed as a small static helper
(`BackupManager.sanitizeHostnameForFilename`) so it can be unit
tested in isolation. Top-level cleanup hooks in the existing test
file are now gated on `process.env.CI`, matching how the
integration tests in the same file are gated, so the new pure
helper tests can run on a developer workstation.
Closes caprover#1257
hamza-younas94
force-pushed
the
backup-filename-include-hostname
branch
from
4d8f6e0 to
bf1beb5
Compare
hamza-younas94
changed the title
|
Friendly ping - this is a small, additive patch for the good-first-issue #1257. It only appends |
|
Thanks! |
Summary
Backup tar files currently use a filename that only encodes a timestamp and the leader's IP, e.g.
When you run several CapRover instances behind a NAT or with similar IPs, that filename is not enough to tell the backups apart after downloading them. This appends the swarm leader's hostname to the existing filename:
Closes #1257
Implementation notes
-host-<hostname>segment right before the.tarextension.[A-Za-z0-9._-]) so it is safe in filesystems and HTTPContent-Dispositionheaders.BackupManager.sanitizeHostnameForFilenameso it can be unit tested in isolation without spinning up a Docker swarm.beforeEach/afterEachcleanup hooks intests/backup.test.tsare now gated onprocess.env.CI, matching how the integration tests in the same file are already gated. This lets the new pure helper tests run on a developer workstation withoutmkdir /captainfailing.Test plan
npx tsc --noEmitcleannpx jest tests/backup.test.ts— 4 new tests pass, 2 CI-only integration tests correctly skipped-host-<hostname>.tar