From 73290bd032bd8e92d25c912591d88d9bcfaae3c5 Mon Sep 17 00:00:00 2001 From: AnitaErnszt Date: Thu, 29 Dec 2022 11:31:19 +0000 Subject: [PATCH 1/8] Handle the 422 error with retry --- github/github-accessors.go | 8 ++++++++ github/github-accessors_test.go | 7 +++++++ github/repos_environments.go | 34 ++++++++++++++++++++++++++++++++- 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/github/github-accessors.go b/github/github-accessors.go index 422d8bc0d3..745c926322 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -3702,6 +3702,14 @@ func (c *CreateUpdateEnvironment) GetWaitTimer() int { return *c.WaitTimer } +// GetDeploymentBranchPolicy returns the DeploymentBranchPolicy field. +func (c *CreateUpdateEnvironmentWithoutEnterprise) GetDeploymentBranchPolicy() *BranchPolicy { + if c == nil { + return nil + } + return c.DeploymentBranchPolicy +} + // GetBody returns the Body field if it's non-nil, zero value otherwise. func (c *CreateUserProjectOptions) GetBody() string { if c == nil || c.Body == nil { diff --git a/github/github-accessors_test.go b/github/github-accessors_test.go index efa66e7875..437c189c80 100644 --- a/github/github-accessors_test.go +++ b/github/github-accessors_test.go @@ -4334,6 +4334,13 @@ func TestCreateUpdateEnvironment_GetWaitTimer(tt *testing.T) { c.GetWaitTimer() } +func TestCreateUpdateEnvironmentWithoutEnterprise_GetDeploymentBranchPolicy(tt *testing.T) { + c := &CreateUpdateEnvironmentWithoutEnterprise{} + c.GetDeploymentBranchPolicy() + c = nil + c.GetDeploymentBranchPolicy() +} + func TestCreateUserProjectOptions_GetBody(tt *testing.T) { var zeroValue string c := &CreateUserProjectOptions{Body: &zeroValue} diff --git a/github/repos_environments.go b/github/repos_environments.go index 365f8d9202..ef27f54025 100644 --- a/github/repos_environments.go +++ b/github/repos_environments.go @@ -168,6 +168,13 @@ type CreateUpdateEnvironment struct { DeploymentBranchPolicy *BranchPolicy `json:"deployment_branch_policy"` } +// CreateUpdateEnvironmentWithoutEnterprise represents the fields accepted for Pro/Teams private repos. +// Ref: https://docs.github.com/en/actions/deployment/targeting-different-environments/using-environments-for-deployment +// See https://github.com/google/go-github/issues/2602 for more information. +type CreateUpdateEnvironmentWithoutEnterprise struct { + DeploymentBranchPolicy *BranchPolicy `json:"deployment_branch_policy"` +} + // CreateUpdateEnvironment create or update a new environment for a repository. // // GitHub API docs: https://docs.github.com/en/rest/deployments/environments#create-or-update-an-environment @@ -181,8 +188,33 @@ func (s *RepositoriesService) CreateUpdateEnvironment(ctx context.Context, owner e := new(Environment) resp, err := s.client.Do(ctx, req, e) + + fmt.Printf("%#v", environment.Reviewers) + fmt.Printf("runs") + if err != nil { - return nil, resp, err + // The API returns 422 when the pricing plan doesn't support all the fields sent. + // This path will be executed for Pro/Teams private repos. + // For public repos, regardless of the pricing plan, all fields supported. + // For Free plan private repos the returned error code is 404. + if resp.StatusCode == 422 { + + req, err = s.client.NewRequest("PUT", u, &CreateUpdateEnvironmentWithoutEnterprise{ + DeploymentBranchPolicy: environment.DeploymentBranchPolicy, + }) + + if err != nil { + return nil, nil, err + } + + resp, err = s.client.Do(ctx, req, e) + if err != nil { + return nil, resp, err + } + } else { + return nil, resp, err + } + } return e, resp, nil } From 0246f7d7848f8b88377f61bead7ebb9177396563 Mon Sep 17 00:00:00 2001 From: AnitaErnszt Date: Thu, 29 Dec 2022 12:24:29 +0000 Subject: [PATCH 2/8] check if no values were sent for Reviewers & WaitTimer and has default value --- github/repos_environments.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/github/repos_environments.go b/github/repos_environments.go index ef27f54025..5522b3a546 100644 --- a/github/repos_environments.go +++ b/github/repos_environments.go @@ -189,15 +189,14 @@ func (s *RepositoriesService) CreateUpdateEnvironment(ctx context.Context, owner e := new(Environment) resp, err := s.client.Do(ctx, req, e) - fmt.Printf("%#v", environment.Reviewers) - fmt.Printf("runs") - if err != nil { // The API returns 422 when the pricing plan doesn't support all the fields sent. // This path will be executed for Pro/Teams private repos. // For public repos, regardless of the pricing plan, all fields supported. // For Free plan private repos the returned error code is 404. - if resp.StatusCode == 422 { + // We are checking that the user didn't try to send a value for unsupported fields, + // and return an error if they did. + if resp.StatusCode == 422 && len(environment.Reviewers) == 0 && *environment.WaitTimer == 0 { req, err = s.client.NewRequest("PUT", u, &CreateUpdateEnvironmentWithoutEnterprise{ DeploymentBranchPolicy: environment.DeploymentBranchPolicy, From 0ecef861747d37cfaa884dd4480866b35dc899d9 Mon Sep 17 00:00:00 2001 From: AnitaErnszt Date: Fri, 30 Dec 2022 00:29:49 +0000 Subject: [PATCH 3/8] Create new function for `createUpdateEnvironmentNoEnterprise` --- github/github-accessors.go | 8 ------- github/github-accessors_test.go | 7 ------ github/repos_environments.go | 38 +++++++++++++++++++++------------ 3 files changed, 24 insertions(+), 29 deletions(-) diff --git a/github/github-accessors.go b/github/github-accessors.go index 745c926322..422d8bc0d3 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -3702,14 +3702,6 @@ func (c *CreateUpdateEnvironment) GetWaitTimer() int { return *c.WaitTimer } -// GetDeploymentBranchPolicy returns the DeploymentBranchPolicy field. -func (c *CreateUpdateEnvironmentWithoutEnterprise) GetDeploymentBranchPolicy() *BranchPolicy { - if c == nil { - return nil - } - return c.DeploymentBranchPolicy -} - // GetBody returns the Body field if it's non-nil, zero value otherwise. func (c *CreateUserProjectOptions) GetBody() string { if c == nil || c.Body == nil { diff --git a/github/github-accessors_test.go b/github/github-accessors_test.go index 437c189c80..efa66e7875 100644 --- a/github/github-accessors_test.go +++ b/github/github-accessors_test.go @@ -4334,13 +4334,6 @@ func TestCreateUpdateEnvironment_GetWaitTimer(tt *testing.T) { c.GetWaitTimer() } -func TestCreateUpdateEnvironmentWithoutEnterprise_GetDeploymentBranchPolicy(tt *testing.T) { - c := &CreateUpdateEnvironmentWithoutEnterprise{} - c.GetDeploymentBranchPolicy() - c = nil - c.GetDeploymentBranchPolicy() -} - func TestCreateUserProjectOptions_GetBody(tt *testing.T) { var zeroValue string c := &CreateUserProjectOptions{Body: &zeroValue} diff --git a/github/repos_environments.go b/github/repos_environments.go index 5522b3a546..7a67696bba 100644 --- a/github/repos_environments.go +++ b/github/repos_environments.go @@ -9,6 +9,7 @@ import ( "context" "encoding/json" "fmt" + "net/http" ) // Environment represents a single environment in a repository. @@ -171,7 +172,7 @@ type CreateUpdateEnvironment struct { // CreateUpdateEnvironmentWithoutEnterprise represents the fields accepted for Pro/Teams private repos. // Ref: https://docs.github.com/en/actions/deployment/targeting-different-environments/using-environments-for-deployment // See https://github.com/google/go-github/issues/2602 for more information. -type CreateUpdateEnvironmentWithoutEnterprise struct { +type createUpdateEnvironmentNoEnterprise struct { DeploymentBranchPolicy *BranchPolicy `json:"deployment_branch_policy"` } @@ -196,20 +197,9 @@ func (s *RepositoriesService) CreateUpdateEnvironment(ctx context.Context, owner // For Free plan private repos the returned error code is 404. // We are checking that the user didn't try to send a value for unsupported fields, // and return an error if they did. - if resp.StatusCode == 422 && len(environment.Reviewers) == 0 && *environment.WaitTimer == 0 { + if resp != nil && resp.StatusCode == http.StatusUnprocessableEntity && environment != nil && len(environment.Reviewers) == 0 && environment.GetWaitTimer() == 0 { + return s.createNewEnvNoEnterprise(ctx, u, environment) - req, err = s.client.NewRequest("PUT", u, &CreateUpdateEnvironmentWithoutEnterprise{ - DeploymentBranchPolicy: environment.DeploymentBranchPolicy, - }) - - if err != nil { - return nil, nil, err - } - - resp, err = s.client.Do(ctx, req, e) - if err != nil { - return nil, resp, err - } } else { return nil, resp, err } @@ -218,6 +208,26 @@ func (s *RepositoriesService) CreateUpdateEnvironment(ctx context.Context, owner return e, resp, nil } +// Creating an internal function for cases where the original call returned 422 +// Currently only the `deployment_branch_policy` paramter is supported for Pro/Team private repos +func (s *RepositoriesService) createNewEnvNoEnterprise(ctx context.Context, u string, environment *CreateUpdateEnvironment) (*Environment, *Response, error) { + req, err := s.client.NewRequest("PUT", u, &createUpdateEnvironmentNoEnterprise{ + DeploymentBranchPolicy: environment.DeploymentBranchPolicy, + }) + + if err != nil { + return nil, nil, err + } + + e := new(Environment) + + resp, err := s.client.Do(ctx, req, e) + if err != nil { + return nil, resp, err + } + return e, resp, nil +} + // DeleteEnvironment delete an environment from a repository. // // GitHub API docs: https://docs.github.com/en/rest/deployments/environments#delete-an-environment From c77d092cd198ab6a1283ae083949468cf1647551 Mon Sep 17 00:00:00 2001 From: AnitaErnszt <56001200+AnitaErnszt@users.noreply.github.com> Date: Mon, 2 Jan 2023 13:39:42 +0000 Subject: [PATCH 4/8] Update github/repos_environments.go Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com> --- github/repos_environments.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/github/repos_environments.go b/github/repos_environments.go index 7a67696bba..9151e8b4a3 100644 --- a/github/repos_environments.go +++ b/github/repos_environments.go @@ -208,8 +208,8 @@ func (s *RepositoriesService) CreateUpdateEnvironment(ctx context.Context, owner return e, resp, nil } -// Creating an internal function for cases where the original call returned 422 -// Currently only the `deployment_branch_policy` paramter is supported for Pro/Team private repos +// createNewEnvNoEnterprise is an internal function for cases where the original call returned 422. +// Currently only the `deployment_branch_policy` parameter is supported for Pro/Team private repos. func (s *RepositoriesService) createNewEnvNoEnterprise(ctx context.Context, u string, environment *CreateUpdateEnvironment) (*Environment, *Response, error) { req, err := s.client.NewRequest("PUT", u, &createUpdateEnvironmentNoEnterprise{ DeploymentBranchPolicy: environment.DeploymentBranchPolicy, From aa8d1358f8b7d05ce356d253538febd7a8043648 Mon Sep 17 00:00:00 2001 From: AnitaErnszt <56001200+AnitaErnszt@users.noreply.github.com> Date: Mon, 2 Jan 2023 13:40:01 +0000 Subject: [PATCH 5/8] Update github/repos_environments.go Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com> --- github/repos_environments.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github/repos_environments.go b/github/repos_environments.go index 9151e8b4a3..ca551e12d6 100644 --- a/github/repos_environments.go +++ b/github/repos_environments.go @@ -169,7 +169,7 @@ type CreateUpdateEnvironment struct { DeploymentBranchPolicy *BranchPolicy `json:"deployment_branch_policy"` } -// CreateUpdateEnvironmentWithoutEnterprise represents the fields accepted for Pro/Teams private repos. +// createUpdateEnvironmentNoEnterprise represents the fields accepted for Pro/Teams private repos. // Ref: https://docs.github.com/en/actions/deployment/targeting-different-environments/using-environments-for-deployment // See https://github.com/google/go-github/issues/2602 for more information. type createUpdateEnvironmentNoEnterprise struct { From 870398909f2467e2243b0e8c6e5970ebdee6ff9f Mon Sep 17 00:00:00 2001 From: AnitaErnszt Date: Mon, 2 Jan 2023 13:44:18 +0000 Subject: [PATCH 6/8] Add requested changes --- github/repos_environments.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/github/repos_environments.go b/github/repos_environments.go index 7a67696bba..c2323ae778 100644 --- a/github/repos_environments.go +++ b/github/repos_environments.go @@ -189,7 +189,6 @@ func (s *RepositoriesService) CreateUpdateEnvironment(ctx context.Context, owner e := new(Environment) resp, err := s.client.Do(ctx, req, e) - if err != nil { // The API returns 422 when the pricing plan doesn't support all the fields sent. // This path will be executed for Pro/Teams private repos. @@ -199,28 +198,23 @@ func (s *RepositoriesService) CreateUpdateEnvironment(ctx context.Context, owner // and return an error if they did. if resp != nil && resp.StatusCode == http.StatusUnprocessableEntity && environment != nil && len(environment.Reviewers) == 0 && environment.GetWaitTimer() == 0 { return s.createNewEnvNoEnterprise(ctx, u, environment) - - } else { - return nil, resp, err } - + return nil, resp, err } return e, resp, nil } -// Creating an internal function for cases where the original call returned 422 +// createNewEnvNoEnterprise is an internal function for cases where the original call returned 422. // Currently only the `deployment_branch_policy` paramter is supported for Pro/Team private repos func (s *RepositoriesService) createNewEnvNoEnterprise(ctx context.Context, u string, environment *CreateUpdateEnvironment) (*Environment, *Response, error) { req, err := s.client.NewRequest("PUT", u, &createUpdateEnvironmentNoEnterprise{ DeploymentBranchPolicy: environment.DeploymentBranchPolicy, }) - if err != nil { return nil, nil, err } e := new(Environment) - resp, err := s.client.Do(ctx, req, e) if err != nil { return nil, resp, err From daea9a88a01c2e58ce95f35766765fe997d81cdf Mon Sep 17 00:00:00 2001 From: AnitaErnszt Date: Mon, 2 Jan 2023 14:31:12 +0000 Subject: [PATCH 7/8] Add test to function --- github/repos_environments_test.go | 68 +++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/github/repos_environments_test.go b/github/repos_environments_test.go index 93d0fc25eb..e1fdeb8fcf 100644 --- a/github/repos_environments_test.go +++ b/github/repos_environments_test.go @@ -220,6 +220,74 @@ func TestRepositoriesService_CreateEnvironment(t *testing.T) { }) } +func TestRepositoriesService_createNewEnvNoEnterprise(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + input := &CreateUpdateEnvironment{ + DeploymentBranchPolicy: &BranchPolicy{ + ProtectedBranches: Bool(true), + CustomBranchPolicies: Bool(false), + }, + } + + mux.HandleFunc("/repos/o/r/environments/e", func(w http.ResponseWriter, r *http.Request) { + v := new(createUpdateEnvironmentNoEnterprise) + json.NewDecoder(r.Body).Decode(v) + + testMethod(t, r, "PUT") + want := &createUpdateEnvironmentNoEnterprise{ + DeploymentBranchPolicy: &BranchPolicy{ + ProtectedBranches: Bool(true), + CustomBranchPolicies: Bool(false), + }, + } + if !cmp.Equal(v, want) { + t.Errorf("Request body = %+v, want %+v", v, want) + } + fmt.Fprint(w, `{"id": 1, "name": "staging", "protection_rules": [{"id": 1, "node_id": "id", "type": "branch_policy"}], "deployment_branch_policy": {"protected_branches": true, "custom_branch_policies": false}}`) + }) + + ctx := context.Background() + release, _, err := client.Repositories.createNewEnvNoEnterprise(ctx, "repos/o/r/environments/e", input) + if err != nil { + t.Errorf("Repositories.createNewEnvNoEnterprise returned error: %v", err) + } + + want := &Environment{ + ID: Int64(1), + Name: String("staging"), + ProtectionRules: []*ProtectionRule{ + { + ID: Int64(1), + NodeID: String("id"), + Type: String("branch_policy"), + }, + }, + DeploymentBranchPolicy: &BranchPolicy{ + ProtectedBranches: Bool(true), + CustomBranchPolicies: Bool(false), + }, + } + if !cmp.Equal(release, want) { + t.Errorf("Repositories.createNewEnvNoEnterprise returned %+v, want %+v", release, want) + } + + const methodName = "createNewEnvNoEnterprise" + testBadOptions(t, methodName, func() (err error) { + _, _, err = client.Repositories.createNewEnvNoEnterprise(ctx, "\n", input) + return err + }) + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Repositories.createNewEnvNoEnterprise(ctx, "repos/o/r/environments/e", input) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) +} + func TestRepositoriesService_DeleteEnvironment(t *testing.T) { client, mux, _, teardown := setup() defer teardown() From aa375bd653f682b06543fbc038f719ec376768dd Mon Sep 17 00:00:00 2001 From: AnitaErnszt Date: Mon, 2 Jan 2023 16:55:24 +0000 Subject: [PATCH 8/8] Add test for noEnterprise environment creation path --- github/repos_environments_test.go | 36 +++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/github/repos_environments_test.go b/github/repos_environments_test.go index e1fdeb8fcf..0b27da933d 100644 --- a/github/repos_environments_test.go +++ b/github/repos_environments_test.go @@ -220,6 +220,42 @@ func TestRepositoriesService_CreateEnvironment(t *testing.T) { }) } +func TestRepositoriesService_CreateEnvironment_noEnterprise(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + input := &CreateUpdateEnvironment{} + callCount := 0 + + mux.HandleFunc("/repos/o/r/environments/e", func(w http.ResponseWriter, r *http.Request) { + v := new(CreateUpdateEnvironment) + json.NewDecoder(r.Body).Decode(v) + + testMethod(t, r, "PUT") + if callCount == 0 { + w.WriteHeader(http.StatusUnprocessableEntity) + callCount++ + } else { + want := &CreateUpdateEnvironment{} + if !cmp.Equal(v, want) { + t.Errorf("Request body = %+v, want %+v", v, want) + } + fmt.Fprint(w, `{"id": 1, "name": "staging", "protection_rules": []}`) + } + }) + + ctx := context.Background() + release, _, err := client.Repositories.CreateUpdateEnvironment(ctx, "o", "r", "e", input) + if err != nil { + t.Errorf("Repositories.CreateUpdateEnvironment returned error: %v", err) + } + + want := &Environment{ID: Int64(1), Name: String("staging"), ProtectionRules: []*ProtectionRule{}} + if !cmp.Equal(release, want) { + t.Errorf("Repositories.CreateUpdateEnvironment returned %+v, want %+v", release, want) + } +} + func TestRepositoriesService_createNewEnvNoEnterprise(t *testing.T) { client, mux, _, teardown := setup() defer teardown()