From dfdbc921b9fc3e6cd4865769357c4570c5ef5cdf Mon Sep 17 00:00:00 2001 From: Daniel Nilsson Date: Mon, 29 Oct 2018 15:54:54 +0100 Subject: [PATCH 1/9] Added support for github api preview signed commits. Preview required commit signatures for Protected Branches API https://developer.github.com/changes/2018-02-22-protected-branches-required-signatures/ Signed-off-by: Daniel Nilsson --- github/github.go | 3 ++ github/repos.go | 68 ++++++++++++++++++++++++++++++++++++++++++++ github/repos_test.go | 65 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 136 insertions(+) diff --git a/github/github.go b/github/github.go index 320b8104b67..d00c5e0c2a4 100644 --- a/github/github.go +++ b/github/github.go @@ -122,6 +122,9 @@ const ( // https://developer.github.com/enterprise/2.13/v3/repos/pre_receive_hooks/ mediaTypePreReceiveHooksPreview = "application/vnd.github.eye-scream-preview" + + // https://developer.github.com/changes/2018-02-22-protected-branches-required-signatures/ + mediaTypeSignaturePreview = "application/vnd.github.zzzax-preview+json" ) // A Client manages communication with the GitHub API. diff --git a/github/repos.go b/github/repos.go index e783ccbea03..21be6c685b2 100644 --- a/github/repos.go +++ b/github/repos.go @@ -698,6 +698,13 @@ type DismissalRestrictionsRequest struct { Teams *[]string `json:"teams,omitempty"` } +// SignaturesProtectedBranch represents the protection status of a individual branch. +type SignaturesProtectedBranch struct { + URL *string `json:"url,omitempty"` + // Commits pushed to matching branches must have verified signatures. + Enabled bool `json:"strict"` +} + // ListBranches lists branches for the specified repository. // // GitHub API docs: https://developer.github.com/v3/repos/#list-branches @@ -850,6 +857,67 @@ func (s *RepositoriesService) RemoveBranchProtection(ctx context.Context, owner, return s.client.Do(ctx, req, nil) } +// GetSignaturesProtectedBranch get required signatures of protected branch +// +// GitHub API docs: https://developer.github.com/v3/repos/branches/#get-required-signatures-of-protected-branch +func (s *RepositoriesService) GetSignaturesProtectedBranch(ctx context.Context, owner, repo, branch string) (*SignaturesProtectedBranch, *Response, error) { + u := fmt.Sprintf("repos/%v/%v/branches/%v/protection/required_signatures", owner, repo, branch) + req, err := s.client.NewRequest("GET", u, nil) + if err != nil { + return nil, nil, err + } + + // TODO: remove custom Accept header when this API fully launches + req.Header.Set("Accept", mediaTypeSignaturePreview) + + p := new(SignaturesProtectedBranch) + resp, err := s.client.Do(ctx, req, p) + if err != nil { + return nil, resp, err + } + + return p, resp, nil +} + +// AddSignatureProtectedBranch Require signed commits to a protected branch. +// It requires admin access and branch protection to be enabled. +// +// GitHub API docs: https://developer.github.com/v3/repos/branches/#add-required-signatures-of-protected-branch +func (s *RepositoriesService) AddSignatureProtectedBranch(ctx context.Context, owner, repo, branch string) (*SignaturesProtectedBranch, *Response, error) { + u := fmt.Sprintf("repos/%v/%v/branches/%v/protection/required_signatures", owner, repo, branch) + req, err := s.client.NewRequest("POST", u, nil) + if err != nil { + return nil, nil, err + } + + // TODO: remove custom Accept header when this API fully launches + req.Header.Set("Accept", mediaTypeSignaturePreview) + + r := new(SignaturesProtectedBranch) + resp, err := s.client.Do(ctx, req, r) + if err != nil { + return nil, resp, err + } + + return r, resp, err +} + +// RemoveSignatureProtectedBranch removes required signed commits on a given branch. +// +// GitHub API docs: https://developer.github.com/v3/repos/branches/#remove-required-signatures-of-protected-branch +func (s *RepositoriesService) RemoveSignatureProtectedBranch(ctx context.Context, owner, repo, branch string) (*Response, error) { + u := fmt.Sprintf("repos/%v/%v/branches/%v/protection/required_signatures", owner, repo, branch) + req, err := s.client.NewRequest("DELETE", u, nil) + if err != nil { + return nil, err + } + + // TODO: remove custom Accept header when this API fully launches + req.Header.Set("Accept", mediaTypeSignaturePreview) + + return s.client.Do(ctx, req, nil) +} + // UpdateRequiredStatusChecks updates the required status checks for a given protected branch. // // GitHub API docs: https://developer.github.com/v3/repos/branches/#update-required-status-checks-of-protected-branch diff --git a/github/repos_test.go b/github/repos_test.go index 08d64412346..21475a41845 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -1055,6 +1055,71 @@ func TestRepositoriesService_RemoveAdminEnforcement(t *testing.T) { } } +func TestRepositoriesService_GetSignaturesProtectedBranch(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + mux.HandleFunc("/repos/o/r/branches/b/protection/required_signatures", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + testHeader(t, r, "Accept", mediaTypeSignaturePreview) + fmt.Fprintf(w, `{"url":"/repos/o/r/branches/b/protection/required_signatures","enabled":false}`) + }) + + signature, _, err := client.Repositories.GetSignaturesProtectedBranch(context.Background(), "o", "r", "b") + if err != nil { + t.Errorf("Repositories.GetSignaturesProtectedBranch returned error: %v", err) + } + + want := &SignaturesProtectedBranch{ + URL: String("/repos/o/r/branches/b/protection/required_signatures"), + Enabled: false, + } + + if !reflect.DeepEqual(signature, want) { + t.Errorf("Repositories.GetSignaturesProtectedBranch returned %+v, want %+v", signature, want) + } +} + +func TestRepositoriesService_AddSignatureProtectedBranch(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + mux.HandleFunc("/repos/o/r/branches/b/protection/required_signatures", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "POST") + testHeader(t, r, "Accept", mediaTypeSignaturePreview) + fmt.Fprintf(w, `{"url":"/repos/o/r/branches/b/protection/required_signatures","enabled":false}`) + }) + + signature, _, err := client.Repositories.AddSignatureProtectedBranch(context.Background(), "o", "r", "b") + if err != nil { + t.Errorf("Repositories.AddSignatureProtectedBranch returned error: %v", err) + } + + want := &SignaturesProtectedBranch{ + URL: String("/repos/o/r/branches/b/protection/required_signatures"), + Enabled: false, + } + if !reflect.DeepEqual(signature, want) { + t.Errorf("Repositories.AddSignatureProtectedBranch returned %+v, want %+v", signature, want) + } +} + +func TestRepositoriesService_RemoveSignatureProtectedBranch(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + mux.HandleFunc("/repos/o/r/branches/b/protection/required_signatures", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "DELETE") + testHeader(t, r, "Accept", mediaTypeSignaturePreview) + w.WriteHeader(http.StatusNoContent) + }) + + _, err := client.Repositories.RemoveSignatureProtectedBranch(context.Background(), "o", "r", "b") + if err != nil { + t.Errorf("Repositories.RemoveSignatureProtectedBranch returned error: %v", err) + } +} + func TestPullRequestReviewsEnforcementRequest_MarshalJSON_nilDismissalRestirctions(t *testing.T) { req := PullRequestReviewsEnforcementRequest{} From c5c33fc3cedc7e17852b99a7666700a53cabe966 Mon Sep 17 00:00:00 2001 From: Daniel Nilsson Date: Mon, 29 Oct 2018 16:09:31 +0100 Subject: [PATCH 2/9] added SignaturesProtectedBranch getURL Signed-off-by: Daniel Nilsson --- github/github-accessors.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/github/github-accessors.go b/github/github-accessors.go index 3ec79c2ca33..2e4a2c21def 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -10100,6 +10100,14 @@ func (s *ServiceHook) GetName() string { return *s.Name } +// GetURL returns the URL field if it's non-nil, zero value otherwise. +func (s *SignaturesProtectedBranch) GetURL() string { + if s == nil || s.URL == nil { + return "" + } + return *s.URL +} + // GetPayload returns the Payload field if it's non-nil, zero value otherwise. func (s *SignatureVerification) GetPayload() string { if s == nil || s.Payload == nil { From 00f96c6df12c2de453dafddf3345e43ac0dde1de Mon Sep 17 00:00:00 2001 From: Daniel Nilsson Date: Fri, 2 Nov 2018 08:26:44 +0100 Subject: [PATCH 3/9] Corrected grammar and naming convention. Signed-off-by: Daniel Nilsson --- github/repos.go | 14 +++++++------- github/repos_test.go | 10 +++++----- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/github/repos.go b/github/repos.go index 21be6c685b2..976d8b0610a 100644 --- a/github/repos.go +++ b/github/repos.go @@ -698,11 +698,11 @@ type DismissalRestrictionsRequest struct { Teams *[]string `json:"teams,omitempty"` } -// SignaturesProtectedBranch represents the protection status of a individual branch. +// SignaturesProtectedBranch represents the protection status of an individual branch. type SignaturesProtectedBranch struct { URL *string `json:"url,omitempty"` // Commits pushed to matching branches must have verified signatures. - Enabled bool `json:"strict"` + Enabled bool `json:"strict,omitempty"` } // ListBranches lists branches for the specified repository. @@ -857,7 +857,7 @@ func (s *RepositoriesService) RemoveBranchProtection(ctx context.Context, owner, return s.client.Do(ctx, req, nil) } -// GetSignaturesProtectedBranch get required signatures of protected branch +// GetSignaturesProtectedBranch gets required signatures of protected branch. // // GitHub API docs: https://developer.github.com/v3/repos/branches/#get-required-signatures-of-protected-branch func (s *RepositoriesService) GetSignaturesProtectedBranch(ctx context.Context, owner, repo, branch string) (*SignaturesProtectedBranch, *Response, error) { @@ -879,11 +879,11 @@ func (s *RepositoriesService) GetSignaturesProtectedBranch(ctx context.Context, return p, resp, nil } -// AddSignatureProtectedBranch Require signed commits to a protected branch. +// RequireSignaturesOnProtectedBranch makes signed commits required on a protected branch. // It requires admin access and branch protection to be enabled. // // GitHub API docs: https://developer.github.com/v3/repos/branches/#add-required-signatures-of-protected-branch -func (s *RepositoriesService) AddSignatureProtectedBranch(ctx context.Context, owner, repo, branch string) (*SignaturesProtectedBranch, *Response, error) { +func (s *RepositoriesService) RequireSignaturesOnProtectedBranch(ctx context.Context, owner, repo, branch string) (*SignaturesProtectedBranch, *Response, error) { u := fmt.Sprintf("repos/%v/%v/branches/%v/protection/required_signatures", owner, repo, branch) req, err := s.client.NewRequest("POST", u, nil) if err != nil { @@ -902,10 +902,10 @@ func (s *RepositoriesService) AddSignatureProtectedBranch(ctx context.Context, o return r, resp, err } -// RemoveSignatureProtectedBranch removes required signed commits on a given branch. +// OptionalSignaturesOnProtectedBranch removes required signed commits on a given branch. // // GitHub API docs: https://developer.github.com/v3/repos/branches/#remove-required-signatures-of-protected-branch -func (s *RepositoriesService) RemoveSignatureProtectedBranch(ctx context.Context, owner, repo, branch string) (*Response, error) { +func (s *RepositoriesService) OptionalSignaturesOnProtectedBranch(ctx context.Context, owner, repo, branch string) (*Response, error) { u := fmt.Sprintf("repos/%v/%v/branches/%v/protection/required_signatures", owner, repo, branch) req, err := s.client.NewRequest("DELETE", u, nil) if err != nil { diff --git a/github/repos_test.go b/github/repos_test.go index 21475a41845..ecd6fe32a70 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -1090,9 +1090,9 @@ func TestRepositoriesService_AddSignatureProtectedBranch(t *testing.T) { fmt.Fprintf(w, `{"url":"/repos/o/r/branches/b/protection/required_signatures","enabled":false}`) }) - signature, _, err := client.Repositories.AddSignatureProtectedBranch(context.Background(), "o", "r", "b") + signature, _, err := client.Repositories.RequireSignaturesOnProtectedBranch(context.Background(), "o", "r", "b") if err != nil { - t.Errorf("Repositories.AddSignatureProtectedBranch returned error: %v", err) + t.Errorf("Repositories.RequireSignaturesOnProtectedBranch returned error: %v", err) } want := &SignaturesProtectedBranch{ @@ -1100,7 +1100,7 @@ func TestRepositoriesService_AddSignatureProtectedBranch(t *testing.T) { Enabled: false, } if !reflect.DeepEqual(signature, want) { - t.Errorf("Repositories.AddSignatureProtectedBranch returned %+v, want %+v", signature, want) + t.Errorf("Repositories.RequireSignaturesOnProtectedBranch returned %+v, want %+v", signature, want) } } @@ -1114,9 +1114,9 @@ func TestRepositoriesService_RemoveSignatureProtectedBranch(t *testing.T) { w.WriteHeader(http.StatusNoContent) }) - _, err := client.Repositories.RemoveSignatureProtectedBranch(context.Background(), "o", "r", "b") + _, err := client.Repositories.OptionalSignaturesOnProtectedBranch(context.Background(), "o", "r", "b") if err != nil { - t.Errorf("Repositories.RemoveSignatureProtectedBranch returned error: %v", err) + t.Errorf("Repositories.OptionalSignaturesOnProtectedBranch returned error: %v", err) } } From 302d64282e46be34622ef860320392b9a7219723 Mon Sep 17 00:00:00 2001 From: Daniel Nilsson Date: Fri, 2 Nov 2018 12:23:49 +0100 Subject: [PATCH 4/9] Added pointer for Enabled bool. Signed-off-by: Daniel Nilsson --- github/repos.go | 2 +- github/repos_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/github/repos.go b/github/repos.go index 976d8b0610a..c460592da21 100644 --- a/github/repos.go +++ b/github/repos.go @@ -702,7 +702,7 @@ type DismissalRestrictionsRequest struct { type SignaturesProtectedBranch struct { URL *string `json:"url,omitempty"` // Commits pushed to matching branches must have verified signatures. - Enabled bool `json:"strict,omitempty"` + Enabled *bool `json:"strict,omitempty"` } // ListBranches lists branches for the specified repository. diff --git a/github/repos_test.go b/github/repos_test.go index ecd6fe32a70..8a4e3293ad5 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -1066,13 +1066,13 @@ func TestRepositoriesService_GetSignaturesProtectedBranch(t *testing.T) { }) signature, _, err := client.Repositories.GetSignaturesProtectedBranch(context.Background(), "o", "r", "b") + if err != nil { t.Errorf("Repositories.GetSignaturesProtectedBranch returned error: %v", err) } want := &SignaturesProtectedBranch{ URL: String("/repos/o/r/branches/b/protection/required_signatures"), - Enabled: false, } if !reflect.DeepEqual(signature, want) { @@ -1097,8 +1097,8 @@ func TestRepositoriesService_AddSignatureProtectedBranch(t *testing.T) { want := &SignaturesProtectedBranch{ URL: String("/repos/o/r/branches/b/protection/required_signatures"), - Enabled: false, } + if !reflect.DeepEqual(signature, want) { t.Errorf("Repositories.RequireSignaturesOnProtectedBranch returned %+v, want %+v", signature, want) } From ae38ea79e0e4dfe9b6de178b53918a45f8d8125f Mon Sep 17 00:00:00 2001 From: Daniel Nilsson Date: Fri, 2 Nov 2018 13:59:10 +0100 Subject: [PATCH 5/9] added github-accessor for GetEnabled() on SignaturesProtectedBranch Signed-off-by: Daniel Nilsson --- github/github-accessors.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/github/github-accessors.go b/github/github-accessors.go index 2e4a2c21def..62b5ff7f907 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -10100,6 +10100,14 @@ func (s *ServiceHook) GetName() string { return *s.Name } +// GetEnabled returns the Enabled field if it's non-nil, zero value otherwise. +func (s *SignaturesProtectedBranch) GetEnabled() bool { + if s == nil || s.Enabled == nil { + return false + } + return *s.Enabled +} + // GetURL returns the URL field if it's non-nil, zero value otherwise. func (s *SignaturesProtectedBranch) GetURL() string { if s == nil || s.URL == nil { From d697b01bfd7701ce8b3548665dff10759f08bdd6 Mon Sep 17 00:00:00 2001 From: Daniel Nilsson Date: Fri, 2 Nov 2018 14:02:56 +0100 Subject: [PATCH 6/9] go fmt Signed-off-by: Daniel Nilsson --- github/repos_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/github/repos_test.go b/github/repos_test.go index 8a4e3293ad5..43bf6ab847b 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -1072,7 +1072,7 @@ func TestRepositoriesService_GetSignaturesProtectedBranch(t *testing.T) { } want := &SignaturesProtectedBranch{ - URL: String("/repos/o/r/branches/b/protection/required_signatures"), + URL: String("/repos/o/r/branches/b/protection/required_signatures"), } if !reflect.DeepEqual(signature, want) { @@ -1096,7 +1096,7 @@ func TestRepositoriesService_AddSignatureProtectedBranch(t *testing.T) { } want := &SignaturesProtectedBranch{ - URL: String("/repos/o/r/branches/b/protection/required_signatures"), + URL: String("/repos/o/r/branches/b/protection/required_signatures"), } if !reflect.DeepEqual(signature, want) { From b521515fefc8a29b8604417f599b32bc2a22b517 Mon Sep 17 00:00:00 2001 From: Daniel Nilsson Date: Wed, 7 Nov 2018 16:58:50 +0100 Subject: [PATCH 7/9] re-added assertion of Enabled Signed-off-by: Daniel Nilsson --- github/repos_test.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/github/repos_test.go b/github/repos_test.go index 43bf6ab847b..a10d48b4df1 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -1066,13 +1066,13 @@ func TestRepositoriesService_GetSignaturesProtectedBranch(t *testing.T) { }) signature, _, err := client.Repositories.GetSignaturesProtectedBranch(context.Background(), "o", "r", "b") - if err != nil { t.Errorf("Repositories.GetSignaturesProtectedBranch returned error: %v", err) } want := &SignaturesProtectedBranch{ - URL: String("/repos/o/r/branches/b/protection/required_signatures"), + URL: String("/repos/o/r/branches/b/protection/required_signatures"), + Enabled: Bool(false), } if !reflect.DeepEqual(signature, want) { @@ -1080,7 +1080,7 @@ func TestRepositoriesService_GetSignaturesProtectedBranch(t *testing.T) { } } -func TestRepositoriesService_AddSignatureProtectedBranch(t *testing.T) { +func TestRepositoriesService_RequireSignaturesOnProtectedBranch(t *testing.T) { client, mux, _, teardown := setup() defer teardown() @@ -1095,8 +1095,9 @@ func TestRepositoriesService_AddSignatureProtectedBranch(t *testing.T) { t.Errorf("Repositories.RequireSignaturesOnProtectedBranch returned error: %v", err) } - want := &SignaturesProtectedBranch{ - URL: String("/repos/o/r/branches/b/protection/required_signatures"), + want := SignaturesProtectedBranch{ + URL: String("/repos/o/r/branches/b/protection/required_signatures"), + Enabled: Bool(false), } if !reflect.DeepEqual(signature, want) { From 9d0cbac26c44c6459ca5e3d6e87277275f909d30 Mon Sep 17 00:00:00 2001 From: Daniel Nilsson Date: Thu, 8 Nov 2018 14:36:25 +0100 Subject: [PATCH 8/9] rename test func name to reflect refactor. Signed-off-by: Daniel Nilsson --- github/repos_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github/repos_test.go b/github/repos_test.go index a10d48b4df1..6a077518a5d 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -1105,7 +1105,7 @@ func TestRepositoriesService_RequireSignaturesOnProtectedBranch(t *testing.T) { } } -func TestRepositoriesService_RemoveSignatureProtectedBranch(t *testing.T) { +func TestRepositoriesService_OptionalSignaturesOnProtectedBranch(t *testing.T) { client, mux, _, teardown := setup() defer teardown() From 6b75719961bd769ca8e63b13175779a20635b50c Mon Sep 17 00:00:00 2001 From: Daniel Nilsson Date: Thu, 8 Nov 2018 14:40:42 +0100 Subject: [PATCH 9/9] keep Enabled json key if empty. Signed-off-by: Daniel Nilsson --- github/repos.go | 2 +- github/repos_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/github/repos.go b/github/repos.go index c460592da21..d09b5cc87e6 100644 --- a/github/repos.go +++ b/github/repos.go @@ -702,7 +702,7 @@ type DismissalRestrictionsRequest struct { type SignaturesProtectedBranch struct { URL *string `json:"url,omitempty"` // Commits pushed to matching branches must have verified signatures. - Enabled *bool `json:"strict,omitempty"` + Enabled *bool `json:"enabled,omitempty"` } // ListBranches lists branches for the specified repository. diff --git a/github/repos_test.go b/github/repos_test.go index 6a077518a5d..4109eda9321 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -1087,7 +1087,7 @@ func TestRepositoriesService_RequireSignaturesOnProtectedBranch(t *testing.T) { mux.HandleFunc("/repos/o/r/branches/b/protection/required_signatures", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "POST") testHeader(t, r, "Accept", mediaTypeSignaturePreview) - fmt.Fprintf(w, `{"url":"/repos/o/r/branches/b/protection/required_signatures","enabled":false}`) + fmt.Fprintf(w, `{"url":"/repos/o/r/branches/b/protection/required_signatures","enabled":true}`) }) signature, _, err := client.Repositories.RequireSignaturesOnProtectedBranch(context.Background(), "o", "r", "b") @@ -1095,9 +1095,9 @@ func TestRepositoriesService_RequireSignaturesOnProtectedBranch(t *testing.T) { t.Errorf("Repositories.RequireSignaturesOnProtectedBranch returned error: %v", err) } - want := SignaturesProtectedBranch{ + want := &SignaturesProtectedBranch{ URL: String("/repos/o/r/branches/b/protection/required_signatures"), - Enabled: Bool(false), + Enabled: Bool(true), } if !reflect.DeepEqual(signature, want) {