* Fix PodVolumeBackup metadata loss on fs-backup timeout
When a backup hits the fs-backup timeout, WaitAllPodVolumesProcessed
returned nil because PVBs were only collected from the indexer in the
done branch of the select. This discarded all PVB metadata including
already-completed PVBs, making their data unrestorable.
Move the PVB collection loop to run after the select so tracked PVBs
are always persisted regardless of timeout.
Fixes#9986
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
* Add changelog for PR #9995
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
* Filter non-completed PVBs in hasPodVolumeBackup
After preserving tracked PVBs on timeout, non-completed PVBs (in-progress
or with no snapshot ID) would cause hasPodVolumeBackup to return true,
leading the restore to skip the original PV and dynamically re-provision
it without any data to restore from.
Only match PVBs that are Completed with a valid SnapshotID.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
* Add unit tests for hasPodVolumeBackup phase filtering
Verify that hasPodVolumeBackup only matches PVBs that are Completed
with a valid SnapshotID, and rejects in-progress, failed, or
empty-snapshot PVBs.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
---------
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
(cherry picked from commit e593ba73f9)
Commit 8ac8f49b5 ("Remove wildcard check from getNamespacesToList")
removed the optimization that prevented "*" from being expanded to
individual namespace names. This caused getNamespacesToList to return
all namespace names instead of "" (cross-namespace listing), resulting
in N separate API list calls per resource type instead of 1.
On clusters with many namespaces (e.g. 178 on an ACM cluster), this
means ~35,000 API calls instead of ~200, causing backups to take
18-20 minutes for just 8 items.
Restore the "*" special case in ShouldExpandWildcards so that plain
"*" is not expanded, and restore the ShouldInclude("*") check in
getNamespacesToList so that cross-namespace listing is used.
The restore fix from 8ac8f49b5 (fromBackup flag) is preserved since
restores already return false before reaching the "*" check.
Namespace exclusion continues to work correctly: the nsTracker filters
excluded namespace objects via ShouldInclude, and backupItem filters
namespace-scoped resources at line 124 of item_backupper.go.
Fixes#9869
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
When a backup tarball incidentally contains DataUpload CRs that belong to
a different backup (common when a schedule includes the velero namespace
where DataUploads live), DataUploadDeleteAction.Execute used to create a
"<du-name>-info" ConfigMap labeled with the *executing* backup's name
instead of the DataUpload's true owning backup. The ConfigMap is
created with Create-only semantics, so the wrong label is never
corrected.
deleteMovedSnapshots in the backup-deletion controller looks up these
ConfigMaps by velero.io/backup-name to discover which Kopia snapshots
to delete. With the wrong label, the real owning backup's expiry pass
finds no ConfigMaps for its DataUploads and silently leaves their Kopia
snapshots in object storage, leaking data over time.
Fix: in DataUploadDeleteAction.Execute, compare the DataUpload's
velero.io/backup-name label against input.Backup.Name (using
label.GetValidName to handle DNS-1035 truncation for long backup names).
If the label is present and differs, skip the DataUpload entirely; this
prevents the over-eager creation of misnamed ConfigMaps without changing
behavior for DataUploads that legitimately belong to the executing
backup, or for legacy DataUploads with no backup-name label.
Refs: #9472
Signed-off-by: Christian Schlichtherle <cs@bsure-analytics.de>
* Fix wildcard expansion when includes is empty and excludes has wildcards
When a Backup CR is applied via kubectl with empty includedNamespaces
and a wildcard in excludedNamespaces, ShouldExpandWildcards triggers
expansion. The empty includes expands to nil, but wildcardExpanded is
set to true, causing ShouldInclude to return false for all namespaces.
Populate expanded includes with all active namespaces when the original
includes was empty (meaning "include all") so that the wildcardExpanded
check does not falsely reject everything.
Signed-off-by: Joseph <jvaikath@redhat.com>
* Changelog
Signed-off-by: Joseph <jvaikath@redhat.com>
* Normalize empty includes to * instead of active namespaces list
This ensures consistent behavior between CLI and kubectl-apply paths
for Namespace CR inclusion when excludes contain wildcards.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Joseph <jvaikath@redhat.com>
* Move empty includes normalization to backup controller
Instead of normalizing empty IncludedNamespaces to ["*"] in the
collections layer's ExpandIncludesExcludes, do it earlier in
prepareBackupRequest. This ensures the spec is correct before any
downstream processing.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Joseph <jvaikath@redhat.com>
* Update TestProcessBackupCompletions for wildcard normalization
Add IncludedNamespaces: []string{"*"} to all expected BackupSpec
structs, reflecting the new prepareBackupRequest normalization.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Joseph <jvaikath@redhat.com>
* Add checks around empty includenamespaces
Signed-off-by: Joseph <jvaikath@redhat.com>
* gofmt
Signed-off-by: Joseph <jvaikath@redhat.com>
---------
Signed-off-by: Joseph <jvaikath@redhat.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Xun Jiang <xun.jiang@broadcom.com>
* Add CI check for invalid characters in file paths
Go's module zip rejects filenames containing certain characters (shell
special chars like " ' * < > ? ` |, path separators : \, and non-letter
Unicode such as control/format characters). This caused a build failure
when a changelog file contained an invisible U+200E LEFT-TO-RIGHT MARK
(see PR #9552).
Add a GitHub Actions workflow that validates all tracked file paths on
every PR to catch these issues before they reach downstream consumers.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
(cherry picked from commit 6d18d9b303)
* Fix changelog filenames containing invisible U+200E characters
Remove LEFT-TO-RIGHT MARK unicode characters from changelog filenames
that would cause Go module zip failures.
Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
---------
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Happy <yesreply@happy.engineering>
Co-authored-by: Scott Seago <sseago@redhat.com>
The `getDataUpload` function in the CSI PVC backup plugin was
previously making a cluster-scoped list query to retrieve DataUpload
CRs. In environments with strict minimum-privilege RBAC, this would
fail with forbidden errors.
This explicitly passes the backup namespace into the `ListOptions`
when calling `crClient.List`, correctly scoping the queries to the
backup's namespace. Unit tests have also been updated to ensure
cross-namespace queries are rejected appropriately.
Signed-off-by: Adam Zhang <adam.zhang@broadcom.com>
Restrict the listing of PodVolumeBackup resources to the specific
restore namespace in both the core restore controller and the pod
volume restore action plugin. This prevents "Forbidden" errors when
Velero is configured with namespace-scoped minimum privileges,
avoiding the need for cluster-scoped list permissions for
PodVolumeBackups.
Fixes: #9681
Signed-off-by: Adam Zhang <adam.zhang@broadcom.com>
* Fix VolumeGroupSnapshot restore failure with Ceph RBD CSI driver (#9516)
* Fix VolumeGroupSnapshot restore on Ceph RBD
This PR fixes two related issues affecting CSI snapshot restore on Ceph RBD:
1. VolumeGroupSnapshot restore fails because Ceph RBD populates
volumeGroupSnapshotHandle on pre-provisioned VSCs, but Velero doesn't
create the required VGSC during restore.
2. CSI snapshot restore fails because VolumeSnapshotClassName is removed
from restored VSCs, preventing the CSI controller from getting
credentials for snapshot verification.
Changes:
- Capture volumeGroupSnapshotHandle during backup as VS annotation
- Create stub VGSC during restore with matching handle in status
- Look up VolumeSnapshotClass by driver and set on restored VSC
Fixes#9512Fixes#9515
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
* Add changelog for VGS restore fix
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
* Fix gofmt import order
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
* Add changelog for VGS restore fix
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
* Fix import alias corev1 to corev1api per lint config
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
* Fix: Add snapshot handles to existing stub VGSC and add unit tests
When multiple VolumeSnapshots from the same VolumeGroupSnapshot are
restored, they share the same VolumeGroupSnapshotHandle but have
different individual snapshot handles. This commit:
1. Fixes incomplete logic where existing VGSC wasn't updated with
new snapshot handles (addresses review feedback)
2. Fixes race condition where Create returning AlreadyExists would
skip adding the snapshot handle
3. Adds comprehensive unit tests for ensureStubVGSCExists (5 cases)
and addSnapshotHandleToVGSC (4 cases) functions
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
* Clean up stub VolumeGroupSnapshotContents during restore finalization
Add cleanup logic for stub VGSCs created during VolumeGroupSnapshot restore.
The stub VGSCs are temporary objects needed to satisfy CSI controller
validation during VSC reconciliation. Once all related VSCs become
ReadyToUse, the stub VGSCs are no longer needed and should be removed.
The cleanup runs in the restore finalizer controller's execute() phase.
Before deleting each VGSC, it polls until all related VolumeSnapshotContents
(correlated by snapshot handle) are ReadyToUse, with a timeout fallback.
Deletion failures and CRD-not-installed scenarios are treated as warnings
rather than errors to avoid failing the restore.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
* Fix lint: remove unused nolint directive and simplify cleanupStubVGSC return
The cleanupStubVGSC function only produces warnings (not errors), so
simplify its return signature. Also remove the now-unused nolint:unparam
directive on execute() since warnings are no longer always nil.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
---------
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
* Rename changelog file to match cherry-pick PR number
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
---------
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
Expand wildcard in namespace filter only for backup scenario.
Restore doesn't need that now, because restore has logic to rely on
IncludeEverything function to check whether cluster-scoped resources
should be restored. Expand wildcard will break the logic.
Signed-off-by: Xun Jiang <xun.jiang@broadcom.com>
o.Namespace is empty when Validate runs (Complete hasn't been called yet),
causing VerifyJSONConfigs to query the default namespace instead of the
intended one. Replace o.Namespace with f.Namespace() in all three ConfigMap
validation calls so the factory's already-resolved namespace is used.
Signed-off-by: Adam Zhang <adam.zhang@broadcom.com>
* Fix DBR stuck when CSI snapshot no longer exists in cloud provider
During backup deletion, VolumeSnapshotContentDeleteItemAction creates a
new VSC with the snapshot handle from the backup and polls for readiness.
If the underlying snapshot no longer exists (e.g., deleted externally),
the CSI driver reports Status.Error but checkVSCReadiness() only checks
ReadyToUse, causing it to poll for the full 10-minute timeout instead of
failing fast. Additionally, the newly created VSC is never cleaned up on
failure, leaving orphaned resources in the cluster.
This commit:
- Adds Status.Error detection in checkVSCReadiness() to fail immediately
on permanent CSI driver errors (e.g., InvalidSnapshot.NotFound)
- Cleans up the dangling VSC when readiness polling fails
Fixes#9579
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
* Add changelog for PR #9581
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
* Fix typo in pod_volume_test.go: colume -> volume
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
---------
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
chlins
Adam Zhang
Shubham Pampattiwar
Lyndon-Li
Tiger KaovilaiandLyndon-Li
Christian SchlichtherleandLyndon-Li
Priyansh ChoudharyandGitHub
lyndon-liandGitHub
