Skip to content
Permalink
Browse files

Validate restore name label length

Velero should handle cases when the label length exceeds 63 characters.

- if the length of the backup/restore name is <= 63 characters, use it as the value of the label
- if it's > 63 characters, take the SHA256 hash of the name. the value of
  the label will be the first 57 characters of the backup/restore name
  plus the first six characters of the SHA256 hash.

Fixes #1021

Signed-off-by: Anshul Chandra <anshulc@vmware.com>
  • Loading branch information...
canshul08 committed Apr 23, 2019
1 parent 134323f commit d4df312eaaa3f58f9d2c039580917028261d7663
@@ -0,0 +1 @@
Validate restore name label length
@@ -18,10 +18,10 @@ package backup

import (
"fmt"

"github.com/heptio/velero/pkg/labels"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

v1 "github.com/heptio/velero/pkg/apis/velero/v1"
"github.com/heptio/velero/pkg/apis/velero/v1"
)

// NewDeleteBackupRequest creates a DeleteBackupRequest for the backup identified by name and uid.
@@ -30,7 +30,7 @@ func NewDeleteBackupRequest(name string, uid string) *v1.DeleteBackupRequest {
ObjectMeta: metav1.ObjectMeta{
GenerateName: name + "-",
Labels: map[string]string{
v1.BackupNameLabel: name,
v1.BackupNameLabel: labels.GetValidLabelName(name),
v1.BackupUIDLabel: uid,
},
},
@@ -44,6 +44,6 @@ func NewDeleteBackupRequest(name string, uid string) *v1.DeleteBackupRequest {
// find DeleteBackupRequests for the backup identified by name and uid.
func NewDeleteBackupRequestListOptions(name, uid string) metav1.ListOptions {
return metav1.ListOptions{
LabelSelector: fmt.Sprintf("%s=%s,%s=%s", v1.BackupNameLabel, name, v1.BackupUIDLabel, uid),
LabelSelector: fmt.Sprintf("%s=%s,%s=%s", v1.BackupNameLabel, labels.GetValidLabelName(name), v1.BackupUIDLabel, uid),
}
}
@@ -42,6 +42,7 @@ import (
velerov1client "github.com/heptio/velero/pkg/generated/clientset/versioned/typed/velero/v1"
informers "github.com/heptio/velero/pkg/generated/informers/externalversions/velero/v1"
listers "github.com/heptio/velero/pkg/generated/listers/velero/v1"
labelValidation "github.com/heptio/velero/pkg/labels"
"github.com/heptio/velero/pkg/metrics"
"github.com/heptio/velero/pkg/persistence"
"github.com/heptio/velero/pkg/plugin/clientmgmt"
@@ -278,7 +279,7 @@ func (c *backupController) prepareBackupRequest(backup *velerov1api.Backup) *pkg
if request.Labels == nil {
request.Labels = make(map[string]string)
}
request.Labels[velerov1api.StorageLocationLabel] = request.Spec.StorageLocation
request.Labels[velerov1api.StorageLocationLabel] = labelValidation.GetValidLabelName(request.Spec.StorageLocation)

// validate the included/excluded resources and namespaces
for _, err := range collections.ValidateIncludesExcludes(request.Spec.IncludedResources, request.Spec.ExcludedResources) {
@@ -34,6 +34,7 @@ import (
"k8s.io/apimachinery/pkg/util/clock"

v1 "github.com/heptio/velero/pkg/apis/velero/v1"
velerov1api "github.com/heptio/velero/pkg/apis/velero/v1"
pkgbackup "github.com/heptio/velero/pkg/backup"
"github.com/heptio/velero/pkg/generated/clientset/versioned/fake"
informers "github.com/heptio/velero/pkg/generated/informers/externalversions"
@@ -199,6 +200,53 @@ func TestProcessBackupValidationFailures(t *testing.T) {
}
}

func TestBackupLocationLabel(t *testing.T) {
tests := []struct {
name string
backup *v1.Backup
backupLocation *v1.BackupStorageLocation
expectedBackupLocation string
}{
{
name: "valid backup location name should be used as a label",
backup: velerotest.NewTestBackup().WithName("backup-1").Backup,
backupLocation: velerotest.NewTestBackupStorageLocation().WithName("loc-1").BackupStorageLocation,
expectedBackupLocation: "loc-1",
},
{
name: "invalid storage location name should be handled while creating label",
backup: velerotest.NewTestBackup().WithName("backup-1").Backup,
backupLocation: velerotest.NewTestBackupStorageLocation().
WithName("defaultdefaultdefaultdefaultdefaultdefaultdefaultdefaultdefaultdefault").BackupStorageLocation,
expectedBackupLocation: "defaultdefaultdefaultdefaultdefaultdefaultdefaultdefaultd58343f",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
var (
clientset = fake.NewSimpleClientset(test.backup)
sharedInformers = informers.NewSharedInformerFactory(clientset, 0)
logger = logging.DefaultLogger(logrus.DebugLevel)
)

c := &backupController{
genericController: newGenericController("backup-test", logger),
client: clientset.VeleroV1(),
lister: sharedInformers.Velero().V1().Backups().Lister(),
backupLocationLister: sharedInformers.Velero().V1().BackupStorageLocations().Lister(),
snapshotLocationLister: sharedInformers.Velero().V1().VolumeSnapshotLocations().Lister(),
defaultBackupLocation: test.backupLocation.Name,
clock: &clock.RealClock{},
}

res := c.prepareBackupRequest(test.backup)
assert.NotNil(t, res)
assert.Equal(t, test.expectedBackupLocation, res.Labels[velerov1api.StorageLocationLabel])
})
}
}

func TestDefaultBackupTTL(t *testing.T) {

var (
@@ -21,7 +21,7 @@ import (
"encoding/json"
"time"

jsonpatch "github.com/evanphx/json-patch"
"github.com/evanphx/json-patch"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -32,11 +32,12 @@ import (
kubeerrs "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/client-go/tools/cache"

v1 "github.com/heptio/velero/pkg/apis/velero/v1"
"github.com/heptio/velero/pkg/apis/velero/v1"
pkgbackup "github.com/heptio/velero/pkg/backup"
velerov1client "github.com/heptio/velero/pkg/generated/clientset/versioned/typed/velero/v1"
informers "github.com/heptio/velero/pkg/generated/informers/externalversions/velero/v1"
listers "github.com/heptio/velero/pkg/generated/listers/velero/v1"
labelUtil "github.com/heptio/velero/pkg/labels"
"github.com/heptio/velero/pkg/metrics"
"github.com/heptio/velero/pkg/persistence"
"github.com/heptio/velero/pkg/plugin/clientmgmt"
@@ -196,7 +197,7 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e
r.Status.Phase = v1.DeleteBackupRequestPhaseInProgress

if req.Labels[v1.BackupNameLabel] == "" {
req.Labels[v1.BackupNameLabel] = req.Spec.BackupName
req.Labels[v1.BackupNameLabel] = labelUtil.GetValidLabelName(req.Spec.BackupName)
}
})
if err != nil {
@@ -390,7 +391,7 @@ func (c *backupDeletionController) backupStoreForBackup(backup *v1.Backup, plugi
func (c *backupDeletionController) deleteExistingDeletionRequests(req *v1.DeleteBackupRequest, log logrus.FieldLogger) []error {
log.Info("Removing existing deletion requests for backup")
selector := labels.SelectorFromSet(labels.Set(map[string]string{
v1.BackupNameLabel: req.Spec.BackupName,
v1.BackupNameLabel: labelUtil.GetValidLabelName(req.Spec.BackupName),
}))
dbrs, err := c.deleteBackupRequestLister.DeleteBackupRequests(req.Namespace).List(selector)
if err != nil {
@@ -465,6 +465,151 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) {
// Make sure snapshot was deleted
assert.Equal(t, 0, td.volumeSnapshotter.SnapshotsTaken.Len())
})

t.Run("full delete, no errors, with backup name greater than 63 chars", func(t *testing.T) {
backup := velerotest.NewTestBackup().WithName("the-really-long-backup-name-that-is-much-more-than-63-characters").Backup
backup.UID = "uid"
backup.Spec.StorageLocation = "primary"

restore1 := velerotest.NewTestRestore("velero", "restore-1", v1.RestorePhaseCompleted).
WithBackup("the-really-long-backup-name-that-is-much-more-than-63-characters").Restore
restore2 := velerotest.NewTestRestore("velero", "restore-2", v1.RestorePhaseCompleted).
WithBackup("the-really-long-backup-name-that-is-much-more-than-63-characters").Restore
restore3 := velerotest.NewTestRestore("velero", "restore-3", v1.RestorePhaseCompleted).
WithBackup("some-other-backup").Restore

td := setupBackupDeletionControllerTest(backup, restore1, restore2, restore3)
td.req = pkgbackup.NewDeleteBackupRequest(backup.Name, string(backup.UID))
td.req.Namespace = "velero"
td.req.Name = "foo-abcde"
td.sharedInformers.Velero().V1().Restores().Informer().GetStore().Add(restore1)
td.sharedInformers.Velero().V1().Restores().Informer().GetStore().Add(restore2)
td.sharedInformers.Velero().V1().Restores().Informer().GetStore().Add(restore3)

location := &v1.BackupStorageLocation{
ObjectMeta: metav1.ObjectMeta{
Namespace: backup.Namespace,
Name: backup.Spec.StorageLocation,
},
Spec: v1.BackupStorageLocationSpec{
Provider: "objStoreProvider",
StorageType: v1.StorageType{
ObjectStorage: &v1.ObjectStorageLocation{
Bucket: "bucket",
},
},
},
}
require.NoError(t, td.sharedInformers.Velero().V1().BackupStorageLocations().Informer().GetStore().Add(location))

snapshotLocation := &v1.VolumeSnapshotLocation{
ObjectMeta: metav1.ObjectMeta{
Namespace: backup.Namespace,
Name: "vsl-1",
},
Spec: v1.VolumeSnapshotLocationSpec{
Provider: "provider-1",
},
}
require.NoError(t, td.sharedInformers.Velero().V1().VolumeSnapshotLocations().Informer().GetStore().Add(snapshotLocation))

// Clear out req labels to make sure the controller adds them
td.req.Labels = make(map[string]string)

td.client.PrependReactor("get", "backups", func(action core.Action) (bool, runtime.Object, error) {
return true, backup, nil
})
td.volumeSnapshotter.SnapshotsTaken.Insert("snap-1")

td.client.PrependReactor("patch", "deletebackuprequests", func(action core.Action) (bool, runtime.Object, error) {
return true, td.req, nil
})

td.client.PrependReactor("patch", "backups", func(action core.Action) (bool, runtime.Object, error) {
return true, backup, nil
})

snapshots := []*volume.Snapshot{
{
Spec: volume.SnapshotSpec{
Location: "vsl-1",
},
Status: volume.SnapshotStatus{
ProviderSnapshotID: "snap-1",
},
},
}

pluginManager := &pluginmocks.Manager{}
pluginManager.On("GetVolumeSnapshotter", "provider-1").Return(td.volumeSnapshotter, nil)
pluginManager.On("CleanupClients")
td.controller.newPluginManager = func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager }

td.backupStore.On("GetBackupVolumeSnapshots", td.req.Spec.BackupName).Return(snapshots, nil)
td.backupStore.On("DeleteBackup", td.req.Spec.BackupName).Return(nil)
td.backupStore.On("DeleteRestore", "restore-1").Return(nil)
td.backupStore.On("DeleteRestore", "restore-2").Return(nil)

err := td.controller.processRequest(td.req)
require.NoError(t, err)

expectedActions := []core.Action{
core.NewPatchAction(
v1.SchemeGroupVersion.WithResource("deletebackuprequests"),
td.req.Namespace,
td.req.Name,
[]byte(`{"metadata":{"labels":{"velero.io/backup-name":"the-really-long-backup-name-that-is-much-more-than-63-cha6ca4bc"}},"status":{"phase":"InProgress"}}`),
),
core.NewGetAction(
v1.SchemeGroupVersion.WithResource("backups"),
td.req.Namespace,
td.req.Spec.BackupName,
),
core.NewPatchAction(
v1.SchemeGroupVersion.WithResource("deletebackuprequests"),
td.req.Namespace,
td.req.Name,
[]byte(`{"metadata":{"labels":{"velero.io/backup-uid":"uid"}}}`),
),
core.NewPatchAction(
v1.SchemeGroupVersion.WithResource("backups"),
td.req.Namespace,
td.req.Spec.BackupName,
[]byte(`{"status":{"phase":"Deleting"}}`),
),
core.NewDeleteAction(
v1.SchemeGroupVersion.WithResource("restores"),
td.req.Namespace,
"restore-1",
),
core.NewDeleteAction(
v1.SchemeGroupVersion.WithResource("restores"),
td.req.Namespace,
"restore-2",
),
core.NewDeleteAction(
v1.SchemeGroupVersion.WithResource("backups"),
td.req.Namespace,
td.req.Spec.BackupName,
),
core.NewPatchAction(
v1.SchemeGroupVersion.WithResource("deletebackuprequests"),
td.req.Namespace,
td.req.Name,
[]byte(`{"status":{"phase":"Processed"}}`),
),
core.NewDeleteCollectionAction(
v1.SchemeGroupVersion.WithResource("deletebackuprequests"),
td.req.Namespace,
pkgbackup.NewDeleteBackupRequestListOptions(td.req.Spec.BackupName, "uid"),
),
}

velerotest.CompareActions(t, expectedActions, td.client.Actions())

// Make sure snapshot was deleted
assert.Equal(t, 0, td.volumeSnapshotter.SnapshotsTaken.Len())
})
}

func TestBackupDeletionControllerDeleteExpiredRequests(t *testing.T) {
@@ -33,6 +33,7 @@ import (
velerov1client "github.com/heptio/velero/pkg/generated/clientset/versioned/typed/velero/v1"
informers "github.com/heptio/velero/pkg/generated/informers/externalversions/velero/v1"
listers "github.com/heptio/velero/pkg/generated/listers/velero/v1"
labelValidation "github.com/heptio/velero/pkg/labels"
"github.com/heptio/velero/pkg/persistence"
"github.com/heptio/velero/pkg/plugin/clientmgmt"
)
@@ -218,7 +219,7 @@ func (c *backupSyncController) run() {
if backup.Labels == nil {
backup.Labels = make(map[string]string)
}
backup.Labels[velerov1api.StorageLocationLabel] = backup.Spec.StorageLocation
backup.Labels[velerov1api.StorageLocationLabel] = labelValidation.GetValidLabelName(backup.Spec.StorageLocation)

_, err = c.backupClient.Backups(backup.Namespace).Create(backup)
switch {
@@ -283,7 +284,7 @@ func patchStorageLocation(backup *velerov1api.Backup, client velerov1client.Back
// and a phase of Completed, but no corresponding backup in object storage.
func (c *backupSyncController) deleteOrphanedBackups(locationName string, cloudBackupNames sets.String, log logrus.FieldLogger) {
locationSelector := labels.Set(map[string]string{
velerov1api.StorageLocationLabel: locationName,
velerov1api.StorageLocationLabel: labelValidation.GetValidLabelName(locationName),
}).AsSelector()

backups, err := c.backupLister.Backups(c.namespace).List(locationSelector)

0 comments on commit d4df312

Please sign in to comment.
You can’t perform that action at this time.