From bac2ce2bc3de5dadc997126e4343a15c04e80a5e Mon Sep 17 00:00:00 2001 From: Idan Novogroder Date: Mon, 19 Feb 2024 18:22:42 +0200 Subject: [PATCH] Skip actions for read-only repositories --- go.mod | 1 + go.sum | 2 + pkg/graveler/graveler.go | 435 ++++++++++++++++++---------------- pkg/graveler/graveler_test.go | 180 +++++++++----- 4 files changed, 356 insertions(+), 262 deletions(-) diff --git a/go.mod b/go.mod index 8cbb2078d03..3b26500fae8 100644 --- a/go.mod +++ b/go.mod @@ -93,6 +93,7 @@ require ( github.com/hashicorp/go-retryablehttp v0.7.4 github.com/hashicorp/go-version v1.6.0 github.com/jackc/pgx/v5 v5.4.3 + github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 github.com/puzpuzpuz/xsync v1.5.2 go.uber.org/ratelimit v0.3.0 ) diff --git a/go.sum b/go.sum index f4519df6679..ec37b242b5b 100644 --- a/go.sum +++ b/go.sum @@ -871,6 +871,8 @@ github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3Rllmb github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9Gz0M= github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk= github.com/modocache/gover v0.0.0-20171022184752-b58185e213c5/go.mod h1:caMODM3PzxT8aQXRPkAt8xlV/e7d7w8GM5g0fa5F0D8= +github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 h1:RWengNIwukTxcDr9M+97sNutRR1RKhG96O6jWumTTnw= +github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826/go.mod h1:TaXosZuwdSHYgviHp1DAtfrULt5eUgsSMsZf+YrPgl8= github.com/montanaflynn/stats v0.6.6/go.mod h1:etXPPgVO6n31NxCd9KQUMvCM+ve0ruNzt6R8Bnaayow= github.com/moul/http2curl v1.0.0/go.mod h1:8UbvGypXm98wA/IqH45anm5Y2Z6ep6O31QGOAZ3H0fQ= github.com/nats-io/jwt v0.3.0/go.mod h1:fRYCDE99xlTsqUzISS1Bi75UBJ6ljOJQOAAu5VglpSg= diff --git a/pkg/graveler/graveler.go b/pkg/graveler/graveler.go index 547b9799125..11047ab3d29 100644 --- a/pkg/graveler/graveler.go +++ b/pkg/graveler/graveler.go @@ -1196,21 +1196,24 @@ func (g *Graveler) CreateBranch(ctx context.Context, repository *RepositoryRecor SealedTokens: make([]StagingToken, 0), } storageNamespace := repository.StorageNamespace - preRunID := g.hooks.NewRunID() - err = g.hooks.PreCreateBranchHook(ctx, HookRecord{ - RunID: preRunID, - StorageNamespace: storageNamespace, - EventType: EventTypePreCreateBranch, - SourceRef: ref, - RepositoryID: repository.RepositoryID, - BranchID: branchID, - CommitID: reference.CommitID, - }) - if err != nil { - return nil, &HookAbortError{ - EventType: EventTypePreCreateBranch, - RunID: preRunID, - Err: err, + var preRunID string + if !repository.ReadOnly { + preRunID = g.hooks.NewRunID() + err = g.hooks.PreCreateBranchHook(ctx, HookRecord{ + RunID: preRunID, + StorageNamespace: storageNamespace, + EventType: EventTypePreCreateBranch, + SourceRef: ref, + RepositoryID: repository.RepositoryID, + BranchID: branchID, + CommitID: reference.CommitID, + }) + if err != nil { + return nil, &HookAbortError{ + EventType: EventTypePreCreateBranch, + RunID: preRunID, + Err: err, + } } } @@ -1218,18 +1221,19 @@ func (g *Graveler) CreateBranch(ctx context.Context, repository *RepositoryRecor if err != nil { return nil, fmt.Errorf("set branch '%s' to '%v': %w", branchID, newBranch, err) } - - postRunID := g.hooks.NewRunID() - g.hooks.PostCreateBranchHook(ctx, HookRecord{ - RunID: postRunID, - StorageNamespace: storageNamespace, - EventType: EventTypePostCreateBranch, - SourceRef: ref, - RepositoryID: repository.RepositoryID, - BranchID: branchID, - CommitID: reference.CommitID, - PreRunID: preRunID, - }) + if !repository.ReadOnly { + postRunID := g.hooks.NewRunID() + g.hooks.PostCreateBranchHook(ctx, HookRecord{ + RunID: postRunID, + StorageNamespace: storageNamespace, + EventType: EventTypePostCreateBranch, + SourceRef: ref, + RepositoryID: repository.RepositoryID, + BranchID: branchID, + CommitID: reference.CommitID, + PreRunID: preRunID, + }) + } return &newBranch, nil } @@ -1349,40 +1353,45 @@ func (g *Graveler) CreateTag(ctx context.Context, repository *RepositoryRecord, return err } - preRunID := g.hooks.NewRunID() - err = g.hooks.PreCreateTagHook(ctx, HookRecord{ - RunID: preRunID, - StorageNamespace: storageNamespace, - EventType: EventTypePreCreateTag, - RepositoryID: repository.RepositoryID, - CommitID: commitID, - SourceRef: commitID.Ref(), - TagID: tagID, - }) - if err != nil { - return &HookAbortError{ - EventType: EventTypePreCreateTag, - RunID: preRunID, - Err: err, + var preRunID string + + if !repository.ReadOnly { + preRunID = g.hooks.NewRunID() + err = g.hooks.PreCreateTagHook(ctx, HookRecord{ + RunID: preRunID, + StorageNamespace: storageNamespace, + EventType: EventTypePreCreateTag, + RepositoryID: repository.RepositoryID, + CommitID: commitID, + SourceRef: commitID.Ref(), + TagID: tagID, + }) + if err != nil { + return &HookAbortError{ + EventType: EventTypePreCreateTag, + RunID: preRunID, + Err: err, + } } } - err = g.RefManager.CreateTag(ctx, repository, tagID, commitID) if err != nil { return err } - postRunID := g.hooks.NewRunID() - g.hooks.PostCreateTagHook(ctx, HookRecord{ - RunID: postRunID, - StorageNamespace: storageNamespace, - EventType: EventTypePostCreateTag, - RepositoryID: repository.RepositoryID, - CommitID: commitID, - SourceRef: commitID.Ref(), - TagID: tagID, - PreRunID: preRunID, - }) + if !repository.ReadOnly { + postRunID := g.hooks.NewRunID() + g.hooks.PostCreateTagHook(ctx, HookRecord{ + RunID: postRunID, + StorageNamespace: storageNamespace, + EventType: EventTypePostCreateTag, + RepositoryID: repository.RepositoryID, + CommitID: commitID, + SourceRef: commitID.Ref(), + TagID: tagID, + PreRunID: preRunID, + }) + } return nil } @@ -1403,21 +1412,24 @@ func (g *Graveler) DeleteTag(ctx context.Context, repository *RepositoryRecord, return err } - preRunID := g.hooks.NewRunID() - err = g.hooks.PreDeleteTagHook(ctx, HookRecord{ - RunID: preRunID, - StorageNamespace: storageNamespace, - EventType: EventTypePreDeleteTag, - RepositoryID: repository.RepositoryID, - SourceRef: commitID.Ref(), - CommitID: *commitID, - TagID: tagID, - }) - if err != nil { - return &HookAbortError{ - EventType: EventTypePreDeleteTag, - RunID: preRunID, - Err: err, + var preRunID string + if !repository.ReadOnly { + preRunID = g.hooks.NewRunID() + err = g.hooks.PreDeleteTagHook(ctx, HookRecord{ + RunID: preRunID, + StorageNamespace: storageNamespace, + EventType: EventTypePreDeleteTag, + RepositoryID: repository.RepositoryID, + SourceRef: commitID.Ref(), + CommitID: *commitID, + TagID: tagID, + }) + if err != nil { + return &HookAbortError{ + EventType: EventTypePreDeleteTag, + RunID: preRunID, + Err: err, + } } } @@ -1426,17 +1438,19 @@ func (g *Graveler) DeleteTag(ctx context.Context, repository *RepositoryRecord, return err } - postRunID := g.hooks.NewRunID() - g.hooks.PostDeleteTagHook(ctx, HookRecord{ - RunID: postRunID, - StorageNamespace: storageNamespace, - EventType: EventTypePostDeleteTag, - RepositoryID: repository.RepositoryID, - SourceRef: commitID.Ref(), - CommitID: *commitID, - TagID: tagID, - PreRunID: preRunID, - }) + if !repository.ReadOnly { + postRunID := g.hooks.NewRunID() + g.hooks.PostDeleteTagHook(ctx, HookRecord{ + RunID: postRunID, + StorageNamespace: storageNamespace, + EventType: EventTypePostDeleteTag, + RepositoryID: repository.RepositoryID, + SourceRef: commitID.Ref(), + CommitID: *commitID, + TagID: tagID, + PreRunID: preRunID, + }) + } return nil } @@ -1486,21 +1500,24 @@ func (g *Graveler) DeleteBranch(ctx context.Context, repository *RepositoryRecor commitID := branch.CommitID storageNamespace := repository.StorageNamespace - preRunID := g.hooks.NewRunID() - preHookRecord := HookRecord{ - RunID: preRunID, - StorageNamespace: storageNamespace, - EventType: EventTypePreDeleteBranch, - RepositoryID: repository.RepositoryID, - SourceRef: commitID.Ref(), - BranchID: branchID, - } - err = g.hooks.PreDeleteBranchHook(ctx, preHookRecord) - if err != nil { - return &HookAbortError{ - EventType: EventTypePreDeleteBranch, - RunID: preRunID, - Err: err, + var preRunID string + if !repository.ReadOnly { + preRunID = g.hooks.NewRunID() + preHookRecord := HookRecord{ + RunID: preRunID, + StorageNamespace: storageNamespace, + EventType: EventTypePreDeleteBranch, + RepositoryID: repository.RepositoryID, + SourceRef: commitID.Ref(), + BranchID: branchID, + } + err = g.hooks.PreDeleteBranchHook(ctx, preHookRecord) + if err != nil { + return &HookAbortError{ + EventType: EventTypePreDeleteBranch, + RunID: preRunID, + Err: err, + } } } @@ -1514,16 +1531,18 @@ func (g *Graveler) DeleteBranch(ctx context.Context, repository *RepositoryRecor tokens = append(tokens, branch.StagingToken) g.dropTokens(ctx, tokens...) - postRunID := g.hooks.NewRunID() - g.hooks.PostDeleteBranchHook(ctx, HookRecord{ - RunID: postRunID, - StorageNamespace: storageNamespace, - EventType: EventTypePostDeleteBranch, - RepositoryID: repository.RepositoryID, - SourceRef: commitID.Ref(), - BranchID: branchID, - PreRunID: preRunID, - }) + if !repository.ReadOnly { + postRunID := g.hooks.NewRunID() + g.hooks.PostDeleteBranchHook(ctx, HookRecord{ + RunID: postRunID, + StorageNamespace: storageNamespace, + EventType: EventTypePostDeleteBranch, + RepositoryID: repository.RepositoryID, + SourceRef: commitID.Ref(), + BranchID: branchID, + PreRunID: preRunID, + }) + } return nil } @@ -1994,21 +2013,23 @@ func (g *Graveler) Commit(ctx context.Context, repository *RepositoryRecord, bra commit.Parents = CommitParents{branch.CommitID} } - preRunID = g.hooks.NewRunID() - err = g.hooks.PreCommitHook(ctx, HookRecord{ - RunID: preRunID, - EventType: EventTypePreCommit, - SourceRef: branchID.Ref(), - RepositoryID: repository.RepositoryID, - StorageNamespace: storageNamespace, - BranchID: branchID, - Commit: commit, - }) - if err != nil { - return nil, &HookAbortError{ - EventType: EventTypePreCommit, - RunID: preRunID, - Err: err, + if !repository.ReadOnly { + preRunID = g.hooks.NewRunID() + err = g.hooks.PreCommitHook(ctx, HookRecord{ + RunID: preRunID, + EventType: EventTypePreCommit, + SourceRef: branchID.Ref(), + RepositoryID: repository.RepositoryID, + StorageNamespace: storageNamespace, + BranchID: branchID, + Commit: commit, + }) + if err != nil { + return nil, &HookAbortError{ + EventType: EventTypePreCommit, + RunID: preRunID, + Err: err, + } } } @@ -2062,23 +2083,25 @@ func (g *Graveler) Commit(ctx context.Context, repository *RepositoryRecord, bra g.dropTokens(ctx, sealedToDrop...) - postRunID := g.hooks.NewRunID() - err = g.hooks.PostCommitHook(ctx, HookRecord{ - EventType: EventTypePostCommit, - RunID: postRunID, - RepositoryID: repository.RepositoryID, - StorageNamespace: storageNamespace, - SourceRef: newCommitID.Ref(), - BranchID: branchID, - Commit: commit, - CommitID: newCommitID, - PreRunID: preRunID, - }) - if err != nil { - g.log(ctx).WithError(err). - WithField("run_id", postRunID). - WithField("pre_run_id", preRunID). - Error("Post-commit hook failed") + if !repository.ReadOnly { + postRunID := g.hooks.NewRunID() + err = g.hooks.PostCommitHook(ctx, HookRecord{ + EventType: EventTypePostCommit, + RunID: postRunID, + RepositoryID: repository.RepositoryID, + StorageNamespace: storageNamespace, + SourceRef: newCommitID.Ref(), + BranchID: branchID, + Commit: commit, + CommitID: newCommitID, + PreRunID: preRunID, + }) + if err != nil { + g.log(ctx).WithError(err). + WithField("run_id", postRunID). + WithField("pre_run_id", preRunID). + Error("Post-commit hook failed") + } } return newCommitID, nil } @@ -2769,21 +2792,23 @@ func (g *Graveler) Merge(ctx context.Context, repository *RepositoryRecord, dest } metadata[MergeStrategyMetadataKey] = mergeStrategyString[mergeStrategy] commit.Metadata = metadata - preRunID = g.hooks.NewRunID() - err = g.hooks.PreMergeHook(ctx, HookRecord{ - EventType: EventTypePreMerge, - RunID: preRunID, - RepositoryID: repository.RepositoryID, - StorageNamespace: storageNamespace, - BranchID: destination, - SourceRef: fromCommit.CommitID.Ref(), - Commit: commit, - }) - if err != nil { - return nil, &HookAbortError{ - EventType: EventTypePreMerge, - RunID: preRunID, - Err: err, + if !repository.ReadOnly { + preRunID = g.hooks.NewRunID() + err = g.hooks.PreMergeHook(ctx, HookRecord{ + EventType: EventTypePreMerge, + RunID: preRunID, + RepositoryID: repository.RepositoryID, + StorageNamespace: storageNamespace, + BranchID: destination, + SourceRef: fromCommit.CommitID.Ref(), + Commit: commit, + }) + if err != nil { + return nil, &HookAbortError{ + EventType: EventTypePreMerge, + RunID: preRunID, + Err: err, + } } } commitID, err = g.RefManager.AddCommit(ctx, repository, commit) @@ -2801,25 +2826,27 @@ func (g *Graveler) Merge(ctx context.Context, repository *RepositoryRecord, dest } g.dropTokens(ctx, tokensToDrop...) - postRunID := g.hooks.NewRunID() - err = g.hooks.PostMergeHook(ctx, HookRecord{ - EventType: EventTypePostMerge, - RunID: postRunID, - RepositoryID: repository.RepositoryID, - StorageNamespace: storageNamespace, - BranchID: destination, + if !repository.ReadOnly { + postRunID := g.hooks.NewRunID() + err = g.hooks.PostMergeHook(ctx, HookRecord{ + EventType: EventTypePostMerge, + RunID: postRunID, + RepositoryID: repository.RepositoryID, + StorageNamespace: storageNamespace, + BranchID: destination, - SourceRef: commitID.Ref(), - Commit: commit, - CommitID: commitID, - PreRunID: preRunID, - }) - if err != nil { - g.log(ctx). - WithError(err). - WithField("run_id", postRunID). - WithField("pre_run_id", preRunID). - Error("Post-merge hook failed") + SourceRef: commitID.Ref(), + Commit: commit, + CommitID: commitID, + PreRunID: preRunID, + }) + if err != nil { + g.log(ctx). + WithError(err). + WithField("run_id", postRunID). + WithField("pre_run_id", preRunID). + Error("Post-merge hook failed") + } } return commitID, nil } @@ -2914,21 +2941,23 @@ func (g *Graveler) Import(ctx context.Context, repository *RepositoryRecord, des commit.Metadata = make(Metadata) } commit.Metadata[MergeStrategyMetadataKey] = MergeStrategySrcWinsStr - preRunID = g.hooks.NewRunID() - err = g.hooks.PreCommitHook(ctx, HookRecord{ - RunID: preRunID, - EventType: EventTypePreCommit, - SourceRef: destination.Ref(), - RepositoryID: repository.RepositoryID, - StorageNamespace: storageNamespace, - BranchID: destination, - Commit: commit, - }) - if err != nil { - return nil, &HookAbortError{ - EventType: EventTypePreCommit, - RunID: preRunID, - Err: err, + if !repository.ReadOnly { + preRunID = g.hooks.NewRunID() + err = g.hooks.PreCommitHook(ctx, HookRecord{ + RunID: preRunID, + EventType: EventTypePreCommit, + SourceRef: destination.Ref(), + RepositoryID: repository.RepositoryID, + StorageNamespace: storageNamespace, + BranchID: destination, + Commit: commit, + }) + if err != nil { + return nil, &HookAbortError{ + EventType: EventTypePreCommit, + RunID: preRunID, + Err: err, + } } } @@ -2947,23 +2976,25 @@ func (g *Graveler) Import(ctx context.Context, repository *RepositoryRecord, des } g.dropTokens(ctx, tokensToDrop...) - postRunID := g.hooks.NewRunID() - err = g.hooks.PostCommitHook(ctx, HookRecord{ - EventType: EventTypePostCommit, - RunID: postRunID, - RepositoryID: repository.RepositoryID, - StorageNamespace: storageNamespace, - SourceRef: commitID.Ref(), - BranchID: destination, - Commit: commit, - CommitID: commitID, - PreRunID: preRunID, - }) - if err != nil { - g.log(ctx).WithError(err). - WithField("run_id", postRunID). - WithField("pre_run_id", preRunID). - Error("Post-commit hook failed") + if !repository.ReadOnly { + postRunID := g.hooks.NewRunID() + err = g.hooks.PostCommitHook(ctx, HookRecord{ + EventType: EventTypePostCommit, + RunID: postRunID, + RepositoryID: repository.RepositoryID, + StorageNamespace: storageNamespace, + SourceRef: commitID.Ref(), + BranchID: destination, + Commit: commit, + CommitID: commitID, + PreRunID: preRunID, + }) + if err != nil { + g.log(ctx).WithError(err). + WithField("run_id", postRunID). + WithField("pre_run_id", preRunID). + Error("Post-commit hook failed") + } } if err = g.retryRepoMetadataUpdate(ctx, repository, func(metadata RepositoryMetadata) (RepositoryMetadata, error) { diff --git a/pkg/graveler/graveler_test.go b/pkg/graveler/graveler_test.go index 14709b8a9f0..8d55e299d86 100644 --- a/pkg/graveler/graveler_test.go +++ b/pkg/graveler/graveler_test.go @@ -3,6 +3,7 @@ package graveler_test import ( "context" "errors" + "github.com/mohae/deepcopy" "strconv" "testing" "time" @@ -1708,24 +1709,34 @@ func TestGraveler_PreCommitHook(t *testing.T) { const commitMessage = "message" commitMetadata := graveler.Metadata{"key1": "val1"} tests := []struct { - name string - hook bool - err error + name string + hook bool + err error + readOnlyRepo bool }{ { - name: "without hook", - hook: false, - err: nil, + name: "without hook", + hook: false, + err: nil, + readOnlyRepo: false, }, { - name: "hook no error", - hook: true, - err: nil, + name: "hook no error", + hook: true, + err: nil, + readOnlyRepo: false, }, { - name: "hook error", - hook: true, - err: errSomethingBad, + name: "hook read only repo", + hook: true, + err: nil, + readOnlyRepo: true, + }, + { + name: "hook error", + hook: true, + err: errSomethingBad, + readOnlyRepo: false, }, } for _, tt := range tests { @@ -1737,12 +1748,15 @@ func TestGraveler_PreCommitHook(t *testing.T) { if tt.hook { g.SetHooksHandler(h) } + repo := deepcopy.Copy(repository).(*graveler.RepositoryRecord) + repo.ReadOnly = tt.readOnlyRepo // call commit - _, err := g.Commit(ctx, repository, commitBranchID, graveler.CommitParams{ + _, err := g.Commit(ctx, repo, commitBranchID, graveler.CommitParams{ Committer: commitCommitter, Message: commitMessage, Metadata: commitMetadata, - }) + }, graveler.WithForce(tt.readOnlyRepo)) + // check err composition if !errors.Is(err, tt.err) { t.Fatalf("Commit err=%v, expected=%v", err, tt.err) @@ -1751,14 +1765,14 @@ func TestGraveler_PreCommitHook(t *testing.T) { if err != nil && !errors.As(err, &hookErr) { t.Fatalf("Commit err=%v, expected HookAbortError", err) } - if tt.hook != h.Called { - t.Fatalf("Commit invalid pre-hook call, %t expected=%t", h.Called, tt.hook) + if (tt.hook && !tt.readOnlyRepo) != h.Called { + t.Fatalf("Commit invalid pre-hook call, %t expected=%t", h.Called, tt.hook && !tt.readOnlyRepo) } if !h.Called { return } - if h.RepositoryID != repository.RepositoryID { - t.Errorf("Hook repository '%s', expected '%s'", h.RepositoryID, repository.RepositoryID) + if h.RepositoryID != repo.RepositoryID { + t.Errorf("Hook repository '%s', expected '%s'", h.RepositoryID, repo.RepositoryID) } if h.BranchID != commitBranchID { t.Errorf("Hook branch '%s', expected '%s'", h.BranchID, commitBranchID) @@ -1811,24 +1825,34 @@ func TestGraveler_PreMergeHook(t *testing.T) { ".lakefs.merge.strategy": "default", } tests := []struct { - name string - hook bool - err error + name string + hook bool + err error + readOnlyRepo bool }{ { - name: "without hook", - hook: false, - err: nil, + name: "without hook", + hook: false, + err: nil, + readOnlyRepo: false, }, { - name: "hook no error", - hook: true, - err: nil, + name: "hook no error", + hook: true, + err: nil, + readOnlyRepo: false, }, { - name: "hook error", - hook: true, - err: errSomethingBad, + name: "hook read only repo", + hook: true, + err: nil, + readOnlyRepo: true, + }, + { + name: "hook error", + hook: true, + err: errSomethingBad, + readOnlyRepo: false, }, } for _, tt := range tests { @@ -1840,12 +1864,14 @@ func TestGraveler_PreMergeHook(t *testing.T) { if tt.hook { g.SetHooksHandler(h) } + repo := deepcopy.Copy(repository).(*graveler.RepositoryRecord) + repo.ReadOnly = tt.readOnlyRepo // call merge - _, err := g.Merge(ctx, repository, mergeDestination, expectedCommitID.Ref(), graveler.CommitParams{ + _, err := g.Merge(ctx, repo, mergeDestination, expectedCommitID.Ref(), graveler.CommitParams{ Committer: commitCommitter, Message: mergeMessage, Metadata: mergeMetadata, - }, "") + }, "", graveler.WithForce(tt.readOnlyRepo)) // verify we got an error if !errors.Is(err, tt.err) { t.Fatalf("Merge err=%v, pre-merge error expected=%v", err, tt.err) @@ -1865,8 +1891,8 @@ func TestGraveler_PreMergeHook(t *testing.T) { t.Fatalf("Wrong CommitParents order, expected: (%s, %s), got: (%s, %s)", destinationCommitID, expectedCommitID, parents[0], parents[1]) } // verify that calls made until the first error - if tt.hook != h.Called { - t.Fatalf("Merge hook h.Called=%t, expected=%t", h.Called, tt.hook) + if (tt.hook && !tt.readOnlyRepo) != h.Called { + t.Fatalf("Merge hook h.Called=%t, expected=%t", h.Called, tt.hook && !tt.readOnlyRepo) } if !h.Called { return @@ -1955,9 +1981,10 @@ func TestGraveler_PreCreateTagHook(t *testing.T) { // tests errSomethingBad := errors.New("first error") tests := []struct { - name string - hook bool - err error + name string + hook bool + err error + readOnlyRepo bool }{ { name: "without hook", @@ -1974,6 +2001,12 @@ func TestGraveler_PreCreateTagHook(t *testing.T) { hook: true, err: errSomethingBad, }, + { + name: "read only repo", + hook: true, + err: nil, + readOnlyRepo: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1984,8 +2017,10 @@ func TestGraveler_PreCreateTagHook(t *testing.T) { if tt.hook { g.SetHooksHandler(h) } + repo := deepcopy.Copy(repository).(*graveler.RepositoryRecord) + repo.ReadOnly = tt.readOnlyRepo - err := g.CreateTag(ctx, repository, expectedTagID, expectedCommitID) + err := g.CreateTag(ctx, repo, expectedTagID, expectedCommitID, graveler.WithForce(tt.readOnlyRepo)) // verify we got an error if !errors.Is(err, tt.err) { @@ -1997,8 +2032,8 @@ func TestGraveler_PreCreateTagHook(t *testing.T) { } // verify that calls made until the first error - if tt.hook != h.Called { - t.Fatalf("Pre-create tag hook h.Called=%t, expected=%t", h.Called, tt.hook) + if (tt.hook && !tt.readOnlyRepo) != h.Called { + t.Fatalf("Pre-create tag hook h.Called=%t, expected=%t", h.Called, tt.hook && !tt.readOnlyRepo) } if !h.Called { return @@ -2033,9 +2068,10 @@ func TestGraveler_PreDeleteTagHook(t *testing.T) { // tests errSomethingBad := errors.New("first error") tests := []struct { - name string - hook bool - err error + name string + hook bool + err error + readOnlyRepo bool }{ { name: "without hook", @@ -2052,6 +2088,12 @@ func TestGraveler_PreDeleteTagHook(t *testing.T) { hook: true, err: errSomethingBad, }, + { + name: "read only repo", + hook: true, + err: nil, + readOnlyRepo: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -2064,8 +2106,9 @@ func TestGraveler_PreDeleteTagHook(t *testing.T) { if tt.hook { g.SetHooksHandler(h) } - - err := g.DeleteTag(ctx, repository, expectedTagID) + repo := deepcopy.Copy(repository).(*graveler.RepositoryRecord) + repo.ReadOnly = tt.readOnlyRepo + err := g.DeleteTag(ctx, repo, expectedTagID, graveler.WithForce(tt.readOnlyRepo)) // verify we got an error if !errors.Is(err, tt.err) { @@ -2077,8 +2120,8 @@ func TestGraveler_PreDeleteTagHook(t *testing.T) { } // verify that calls made until the first error - if tt.hook != h.Called { - t.Fatalf("Pre delete Tag hook h.Called=%t, expected=%t", h.Called, tt.hook) + if (tt.hook && !tt.readOnlyRepo) != h.Called { + t.Fatalf("Pre delete Tag hook h.Called=%t, expected=%t", h.Called, tt.hook && !tt.readOnlyRepo) } if !h.Called { return @@ -2115,9 +2158,10 @@ func TestGraveler_PreCreateBranchHook(t *testing.T) { // tests errSomethingBad := errors.New("first error") tests := []struct { - name string - hook bool - err error + name string + hook bool + err error + readOnlyRepo bool }{ { name: "without hook", @@ -2134,6 +2178,12 @@ func TestGraveler_PreCreateBranchHook(t *testing.T) { hook: true, err: errSomethingBad, }, + { + name: "read only repo", + hook: true, + err: nil, + readOnlyRepo: true, + }, } for i, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -2150,7 +2200,9 @@ func TestGraveler_PreCreateBranchHook(t *testing.T) { refManager.Err = graveler.ErrBranchNotFound newBranch := newBranchPrefix + strconv.Itoa(i) - _, err := g.CreateBranch(ctx, repository, graveler.BranchID(newBranch), graveler.Ref(sourceBranchID)) + repo := deepcopy.Copy(repository).(*graveler.RepositoryRecord) + repo.ReadOnly = tt.readOnlyRepo + _, err := g.CreateBranch(ctx, repo, graveler.BranchID(newBranch), graveler.Ref(sourceBranchID), graveler.WithForce(tt.readOnlyRepo)) // verify we got an error if !errors.Is(err, tt.err) { @@ -2162,8 +2214,8 @@ func TestGraveler_PreCreateBranchHook(t *testing.T) { } // verify that calls made until the first error - if tt.hook != h.Called { - t.Fatalf("Pre-create branch hook h.Called=%t, expected=%t", h.Called, tt.hook) + if (tt.hook && !tt.readOnlyRepo) != h.Called { + t.Fatalf("Pre-create branch hook h.Called=%t, expected=%t", h.Called, tt.hook && !tt.readOnlyRepo) } if !h.Called { return @@ -2207,9 +2259,10 @@ func TestGraveler_PreDeleteBranchHook(t *testing.T) { // tests errSomethingBad := errors.New("first error") tests := []struct { - name string - hook bool - err error + name string + hook bool + err error + readOnlyRepo bool }{ { name: "without hook", @@ -2226,6 +2279,12 @@ func TestGraveler_PreDeleteBranchHook(t *testing.T) { hook: true, err: errSomethingBad, }, + { + name: "read only repo", + hook: true, + err: nil, + readOnlyRepo: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -2236,8 +2295,9 @@ func TestGraveler_PreDeleteBranchHook(t *testing.T) { if tt.hook { g.SetHooksHandler(h) } - - err := g.DeleteBranch(ctx, repository, graveler.BranchID(sourceBranchID)) + repo := deepcopy.Copy(repository).(*graveler.RepositoryRecord) + repo.ReadOnly = tt.readOnlyRepo + err := g.DeleteBranch(ctx, repo, graveler.BranchID(sourceBranchID), graveler.WithForce(tt.readOnlyRepo)) // verify we got an error if !errors.Is(err, tt.err) { @@ -2249,8 +2309,8 @@ func TestGraveler_PreDeleteBranchHook(t *testing.T) { } // verify that calls made until the first error - if tt.hook != h.Called { - t.Fatalf("Pre-delete branch hook h.Called=%t, expected=%t", h.Called, tt.hook) + if (tt.hook && !tt.readOnlyRepo) != h.Called { + t.Fatalf("Pre-delete branch hook h.Called=%t, expected=%t", h.Called, tt.hook && !tt.readOnlyRepo) } if !h.Called { return