vmware-tanzu
/
velero
mirror of https://github.com/vmware-tanzu/velero.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

strip leading/trailing slashes from BSL bucket & prefix vals (#1694) 

* strip leading/trailing slashes from BSL bucket & prefix vals

Signed-off-by: Steve Kriss <krisss@vmware.com>
pull/1721/head
Steve Kriss 2019-07-31 09:27:12 -06:00 committed by KubeKween
parent 4543258970
commit f2d06bc5e9
6 changed files with 129 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
changelogs/unreleased/1694-skriss Normal file
View File

@ -0,0 +1 @@
error if backup storage location's Bucket field also contains a prefix, and gracefully handle leading/trailing slashes on Bucket and Prefix fields.

15
pkg/builder/backup_storage_location_builder.go
View File

@ -63,12 +63,21 @@ func (b *BackupStorageLocationBuilder) Provider(name string) *BackupStorageLocat
return b
}
// ObjectStorage sets the BackupStorageLocation's object storage.
func (b *BackupStorageLocationBuilder) ObjectStorage(bucketName string) *BackupStorageLocationBuilder {
// Bucket sets the BackupStorageLocation's object storage bucket.
func (b *BackupStorageLocationBuilder) Bucket(val string) *BackupStorageLocationBuilder {
if b.object.Spec.StorageType.ObjectStorage == nil {
b.object.Spec.StorageType.ObjectStorage = new(velerov1api.ObjectStorageLocation)
}
b.object.Spec.ObjectStorage.Bucket = bucketName
b.object.Spec.ObjectStorage.Bucket = val
return b
}
// Prefix sets the BackupStorageLocation's object storage prefix.
func (b *BackupStorageLocationBuilder) Prefix(val string) *BackupStorageLocationBuilder {
if b.object.Spec.StorageType.ObjectStorage == nil {
b.object.Spec.StorageType.ObjectStorage = new(velerov1api.ObjectStorageLocation)
}
b.object.Spec.ObjectStorage.Prefix = val
return b
}

6
pkg/controller/backup_controller_test.go
View File

@ -311,7 +311,7 @@ func TestDefaultBackupTTL(t *testing.T) {
}
func TestProcessBackupCompletions(t *testing.T) {
defaultBackupLocation := builder.ForBackupStorageLocation("velero", "loc-1").ObjectStorage("store-1").Result()
defaultBackupLocation := builder.ForBackupStorageLocation("velero", "loc-1").Bucket("store-1").Result()
now, err := time.Parse(time.RFC1123Z, time.RFC1123Z)
require.NoError(t, err)
@ -357,7 +357,7 @@ func TestProcessBackupCompletions(t *testing.T) {
{
name: "backup with a specific backup location keeps it",
backup: defaultBackup().StorageLocation("alt-loc").Result(),
backupLocation: builder.ForBackupStorageLocation("velero", "alt-loc").ObjectStorage("store-1").Result(),
backupLocation: builder.ForBackupStorageLocation("velero", "alt-loc").Bucket("store-1").Result(),
expectedResult: &velerov1api.Backup{
TypeMeta: metav1.TypeMeta{
Kind: "Backup",
@ -386,7 +386,7 @@ func TestProcessBackupCompletions(t *testing.T) {
name: "backup for a location with ReadWrite access mode gets processed",
backup: defaultBackup().StorageLocation("read-write").Result(),
backupLocation: builder.ForBackupStorageLocation("velero", "read-write").
ObjectStorage("store-1").
Bucket("store-1").
AccessMode(velerov1api.BackupStorageLocationAccessModeReadWrite).
Result(),
expectedResult: &velerov1api.Backup{

6
pkg/controller/restore_controller_test.go
View File

@ -66,7 +66,7 @@ func TestFetchBackupInfo(t *testing.T) {
{
name: "lister has backup",
backupName: "backup-1",
informerLocations: []*api.BackupStorageLocation{builder.ForBackupStorageLocation("velero", "default").Provider("myCloud").ObjectStorage("bucket").Result()},
informerLocations: []*api.BackupStorageLocation{builder.ForBackupStorageLocation("velero", "default").Provider("myCloud").Bucket("bucket").Result()},
informerBackups: []*api.Backup{defaultBackup().StorageLocation("default").Result()},
expectedRes: defaultBackup().StorageLocation("default").Result(),
},
@ -74,7 +74,7 @@ func TestFetchBackupInfo(t *testing.T) {
name: "lister does not have a backup, but backupSvc does",
backupName: "backup-1",
backupStoreBackup: defaultBackup().StorageLocation("default").Result(),
informerLocations: []*api.BackupStorageLocation{builder.ForBackupStorageLocation("velero", "default").Provider("myCloud").ObjectStorage("bucket").Result()},
informerLocations: []*api.BackupStorageLocation{builder.ForBackupStorageLocation("velero", "default").Provider("myCloud").Bucket("bucket").Result()},
informerBackups: []*api.Backup{defaultBackup().StorageLocation("default").Result()},
expectedRes: defaultBackup().StorageLocation("default").Result(),
},
@ -227,7 +227,7 @@ func TestProcessQueueItemSkips(t *testing.T) {
}
func TestProcessQueueItem(t *testing.T) {
defaultStorageLocation := builder.ForBackupStorageLocation("velero", "default").Provider("myCloud").ObjectStorage("bucket").Result()
defaultStorageLocation := builder.ForBackupStorageLocation("velero", "default").Provider("myCloud").Bucket("bucket").Result()
tests := []struct {
name string

27
pkg/persistence/object_store.go
View File

@ -91,9 +91,15 @@ func NewObjectBackupStore(location *velerov1api.BackupStorageLocation, objectSto
return nil, errors.New("object storage provider name must not be empty")
}
objectStore, err := objectStoreGetter.GetObjectStore(location.Spec.Provider)
if err != nil {
return nil, err
// trim off any leading/trailing slashes
bucket := strings.Trim(location.Spec.ObjectStorage.Bucket, "/")
prefix := strings.Trim(location.Spec.ObjectStorage.Prefix, "/")
// if there are any slashes in the middle of 'bucket', the user
// probably put <bucket>/<prefix> in the bucket field, which we
// don't support.
if strings.Contains(bucket, "/") {
return nil, errors.Errorf("backup storage location's bucket name %q must not contain a '/' (if using a prefix, put it in the 'Prefix' field instead)", location.Spec.ObjectStorage.Bucket)
}
// add the bucket name to the config map so that object stores can use
@ -103,7 +109,12 @@ func NewObjectBackupStore(location *velerov1api.BackupStorageLocation, objectSto
if location.Spec.Config == nil {
location.Spec.Config = make(map[string]string)
}
location.Spec.Config["bucket"] = location.Spec.ObjectStorage.Bucket
location.Spec.Config["bucket"] = bucket
}
objectStore, err := objectStoreGetter.GetObjectStore(location.Spec.Provider)
if err != nil {
return nil, err
}
if err := objectStore.Init(location.Spec.Config); err != nil {
@ -111,14 +122,14 @@ func NewObjectBackupStore(location *velerov1api.BackupStorageLocation, objectSto
}
log := logger.WithFields(logrus.Fields(map[string]interface{}{
"bucket": location.Spec.ObjectStorage.Bucket,
"prefix": location.Spec.ObjectStorage.Prefix,
"bucket": bucket,
"prefix": prefix,
}))
return &objectBackupStore{
objectStore: objectStore,
bucket: location.Spec.ObjectStorage.Bucket,
layout: NewObjectStoreLayout(location.Spec.ObjectStorage.Prefix),
bucket: bucket,
layout: NewObjectStoreLayout(prefix),
logger: log,
}, nil
}

105
pkg/persistence/object_store_test.go
View File

@ -35,8 +35,10 @@ import (
"k8s.io/apimachinery/pkg/runtime"
velerov1api "github.com/heptio/velero/pkg/apis/velero/v1"
"github.com/heptio/velero/pkg/builder"
"github.com/heptio/velero/pkg/cloudprovider"
cloudprovidermocks "github.com/heptio/velero/pkg/cloudprovider/mocks"
"github.com/heptio/velero/pkg/plugin/velero"
"github.com/heptio/velero/pkg/util/encode"
velerotest "github.com/heptio/velero/pkg/util/test"
"github.com/heptio/velero/pkg/volume"
@ -170,8 +172,8 @@ func TestListBackups(t *testing.T) {
{
name: "normal case",
storageData: map[string][]byte{
"backups/backup-1/velero-backup.json": encodeToBytes(&velerov1api.Backup{ObjectMeta: metav1.ObjectMeta{Name: "backup-1"}}),
"backups/backup-2/velero-backup.json": encodeToBytes(&velerov1api.Backup{ObjectMeta: metav1.ObjectMeta{Name: "backup-2"}}),
"backups/backup-1/velero-backup.json": encodeToBytes(builder.ForBackup("", "backup-1").Result()),
"backups/backup-2/velero-backup.json": encodeToBytes(builder.ForBackup("", "backup-2").Result()),
},
expectedRes: []string{"backup-1", "backup-2"},
},
@ -179,8 +181,8 @@ func TestListBackups(t *testing.T) {
name: "normal case with backup store prefix",
prefix: "velero-backups/",
storageData: map[string][]byte{
"velero-backups/backups/backup-1/velero-backup.json": encodeToBytes(&velerov1api.Backup{ObjectMeta: metav1.ObjectMeta{Name: "backup-1"}}),
"velero-backups/backups/backup-2/velero-backup.json": encodeToBytes(&velerov1api.Backup{ObjectMeta: metav1.ObjectMeta{Name: "backup-2"}}),
"velero-backups/backups/backup-1/velero-backup.json": encodeToBytes(builder.ForBackup("", "backup-1").Result()),
"velero-backups/backups/backup-2/velero-backup.json": encodeToBytes(builder.ForBackup("", "backup-2").Result()),
},
expectedRes: []string{"backup-1", "backup-2"},
},
@ -335,16 +337,7 @@ func TestGetBackupMetadata(t *testing.T) {
name: "metadata file returns correctly",
backupName: "foo",
key: "backups/foo/velero-backup.json",
obj: &velerov1api.Backup{
TypeMeta: metav1.TypeMeta{
Kind: "Backup",
APIVersion: velerov1api.SchemeGroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Namespace: velerov1api.DefaultNamespace,
Name: "foo",
},
},
obj: builder.ForBackup(velerov1api.DefaultNamespace, "foo").Result(),
},
{
name: "no metadata file returns an error",
@ -558,6 +551,90 @@ func TestGetDownloadURL(t *testing.T) {
}
}
type objectStoreGetter map[string]velero.ObjectStore
func (osg objectStoreGetter) GetObjectStore(provider string) (velero.ObjectStore, error) {
res, ok := osg[provider]
if !ok {
return nil, errors.New("object store not found")
}
return res, nil
}
// TestNewObjectBackupStore runs the NewObjectBackupStore constructor and ensures
// that an ObjectBackupStore is constructed correctly or an appropriate error is
// returned.
func TestNewObjectBackupStore(t *testing.T) {
tests := []struct {
name string
location *velerov1api.BackupStorageLocation
objectStoreGetter objectStoreGetter
wantBucket string
wantPrefix string
wantErr string
}{
{
name: "location with no ObjectStorage field results in an error",
location: new(velerov1api.BackupStorageLocation),
wantErr: "backup storage location does not use object storage",
},
{
name: "location with no Provider field results in an error",
location: builder.ForBackupStorageLocation("", "").Bucket("").Result(),
wantErr: "object storage provider name must not be empty",
},
{
name: "location with a Bucket field with a '/' in the middle results in an error",
location: builder.ForBackupStorageLocation("", "").Provider("provider-1").Bucket("invalid/bucket").Result(),
wantErr: "backup storage location's bucket name \"invalid/bucket\" must not contain a '/' (if using a prefix, put it in the 'Prefix' field instead)",
},
{
name: "when Bucket has a leading and trailing slash, they are both stripped",
location: builder.ForBackupStorageLocation("", "").Provider("provider-1").Bucket("/bucket/").Result(),
objectStoreGetter: objectStoreGetter{
"provider-1": cloudprovider.NewInMemoryObjectStore("bucket"),
},
wantBucket: "bucket",
},
{
name: "when Prefix has a leading and trailing slash, the leading slash is stripped and the trailing slash is left",
location: builder.ForBackupStorageLocation("", "").Provider("provider-1").Bucket("bucket").Prefix("/prefix/").Result(),
objectStoreGetter: objectStoreGetter{
"provider-1": cloudprovider.NewInMemoryObjectStore("bucket"),
},
wantBucket: "bucket",
wantPrefix: "prefix/",
},
{
name: "when Prefix has no leading or trailing slash, a trailing slash is added",
location: builder.ForBackupStorageLocation("", "").Provider("provider-1").Bucket("bucket").Prefix("prefix").Result(),
objectStoreGetter: objectStoreGetter{
"provider-1": cloudprovider.NewInMemoryObjectStore("bucket"),
},
wantBucket: "bucket",
wantPrefix: "prefix/",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
res, err := NewObjectBackupStore(tc.location, tc.objectStoreGetter, velerotest.NewLogger())
if tc.wantErr != "" {
require.Equal(t, tc.wantErr, err.Error())
} else {
require.Nil(t, err)
store, ok := res.(*objectBackupStore)
require.True(t, ok)
assert.Equal(t, tc.wantBucket, store.bucket)
assert.Equal(t, tc.wantPrefix, store.layout.rootPrefix)
}
})
}
}
func encodeToBytes(obj runtime.Object) []byte {
res, err := encode.Encode(obj, "json")
if err != nil {