- Fix premature deletion of shared secrets/configmaps: check owner label
in addition to data equality. Same data + different owner is now a
collision, preventing one DataUpload's CleanUp from removing resources
another DataUpload is still using.
- Copy BinaryData in CopyConfigMap and include it in the equality check,
so configmaps with binary payloads (e.g., CA bundles) are not silently
truncated.
- Add UID preconditions to DeleteSecretsWithLabel and
DeleteConfigMapsWithLabel to avoid TOCTOU races where a recreated
object with the same name could be deleted.
- Move secret/configmap copy to the beginning of Expose(), before any
intermediate objects are created, so failure doesn't require cleanup.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
Add unit tests for CopyConfigMap, DeleteConfigMapIfAny, and
DeleteConfigMapsWithLabel mirroring the existing secret tests.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
Move the secret and configmap copy logic from the DataUpload controller
into the CSI snapshot exposer's Expose() method. This keeps all
CSI-specific logic in the exposer and maintains symmetry with CleanUp()
which already handles the cleanup of copied resources.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
- Fix import ordering in test file (gofmt)
- Add nolint:gosec for BackupPVCSecretLabel constant (not a credential)
- Use assert.Error instead of assert.True(err != nil) (testifylint)
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
Add label-based secret cleanup in CleanUp() to delete any secrets
that were copied to the Velero namespace for backup PVC provisioning.
Uses the velero.io/backup-pvc-secret label to find secrets associated
with the DataUpload being cleaned up.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
Copy configured secrets from the source namespace to the Velero
namespace in the New phase of the DataUpload reconcile loop, before
calling Expose(). This is done in the controller rather than the
exposer because Expose() errors are non-retryable (marked as permanent
failure), while the controller can requeue on collision.
On secret collision (same name, different data from another
DataUpload), the controller requeues with a 5s delay, matching the
existing pattern used for VGDP constraint checking.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
Add CopySecret, DeleteSecretIfAny, and DeleteSecretsWithLabel utilities
for copying namespace-scoped secrets to the Velero namespace during
datamover backup PVC creation.
CopySecret handles three cases:
- Secret does not exist in target: copies it with a tracking label
- Secret exists with same data: no-op (same source namespace)
- Secret exists with different data: returns ErrSecretCollision so the
caller can requeue
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
Add a SecretNames field to the BackupPVC type to allow users to specify
secrets that need to be copied from the source PVC namespace to the
Velero namespace before creating the backup PVC. This is needed for CSI
drivers that require namespace-scoped secrets for volume provisioning,
such as encrypted volumes with KMS.
Fixes#9879
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
This commit updates the func buildFinalTarball so it won't use
io.ReadAll, in order to optimize memory usage.
Signed-off-by: Daniel Jiang <daniel.jiang@broadcom.com>
Surface adoption signals (500M+ Docker Hub pulls, 10K+ GitHub stars)
at the top of the README, matching the org profile README. Also make
the release badge clickable.
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
DownloadRequest and ServerStatusRequest were the last two Velero CRDs
without printer columns, so kubectl showed only NAME and AGE for both.
DownloadRequest gains the target kind and name, its phase, and age.
ServerStatusRequest gains its phase, the reported server version, the
time the controller processed it, and age.
status.downloadURL is deliberately left out: it is a pre-signed URL that
grants access to the object, and a default list view is the wrong place
for it. status.expiration is left out because kubectl renders a date
column as time elapsed, so a future timestamp prints <invalid>.
Signed-off-by: saral <ilovegojo2580@gmail.com>
* cap the unzip of metadata download to avoid oom kill
Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
* detect when EOF is retuend because of cap
Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
---------
Signed-off-by: Lyndon-Li <lyonghui@vmware.com>
* refactor: use k8s.io/api well-known label constants
Several well-known Kubernetes label strings were hardcoded across the
codebase instead of using the constants already exported by
k8s.io/api/core/v1, which is an existing dependency:
"kubernetes.io/hostname" -> corev1api.LabelHostname
"kubernetes.io/os" -> corev1api.LabelOSStable
"topology.kubernetes.io/zone" -> corev1api.LabelTopologyZone
The local kube.NodeOSLabel and zoneLabel consts, which duplicated the
upstream values verbatim, are now defined in terms of the upstream
constants rather than repeating the literal. Both are kept: NodeOSLabel
is exported and referenced from four packages alongside NodeOSLinux and
NodeOSWindows, which have no upstream equivalent, and zoneLabel sits
beside the deprecated-label fallback it is compared against.
No functional change - every replacement is a constant with an identical
value.
Signed-off-by: Harshit saini <harshitsaini1188@gmail.com>
* Add changelog for #10279
Signed-off-by: Harshit saini <harshitsaini1188@gmail.com>
* Cover the selected-node path in createRestorePod
TestCreateRestorePod only exercised selectedNode == "", so the branch
that pins the restore pod to a node was never executed. Add a case with
a selected node and assert the resulting pod carries the hostname label
in its node selector.
Signed-off-by: Harshit saini <harshitsaini1188@gmail.com>
* Also use constants for the arch and deprecated zone labels
Extends the same replacement to the two remaining well-known labels
raised on the issue:
"kubernetes.io/arch" -> corev1api.LabelArchStable
"failure-domain.beta.kubernetes.io/zone" -> corev1api.LabelFailureDomainBetaZone
zoneLabelDeprecated in item_backupper.go was the last local const still
repeating a literal that upstream already exports, so the zone pair now
reads consistently against k8s.io/api. The deprecation note upstream
applies to the label itself, not the constant; Velero reads that label
deliberately as the fallback for PVs created before the topology labels
existed.
Signed-off-by: Harshit saini <harshitsaini1188@gmail.com>
---------
Signed-off-by: Harshit saini <harshitsaini1188@gmail.com>
This commit hardens the func "patchDynamicPVWithVolumeInfo":
1. Add nil checks for storageClass and the attributes.
2. Remove the double reported errors.
Signed-off-by: Daniel Jiang <daniel.jiang@broadcom.com>
When a Schedule has no explicit spec.skipImmediately, the reconciler
assigned &c.skipImmediately directly into the Schedule's spec pointer.
The subsequent write-through-pointer (*ptr = false) mutated the
reconciler's own shared field, silently disabling
--schedule-skip-immediately for every schedule reconciled afterward
for the life of the process.
Fix: copy the value into a fresh bool before taking its address.
Adds TestReconcileDoesNotCorruptReconcilerSkipImmediately, which
reconciles two schedules against one reconciler instance and asserts
the shared default is preserved.
Signed-off-by: Prajwal <percy38621@gmail.com>
BeforeSuite applies testdata/volume-snapshot-class/<provider>.yaml when
CSI is enabled, and there is no kind.yaml, so the suite fails before any
spec runs and none of the existing CSI tests can run on kind.
This adds a class for csi-driver-host-path. The driver ships its own, but
it lacks the velero.io/csi-volumesnapshot-class label so Velero never
selects it. Nothing sets FEATURES=EnableCSI for kind yet, so no test that
runs today is affected.
Signed-off-by: Sairam Bisoyi <sairamb2007.21@gmail.com>
Processed means the controller signed a URL into status.downloadURL. It
does not mean the object is present: GetDownloadURL builds the key by
convention and signs it, with no existence check, so a request whose
target never produced a file still reaches Processed and the URL 404s.
The CLI never sees this because it filters on backup and restore phase
before creating the request. Other API consumers have nothing in the
status telling them that filter is needed, and the field description
said only "Phase is the current state of the DownloadRequest".
Documentation only. The field comments are what controller-gen writes
into the CRD, so this reaches kubectl explain and generated clients
without anyone reading the Go source.
Refs #10232
Signed-off-by: saral <ilovegojo2580@gmail.com>
velero backup get prints <nil> in CREATED for a backup that never
started, and velero restore get prints it in both STARTED and COMPLETED.
The timestamps are *metav1.Time and are appended to the row unformatted,
so a nil pointer reaches the user as Go's nil literal.
This is reachable in ordinary use. A backup that fails validation never
starts, so StartTimestamp is never set, and a restore that fails
validation gets neither timestamp.
formatTimestamp returns n/a for an unset value, matching
humanReadableTimeFromNow, which already handles a zero expiration in the
same row. A set timestamp is unchanged.
Adds tests for both printers, which had no row-level coverage.
Signed-off-by: saral <ilovegojo2580@gmail.com>
Shubham Pampattiwar
Daniel JiangandGitHub
lyndon-liandGitHub
Joseph Antony VaikathandGitHub
R4mboandGitHub
opbot_xd
RalthosandGitHub
Krishna AwasthiandGitHub
harshit sainiandGitHub
Lyndon-Li
V PrajwalandGitHub
Jay SawantandGitHub
Sairam BisoyiandGitHub
Xun Jiang
Chlins ZhangandGitHub
Xun Jiang/Bruce JiangandGitHub