This commit addresses three review comments on PR #9321:
1. Keep sanitization in controller (response to @ywk253100)
- Maintaining centralized error handling for easier extension
- Azure-specific patterns detected and others passed through unchanged
2. Sanitize unavailableErrors array (@priyansh17)
- Now using sanitizeStorageError() for both unavailableErrors array
and location.Status.Message for consistency
3. Add SAS token scrubbing (@anshulahuja98)
- Scrubs Azure SAS token parameters to prevent credential leakage
- Redacts: sig, se, st, sp, spr, sv, sr, sip, srt, ss
- Example: ?sig=secret becomes ?sig=***REDACTED***
Added comprehensive test coverage for SAS token scrubbing with 4 new
test cases covering various scenarios.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
Azure storage errors include verbose HTTP response details and XML
in error messages, making the BSL status.message field cluttered
and hard to read. This change adds sanitization to extract only
the error code and meaningful message.
Before:
BackupStorageLocation "test" is unavailable: rpc error: code = Unknown
desc = GET https://...
RESPONSE 404: 404 The specified container does not exist.
ERROR CODE: ContainerNotFound
<?xml version="1.0"...>
After:
BackupStorageLocation "test" is unavailable: rpc error: code = Unknown
desc = ContainerNotFound: The specified container does not exist.
AWS and GCP error messages are preserved as-is since they don't
contain verbose HTTP responses.
Fixes#8368
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
- Rename metric constants from maintenance_job_* to repo_maintenance_*
- Update metric help text to clarify these are for repo maintenance
- Rename functions: RegisterMaintenanceJob* → RegisterRepoMaintenance*
- Update all test references to use new names
Addresses review comments from @Lyndon-Li on PR #9414
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
Adds three new Prometheus metrics to track backup repository
maintenance job execution:
- velero_maintenance_job_success_total: Counter for successful jobs
- velero_maintenance_job_failure_total: Counter for failed jobs
- velero_maintenance_job_duration_seconds: Histogram for job duration
Metrics use repository_name label to identify specific BackupRepositories.
Duration is recorded for both successful and failed jobs (when job runs),
but not when job fails to start.
Includes comprehensive unit and integration tests.
Fixes#9225
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
* Add wildcard status fields
Signed-off-by: Joseph <jvaikath@redhat.com>
* Implement wildcard namespace expansion in item collector
- Introduced methods to get active namespaces and expand wildcard includes/excludes in the item collector.
- Updated getNamespacesToList to handle wildcard patterns and return expanded lists.
- Added utility functions for setting includes and excludes in the IncludesExcludes struct.
- Created a new package for wildcard handling, including functions to determine when to expand wildcards and to perform the expansion.
This enhances the backup process by allowing more flexible namespace selection based on wildcard patterns.
Signed-off-by: Joseph <jvaikath@redhat.com>
* Enhance wildcard expansion logic and logging in item collector
- Improved logging to include original includes and excludes when expanding wildcards.
- Updated the ShouldExpandWildcards function to check for wildcard patterns in excludes.
- Added comments for clarity in the expandWildcards function regarding pattern handling.
These changes enhance the clarity and functionality of the wildcard expansion process in the backup system.
Signed-off-by: Joseph <jvaikath@redhat.com>
* Add wildcard namespace fields to Backup CRD and update deepcopy methods
- Introduced `wildcardIncludedNamespaces` and `wildcardExcludedNamespaces` fields to the Backup CRD to support wildcard patterns for namespace inclusion and exclusion.
- Updated deepcopy methods to handle the new fields, ensuring proper copying of data during object manipulation.
These changes enhance the flexibility of namespace selection in backup operations, aligning with recent improvements in wildcard handling.
Signed-off-by: Joseph <jvaikath@redhat.com>
* Refactor Backup CRD to rename wildcard namespace fields
- Updated `BackupStatus` struct to rename `WildcardIncludedNamespaces` to `WildcardExpandedIncludedNamespaces` and `WildcardExcludedNamespaces` to `WildcardExpandedExcludedNamespaces` for clarity.
- Adjusted associated comments to reflect the new naming and ensure consistency in documentation.
- Modified deepcopy methods to accommodate the renamed fields, ensuring proper data handling during object manipulation.
These changes enhance the clarity and maintainability of the Backup CRD, aligning with recent improvements in wildcard handling.
Signed-off-by: Joseph <jvaikath@redhat.com>
* Fix
Signed-off-by: Joseph <jvaikath@redhat.com>
* Refactor where wildcard expansion happens
Signed-off-by: Joseph <jvaikath@redhat.com>
* Refactor Backup CRD and related components for expanded namespace handling
- Updated `BackupStatus` struct to rename fields for clarity: `WildcardExpandedIncludedNamespaces` and `WildcardExpandedExcludedNamespaces` are now `ExpandedIncludedNamespaces` and `ExpandedExcludedNamespaces`, respectively.
- Adjusted associated comments and deepcopy methods to reflect the new naming conventions.
- Removed the `getActiveNamespaces` function from the item collector, streamlining the namespace handling process.
- Enhanced logging during wildcard expansion to provide clearer insights into the process.
These changes improve the clarity and maintainability of the Backup CRD and enhance the functionality of namespace selection in backup operations.
Signed-off-by: Joseph <jvaikath@redhat.com>
* Refactor wildcard expansion logic in item collector and enhance testing
- Moved the wildcard expansion logic into a dedicated method, `expandNamespaceWildcards`, improving code organization and readability.
- Updated logging to provide detailed insights during the wildcard expansion process.
- Introduced comprehensive unit tests for wildcard handling, covering various scenarios and edge cases.
- Enhanced the `ShouldExpandWildcards` function to better identify wildcard patterns and validate inputs.
These changes improve the maintainability and robustness of the wildcard handling in the backup system.
Signed-off-by: Joseph <jvaikath@redhat.com>
* Enhance Restore CRD with expanded namespace fields and update logic
- Added `ExpandedIncludedNamespaces` and `ExpandedExcludedNamespaces` fields to the `RestoreStatus` struct to support expanded wildcard namespace handling.
- Updated the `DeepCopyInto` method to ensure proper copying of the new fields.
- Implemented logic in the restore process to expand wildcard patterns for included and excluded namespaces, improving flexibility in namespace selection during restores.
- Enhanced logging to provide insights into the expanded namespaces.
These changes improve the functionality and maintainability of the restore process, aligning with recent enhancements in wildcard handling.
Signed-off-by: Joseph <jvaikath@redhat.com>
* Refactor Backup and Restore CRDs to enhance wildcard namespace handling
- Renamed fields in `BackupStatus` and `RestoreStatus` from `ExpandedIncludedNamespaces` and `ExpandedExcludedNamespaces` to `IncludeWildcardMatches` and `ExcludeWildcardMatches` for clarity.
- Introduced a new field `WildcardResult` to record the final namespaces after applying wildcard logic.
- Updated the `DeepCopyInto` methods to accommodate the new field names and ensure proper data handling.
- Enhanced comments to reflect the changes and improve documentation clarity.
These updates improve the maintainability and clarity of the CRDs, aligning with recent enhancements in wildcard handling.
Signed-off-by: Joseph <jvaikath@redhat.com>
* Enhance wildcard namespace handling in Backup and Restore processes
- Updated `BackupRequest` and `Restore` status structures to include a new field `WildcardResult`, which captures the final list of namespaces after applying wildcard logic.
- Renamed existing fields to `IncludeWildcardMatches` and `ExcludeWildcardMatches` for improved clarity.
- Enhanced logging to provide detailed insights into the expanded namespaces and final results during backup and restore operations.
- Introduced a new utility function `GetWildcardResult` to streamline the selection of namespaces based on include/exclude criteria.
These changes improve the clarity and functionality of namespace selection in both backup and restore processes, aligning with recent enhancements in wildcard handling.
Signed-off-by: Joseph <jvaikath@redhat.com>
* Refactor namespace wildcard expansion logic in restore process
- Moved the wildcard expansion logic into a dedicated method, `expandNamespaceWildcards`, improving code organization and readability.
- Enhanced error handling and logging to provide detailed insights into the expanded namespaces during the restore operation.
- Updated the restore context with expanded namespace patterns and final results, ensuring clarity in the restore status.
These changes improve the maintainability and clarity of the restore process, aligning with recent enhancements in wildcard handling.
Signed-off-by: Joseph <jvaikath@redhat.com>
* Add checks for "*" in exclude
Signed-off-by: Joseph <jvaikath@redhat.com>
* Rebase
Signed-off-by: Joseph <jvaikath@redhat.com>
* Create NamespaceIncludesExcludes to get full NS listing for backup w/
Signed-off-by: Scott Seago <sseago@redhat.com>
Signed-off-by: Joseph <jvaikath@redhat.com>
* Add new NamespaceIncludesExcludes struct
Signed-off-by: Joseph <jvaikath@redhat.com>
* Move namespace expansion logic
Signed-off-by: Joseph <jvaikath@redhat.com>
* Update backup status with expansion
Signed-off-by: Joseph <jvaikath@redhat.com>
* Wildcard status update
Signed-off-by: Joseph <jvaikath@redhat.com>
* Skip ns check if wildcard expansion
Signed-off-by: Joseph <jvaikath@redhat.com>
* Move wildcard expansion to getResourceItems
Signed-off-by: Joseph <jvaikath@redhat.com>
* lint
Signed-off-by: Joseph <jvaikath@redhat.com>
* Changelog
Signed-off-by: Joseph <jvaikath@redhat.com>
* linting issues
Signed-off-by: Joseph <jvaikath@redhat.com>
* Remove wildcard restore to check if tests pass
Signed-off-by: Joseph <jvaikath@redhat.com>
* Fix namespace mapping test bug from lint fix
The previous commit (0a4aabcf4) attempted to fix linting issues by
using strings.Builder, but incorrectly wrote commas to a separate
builder and concatenated them at the end instead of between namespace
mappings.
This caused the namespace mapping string to be malformed:
Before: ns-1:ns-1-mapped,ns-2:ns-2-mapped
Bug: ns-1:ns-1-mappedns-2:ns-2-mapped,,
The malformed string was parsed as a single mapping with an invalid
namespace name containing a colon, causing Kubernetes to reject it:
"ns-1-mappedns-2:ns-2-mapped" is invalid
Fix by properly using strings.Builder to construct the mapping string
with commas between entries, addressing both the linting concern and
the functional bug.
Fixes the MultiNamespacesMappingResticTest and
MultiNamespacesMappingSnapshotTest failures.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
Signed-off-by: Joseph <jvaikath@redhat.com>
* Fix wildcard namespace expansion edge cases
This commit fixes two bugs in the wildcard namespace expansion feature:
1. Empty wildcard results: When a wildcard pattern (e.g., "invalid*")
matched no namespaces, the backup would incorrectly back up ALL
namespaces instead of backing up nothing. This was because the empty
includes list was indistinguishable from "no filter specified".
Fix: Added wildcardExpanded flag to NamespaceIncludesExcludes to
track when wildcard expansion has occurred. When true and the
includes list is empty, ShouldInclude now correctly returns false.
2. Premature namespace filtering: An earlier attempt to fix bug #1
filtered namespaces too early in collectNamespaces, breaking
LabelSelector tests where namespaces should be included based on
resources within them matching the label selector.
Fix: Removed the premature filtering and rely on the existing
filterNamespaces call at the end of getAllItems, which correctly
handles both wildcard expansion and label selector scenarios.
The fixes ensure:
- Wildcard patterns matching nothing result in empty backups
- Label selectors still work correctly (namespace included if any
resource in it matches the selector)
- State is preserved across multiple ResolveNamespaceList calls
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
Signed-off-by: Joseph <jvaikath@redhat.com>
* Run wildcard expansion during backup processing
Signed-off-by: Joseph <jvaikath@redhat.com>
* Lint fix
Signed-off-by: Joseph <jvaikath@redhat.com>
* Improve coverage
Signed-off-by: Joseph <jvaikath@redhat.com>
* gofmt fix
Signed-off-by: Joseph <jvaikath@redhat.com>
* Add wildcard details to describe backup status
Signed-off-by: Joseph <jvaikath@redhat.com>
* Revert "Remove wildcard restore to check if tests pass"
This reverts commit 4e22c2af855b71447762cb0a9fab7e7049f38a5f.
Signed-off-by: Joseph <jvaikath@redhat.com>
* Add restore describe for wildcard namespaces Revert restore wildcard removal
Signed-off-by: Joseph <jvaikath@redhat.com>
* Add coverage
Signed-off-by: Joseph <jvaikath@redhat.com>
* Lint
Signed-off-by: Joseph <jvaikath@redhat.com>
* Remove unintentional changes
Signed-off-by: Joseph <jvaikath@redhat.com>
* Remove wildcard status fields and mentionsRemove usage of wildcard fields for backup and restore status.
Signed-off-by: Joseph <jvaikath@redhat.com>
* Remove status update changelog line
Signed-off-by: Joseph <jvaikath@redhat.com>
* Rename getNamespaceIncludesExcludes
Signed-off-by: Scott Seago <sseago@redhat.com>
Signed-off-by: Scott Seago <sseago@redhat.com>
* Rewrite brace pattern validation
Signed-off-by: Joseph <jvaikath@redhat.com>
* Different var for internal loop
Signed-off-by: Joseph <jvaikath@redhat.com>
---------
Signed-off-by: Joseph <jvaikath@redhat.com>
Signed-off-by: Scott Seago <sseago@redhat.com>
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
Co-authored-by: Scott Seago <sseago@redhat.com>
Co-authored-by: Tiger Kaovilai <tkaovila@redhat.com>
Co-authored-by: Claude <noreply@anthropic.com>
VolumeGroupSnapshots were querying all PVCs with matching labels
directly from the cluster without respecting volume policies. This
caused errors when labeled PVCs included both CSI and non-CSI volumes,
or volumes from different CSI drivers that were excluded by policies.
This change filters PVCs by volume policy before VGS grouping,
ensuring only PVCs that should be snapshotted are included in the
group. A warning is logged when PVCs are excluded from VGS due to
volume policy.
Fixes#9344
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
* Fix the Job build error when BackupReposiotry name longer than 63.
Fix the Job build error.
Consider the name length limitation change in job list code.
Use hash to replace the GetValidName function.
Signed-off-by: Xun Jiang <xun.jiang@broadcom.com>
* Use ref_name to replace ref.
Signed-off-by: Xun Jiang <xun.jiang@broadcom.com>
---------
Signed-off-by: Xun Jiang <xun.jiang@broadcom.com>
Update test expectations to include createdName field for resources
with action 'created'. Also ensure namespaces track their created
names when created via EnsureNamespaceExistsAndIsReady.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
When restoring resources with GenerateName, Kubernetes assigns the actual name
after creation, but Velero only tracked the original name from the backup in
itemKey. This caused volume information collection to fail when trying to fetch
PVCs using the original name instead of the actual created name.
Example:
- Original PVC name from backup: "test-vm-disk-1"
- Actual created PVC name: "test-vm-backup-2025-10-27-test-vm-disk-1-mdjkd"
- Volume info tried to fetch: "test-vm-disk-1" → Failed with "not found"
This affects any plugin or workflow using GenerateName during restore:
- kubevirt-velero-plugin (VMFR use case with PVC collision avoidance)
- Custom restore item actions using generateName
- Secrets/ConfigMaps restored with generateName
Changes:
1. Add createdName field to restoredItemStatus struct (pkg/restore/request.go)
2. Capture actual name from createdObj.GetName() (pkg/restore/restore.go:1520)
3. Use createdName in RestoredResourceList() when available (pkg/restore/request.go:93-95)
This fix is backwards compatible:
- createdName defaults to empty string
- When empty, falls back to itemKey.name (original behavior)
- Only populated for GenerateName resources where needed
Fixes volume information collection errors like:
"Failed to get PVC" error="persistentvolumeclaims \"<original-name>\" not found"
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
When restoring resources with GenerateName (where name is empty and K8s
assigns the actual name), the managed fields patch was failing with error
"name is required" because it was using obj.GetName() which returns empty
for GenerateName resources.
The fix uses createdObj.GetName() instead, which contains the actual name
assigned by Kubernetes after resource creation.
This affects any resource using GenerateName for restore, including:
- PersistentVolumeClaims restored by kubevirt-velero-plugin
- Secrets and ConfigMaps created with generateName
- Any custom resources using generateName
Changes:
- Line 1707: Use createdObj.GetName() instead of obj.GetName() in Patch call
- Lines 1702, 1709, 1713, 1716: Use createdObj in error/info messages for accuracy
This is a backwards-compatible fix since:
- For resources WITHOUT generateName: obj.GetName() == createdObj.GetName()
- For resources WITH generateName: createdObj.GetName() has the actual name
The managed fields patch was already correctly using createdObj (lines 1698-1700),
only the Patch() call was incorrectly using obj.
Fixes restore status showing FinalizingPartiallyFailed with "name is required"
error when restoring resources with GenerateName.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
