fix: do not restore PV verbatim when volume data was skipped via VolumePolicy

[mateenali66]

Summary

Fixes #9318

When a VolumePolicy action=skip is used during backup, the volume data is intentionally not backed up. This PR adds defense-in-depth to prevent restoring the original PV identity (VolumeHandle) when there's no snapshot data to back it:

• pv_restorer.go: When snapshotInfo == nil and the PV has a Delete reclaim policy, returns errPVNeedsReprovisioning instead of the PV object. This prevents restoring a PV whose underlying storage may no longer exist.
• restore.go: Both the new (BackupVolumeInfo) and legacy snapshot paths in handlePVHasNativeSnapshot now catch errPVNeedsReprovisioning and trigger dynamic re-provisioning via pvsToProvision.
• handleSkippedPVHasRetainPolicy: Added a warning log about cross-cluster restore risks when restoring a PV with its original volume identity. The Retain policy case preserves existing behavior (restoring as-is) since changing it would be a breaking change.

Why this is safe

• The Delete reclaim policy + no snapshot case was already handled correctly in the main restore flow (restoreItem). This change adds a safety net in executePVAction for defense-in-depth.
• The Retain policy behavior is unchanged - only a warning is added. Per maintainer feedback, this is acceptable for now.
• Existing tests all pass, and new test cases cover both Delete and Retain reclaim policy scenarios.

Test plan

• Added test: no snapshot and Delete reclaim policy: return errPVNeedsReprovisioning
• Added test: no snapshot and Retain reclaim policy: return PV as-is
• All existing TestExecutePVAction_NoSnapshotRestores tests pass
• All existing TestExecutePVAction_SnapshotRestores tests pass
• Package compiles without errors

[Lyndon-Li]

From the fist glance of #9318, I regard it as an expected behavior:

• action=skip means the volume backup won't go with either CSI snapshot or fs-backup
• On the other hand, Velero has another backup method --- native snapshot which is actually the default backup method
• So in this case, the backup fallbacks to native snapshot
• And during restore, the result matches to the behavior of native snapshot

So the point is, action=skip is not skip backing up the volume.

[mateenali66]

Thanks @Lyndon-Li — that's a helpful clarification. If action=skip is a fallback to native snapshot rather than a full "skip all backup methods" directive, then the restore behavior is working as designed and this PR is based on a misread of the semantics.

Closing this. The original issue (#9318) may be worth revisiting from a documentation or UX angle — it seems the reporters expected skip to mean "don't backup data at all", which suggests the distinction between skip-CSI/fs-backup vs. skip-all isn't obvious. But that's a separate conversation for the issue thread.

Thanks for the quick review.

[Lyndon-Li]

@mateenali66
Sorry, my mistake. The volume policy should include native snapshot, so for action=skip it should skip any volume backup, including volume snapshot.
And the current behavior is like this because volume policy only defines the behavior of volume backup, so even though no volume data is backed up, the PVC/PV resource are still backed up. As a result, during restore, they are restored as is.

I reopened your PR.

[mateenali66]

Hi, wanted to follow up on this. @Lyndon-Li confirmed in the comments that the current behavior is a bug and that action=skip should skip all volume backup methods including native snapshot. The fix addresses exactly that. Would appreciate a review when you get a chance. Happy to add tests or adjust the approach if needed.

[kaovilai]

Hi, fyi that some maintainers are at Kubecon EU this week.

[codecov]

Codecov Report

❌ Patch coverage is 77.77778% with 4 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
pkg/restore/restore.go	73.33%	4 Missing ⚠️

📢 Thoughts on this report? Let us know!

[blackpiglet]

@mateenali66
Please create a changelog file.
https://velero.io/docs/main/code-standards/#adding-a-changelog

[blackpiglet]

@mateenali66
Change looks good, but CI action had an error. Could you help take a look?

[mateenali66]

@kaovilai @blackpiglet gentle reminder, CI is green now, this can be approved/merged whenever you feel easy

[mateenali66]

@kaovilai when you get a moment, this one's been sitting since @blackpiglet approved on 4/1. would appreciate a second review if you're happy with the approach. CI green, coverage 77.8%, changelog in. fix addresses the PV restore-verbatim bug that @Lyndon-Li confirmed in #9588 (comment).