Skip to content

Commit

Permalink
Merge pull request #8615 from Lyndon-Li/avoid-creating-repo-when-bsl-…
Browse files Browse the repository at this point in the history 
…is-readonly

Avoid to create new repo when BSL is readonly
  • Loading branch information
@Lyndon-Li
Lyndon-Li authored Jan 15, 2025
2 parents 1d3af6d + 34c26dd commit 0543750
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 4 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/8615-Lyndon-Li
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix issue #8091, avoid to create new repo when BSL is readonly
8 changes: 8 additions & 0 deletions pkg/repository/provider/unified_repo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ func (urp *unifiedRepoProvider) InitRepo(ctx context.Context, param RepoParam) e

log.Debug("Start to init repo")

if param.BackupLocation.Spec.AccessMode == velerov1api.BackupStorageLocationAccessModeReadOnly {
return errors.Errorf("cannot create new backup repo for read-only backup storage location %s/%s", param.BackupLocation.Namespace, param.BackupLocation.Name)
}

repoOption, err := udmrepo.NewRepoOptions(
udmrepo.WithPassword(urp, param),
udmrepo.WithConfigFile(urp.workPath, string(param.BackupRepo.UID)),
Expand Down Expand Up @@ -193,6 +197,10 @@ func (urp *unifiedRepoProvider) PrepareRepo(ctx context.Context, param RepoParam
return errors.Wrap(err, "error to connect to backup repo")
}

if param.BackupLocation.Spec.AccessMode == velerov1api.BackupStorageLocationAccessModeReadOnly {
return errors.Errorf("cannot create new backup repo for read-only backup storage location %s/%s", param.BackupLocation.Namespace, param.BackupLocation.Name)
}

err = urp.repoService.Init(ctx, *repoOption, true)
if err != nil {
return errors.Wrap(err, "error to create backup repo")
Expand Down
84 changes: 80 additions & 4 deletions pkg/repository/provider/unified_repo_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/stretchr/testify/require"

corev1api "k8s.io/api/core/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"

velerocredentials "github.com/vmware-tanzu/velero/internal/credentials"
credmock "github.com/vmware-tanzu/velero/internal/credentials/mocks"
Expand Down Expand Up @@ -600,6 +601,13 @@ func TestGetStoreOptions(t *testing.T) {
}

func TestPrepareRepo(t *testing.T) {
bsl := velerov1api.BackupStorageLocation{
ObjectMeta: v1.ObjectMeta{
Name: "fake-bsl",
Namespace: velerov1api.DefaultNamespace,
},
}

testCases := []struct {
name string
funcTable localFuncTable
Expand All @@ -608,6 +616,7 @@ func TestPrepareRepo(t *testing.T) {
retFuncInit func(context.Context, udmrepo.RepoOptions, bool) error
credStoreReturn string
credStoreError error
readOnlyBSL bool
expectedErr string
}{
{
Expand Down Expand Up @@ -655,7 +664,29 @@ func TestPrepareRepo(t *testing.T) {
},
},
{
name: "initialize fail",
name: "bsl is readonly",
readOnlyBSL: true,
getter: new(credmock.SecretStore),
credStoreReturn: "fake-password",
funcTable: localFuncTable{
getStorageVariables: func(*velerov1api.BackupStorageLocation, string, string, map[string]string) (map[string]string, error) {
return map[string]string{}, nil
},
getStorageCredentials: func(*velerov1api.BackupStorageLocation, velerocredentials.FileStore) (map[string]string, error) {
return map[string]string{}, nil
},
},
repoService: new(reposervicenmocks.BackupRepoService),
retFuncInit: func(ctx context.Context, repoOption udmrepo.RepoOptions, createNew bool) error {
if !createNew {
return repo.ErrRepositoryNotInitialized
}
return errors.New("fake-error-2")
},
expectedErr: "cannot create new backup repo for read-only backup storage location velero/fake-bsl",
},
{
name: "connect fail",
getter: new(credmock.SecretStore),
credStoreReturn: "fake-password",
funcTable: localFuncTable{
Expand All @@ -676,7 +707,7 @@ func TestPrepareRepo(t *testing.T) {
expectedErr: "error to connect to backup repo: fake-error-1",
},
{
name: "not initialize",
name: "initialize error",
getter: new(credmock.SecretStore),
credStoreReturn: "fake-password",
funcTable: localFuncTable{
Expand All @@ -696,6 +727,26 @@ func TestPrepareRepo(t *testing.T) {
},
expectedErr: "error to create backup repo: fake-error-2",
},
{
name: "initialize succeed",
getter: new(credmock.SecretStore),
credStoreReturn: "fake-password",
funcTable: localFuncTable{
getStorageVariables: func(*velerov1api.BackupStorageLocation, string, string, map[string]string) (map[string]string, error) {
return map[string]string{}, nil
},
getStorageCredentials: func(*velerov1api.BackupStorageLocation, velerocredentials.FileStore) (map[string]string, error) {
return map[string]string{}, nil
},
},
repoService: new(reposervicenmocks.BackupRepoService),
retFuncInit: func(ctx context.Context, repoOption udmrepo.RepoOptions, createNew bool) error {
if !createNew {
return repo.ErrRepositoryNotInitialized
}
return nil
},
},
}

for _, tc := range testCases {
Expand All @@ -718,8 +769,14 @@ func TestPrepareRepo(t *testing.T) {

tc.repoService.On("Init", mock.Anything, mock.Anything, mock.Anything).Return(tc.retFuncInit)

if tc.readOnlyBSL {
bsl.Spec.AccessMode = velerov1api.BackupStorageLocationAccessModeReadOnly
} else {
bsl.Spec.AccessMode = velerov1api.BackupStorageLocationAccessModeReadWrite
}

err := urp.PrepareRepo(context.Background(), RepoParam{
BackupLocation: &velerov1api.BackupStorageLocation{},
BackupLocation: &bsl,
BackupRepo: &velerov1api.BackupRepository{},
})

Expand Down Expand Up @@ -1037,6 +1094,13 @@ func TestBatchForget(t *testing.T) {
}

func TestInitRepo(t *testing.T) {
bsl := velerov1api.BackupStorageLocation{
ObjectMeta: v1.ObjectMeta{
Name: "fake-bsl",
Namespace: velerov1api.DefaultNamespace,
},
}

testCases := []struct {
name string
funcTable localFuncTable
Expand All @@ -1045,8 +1109,14 @@ func TestInitRepo(t *testing.T) {
retFuncInit interface{}
credStoreReturn string
credStoreError error
readOnlyBSL bool
expectedErr string
}{
{
name: "bsl is readonly",
readOnlyBSL: true,
expectedErr: "cannot create new backup repo for read-only backup storage location velero/fake-bsl",
},
{
name: "get repo option fail",
expectedErr: "error to get repo options: error to get repo password: invalid credentials interface",
Expand Down Expand Up @@ -1110,8 +1180,14 @@ func TestInitRepo(t *testing.T) {
tc.repoService.On("Init", mock.Anything, mock.Anything, mock.Anything).Return(tc.retFuncInit)
}

if tc.readOnlyBSL {
bsl.Spec.AccessMode = velerov1api.BackupStorageLocationAccessModeReadOnly
} else {
bsl.Spec.AccessMode = velerov1api.BackupStorageLocationAccessModeReadWrite
}

err := urp.InitRepo(context.Background(), RepoParam{
BackupLocation: &velerov1api.BackupStorageLocation{},
BackupLocation: &bsl,
BackupRepo: &velerov1api.BackupRepository{},
})

Expand Down

0 comments on commit 0543750

Please sign in to comment.