From 34c26dd476f78eeec00ba5085d4f5031d732451b Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Tue, 14 Jan 2025 16:02:56 +0800 Subject: [PATCH] avoid to create new repo when BSL is readonly Signed-off-by: Lyndon-Li --- changelogs/unreleased/8615-Lyndon-Li | 1 + pkg/repository/provider/unified_repo.go | 8 ++ pkg/repository/provider/unified_repo_test.go | 84 +++++++++++++++++++- 3 files changed, 89 insertions(+), 4 deletions(-) create mode 100644 changelogs/unreleased/8615-Lyndon-Li diff --git a/changelogs/unreleased/8615-Lyndon-Li b/changelogs/unreleased/8615-Lyndon-Li new file mode 100644 index 0000000000..993c1b1c44 --- /dev/null +++ b/changelogs/unreleased/8615-Lyndon-Li @@ -0,0 +1 @@ +Fix issue #8091, avoid to create new repo when BSL is readonly \ No newline at end of file diff --git a/pkg/repository/provider/unified_repo.go b/pkg/repository/provider/unified_repo.go index 6191c44528..42f100fa97 100644 --- a/pkg/repository/provider/unified_repo.go +++ b/pkg/repository/provider/unified_repo.go @@ -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)), @@ -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") diff --git a/pkg/repository/provider/unified_repo_test.go b/pkg/repository/provider/unified_repo_test.go index 6f87858489..8c9203b5bf 100644 --- a/pkg/repository/provider/unified_repo_test.go +++ b/pkg/repository/provider/unified_repo_test.go @@ -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" @@ -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 @@ -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 }{ { @@ -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{ @@ -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{ @@ -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 { @@ -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{}, }) @@ -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 @@ -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", @@ -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{}, })