Repository navigation
feat: record the backup location in the Backup plugin metadata - #1141
Open
BoxBoxJason wants to merge 1 commit into
Open
BoxBoxJason wants to merge 1 commit into
BoxBoxJason wants to merge 1 commit into
Conversation
Add the `ObjectStore` name, the server name, the `destinationPath` and the `endpointURL` a backup was written to in the backup result metadata, so that they end up in `Backup.status.pluginMetadata`. A recovery cluster can then be pointed at the right location without listing the bucket or digging through old manifests, which matters when `serverName` changes across cluster generations. The keys are only added, and left out when empty, so the metadata of backups taken by older versions keeps the same shape. The plugin writes them but doesn't read them back. Credentials embedded in `destinationPath` or `endpointURL` are masked before being recorded. Closes cloudnative-pg#1140 Assisted-by: Claude Opus 5.5 Signed-off-by: BoxBoxJason <contact@boxboxjason.dev>
BoxBoxJason
marked this pull request as ready for review
October 5, 2026 17:27
This branch has not been deployed
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.
Problem
A
Backuptaken through the plugin doesn't record where it was written.Backup.status.pluginMetadataonly holds the cluster UID, the timeline and the plugin identity, so to create a recovery cluster from a given backup you have to find itsObjectStoreandserverNamesome other way and write them by hand in theexternalClustersentry.This is painful when the
serverNamechanges over time, for example a timestampedserverNameper cluster generation: the recovery default forserverNameis the name of the new cluster, which never matches. The in-tree integration kept this link inBackup.status.serverName,destinationPathandendpointURL, so users migrating to the plugin lose it.Closes #1140
Change
newBackupResultMetadatanow also records the configuration the backup was taken with:barmanObjectNameObjectStoreused for the backupserverNameserverNameparameter, or the cluster name when unset)destinationPathdestinationPathof theObjectStoreat backup timeendpointURLendpointURLof theObjectStoreat backup timedestinationPathorendpointURL(https://user:password@host) are masked withurl.Redacted()before being recorded, becauseBackupobjects are readable by more people than theObjectStore's secrets.usage.mddocuments the keys. It also says thatbarmanObjectNamealways refers to anObjectStorein theCluster's namespace, so restoring into another namespace needs anObjectStorethere that points to the same location.Follow-up
These keys would also let the catalog maintenance (
useSameBackupLocationinretention.go) skipBackupobjects taken against another location. Today it deletes them when the cluster switches to anotherObjectStoreorserverName, which I believe is what #405 reports. That's left out of this PR on purpose; I've detailed it in #405.Testing
Unit tests cover the recorded keys, empty values, a missing
ObjectStoreand credential masking.go vet,go test ./internal/cnpgi/instance/...andgolangci-lintpass.I also ran it on a local kind cluster (Kubernetes v1.37.0, CloudNativePG 1.30.1, cert-manager, RustFS as the S3 store), starting from the released plugin v0.15.1 and then switching the plugin and sidecar images to a build of this commit:
pluginMetadatahas none of the new keys, as expected.barmanObjectName,serverName(a timestamped one set through the parameter),destinationPathandendpointURLare recorded and match theObjectStoreand the folder in the bucket. The backup taken before the upgrade is unchanged and both survive the periodic catalog maintenance.externalClustersentry andrecoveryTarget.backupIDfrom theBackupstatus and nothing else. The new cluster came up healthy with the data from before and after the upgrade.serverNameset: the backup records the backup store and the default server name (the cluster name), not the recovery source.endpointURL(https://user:password@host): the backup completes, andpluginMetadataholdshttps://user:xxxxx@host. The password appears nowhere in theBackupobject.I used an AI assistant (Claude) to help review and write this change, as the AI policy asks for disclosure. The commit carries an
Assisted-by:trailer.