feat(backups): support generic rclone destinations - #5491
tokenjunkielabs wants to merge 43 commits into
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const isGenericRclone = | ||
| form.watch("provider") === RCLONE_CONFIG_PROVIDER; |
There was a problem hiding this comment.
The provider dropdown still renders only S3_PROVIDERS, which contains Rclone but no RcloneConfig entry. Users creating a destination therefore cannot activate the new generic-rclone fields or credential-validation behavior. Add a separately labeled option with the exact RcloneConfig value, preserving the existing Rclone S3 provider.
There was a problem hiding this comment.
Addressed on current head 8d8bcaa86e3aa8a99bcd625a05bf95efed47e21e: the provider menu now appends a dedicated RcloneConfig option after the existing S3_PROVIDERS entries, so the existing Rclone S3 provider remains unchanged while generic configured remotes are selectable.
| export const joinRclonePath = (remoteRoot: string, path = "") => { | ||
| const normalizedRoot = remoteRoot.replace(/\/+$/g, ""); | ||
| const normalizedPath = path.replace(/^\/+/, ""); | ||
| return normalizedPath ? `${normalizedRoot}/${normalizedPath}` : normalizedRoot; |
There was a problem hiding this comment.
For a configured SFTP remote such as sftp:, connection testing lists the user's home directory, but joining a backup filename produces sftp:/app/file.sql.gz, targeting the filesystem root instead. Backups can fail with permission errors or be written outside the directory that was tested and initially listed. Stripping the slash from sftp:/ also loses the distinction between absolute and home-relative roots. Preserve that distinction during resolution and joining, and use the corrected joining behavior in the MySQL/PostgreSQL restore paths too.
Knowledge Base Used: Backups and restore
There was a problem hiding this comment.
Addressed on current head 8d8bcaa86e3aa8a99bcd625a05bf95efed47e21e: normalizeRcloneRemoteRoot preserves the remote:/ absolute-root marker, while joinRclonePath now emits remote:file for home-relative bare remotes and remote:/file for absolute roots. Backup and restore callers, including MySQL/PostgreSQL, now construct paths through this shared joiner.
| .replace(/(--s3-access-key-id=)"[^"]*"/g, '$1"[REDACTED]"') | ||
| .replace(/(--s3-secret-access-key=)"[^"]*"/g, '$1"[REDACTED]"'); | ||
| return command.replace( | ||
| /(--(?:s3-access-key-id|s3-secret-access-key|[a-zA-Z0-9-]*(?:password|secret|token)[a-zA-Z0-9-]*)=)(?:"[^"]*"|'[^']*'|[^\s]+)/gi, |
There was a problem hiding this comment.
The new generic redaction misses rclone password flags such as --sftp-pass, --ftp-pass, and --webdav-pass: none contains password, secret, or token. These flags are accepted by additionalFlags and forwarded into the command, so their credential values survive redaction and enter the structured backup log even on successful backups. Cover these password-flag names, or prohibit credential overrides and require them in the config file; add regression cases for these flags.
How this was verified: An accepted --sftp-pass=credential flag flows unchanged through the generic resolver into getBackupCommand's logged rcloneCommand because the redaction expression does not match its name.
There was a problem hiding this comment.
Addressed on current head 8d8bcaa86e3aa8a99bcd625a05bf95efed47e21e: the shared redactor now covers provider-specific *-pass flags (including --sftp-pass, --ftp-pass, and --webdav-pass) in addition to password/secret/token and S3 credential flags before command or error text reaches structured logs.
| const { flags: rcloneFlags, remoteRoot } = | ||
| getRcloneDestination(input); | ||
| const rcloneCommand = `rclone ls ${rcloneFlags.join(" ")} ${quote([remoteRoot])}`; |
There was a problem hiding this comment.
Switching to the shared resolver drops --retries 1, --low-level-retries 1, --timeout 10s, and --contimeout 5s. Neither resolver branch restores these test-specific limits, and the execution helpers impose no command deadline. Testing an unreachable or stalled destination now uses rclone's normal retry/timeout behavior, leaving the request and loading state pending substantially longer. Apply the connection-test limits after resolving the destination, without imposing them on actual backup transfers.
There was a problem hiding this comment.
Addressed on current head 8d8bcaa86e3aa8a99bcd625a05bf95efed47e21e: testConnection appends --retries 1, --low-level-retries 1, --timeout 10s, and --contimeout 5s only to the resolved connection-test command. Transfer commands continue using the destination's normal flags.
| <FormLabel>{isGenericRclone ? "Rclone Config Path" : "Endpoint"}</FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| placeholder={"https://us.bucket.aws/s3"} | ||
| placeholder={isGenericRclone ? "/etc/dokploy/rclone.conf" : "https://us.bucket.aws/s3"} |
There was a problem hiding this comment.
The new field does not explain where the config file must exist. Self-hosted connection testing reads it locally in Dokploy's execution environment, while backups and restores for remote resources read it on their resource server. If the file exists only in the local environment, a successful connection test can be followed by a failed remote backup. Add generic-mode guidance explaining that the config and named remote must be provisioned at the specified path on each relevant execution host, and clarify where connection testing runs.
Knowledge Base Used: Backups and restore
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Addressed on current head 8d8bcaa86e3aa8a99bcd625a05bf95efed47e21e: the generic-mode config field now explains that both the config path and named remote must exist on every execution host, and distinguishes selected-server connection tests from Dokploy-local execution.
|
Addressed the current review blockers on this bounty carrier: |
|
Carrier follow-through on #416: the remaining live Greptile selector finding is repaired on this exact PR head |
|
The current head |
|
Carrier cleanup complete for #416 at head |
2 participants
/claim #416
Closes #416
Summary
The generic mode uses a dedicated
RcloneConfigprovider key so Dokploy's existingRcloneS3-compatible provider keeps its current behavior.Review follow-up
RcloneConfigas a selectable provider without replacing the existing S3Rcloneproviderremote:and absoluteremote:/roots when joining backup paths*-passcredentials from structured backup logsFix the inaccessible provider option, incorrect bare-remote path handling, and credential-log exposure before merging.
Summary
Adds a shared destination resolver for configured rclone remotes while retaining the existing S3-compatible provider path.
Reviews (1) · Last reviewed commit: "chore(backups): make upload logs destina..."