From 476f6f2664a670ed3cc706c2277420054a9d1a11 Mon Sep 17 00:00:00 2001 From: N-o-Z Date: Thu, 31 Mar 2022 21:10:03 +0300 Subject: [PATCH] Feature/add additional hook locations (#3130) * Add hooks for pre/post create/delete branch/tag Co-authored-by: Nir Ozery Co-authored-by: Barak Amar --- nessie/action_files/action_post_commit.yaml | 12 + .../action_post_create_branch.yaml | 12 + .../action_files/action_post_create_tag.yaml | 10 + .../action_post_delete_branch.yaml | 11 + .../action_files/action_post_delete_tag.yaml | 10 + nessie/action_files/action_post_merge.yaml | 12 + nessie/action_files/action_pre_commit.yaml | 12 + .../action_pre_create_branch.yaml | 12 + .../action_files/action_pre_create_tag.yaml | 10 + .../action_pre_delete_branch.yaml | 11 + .../action_files/action_pre_delete_tag.yaml | 10 + nessie/action_files/action_pre_merge.yaml | 12 + nessie/hooks_test.go | 416 +++++++++++----- nessie/webhook_server_test.go | 8 + pkg/actions/action.go | 52 +- pkg/actions/action_test.go | 64 +++ pkg/actions/event.go | 10 +- pkg/actions/service.go | 34 +- .../action_invalid_param_tag_actions.yaml | 8 + pkg/graveler/graveler.go | 195 +++++++- pkg/graveler/graveler_test.go | 454 +++++++++++++++++- pkg/graveler/hooks_handler.go | 72 ++- 22 files changed, 1302 insertions(+), 145 deletions(-) create mode 100644 nessie/action_files/action_post_commit.yaml create mode 100644 nessie/action_files/action_post_create_branch.yaml create mode 100644 nessie/action_files/action_post_create_tag.yaml create mode 100644 nessie/action_files/action_post_delete_branch.yaml create mode 100644 nessie/action_files/action_post_delete_tag.yaml create mode 100644 nessie/action_files/action_post_merge.yaml create mode 100644 nessie/action_files/action_pre_commit.yaml create mode 100644 nessie/action_files/action_pre_create_branch.yaml create mode 100644 nessie/action_files/action_pre_create_tag.yaml create mode 100644 nessie/action_files/action_pre_delete_branch.yaml create mode 100644 nessie/action_files/action_pre_delete_tag.yaml create mode 100644 nessie/action_files/action_pre_merge.yaml create mode 100644 pkg/actions/testdata/action_invalid_param_tag_actions.yaml diff --git a/nessie/action_files/action_post_commit.yaml b/nessie/action_files/action_post_commit.yaml new file mode 100644 index 00000000000..26b68990ab0 --- /dev/null +++ b/nessie/action_files/action_post_commit.yaml @@ -0,0 +1,12 @@ +name: Test Post Commit +description: a test action description +on: + post-commit: + branches: + - feature-* +hooks: + - id: test_webhook + type: webhook + description: Check webhooks for post-commit works + properties: + url: "{{.URL}}/post-commit" \ No newline at end of file diff --git a/nessie/action_files/action_post_create_branch.yaml b/nessie/action_files/action_post_create_branch.yaml new file mode 100644 index 00000000000..7c8712a4d9e --- /dev/null +++ b/nessie/action_files/action_post_create_branch.yaml @@ -0,0 +1,12 @@ +name: Test Post Create Branch +description: a test action description +on: + post-create-branch: + branches: + - test_branch_delete +hooks: + - id: test_webhook + type: webhook + description: Check webhooks for post-create branch works + properties: + url: "{{.URL}}/post-create-branch" \ No newline at end of file diff --git a/nessie/action_files/action_post_create_tag.yaml b/nessie/action_files/action_post_create_tag.yaml new file mode 100644 index 00000000000..e1434428a6b --- /dev/null +++ b/nessie/action_files/action_post_create_tag.yaml @@ -0,0 +1,10 @@ +name: Test Post Create Tag +description: a test action description +on: + post-create-tag: +hooks: + - id: test_webhook + type: webhook + description: Check webhooks for post-create tag works + properties: + url: "{{.URL}}/post-create-tag" \ No newline at end of file diff --git a/nessie/action_files/action_post_delete_branch.yaml b/nessie/action_files/action_post_delete_branch.yaml new file mode 100644 index 00000000000..0880e988958 --- /dev/null +++ b/nessie/action_files/action_post_delete_branch.yaml @@ -0,0 +1,11 @@ +name: Test Post Delete Branch +description: a test action description +on: + post-delete-branch: + branches: +hooks: + - id: test_webhook + type: webhook + description: Check webhooks for post-delete branch works + properties: + url: "{{.URL}}/post-delete-branch" \ No newline at end of file diff --git a/nessie/action_files/action_post_delete_tag.yaml b/nessie/action_files/action_post_delete_tag.yaml new file mode 100644 index 00000000000..e8aad20d8d4 --- /dev/null +++ b/nessie/action_files/action_post_delete_tag.yaml @@ -0,0 +1,10 @@ +name: Test Post Delete Tag +description: a test action description +on: + post-delete-tag: +hooks: + - id: test_webhook + type: webhook + description: Check webhooks for post-delete tag works + properties: + url: "{{.URL}}/post-delete-tag" \ No newline at end of file diff --git a/nessie/action_files/action_post_merge.yaml b/nessie/action_files/action_post_merge.yaml new file mode 100644 index 00000000000..0c86d1910de --- /dev/null +++ b/nessie/action_files/action_post_merge.yaml @@ -0,0 +1,12 @@ +name: Test Post Merge +description: a test action description +on: + post-merge: + branches: + - main +hooks: + - id: test_webhook + type: webhook + description: Check webhooks for post-merge works + properties: + url: "{{.URL}}/post-merge" \ No newline at end of file diff --git a/nessie/action_files/action_pre_commit.yaml b/nessie/action_files/action_pre_commit.yaml new file mode 100644 index 00000000000..4d61c0a3572 --- /dev/null +++ b/nessie/action_files/action_pre_commit.yaml @@ -0,0 +1,12 @@ +name: Test Pre Commit +description: a test action description +on: + pre-commit: +hooks: + - id: test_webhook + type: webhook + description: Check webhooks for pre-commit works + properties: + url: "{{.URL}}/pre-commit" + query_params: + check_env_vars: "{{"{{ ENV.ACTIONS_VAR }}"}}" \ No newline at end of file diff --git a/nessie/action_files/action_pre_create_branch.yaml b/nessie/action_files/action_pre_create_branch.yaml new file mode 100644 index 00000000000..4e7dd44ed77 --- /dev/null +++ b/nessie/action_files/action_pre_create_branch.yaml @@ -0,0 +1,12 @@ +name: Test Pre Create Branch +description: a test action description +on: + pre-create-branch: + branches: + - test_branch_delete +hooks: + - id: test_webhook + type: webhook + description: Check webhooks for pre-create branch works + properties: + url: "{{.URL}}/pre-create-branch" \ No newline at end of file diff --git a/nessie/action_files/action_pre_create_tag.yaml b/nessie/action_files/action_pre_create_tag.yaml new file mode 100644 index 00000000000..ede4d5817c3 --- /dev/null +++ b/nessie/action_files/action_pre_create_tag.yaml @@ -0,0 +1,10 @@ +name: Test Pre Create Tag +description: a test action description +on: + pre-create-tag: +hooks: + - id: test_webhook + type: webhook + description: Check webhooks for pre-create tag works + properties: + url: "{{.URL}}/pre-create-tag" \ No newline at end of file diff --git a/nessie/action_files/action_pre_delete_branch.yaml b/nessie/action_files/action_pre_delete_branch.yaml new file mode 100644 index 00000000000..f109d134659 --- /dev/null +++ b/nessie/action_files/action_pre_delete_branch.yaml @@ -0,0 +1,11 @@ +name: Test Pre Delete Branch +description: a test action description +on: + pre-delete-branch: + branches: +hooks: + - id: test_webhook + type: webhook + description: Check webhooks for pre-delete branch works + properties: + url: "{{.URL}}/pre-delete-branch" \ No newline at end of file diff --git a/nessie/action_files/action_pre_delete_tag.yaml b/nessie/action_files/action_pre_delete_tag.yaml new file mode 100644 index 00000000000..22e8401b667 --- /dev/null +++ b/nessie/action_files/action_pre_delete_tag.yaml @@ -0,0 +1,10 @@ +name: Test Pre Delete Tag +description: a test action description +on: + pre-delete-tag: +hooks: + - id: test_webhook + type: webhook + description: Check webhooks for pre-delete tag works + properties: + url: "{{.URL}}/pre-delete-tag" \ No newline at end of file diff --git a/nessie/action_files/action_pre_merge.yaml b/nessie/action_files/action_pre_merge.yaml new file mode 100644 index 00000000000..f585705d3f5 --- /dev/null +++ b/nessie/action_files/action_pre_merge.yaml @@ -0,0 +1,12 @@ +name: Test Pre Merge +description: a test action description +on: + pre-merge: + branches: + - main +hooks: + - id: test_webhook + type: webhook + description: Check webhooks for pre-merge works + properties: + url: "{{.URL}}/pre-merge" \ No newline at end of file diff --git a/nessie/hooks_test.go b/nessie/hooks_test.go index a1c755fadaa..0583b9712b5 100644 --- a/nessie/hooks_test.go +++ b/nessie/hooks_test.go @@ -3,7 +3,10 @@ package nessie import ( "bytes" "context" + "embed" "encoding/json" + "fmt" + "io/fs" "net/http" "testing" "text/template" @@ -13,71 +16,36 @@ import ( "github.com/treeverse/lakefs/pkg/api" ) -const actionPreMergeYaml = ` -name: Test Pre Merge -description: set of checks to verify that branch is good -on: - pre-merge: - branches: - - main -hooks: - - id: test_webhook - type: webhook - description: Check webhooks for pre-merge works - properties: - url: "{{.URL}}/pre-merge" -` - -const actionPreCommitYaml = ` -name: Test Pre Commit -description: set of checks to verify that branch is good -on: - pre-commit: -hooks: - - id: test_webhook - type: webhook - description: Check webhooks for pre-commit works - properties: - url: "{{.URL}}/pre-commit" - query_params: - check_env_vars: "{{"{{ ENV.ACTIONS_VAR }}"}}" -` - -const actionPostCommitYaml = ` -name: Test Post Commit -description: set of checks to verify that branch is good -on: - post-commit: - branches: - - feature-* -hooks: - - id: test_webhook - type: webhook - description: Check webhooks for post-commit works - properties: - url: "{{.URL}}/post-commit" -` - -const actionPostMergeYaml = ` -name: Test Post Merge -description: set of checks to verify that branch is good -on: - post-merge: - branches: - - main -hooks: - - id: test_webhook - type: webhook - description: Check webhooks for post-merge works - properties: - url: "{{.URL}}/post-merge" -` +//go:embed action_files/*.yaml +var actions embed.FS func TestHooksSuccess(t *testing.T) { - ctx, logger, repo := setupTest(t) + ctx, _, repo := setupTest(t) + parseAndUploadActions(t, ctx, repo, mainBranch) + commitResp, err := client.CommitWithResponse(ctx, repo, mainBranch, api.CommitJSONRequestBody{ + Message: "Initial content", + }) + require.NoError(t, err, "failed to commit initial content") + require.Equal(t, http.StatusCreated, commitResp.StatusCode()) + + _, err = responseWithTimeout(server, 1*time.Minute) // pre-commit action triggered on action upload, flush buffer + require.NoError(t, err) + + t.Run("commit merge test", func(t *testing.T) { + testCommitMerge(t, ctx, repo) + }) + t.Run("create delete branch test", func(t *testing.T) { + testCreateDeleteBranch(t, ctx, repo) + }) + t.Run("create delete tag test", func(t *testing.T) { + testCreateDeleteTag(t, ctx, repo) + }) +} + +func testCommitMerge(t *testing.T, ctx context.Context, repo string) { const branch = "feature-1" - logger.WithField("branch", branch).Info("Create branch") + t.Log("Create branch", branch) createBranchResp, err := client.CreateBranchWithResponse(ctx, repo, api.CreateBranchJSONRequestBody{ Name: branch, Source: mainBranch, @@ -85,16 +53,13 @@ func TestHooksSuccess(t *testing.T) { require.NoError(t, err, "failed to create branch") require.Equal(t, http.StatusCreated, createBranchResp.StatusCode()) ref := string(createBranchResp.Body) - logger.WithField("branchRef", ref).Info("Branch created") - logger.WithField("branch", branch).Info("Upload initial content") - - parseAndUpload(t, ctx, repo, branch, actionPreMergeYaml, "action-pre-merge", "testing_pre_merge") - parseAndUpload(t, ctx, repo, branch, actionPreCommitYaml, "action-pre-commit", "testing_pre_commit") - parseAndUpload(t, ctx, repo, branch, actionPostCommitYaml, "action-post-commit", "testing_post_commit") - parseAndUpload(t, ctx, repo, branch, actionPostMergeYaml, "action-post-merge", "testing_post_merge") + t.Log("Branch created", ref) - logger.WithField("branch", branch).Info("Commit initial content") + resp, err := uploadContent(ctx, repo, branch, "somefile", "") + require.NoError(t, err) + require.Equal(t, http.StatusCreated, resp.StatusCode()) + t.Log("Commit content", branch) commitResp, err := client.CommitWithResponse(ctx, repo, branch, api.CommitJSONRequestBody{ Message: "Initial content", }) @@ -106,39 +71,45 @@ func TestHooksSuccess(t *testing.T) { require.NoError(t, webhookData.err, "error on pre commit serving") decoder := json.NewDecoder(bytes.NewReader(webhookData.data)) - var preCommitEvent, postCommitEvent, preMergeEvent, postMergeEvent webhookEventInfo + var preCommitEvent webhookEventInfo require.NoError(t, decoder.Decode(&preCommitEvent), "reading pre-commit data") - commitRecord := commitResp.JSON201 - require.Equal(t, "pre-commit", preCommitEvent.EventType) - require.Equal(t, "Test Pre Commit", preCommitEvent.ActionName) - require.Equal(t, "test_webhook", preCommitEvent.HookID) - require.Equal(t, repo, preCommitEvent.RepositoryID) - require.Equal(t, branch, preCommitEvent.BranchID) - require.Equal(t, commitRecord.Committer, preCommitEvent.Committer) - require.Equal(t, commitRecord.Message, preCommitEvent.CommitMessage) - require.Equal(t, branch, preCommitEvent.SourceRef) - require.Equal(t, commitRecord.Metadata.AdditionalProperties, preCommitEvent.Metadata) + require.Equal(t, webhookEventInfo{ + EventTime: preCommitEvent.EventTime, + SourceRef: branch, + EventType: "pre-commit", + ActionName: "Test Pre Commit", + HookID: "test_webhook", + RepositoryID: repo, + BranchID: branch, + Committer: commitRecord.Committer, + CommitMessage: commitRecord.Message, + Metadata: commitRecord.Metadata.AdditionalProperties, + }, preCommitEvent) require.NotNil(t, webhookData.queryParams) require.Contains(t, webhookData.queryParams, "check_env_vars") - require.Equal(t, webhookData.queryParams["check_env_vars"], []string{"this_is_actions_var"}) + require.Equal(t, []string{"this_is_actions_var"}, webhookData.queryParams["check_env_vars"]) webhookData, err = responseWithTimeout(server, 1*time.Minute) require.NoError(t, err) require.NoError(t, webhookData.err, "error on post commit serving") decoder = json.NewDecoder(bytes.NewReader(webhookData.data)) + var postCommitEvent webhookEventInfo require.NoError(t, decoder.Decode(&postCommitEvent), "reading post-commit data") - - require.Equal(t, "post-commit", postCommitEvent.EventType) - require.Equal(t, "Test Post Commit", postCommitEvent.ActionName) - require.Equal(t, "test_webhook", postCommitEvent.HookID) - require.Equal(t, repo, postCommitEvent.RepositoryID) - require.Equal(t, branch, postCommitEvent.BranchID) - require.Equal(t, commitRecord.Committer, postCommitEvent.Committer) - require.Equal(t, commitRecord.Message, postCommitEvent.CommitMessage) - require.Equal(t, branch, postCommitEvent.SourceRef) - require.Equal(t, commitRecord.Metadata.AdditionalProperties, postCommitEvent.Metadata) + require.Equal(t, webhookEventInfo{ + EventTime: postCommitEvent.EventTime, + SourceRef: commitResp.JSON201.Id, + EventType: "post-commit", + ActionName: "Test Post Commit", + HookID: "test_webhook", + RepositoryID: repo, + BranchID: branch, + CommitID: commitRecord.Id, + Committer: commitRecord.Committer, + CommitMessage: commitRecord.Message, + Metadata: commitRecord.Metadata.AdditionalProperties, + }, postCommitEvent) mergeResp, err := client.MergeIntoBranchWithResponse(ctx, repo, branch, mainBranch, api.MergeIntoBranchJSONRequestBody{}) require.NoError(t, err) @@ -148,32 +119,43 @@ func TestHooksSuccess(t *testing.T) { require.Equal(t, http.StatusOK, mergeResp.StatusCode()) require.NoError(t, webhookData.err, "failed to merge branches") mergeRef := mergeResp.JSON200.Reference - logger.WithField("mergeResult", mergeRef).Info("Merged successfully") + t.Log("Merged successfully", mergeRef) require.NoError(t, err, "error on pre commit serving") decoder = json.NewDecoder(bytes.NewReader(webhookData.data)) + var preMergeEvent webhookEventInfo require.NoError(t, decoder.Decode(&preMergeEvent), "reading pre-merge data") - - require.Equal(t, "pre-merge", preMergeEvent.EventType) - require.Equal(t, "Test Pre Merge", preMergeEvent.ActionName) - require.Equal(t, "test_webhook", preMergeEvent.HookID) - require.Equal(t, repo, preMergeEvent.RepositoryID) - require.Equal(t, mainBranch, preMergeEvent.BranchID) - require.Equal(t, commitRecord.Id, preMergeEvent.SourceRef) + require.Equal(t, webhookEventInfo{ + EventTime: preMergeEvent.EventTime, + SourceRef: commitRecord.Id, + EventType: "pre-merge", + ActionName: "Test Pre Merge", + HookID: "test_webhook", + RepositoryID: repo, + BranchID: mainBranch, + Committer: commitRecord.Committer, + CommitMessage: fmt.Sprintf("Merge '%s' into '%s'", branch, mainBranch), + }, preMergeEvent) // Testing post-merge hook response webhookData, err = responseWithTimeout(server, 1*time.Minute) require.NoError(t, err) decoder = json.NewDecoder(bytes.NewReader(webhookData.data)) + var postMergeEvent webhookEventInfo require.NoError(t, decoder.Decode(&postMergeEvent), "reading post-merge data") - - require.Equal(t, "post-merge", postMergeEvent.EventType) - require.Equal(t, "Test Post Merge", postMergeEvent.ActionName) - require.Equal(t, "test_webhook", postMergeEvent.HookID) - require.Equal(t, repo, postMergeEvent.RepositoryID) - require.Equal(t, mainBranch, postMergeEvent.BranchID) - require.Equal(t, branch, postMergeEvent.SourceRef) + require.Equal(t, webhookEventInfo{ + EventTime: postMergeEvent.EventTime, + SourceRef: mergeResp.JSON200.Reference, + EventType: "post-merge", + ActionName: "Test Post Merge", + HookID: "test_webhook", + RepositoryID: repo, + BranchID: mainBranch, + CommitID: mergeResp.JSON200.Reference, + Committer: commitRecord.Committer, + CommitMessage: fmt.Sprintf("Merge '%s' into '%s'", branch, mainBranch), + }, postMergeEvent) t.Log("List repository runs", mergeRef) runsResp, err := client.ListRepositoryRunsWithResponse(ctx, repo, &api.ListRepositoryRunsParams{ @@ -190,9 +172,202 @@ func TestHooksSuccess(t *testing.T) { require.Equal(t, "main", run.Branch) } -func parseAndUpload(t *testing.T, ctx context.Context, repo, branch, yaml, templateName, actionPath string) { - t.Helper() +func testCreateDeleteBranch(t *testing.T, ctx context.Context, repo string) { + const testBranch = "test_branch_delete" + createBranchResp, err := client.CreateBranchWithResponse(ctx, repo, api.CreateBranchJSONRequestBody{ + Name: testBranch, + Source: mainBranch, + }) + require.NoError(t, err, "failed to create branch") + require.Equal(t, http.StatusCreated, createBranchResp.StatusCode()) + + resp, err := client.GetBranchWithResponse(ctx, repo, mainBranch) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode()) + commitID := resp.JSON200.CommitId + + // Testing pre-create branch hook response + webhookData, err := responseWithTimeout(server, 1*time.Minute) + require.NoError(t, err) + + require.NoError(t, webhookData.err, "error on pre create branch") + decoder := json.NewDecoder(bytes.NewReader(webhookData.data)) + var preCreateBranchEvent webhookEventInfo + require.NoError(t, decoder.Decode(&preCreateBranchEvent), "reading pre-create branch data") + require.Equal(t, webhookEventInfo{ + EventTime: preCreateBranchEvent.EventTime, + SourceRef: mainBranch, + EventType: "pre-create-branch", + ActionName: "Test Pre Create Branch", + HookID: "test_webhook", + RepositoryID: repo, + BranchID: testBranch, + CommitID: commitID, + }, preCreateBranchEvent) + + // Testing post-create branch hook response + webhookData, err = responseWithTimeout(server, 1*time.Minute) + require.NoError(t, err) + + require.NoError(t, webhookData.err, "error on post create branch") + decoder = json.NewDecoder(bytes.NewReader(webhookData.data)) + var postCreateBranchEvent webhookEventInfo + require.NoError(t, decoder.Decode(&postCreateBranchEvent), "reading post-create branch data") + require.Equal(t, webhookEventInfo{ + EventTime: postCreateBranchEvent.EventTime, + SourceRef: mainBranch, + EventType: "post-create-branch", + ActionName: "Test Post Create Branch", + HookID: "test_webhook", + RepositoryID: repo, + BranchID: testBranch, + CommitID: commitID, + }, postCreateBranchEvent) + + // Delete branch + deleteBranchResp, err := client.DeleteBranchWithResponse(ctx, repo, testBranch) + + require.NoError(t, err, "failed to delete branch") + require.Equal(t, http.StatusNoContent, deleteBranchResp.StatusCode()) + + // Testing pre-delete branch hook response + webhookData, err = responseWithTimeout(server, 1*time.Minute) + require.NoError(t, err) + require.NoError(t, webhookData.err, "error on pre delete branch") + decoder = json.NewDecoder(bytes.NewReader(webhookData.data)) + var preDeleteBranchEvent webhookEventInfo + require.NoError(t, decoder.Decode(&preDeleteBranchEvent), "reading pre-delete branch data") + require.Equal(t, webhookEventInfo{ + EventTime: preDeleteBranchEvent.EventTime, + SourceRef: commitID, + EventType: "pre-delete-branch", + ActionName: "Test Pre Delete Branch", + HookID: "test_webhook", + RepositoryID: repo, + BranchID: testBranch, + }, preDeleteBranchEvent) + + // Testing post-delete branch hook response + webhookData, err = responseWithTimeout(server, 1*time.Minute) + require.NoError(t, err) + + require.NoError(t, webhookData.err, "error on post delete branch") + decoder = json.NewDecoder(bytes.NewReader(webhookData.data)) + var postDeleteBranchEvent webhookEventInfo + require.NoError(t, decoder.Decode(&postDeleteBranchEvent), "reading post-delete branch data") + require.Equal(t, webhookEventInfo{ + EventTime: postDeleteBranchEvent.EventTime, + SourceRef: commitID, + EventType: "post-delete-branch", + ActionName: "Test Post Delete Branch", + HookID: "test_webhook", + RepositoryID: repo, + BranchID: testBranch, + }, postDeleteBranchEvent) +} + +func testCreateDeleteTag(t *testing.T, ctx context.Context, repo string) { + const tagID = "tag_test_hooks" + + resp, err := client.GetBranchWithResponse(ctx, repo, mainBranch) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode()) + commitID := resp.JSON200.CommitId + + createTagResp, err := client.CreateTagWithResponse(ctx, repo, api.CreateTagJSONRequestBody{ + Id: tagID, + Ref: commitID, + }) + + require.NoError(t, err, "failed to create tag") + require.Equal(t, http.StatusCreated, createTagResp.StatusCode()) + + // Testing pre-create tag hook response + webhookData, err := responseWithTimeout(server, 1*time.Minute) + require.NoError(t, err) + + require.NoError(t, webhookData.err, "error on pre create tag") + decoder := json.NewDecoder(bytes.NewReader(webhookData.data)) + var preCreateTagEvent webhookEventInfo + require.NoError(t, decoder.Decode(&preCreateTagEvent), "reading pre-create tag data") + require.Equal(t, webhookEventInfo{ + EventTime: preCreateTagEvent.EventTime, + SourceRef: commitID, + EventType: "pre-create-tag", + ActionName: "Test Pre Create Tag", + HookID: "test_webhook", + RepositoryID: repo, + CommitID: commitID, + TagID: tagID, + }, preCreateTagEvent) + + // Testing post-create tag hook response + webhookData, err = responseWithTimeout(server, 1*time.Minute) + require.NoError(t, err) + + require.NoError(t, webhookData.err, "error on post create tag") + decoder = json.NewDecoder(bytes.NewReader(webhookData.data)) + var postCreateTagEvent webhookEventInfo + require.NoError(t, decoder.Decode(&postCreateTagEvent), "reading post-create tag data") + require.Equal(t, webhookEventInfo{ + EventTime: postCreateTagEvent.EventTime, + SourceRef: commitID, + EventType: "post-create-tag", + ActionName: "Test Post Create Tag", + HookID: "test_webhook", + RepositoryID: repo, + CommitID: commitID, + TagID: tagID, + }, postCreateTagEvent) + + // Delete tag + deleteTagResp, err := client.DeleteTagWithResponse(ctx, repo, tagID) + + require.NoError(t, err, "failed to delete tag") + require.Equal(t, http.StatusNoContent, deleteTagResp.StatusCode()) + + // Testing pre-delete tag hook response + webhookData, err = responseWithTimeout(server, 1*time.Minute) + require.NoError(t, err) + + require.NoError(t, webhookData.err, "error on pre delete tag") + decoder = json.NewDecoder(bytes.NewReader(webhookData.data)) + var preDeleteTagEvent webhookEventInfo + require.NoError(t, decoder.Decode(&preDeleteTagEvent), "reading pre-delete tag data") + require.Equal(t, webhookEventInfo{ + EventTime: preDeleteTagEvent.EventTime, + SourceRef: commitID, + EventType: "pre-delete-tag", + ActionName: "Test Pre Delete Tag", + HookID: "test_webhook", + RepositoryID: repo, + CommitID: commitID, + TagID: tagID, + }, preDeleteTagEvent) + + // Testing post-delete tag hook response + webhookData, err = responseWithTimeout(server, 1*time.Minute) + require.NoError(t, err) + + require.NoError(t, webhookData.err, "error on post delete tag") + decoder = json.NewDecoder(bytes.NewReader(webhookData.data)) + var postDeleteTagEvent webhookEventInfo + require.NoError(t, decoder.Decode(&postDeleteTagEvent), "reading post-delete tag data") + require.Equal(t, webhookEventInfo{ + EventTime: postDeleteTagEvent.EventTime, + SourceRef: commitID, + EventType: "post-delete-tag", + ActionName: "Test Post Delete Tag", + HookID: "test_webhook", + RepositoryID: repo, + CommitID: commitID, + TagID: tagID, + }, postDeleteTagEvent) +} + +func parseAndUploadActions(t *testing.T, ctx context.Context, repo, branch string) { + t.Helper() // render actions based on templates docData := struct { URL string @@ -200,16 +375,23 @@ func parseAndUpload(t *testing.T, ctx context.Context, repo, branch, yaml, templ URL: server.BaseURL(), } - var doc bytes.Buffer + actionsDir, _ := fs.Sub(actions, "action_files") + ents, _ := fs.Glob(actionsDir, "*.yaml") + for _, ent := range ents { + buf, err := fs.ReadFile(actionsDir, ent) + require.NoError(t, err) - actionPreMergeTmpl := template.Must(template.New(templateName).Parse(yaml)) - err := actionPreMergeTmpl.Execute(&doc, docData) - require.NoError(t, err) - action := doc.String() + actionTmpl, err := template.New(ent).Parse(string(buf)) + require.NoError(t, err, ent) + var doc bytes.Buffer + err = actionTmpl.Execute(&doc, docData) + require.NoError(t, err) - resp, err := uploadContent(ctx, repo, branch, "_lakefs_actions/"+actionPath, action) - require.NoError(t, err) - require.Equal(t, http.StatusCreated, resp.StatusCode()) + action := doc.String() + resp, err := uploadContent(ctx, repo, branch, "_lakefs_actions/"+ent, action) + require.NoError(t, err) + require.Equal(t, http.StatusCreated, resp.StatusCode()) + } } type webhookEventInfo struct { @@ -220,6 +402,8 @@ type webhookEventInfo struct { RepositoryID string `json:"repository_id"` BranchID string `json:"branch_id"` SourceRef string `json:"source_ref"` + TagID string `json:"tag_id"` + CommitID string `json:"commit_id"` CommitMessage string `json:"commit_message"` Committer string `json:"committer"` Metadata map[string]string `json:"metadata"` diff --git a/nessie/webhook_server_test.go b/nessie/webhook_server_test.go index bdfe673f497..317f05c4e14 100644 --- a/nessie/webhook_server_test.go +++ b/nessie/webhook_server_test.go @@ -36,6 +36,14 @@ func startWebhookServer() (*webhookServer, error) { mux.HandleFunc("/post-commit", hookHandlerFunc(respCh)) mux.HandleFunc("/pre-merge", hookHandlerFunc(respCh)) mux.HandleFunc("/post-merge", hookHandlerFunc(respCh)) + mux.HandleFunc("/pre-create-branch", hookHandlerFunc(respCh)) + mux.HandleFunc("/post-create-branch", hookHandlerFunc(respCh)) + mux.HandleFunc("/pre-delete-branch", hookHandlerFunc(respCh)) + mux.HandleFunc("/post-delete-branch", hookHandlerFunc(respCh)) + mux.HandleFunc("/pre-create-tag", hookHandlerFunc(respCh)) + mux.HandleFunc("/post-create-tag", hookHandlerFunc(respCh)) + mux.HandleFunc("/pre-delete-tag", hookHandlerFunc(respCh)) + mux.HandleFunc("/post-delete-tag", hookHandlerFunc(respCh)) mux.HandleFunc("/timeout", timeoutHandlerFunc(respCh)) mux.HandleFunc("/fail", failHandlerFunc(respCh)) listener, err := net.Listen("tcp", ":0") diff --git a/pkg/actions/action.go b/pkg/actions/action.go index 5a161f95138..c383ef6ff32 100644 --- a/pkg/actions/action.go +++ b/pkg/actions/action.go @@ -6,6 +6,7 @@ import ( "fmt" "path" "regexp" + "strings" "github.com/hashicorp/go-multierror" "github.com/treeverse/lakefs/pkg/graveler" @@ -65,15 +66,27 @@ var ( reName = regexp.MustCompile(`^\w[\w\-. ]+$`) reHookID = regexp.MustCompile(`^[_a-zA-Z][\-_a-zA-Z0-9]{1,255}$`) - ErrInvalidAction = errors.New("invalid action") - ErrInvalidEventType = errors.New("invalid event type") + ErrInvalidAction = errors.New("invalid action") + ErrInvalidEventParameter = errors.New("invalid event parameter") ) -var supportedEvents = map[graveler.EventType]bool{ - graveler.EventTypePreCommit: true, - graveler.EventTypePostMerge: true, - graveler.EventTypePreMerge: true, - graveler.EventTypePostCommit: true, +func isEventSupported(event graveler.EventType) bool { + switch event { + case graveler.EventTypePreCommit, + graveler.EventTypePostMerge, + graveler.EventTypePreMerge, + graveler.EventTypePostCommit, + graveler.EventTypePreCreateBranch, + graveler.EventTypePostCreateBranch, + graveler.EventTypePreDeleteBranch, + graveler.EventTypePostDeleteBranch, + graveler.EventTypePreCreateTag, + graveler.EventTypePostCreateTag, + graveler.EventTypePreDeleteTag, + graveler.EventTypePostDeleteTag: + return true + } + return false } func (a *Action) Validate() error { @@ -86,9 +99,10 @@ func (a *Action) Validate() error { if len(a.On) == 0 { return fmt.Errorf("'on' is required: %w", ErrInvalidAction) } - for o := range a.On { - if !supportedEvents[o] { - return fmt.Errorf("event '%s' is not supported: %w", o, ErrInvalidAction) + for k, v := range a.On { + err := validateEvent(k, v) + if err != nil { + return err } } ids := make(map[string]struct{}) @@ -107,6 +121,24 @@ func (a *Action) Validate() error { return nil } +func validateEvent(event graveler.EventType, on *ActionOn) error { + if !isEventSupported(event) { + return fmt.Errorf("event '%s' is not supported: %w", event, ErrInvalidAction) + } + if on != nil { + switch { + // Add a case for any additional field added to ActionOn struct + case len(on.Branches) > 0: + if strings.HasSuffix(string(event), "-tag") { + return fmt.Errorf("'branches' is not supported in tag event types. %w", ErrInvalidEventParameter) + } + default: + // Nothing to do + } + } + return nil +} + func (a *Action) Match(spec MatchSpec) (bool, error) { // at least one matched event definition actionOn, ok := a.On[spec.EventType] diff --git a/pkg/actions/action_test.go b/pkg/actions/action_test.go index 1c703c43dc4..555bc438828 100644 --- a/pkg/actions/action_test.go +++ b/pkg/actions/action_test.go @@ -31,6 +31,7 @@ func TestAction_ReadAction(t *testing.T) { {name: "invalid hook type", filename: "action_invalid_type.yaml", errStr: "type 'no_temp' unknown: invalid action"}, {name: "invalid event type", filename: "action_invalid_event.yaml", errStr: "event 'not-a-valid-event' is not supported: invalid action"}, {name: "invalid yaml", filename: "action_invalid_yaml.yaml", errStr: "yaml: unmarshal errors"}, + {name: "invalid parameter in tag event", filename: "action_invalid_param_tag_actions.yaml", errStr: "'branches' is not supported in tag event types"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -169,6 +170,69 @@ func TestAction_Match(t *testing.T) { want: false, wantErr: true, }, + { + name: "pre create tag - on pre-create tag without branch", + on: map[graveler.EventType]*actions.ActionOn{graveler.EventTypePreCreateTag: {}}, + spec: actions.MatchSpec{EventType: graveler.EventTypePreCreateTag}, + want: true, + wantErr: false, + }, + { + name: "pre create tag main - on pre-create main", + on: map[graveler.EventType]*actions.ActionOn{graveler.EventTypePreCreateTag: {}}, + spec: actions.MatchSpec{EventType: graveler.EventTypePreCreateTag, BranchID: "main"}, + want: true, + wantErr: false, + }, + { + name: "post create tag main - on post-create main", + on: map[graveler.EventType]*actions.ActionOn{graveler.EventTypePostCreateTag: {}}, + spec: actions.MatchSpec{EventType: graveler.EventTypePostCreateTag, BranchID: "main"}, + want: true, + wantErr: false, + }, + { + name: "pre delete tag main - on pre-delete main", + on: map[graveler.EventType]*actions.ActionOn{graveler.EventTypePreDeleteTag: {}}, + spec: actions.MatchSpec{EventType: graveler.EventTypePreDeleteTag, BranchID: "main"}, + want: true, + wantErr: false, + }, + { + name: "post delete tag main - on post-delete main", + on: map[graveler.EventType]*actions.ActionOn{graveler.EventTypePostDeleteTag: {}}, + spec: actions.MatchSpec{EventType: graveler.EventTypePostDeleteTag, BranchID: "main"}, + want: true, + wantErr: false, + }, + { + name: "pre create branch main - on pre-create main", + on: map[graveler.EventType]*actions.ActionOn{graveler.EventTypePreCreateBranch: {Branches: []string{"main"}}}, + spec: actions.MatchSpec{EventType: graveler.EventTypePreCreateBranch, BranchID: "main"}, + want: true, + wantErr: false, + }, + { + name: "post create branch main - on post-create main", + on: map[graveler.EventType]*actions.ActionOn{graveler.EventTypePostCreateBranch: {Branches: []string{"main"}}}, + spec: actions.MatchSpec{EventType: graveler.EventTypePostCreateBranch, BranchID: "main"}, + want: true, + wantErr: false, + }, + { + name: "pre delete branch main - on pre-delete main", + on: map[graveler.EventType]*actions.ActionOn{graveler.EventTypePreDeleteBranch: {Branches: []string{"main"}}}, + spec: actions.MatchSpec{EventType: graveler.EventTypePreDeleteBranch, BranchID: "main"}, + want: true, + wantErr: false, + }, + { + name: "post delete branch main - on post-delete main", + on: map[graveler.EventType]*actions.ActionOn{graveler.EventTypePostDeleteBranch: {Branches: []string{"main"}}}, + spec: actions.MatchSpec{EventType: graveler.EventTypePostDeleteBranch, BranchID: "main"}, + want: true, + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/actions/event.go b/pkg/actions/event.go index e3c25aab065..c741de89f07 100644 --- a/pkg/actions/event.go +++ b/pkg/actions/event.go @@ -13,10 +13,12 @@ type EventInfo struct { ActionName string `json:"action_name"` HookID string `json:"hook_id"` RepositoryID string `json:"repository_id"` - BranchID string `json:"branch_id"` + BranchID string `json:"branch_id,omitempty"` SourceRef string `json:"source_ref,omitempty"` - CommitMessage string `json:"commit_message"` - Committer string `json:"committer"` + TagID string `json:"tag_id,omitempty"` + CommitID string `json:"commit_id,omitempty"` + CommitMessage string `json:"commit_message,omitempty"` + Committer string `json:"committer,omitempty"` CommitMetadata map[string]string `json:"commit_metadata,omitempty"` } @@ -30,6 +32,8 @@ func marshalEventInformation(actionName, hookID string, record graveler.HookReco RepositoryID: record.RepositoryID.String(), BranchID: record.BranchID.String(), SourceRef: record.SourceRef.String(), + TagID: record.TagID.String(), + CommitID: record.CommitID.String(), CommitMessage: record.Commit.Message, Committer: record.Commit.Committer, CommitMetadata: record.Commit.Metadata, diff --git a/pkg/actions/service.go b/pkg/actions/service.go index 64a798e9546..0df0e8898bb 100644 --- a/pkg/actions/service.go +++ b/pkg/actions/service.go @@ -114,7 +114,7 @@ func (s *Service) asyncRun(record graveler.HookRecord) { // passing the global context for cancelling all runs when lakeFS shuts down if err := s.Run(s.ctx, record); err != nil { - logging.Default().WithField("record", record). + logging.Default().WithError(err).WithField("record", record). Info("Async run of hook failed") } }() @@ -453,6 +453,38 @@ func (s *Service) PostMergeHook(ctx context.Context, record graveler.HookRecord) return nil } +func (s *Service) PreCreateTagHook(ctx context.Context, record graveler.HookRecord) error { + return s.Run(ctx, record) +} + +func (s *Service) PostCreateTagHook(_ context.Context, record graveler.HookRecord) { + s.asyncRun(record) +} + +func (s *Service) PreDeleteTagHook(ctx context.Context, record graveler.HookRecord) error { + return s.Run(ctx, record) +} + +func (s *Service) PostDeleteTagHook(_ context.Context, record graveler.HookRecord) { + s.asyncRun(record) +} + +func (s *Service) PreCreateBranchHook(ctx context.Context, record graveler.HookRecord) error { + return s.Run(ctx, record) +} + +func (s *Service) PostCreateBranchHook(_ context.Context, record graveler.HookRecord) { + s.asyncRun(record) +} + +func (s *Service) PreDeleteBranchHook(ctx context.Context, record graveler.HookRecord) error { + return s.Run(ctx, record) +} + +func (s *Service) PostDeleteBranchHook(_ context.Context, record graveler.HookRecord) { + s.asyncRun(record) +} + func NewHookRunID(actionIdx, hookIdx int) string { return fmt.Sprintf("%04d_%04d", actionIdx, hookIdx) } diff --git a/pkg/actions/testdata/action_invalid_param_tag_actions.yaml b/pkg/actions/testdata/action_invalid_param_tag_actions.yaml new file mode 100644 index 00000000000..7645eb892fc --- /dev/null +++ b/pkg/actions/testdata/action_invalid_param_tag_actions.yaml @@ -0,0 +1,8 @@ +name: invalid parameter for tag event +on: + pre-create-tag: + branches: + - main +hooks: + - id: no_temp + type: webhook \ No newline at end of file diff --git a/pkg/graveler/graveler.go b/pkg/graveler/graveler.go index 5ef0b70c373..c33008fc030 100644 --- a/pkg/graveler/graveler.go +++ b/pkg/graveler/graveler.go @@ -819,10 +819,25 @@ func generateStagingToken(repositoryID RepositoryID, branchID BranchID) StagingT } func (g *Graveler) CreateBranch(ctx context.Context, repositoryID RepositoryID, branchID BranchID, ref Ref) (*Branch, error) { + repo, err := g.RefManager.GetRepository(ctx, repositoryID) + if err != nil { + return nil, fmt.Errorf("get repository: %w", err) + } + storageNamespace := repo.StorageNamespace + reference, err := g.Dereference(ctx, repositoryID, ref) if err != nil { return nil, fmt.Errorf("source reference '%s': %w", ref, err) } + + _, err = g.RefManager.GetBranch(ctx, repositoryID, branchID) + if err == nil { + return nil, ErrBranchExists + } + if !errors.Is(err, ErrBranchNotFound) { + return nil, err + } + if reference.ResolvedBranchModifier == ResolvedBranchModifierStaging { return nil, fmt.Errorf("source reference '%s': %w", ref, ErrCreateBranchNoCommit) } @@ -830,10 +845,42 @@ func (g *Graveler) CreateBranch(ctx context.Context, repositoryID RepositoryID, CommitID: reference.CommitID, StagingToken: generateStagingToken(repositoryID, branchID), } + + preRunID := NewRunID() + err = g.hooks.PreCreateBranchHook(ctx, HookRecord{ + RunID: preRunID, + StorageNamespace: storageNamespace, + EventType: EventTypePreCreateBranch, + SourceRef: ref, + RepositoryID: repositoryID, + BranchID: branchID, + CommitID: reference.CommitID, + }) + if err != nil { + return nil, &HookAbortError{ + EventType: EventTypePreCreateBranch, + RunID: preRunID, + Err: err, + } + } + err = g.RefManager.CreateBranch(ctx, repositoryID, branchID, newBranch) if err != nil { return nil, fmt.Errorf("set branch '%s' to '%s': %w", branchID, newBranch, err) } + + postRunID := NewRunID() + g.hooks.PostCreateBranchHook(ctx, HookRecord{ + RunID: postRunID, + StorageNamespace: storageNamespace, + EventType: EventTypePostCreateBranch, + SourceRef: ref, + RepositoryID: repositoryID, + BranchID: branchID, + CommitID: reference.CommitID, + PreRunID: preRunID, + }) + return &newBranch, nil } @@ -891,11 +938,107 @@ func (g *Graveler) GetTag(ctx context.Context, repositoryID RepositoryID, tagID } func (g *Graveler) CreateTag(ctx context.Context, repositoryID RepositoryID, tagID TagID, commitID CommitID) error { - return g.RefManager.CreateTag(ctx, repositoryID, tagID, commitID) + repo, err := g.RefManager.GetRepository(ctx, repositoryID) + if err != nil { + return fmt.Errorf("get repository: %w", err) + } + storageNamespace := repo.StorageNamespace + + // Check that Tag doesn't exist before running hook + _, err = g.RefManager.GetTag(ctx, repositoryID, tagID) + if err == nil { + return ErrTagAlreadyExists + } + if !errors.Is(err, ErrTagNotFound) { + return err + } + + preRunID := NewRunID() + err = g.hooks.PreCreateTagHook(ctx, HookRecord{ + RunID: preRunID, + StorageNamespace: storageNamespace, + EventType: EventTypePreCreateTag, + RepositoryID: repositoryID, + CommitID: commitID, + SourceRef: commitID.Ref(), + TagID: tagID, + }) + if err != nil { + return &HookAbortError{ + EventType: EventTypePreCreateTag, + RunID: preRunID, + Err: err, + } + } + + err = g.RefManager.CreateTag(ctx, repositoryID, tagID, commitID) + if err != nil { + return err + } + + postRunID := NewRunID() + g.hooks.PostCreateTagHook(ctx, HookRecord{ + RunID: postRunID, + StorageNamespace: storageNamespace, + EventType: EventTypePostCreateTag, + RepositoryID: repositoryID, + CommitID: commitID, + SourceRef: commitID.Ref(), + TagID: tagID, + PreRunID: preRunID, + }) + + return nil } func (g *Graveler) DeleteTag(ctx context.Context, repositoryID RepositoryID, tagID TagID) error { - return g.RefManager.DeleteTag(ctx, repositoryID, tagID) + repo, err := g.RefManager.GetRepository(ctx, repositoryID) + if err != nil { + return fmt.Errorf("get repository: %w", err) + } + storageNamespace := repo.StorageNamespace + + // Sanity check that Tag exists before running hook. + commitID, err := g.RefManager.GetTag(ctx, repositoryID, tagID) + if err != nil { + return err + } + + preRunID := NewRunID() + err = g.hooks.PreDeleteTagHook(ctx, HookRecord{ + RunID: preRunID, + StorageNamespace: storageNamespace, + EventType: EventTypePreDeleteTag, + RepositoryID: repositoryID, + SourceRef: commitID.Ref(), + CommitID: *commitID, + TagID: tagID, + }) + if err != nil { + return &HookAbortError{ + EventType: EventTypePreDeleteTag, + RunID: preRunID, + Err: err, + } + } + + err = g.RefManager.DeleteTag(ctx, repositoryID, tagID) + if err != nil { + return err + } + + postRunID := NewRunID() + g.hooks.PostDeleteTagHook(ctx, HookRecord{ + RunID: postRunID, + StorageNamespace: storageNamespace, + EventType: EventTypePostDeleteTag, + RepositoryID: repositoryID, + SourceRef: commitID.Ref(), + CommitID: *commitID, + TagID: tagID, + PreRunID: preRunID, + }) + return nil } func (g *Graveler) ListTags(ctx context.Context, repositoryID RepositoryID) (TagIterator, error) { @@ -931,6 +1074,11 @@ func (g *Graveler) ListBranches(ctx context.Context, repositoryID RepositoryID) } func (g *Graveler) DeleteBranch(ctx context.Context, repositoryID RepositoryID, branchID BranchID) error { + var ( + preRunID string + storageNamespace StorageNamespace + commitID CommitID + ) _, err := g.branchLocker.MetadataUpdater(ctx, repositoryID, branchID, func() (interface{}, error) { repo, err := g.RefManager.GetRepository(ctx, repositoryID) if err != nil { @@ -943,13 +1091,50 @@ func (g *Graveler) DeleteBranch(ctx context.Context, repositoryID RepositoryID, if err != nil { return nil, err } + commitID = branch.CommitID err = g.StagingManager.Drop(ctx, branch.StagingToken) if err != nil && !errors.Is(err, ErrNotFound) { return nil, err } + + storageNamespace = repo.StorageNamespace + preRunID = NewRunID() + preHookRecord := HookRecord{ + RunID: preRunID, + StorageNamespace: storageNamespace, + EventType: EventTypePreDeleteBranch, + RepositoryID: repositoryID, + SourceRef: commitID.Ref(), + BranchID: branchID, + } + err = g.hooks.PreDeleteBranchHook(ctx, preHookRecord) + if err != nil { + return nil, &HookAbortError{ + EventType: EventTypePreDeleteBranch, + RunID: preRunID, + Err: err, + } + } + return nil, g.RefManager.DeleteBranch(ctx, repositoryID, branchID) }) - return err + + if err != nil { // Don't perform post action hook if operation finished with error + return err + } + + postRunID := NewRunID() + g.hooks.PostDeleteBranchHook(ctx, HookRecord{ + RunID: postRunID, + StorageNamespace: storageNamespace, + EventType: EventTypePostDeleteBranch, + RepositoryID: repositoryID, + SourceRef: commitID.Ref(), + BranchID: branchID, + PreRunID: preRunID, + }) + + return nil } func (g *Graveler) GetStagingToken(ctx context.Context, repositoryID RepositoryID, branchID BranchID) (*StagingToken, error) { @@ -1297,7 +1482,7 @@ func (g *Graveler) Commit(ctx context.Context, repositoryID RepositoryID, branch RunID: postRunID, RepositoryID: repositoryID, StorageNamespace: storageNamespace, - SourceRef: branchID.Ref(), + SourceRef: res.(CommitID).Ref(), BranchID: branchID, Commit: commit, CommitID: newCommitID, @@ -1689,7 +1874,7 @@ func (g *Graveler) Merge(ctx context.Context, repositoryID RepositoryID, destina RepositoryID: repositoryID, StorageNamespace: storageNamespace, BranchID: destination, - SourceRef: source, + SourceRef: res.(CommitID).Ref(), Commit: commit, CommitID: res.(CommitID), PreRunID: preRunID, diff --git a/pkg/graveler/graveler_test.go b/pkg/graveler/graveler_test.go index 1c20826b640..9b70070c6df 100644 --- a/pkg/graveler/graveler_test.go +++ b/pkg/graveler/graveler_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "errors" + "strconv" "testing" "github.com/go-test/deep" @@ -23,6 +24,7 @@ type Hooks struct { SourceRef graveler.Ref CommitID graveler.CommitID Commit graveler.Commit + TagID graveler.TagID } func (h *Hooks) PreCommitHook(_ context.Context, record graveler.HookRecord) error { @@ -64,6 +66,72 @@ func (h *Hooks) PostMergeHook(_ context.Context, record graveler.HookRecord) err return h.Err } +func (h *Hooks) PreCreateTagHook(_ context.Context, record graveler.HookRecord) error { + h.Called = true + h.StorageNamespace = record.StorageNamespace + h.RepositoryID = record.RepositoryID + h.CommitID = record.CommitID + h.TagID = record.TagID + return h.Err +} + +func (h *Hooks) PostCreateTagHook(_ context.Context, record graveler.HookRecord) { + h.Called = true + h.StorageNamespace = record.StorageNamespace + h.RepositoryID = record.RepositoryID + h.CommitID = record.CommitID + h.TagID = record.TagID +} + +func (h *Hooks) PreDeleteTagHook(_ context.Context, record graveler.HookRecord) error { + h.Called = true + h.StorageNamespace = record.StorageNamespace + h.RepositoryID = record.RepositoryID + h.TagID = record.TagID + return h.Err +} + +func (h *Hooks) PostDeleteTagHook(_ context.Context, record graveler.HookRecord) { + h.Called = true + h.StorageNamespace = record.StorageNamespace + h.RepositoryID = record.RepositoryID + h.TagID = record.TagID +} + +func (h *Hooks) PreCreateBranchHook(_ context.Context, record graveler.HookRecord) error { + h.Called = true + h.StorageNamespace = record.StorageNamespace + h.RepositoryID = record.RepositoryID + h.BranchID = record.BranchID + h.CommitID = record.CommitID + h.SourceRef = record.SourceRef + return h.Err +} + +func (h *Hooks) PostCreateBranchHook(_ context.Context, record graveler.HookRecord) { + h.Called = true + h.StorageNamespace = record.StorageNamespace + h.RepositoryID = record.RepositoryID + h.BranchID = record.BranchID + h.CommitID = record.CommitID + h.SourceRef = record.SourceRef +} + +func (h *Hooks) PreDeleteBranchHook(_ context.Context, record graveler.HookRecord) error { + h.Called = true + h.StorageNamespace = record.StorageNamespace + h.RepositoryID = record.RepositoryID + h.BranchID = record.BranchID + return h.Err +} + +func (h *Hooks) PostDeleteBranchHook(_ context.Context, record graveler.HookRecord) { + h.Called = true + h.StorageNamespace = record.StorageNamespace + h.RepositoryID = record.RepositoryID + h.BranchID = record.BranchID +} + func TestGraveler_List(t *testing.T) { conn, _ := tu.GetDB(t, databaseURI) branchLocker := ref.NewBranchLocker(conn) @@ -294,7 +362,7 @@ func TestGraveler_CreateBranch(t *testing.T) { gravel := graveler.NewGraveler(branchLocker, nil, nil, &testutil.RefsFake{ - Err: graveler.ErrNotFound, + Err: graveler.ErrBranchNotFound, CommitID: "8888888798e3aeface8e62d1c7072a965314b4", }, nil, nil, ) @@ -1233,3 +1301,387 @@ func TestGraveler_Delete(t *testing.T) { }) } } + +func TestGraveler_CreateTag(t *testing.T) { + // prepare graveler + conn, _ := tu.GetDB(t, databaseURI) + branchLocker := ref.NewBranchLocker(conn) + const repositoryID = "repoID" + const commitID = graveler.CommitID("commitID") + const tagID = graveler.TagID("tagID") + committedManager := &testutil.CommittedFake{} + stagingManager := &testutil.StagingFake{ValueIterator: testutil.NewValueIteratorFake(nil)} + refManager := &testutil.RefsFake{ + Err: graveler.ErrTagNotFound, + } + // tests + errSomethingBad := errors.New("first error") + tests := []struct { + name string + err error + }{ + { + name: "Successful", + err: nil, + }, + { + name: "Tag exists", + err: graveler.ErrTagAlreadyExists, + }, + { + name: "Other error on get tag", + err: errSomethingBad, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // setup + ctx := context.Background() + + if tt.err != nil { + refManager.Err = tt.err + } + g := graveler.NewGraveler(branchLocker, committedManager, stagingManager, refManager, nil, testutil.NewProtectedBranchesManagerFake()) + err := g.CreateTag(ctx, repositoryID, tagID, commitID) + + // verify we got an error + if !errors.Is(err, tt.err) { + t.Fatalf("Create tag err=%v, expected=%v", err, tt.err) + } + }) + } +} + +func TestGraveler_PreCreateTagHook(t *testing.T) { + // prepare graveler + conn, _ := tu.GetDB(t, databaseURI) + branchLocker := ref.NewBranchLocker(conn) + const repositoryID = "repoID" + const expectedRangeID = graveler.MetaRangeID("expectedRangeID") + const expectedCommitID = graveler.CommitID("expectedCommitID") + const expectedTagID = graveler.TagID("expectedTagID") + committedManager := &testutil.CommittedFake{MetaRangeID: expectedRangeID} + stagingManager := &testutil.StagingFake{ValueIterator: testutil.NewValueIteratorFake(nil)} + refManager := &testutil.RefsFake{ + CommitID: expectedCommitID, + Branch: &graveler.Branch{CommitID: expectedCommitID}, + Err: graveler.ErrTagNotFound, + Commits: map[graveler.CommitID]*graveler.Commit{ + expectedCommitID: {MetaRangeID: expectedRangeID}, + }, + } + // tests + errSomethingBad := errors.New("first error") + tests := []struct { + name string + hook bool + err error + }{ + { + name: "without hook", + hook: false, + err: nil, + }, + { + name: "hook no error", + hook: true, + err: nil, + }, + { + name: "hook error", + hook: true, + err: errSomethingBad, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // setup + ctx := context.Background() + g := graveler.NewGraveler(branchLocker, committedManager, stagingManager, refManager, nil, testutil.NewProtectedBranchesManagerFake()) + h := &Hooks{Err: tt.err} + if tt.hook { + g.SetHooksHandler(h) + } + + err := g.CreateTag(ctx, repositoryID, expectedTagID, expectedCommitID) + + // verify we got an error + if !errors.Is(err, tt.err) { + t.Fatalf("Create tag err=%v, expected=%v", err, tt.err) + } + var hookErr *graveler.HookAbortError + if err != nil && !errors.As(err, &hookErr) { + t.Fatalf("Create tag err=%v, expected HookAbortError", err) + } + + // 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 !h.Called { + return + } + if h.RepositoryID != repositoryID { + t.Errorf("Hook repository '%s', expected '%s'", h.RepositoryID, repositoryID) + } + if h.CommitID != expectedCommitID { + t.Errorf("Hook commit ID '%s', expected '%s'", h.BranchID, expectedCommitID) + } + if h.TagID != expectedTagID { + t.Errorf("Hook tag ID '%s', expected '%s'", h.TagID, expectedTagID) + } + }) + } +} + +func TestGraveler_PreDeleteTagHook(t *testing.T) { + // prepare graveler + conn, _ := tu.GetDB(t, databaseURI) + branchLocker := ref.NewBranchLocker(conn) + const repositoryID = "repoID" + const expectedRangeID = graveler.MetaRangeID("expectedRangeID") + const expectedCommitID = graveler.CommitID("expectedCommitID") + const expectedTagID = graveler.TagID("expectedTagID") + committedManager := &testutil.CommittedFake{MetaRangeID: expectedRangeID} + stagingManager := &testutil.StagingFake{ValueIterator: testutil.NewValueIteratorFake(nil)} + refManager := &testutil.RefsFake{ + CommitID: expectedCommitID, + Branch: &graveler.Branch{CommitID: expectedCommitID}, + Commits: map[graveler.CommitID]*graveler.Commit{ + expectedCommitID: {MetaRangeID: expectedRangeID}, + }, + } + // tests + errSomethingBad := errors.New("first error") + tests := []struct { + name string + hook bool + err error + }{ + { + name: "without hook", + hook: false, + err: nil, + }, + { + name: "hook no error", + hook: true, + err: nil, + }, + { + name: "hook error", + hook: true, + err: errSomethingBad, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // setup + ctx := context.Background() + expected := expectedCommitID + refManager.TagCommitID = &expected + g := graveler.NewGraveler(branchLocker, committedManager, stagingManager, refManager, nil, testutil.NewProtectedBranchesManagerFake()) + h := &Hooks{Err: tt.err} + if tt.hook { + g.SetHooksHandler(h) + } + + err := g.DeleteTag(ctx, repositoryID, expectedTagID) + + // verify we got an error + if !errors.Is(err, tt.err) { + t.Fatalf("Delete tag err=%v, expected=%v", err, tt.err) + } + var hookErr *graveler.HookAbortError + if err != nil && !errors.As(err, &hookErr) { + t.Fatalf("Delete Tag err=%v, expected HookAbortError", err) + } + + // 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 !h.Called { + return + } + if h.RepositoryID != repositoryID { + t.Errorf("Hook repository '%s', expected '%s'", h.RepositoryID, repositoryID) + } + if h.TagID != expectedTagID { + t.Errorf("Hook tag ID '%s', expected '%s'", h.TagID, expectedTagID) + } + }) + } +} + +func TestGraveler_PreCreateBranchHook(t *testing.T) { + // prepare graveler + conn, _ := tu.GetDB(t, databaseURI) + branchLocker := ref.NewBranchLocker(conn) + const repositoryID = "repoID" + const expectedRangeID = graveler.MetaRangeID("expectedRangeID") + const sourceCommitID = graveler.CommitID("sourceCommitID") + const sourceBranchID = graveler.CommitID("sourceBranchID") + const newBranchPrefix = "newBranch-" + committedManager := &testutil.CommittedFake{MetaRangeID: expectedRangeID} + stagingManager := &testutil.StagingFake{ValueIterator: testutil.NewValueIteratorFake(nil)} + refManager := &testutil.RefsFake{ + Refs: map[graveler.Ref]*graveler.ResolvedRef{graveler.Ref(sourceBranchID): { + Type: graveler.ReferenceTypeBranch, + BranchID: graveler.BranchID(sourceBranchID), + ResolvedBranchModifier: 0, + CommitID: sourceCommitID, + StagingToken: "", + }}, + } + // tests + errSomethingBad := errors.New("first error") + tests := []struct { + name string + hook bool + err error + }{ + { + name: "without hook", + hook: false, + err: nil, + }, + { + name: "hook no error", + hook: true, + err: nil, + }, + { + name: "hook error", + hook: true, + err: errSomethingBad, + }, + } + for i, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // setup + ctx := context.Background() + g := graveler.NewGraveler(branchLocker, committedManager, stagingManager, refManager, nil, testutil.NewProtectedBranchesManagerFake()) + h := &Hooks{Err: tt.err} + if tt.hook { + g.SetHooksHandler(h) + } + + // WA for CreateBranch fake logic + refManager.Branch = nil + refManager.Err = graveler.ErrBranchNotFound + + newBranch := newBranchPrefix + strconv.Itoa(i) + _, err := g.CreateBranch(ctx, repositoryID, graveler.BranchID(newBranch), graveler.Ref(sourceBranchID)) + + // verify we got an error + if !errors.Is(err, tt.err) { + t.Fatalf("Create branch err=%v, expected=%v", err, tt.err) + } + var hookErr *graveler.HookAbortError + if err != nil && !errors.As(err, &hookErr) { + t.Fatalf("Create branch err=%v, expected HookAbortError", err) + } + + // 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 !h.Called { + return + } + if h.RepositoryID != repositoryID { + t.Errorf("Hook repository '%s', expected '%s'", h.RepositoryID, repositoryID) + } + if h.CommitID != sourceCommitID { + t.Errorf("Hook commit ID '%s', expected '%s'", h.BranchID, sourceCommitID) + } + if h.BranchID != graveler.BranchID(newBranch) { + t.Errorf("Hook branch ID '%s', expected '%s'", h.BranchID, newBranch) + } + }) + } +} + +func TestGraveler_PreDeleteBranchHook(t *testing.T) { + // prepare graveler + conn, _ := tu.GetDB(t, databaseURI) + branchLocker := ref.NewBranchLocker(conn) + const repositoryID = "repoID" + const expectedRangeID = graveler.MetaRangeID("expectedRangeID") + const sourceCommitID = graveler.CommitID("sourceCommitID") + const sourceBranchID = graveler.CommitID("sourceBranchID") + committedManager := &testutil.CommittedFake{MetaRangeID: expectedRangeID} + values := testutil.NewValueIteratorFake([]graveler.ValueRecord{{Key: nil, Value: nil}}) + stagingManager := &testutil.StagingFake{ValueIterator: values} + refManager := &testutil.RefsFake{ + Refs: map[graveler.Ref]*graveler.ResolvedRef{graveler.Ref(sourceBranchID): { + Type: graveler.ReferenceTypeBranch, + BranchID: graveler.BranchID(sourceBranchID), + ResolvedBranchModifier: 0, + CommitID: sourceCommitID, + StagingToken: "token", + }}, + Branch: &graveler.Branch{CommitID: sourceBranchID, StagingToken: "token"}, + StagingToken: "token", + } + // tests + errSomethingBad := errors.New("first error") + tests := []struct { + name string + hook bool + err error + }{ + { + name: "without hook", + hook: false, + err: nil, + }, + { + name: "hook no error", + hook: true, + err: nil, + }, + { + name: "hook error", + hook: true, + err: errSomethingBad, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // setup + ctx := context.Background() + g := graveler.NewGraveler(branchLocker, committedManager, stagingManager, refManager, nil, testutil.NewProtectedBranchesManagerFake()) + h := &Hooks{Err: tt.err} + if tt.hook { + g.SetHooksHandler(h) + } + + err := g.DeleteBranch(ctx, repositoryID, graveler.BranchID(sourceBranchID)) + + // verify we got an error + if !errors.Is(err, tt.err) { + t.Fatalf("Delete branch err=%v, expected=%v", err, tt.err) + } + var hookErr *graveler.HookAbortError + if err != nil && !errors.As(err, &hookErr) { + t.Fatalf("Delete branch err=%v, expected HookAbortError", err) + } + + // 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 !h.Called { + return + } + if h.RepositoryID != repositoryID { + t.Errorf("Hook repository '%s', expected '%s'", h.RepositoryID, repositoryID) + } + if h.BranchID != graveler.BranchID(sourceBranchID) { + t.Errorf("Hook branch ID '%s', expected '%s'", h.BranchID, sourceBranchID) + } + }) + } +} diff --git a/pkg/graveler/hooks_handler.go b/pkg/graveler/hooks_handler.go index d943f6dd364..bb4561567fc 100644 --- a/pkg/graveler/hooks_handler.go +++ b/pkg/graveler/hooks_handler.go @@ -10,22 +10,40 @@ import ( type EventType string const ( - EventTypePreCommit EventType = "pre-commit" - EventTypePostCommit EventType = "post-commit" - EventTypePreMerge EventType = "pre-merge" - EventTypePostMerge EventType = "post-merge" + EventTypePreCommit EventType = "pre-commit" + EventTypePostCommit EventType = "post-commit" + EventTypePreMerge EventType = "pre-merge" + EventTypePostMerge EventType = "post-merge" + EventTypePreCreateTag EventType = "pre-create-tag" + EventTypePostCreateTag EventType = "post-create-tag" + EventTypePreDeleteTag EventType = "pre-delete-tag" + EventTypePostDeleteTag EventType = "post-delete-tag" + EventTypePreCreateBranch EventType = "pre-create-branch" + EventTypePostCreateBranch EventType = "post-create-branch" + EventTypePreDeleteBranch EventType = "pre-delete-branch" + EventTypePostDeleteBranch EventType = "post-delete-branch" ) +// HookRecord is an aggregation of all necessary fields for all event types type HookRecord struct { + // Required fields for all event types: RunID string EventType EventType RepositoryID RepositoryID StorageNamespace StorageNamespace - BranchID BranchID - SourceRef Ref - Commit Commit - CommitID CommitID - PreRunID string + // The reference which the actions files are read from + SourceRef Ref + // Event specific fields: + // Relevant for all event types except tags. For merge events this will be the ID of the destination branch + BranchID BranchID + // Relevant only for commit and merge events. In both it will contain the new commit data created from the operation + Commit Commit + // Not relevant in delete branch. In commit and merge will not exist in pre-action. In post actions will contain the new commit ID + CommitID CommitID + // Exists only in post actions. Contains the ID of the pre-action associated with this post-action + PreRunID string + // Exists only in tag actions. + TagID TagID } type HooksHandler interface { @@ -33,6 +51,14 @@ type HooksHandler interface { PostCommitHook(ctx context.Context, record HookRecord) error PreMergeHook(ctx context.Context, record HookRecord) error PostMergeHook(ctx context.Context, record HookRecord) error + PreCreateTagHook(ctx context.Context, record HookRecord) error + PostCreateTagHook(ctx context.Context, record HookRecord) + PreDeleteTagHook(ctx context.Context, record HookRecord) error + PostDeleteTagHook(ctx context.Context, record HookRecord) + PreCreateBranchHook(ctx context.Context, record HookRecord) error + PostCreateBranchHook(ctx context.Context, record HookRecord) + PreDeleteBranchHook(ctx context.Context, record HookRecord) error + PostDeleteBranchHook(ctx context.Context, record HookRecord) } type HooksNoOp struct{} @@ -53,6 +79,34 @@ func (h *HooksNoOp) PostMergeHook(context.Context, HookRecord) error { return nil } +func (h *HooksNoOp) PreCreateTagHook(context.Context, HookRecord) error { + return nil +} + +func (h *HooksNoOp) PostCreateTagHook(context.Context, HookRecord) { +} + +func (h *HooksNoOp) PreDeleteTagHook(context.Context, HookRecord) error { + return nil +} + +func (h *HooksNoOp) PostDeleteTagHook(context.Context, HookRecord) { +} + +func (h *HooksNoOp) PreCreateBranchHook(context.Context, HookRecord) error { + return nil +} + +func (h *HooksNoOp) PostCreateBranchHook(context.Context, HookRecord) { +} + +func (h *HooksNoOp) PreDeleteBranchHook(context.Context, HookRecord) error { + return nil +} + +func (h *HooksNoOp) PostDeleteBranchHook(context.Context, HookRecord) { +} + func NewRunID() string { const nanoLen = 8 id := nanoid.Must(nanoLen)