diff --git a/pkg/repository/manager/manager.go b/pkg/repository/manager/manager.go index f8b10db5e..f25a1cd29 100644 --- a/pkg/repository/manager/manager.go +++ b/pkg/repository/manager/manager.go @@ -231,11 +231,11 @@ func (m *manager) Forget(ctx context.Context, repo *velerov1api.BackupRepository return errors.WithStack(err) } - if err := prd.BoostRepoConnect(context.Background(), param); err != nil { + if err := prd.BoostRepoConnect(ctx, param); err != nil { return errors.WithStack(err) } - return prd.Forget(context.Background(), snapshot, param) + return prd.Forget(ctx, snapshot, param) } func (m *manager) BatchForget(ctx context.Context, repo *velerov1api.BackupRepository, snapshots []string) []error { @@ -254,15 +254,15 @@ func (m *manager) BatchForget(ctx context.Context, repo *velerov1api.BackupRepos // Disable FIPS-140 compliance check, because Kopia doesn't support FIPS-140 yet. var connectErr error fips140.WithoutEnforcement(func() { - connectErr = prd.BoostRepoConnect(context.Background(), param) + connectErr = prd.BoostRepoConnect(ctx, param) }) if connectErr != nil { - return []error{errors.WithStack(err)} + return []error{errors.WithStack(connectErr)} } forgetErr := make([]error, 0) fips140.WithoutEnforcement(func() { - forgetErr = prd.BatchForget(context.Background(), snapshots, param) + forgetErr = prd.BatchForget(ctx, snapshots, param) }) return forgetErr } diff --git a/pkg/repository/manager/manager_test.go b/pkg/repository/manager/manager_test.go index 88d8db481..585b7e860 100644 --- a/pkg/repository/manager/manager_test.go +++ b/pkg/repository/manager/manager_test.go @@ -17,15 +17,73 @@ limitations under the License. package repository import ( + "context" "testing" + "github.com/cockroachdb/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/runtime" kbclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + "github.com/vmware-tanzu/velero/pkg/repository" + "github.com/vmware-tanzu/velero/pkg/repository/provider" ) +// fakeProvider is a minimal provider.Provider implementation used to +// observe the context passed by the manager into the provider calls. +type fakeProvider struct { + provider.Provider + + gotConnectCtx context.Context + gotForgetCtx context.Context + + connectErr error + forgetErr error + forgetErrs []error +} + +func (f *fakeProvider) BoostRepoConnect(ctx context.Context, _ provider.RepoParam) error { + f.gotConnectCtx = ctx + return f.connectErr +} + +func (f *fakeProvider) Forget(ctx context.Context, _ string, _ provider.RepoParam) error { + f.gotForgetCtx = ctx + return f.forgetErr +} + +func (f *fakeProvider) BatchForget(ctx context.Context, _ []string, _ provider.RepoParam) []error { + f.gotForgetCtx = ctx + return f.forgetErrs +} + +func newTestManager(t *testing.T, prd provider.Provider) *manager { + t.Helper() + + scheme := runtime.NewScheme() + require.NoError(t, velerov1.AddToScheme(scheme)) + + bsl := &velerov1.BackupStorageLocation{} + bsl.Namespace = "velero" + bsl.Name = "fake-bsl" + + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(bsl).Build() + + mgr := NewManager("velero", fakeClient, repository.NewRepoLocker(), nil, nil, nil).(*manager) + mgr.providers[velerov1.BackupRepositoryTypeKopia] = prd + return mgr +} + +func newTestRepo() *velerov1.BackupRepository { + repo := &velerov1.BackupRepository{} + repo.Spec.RepositoryType = velerov1.BackupRepositoryTypeKopia + repo.Spec.BackupStorageLocation = "fake-bsl" + return repo +} + func TestGetRepositoryProvider(t *testing.T) { var fakeClient kbclient.Client mgr := NewManager("", fakeClient, nil, nil, nil, nil).(*manager) @@ -62,3 +120,56 @@ func TestGetRepositoryConfigProvider(t *testing.T) { _, err = mgr.getRepositoryProvider("restic") require.Error(t, err) } + +func TestForgetPropagatesCallerContext(t *testing.T) { + prd := &fakeProvider{} + mgr := newTestManager(t, prd) + + type ctxKeyType string + key := ctxKeyType("test-key") + ctx, cancel := context.WithCancel(context.WithValue(context.Background(), key, "test-value")) + defer cancel() + + err := mgr.Forget(ctx, newTestRepo(), "snapshot-1") + require.NoError(t, err) + + require.NotNil(t, prd.gotConnectCtx) + require.NotNil(t, prd.gotForgetCtx) + assert.Equal(t, "test-value", prd.gotConnectCtx.Value(key)) + assert.Equal(t, "test-value", prd.gotForgetCtx.Value(key)) + + // canceling the caller's context must be observed by the provider calls, + // proving the manager no longer substitutes context.Background(). + cancel() + require.Error(t, prd.gotConnectCtx.Err()) + require.Error(t, prd.gotForgetCtx.Err()) +} + +func TestBatchForgetPropagatesCallerContext(t *testing.T) { + prd := &fakeProvider{forgetErrs: []error{}} + mgr := newTestManager(t, prd) + + type ctxKeyType string + key := ctxKeyType("test-key") + ctx, cancel := context.WithCancel(context.WithValue(context.Background(), key, "test-value")) + defer cancel() + + errs := mgr.BatchForget(ctx, newTestRepo(), []string{"snapshot-1", "snapshot-2"}) + require.Empty(t, errs) + + require.NotNil(t, prd.gotConnectCtx) + require.NotNil(t, prd.gotForgetCtx) + assert.Equal(t, "test-value", prd.gotConnectCtx.Value(key)) + assert.Equal(t, "test-value", prd.gotForgetCtx.Value(key)) +} + +func TestBatchForgetReturnsConnectError(t *testing.T) { + connectErr := errors.New("boom: connection refused") + prd := &fakeProvider{connectErr: connectErr} + mgr := newTestManager(t, prd) + + errs := mgr.BatchForget(context.Background(), newTestRepo(), []string{"snapshot-1"}) + + require.Len(t, errs, 1) + require.ErrorContains(t, errs[0], "boom: connection refused") +}