Skip to content

Commit 993bf8d

Browse files
vmvarelavmvarela
authored
Merge branch 'integrations:main' into enterprise-scim
2 parents d075d01 + 9fc330c commit 993bf8d

File tree

4 files changed

+79
-15
lines changed

4 files changed

+79
-15
lines changed

‎github/resource_github_organization_role.go‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -73,17 +73,26 @@ func resourceGithubOrganizationRoleCreate(ctx context.Context, d *schema.Resourc
7373
permissionsStr[i] = v.(string)
7474
}
7575

76-
role, _, err := client.Organizations.CreateCustomOrgRole(ctx, orgName, &github.CreateOrUpdateOrgRoleOptions{
76+
createOrUpdateOrgRoleOptions := &github.CreateOrUpdateOrgRoleOptions{
7777
Name: github.String(d.Get("name").(string)),
7878
Description: github.String(d.Get("description").(string)),
79-
BaseRole: github.String(d.Get("base_role").(string)),
8079
Permissions: permissionsStr,
81-
})
80+
}
81+
82+
baseRole := d.Get("base_role").(string)
83+
if baseRole != "none" {
84+
createOrUpdateOrgRoleOptions.BaseRole = github.String(baseRole)
85+
}
86+
87+
role, _, err := client.Organizations.CreateCustomOrgRole(ctx, orgName, createOrUpdateOrgRoleOptions)
8288
if err != nil {
8389
return diag.FromErr(fmt.Errorf("error creating organization role (%s/%s): %w", orgName, d.Get("name").(string), err))
8490
}
8591

8692
d.SetId(fmt.Sprint(role.GetID()))
93+
if err = d.Set("role_id", role.GetID()); err != nil {
94+
return diag.FromErr(err)
95+
}
8796
return nil
8897
}
8998

@@ -123,8 +132,14 @@ func resourceGithubOrganizationRoleRead(ctx context.Context, d *schema.ResourceD
123132
if err = d.Set("description", role.Description); err != nil {
124133
return diag.FromErr(err)
125134
}
126-
if err = d.Set("base_role", role.BaseRole); err != nil {
127-
return diag.FromErr(err)
135+
if role.BaseRole != nil {
136+
if err = d.Set("base_role", role.BaseRole); err != nil {
137+
return diag.FromErr(err)
138+
}
139+
} else {
140+
if err = d.Set("base_role", "none"); err != nil {
141+
return diag.FromErr(err)
142+
}
128143
}
129144
if err = d.Set("permissions", role.Permissions); err != nil {
130145
return diag.FromErr(err)

‎github/resource_github_organization_role_test.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,12 @@ func TestAccGithubOrganizationRole(t *testing.T) {
2525
Steps: []resource.TestStep{
2626
{
2727
Config: config,
28-
Check: resource.ComposeTestCheckFunc(
28+
Check: resource.ComposeAggregateTestCheckFunc(
2929
resource.TestCheckResourceAttrSet("github_organization_role.test", "id"),
3030
resource.TestCheckResourceAttrSet("github_organization_role.test", "role_id"),
3131
resource.TestCheckResourceAttr("github_organization_role.test", "name", name),
3232
resource.TestCheckResourceAttr("github_organization_role.test", "base_role", "none"),
33-
resource.TestCheckResourceAttrSet("github_organization_role.test", "permissions.#"),
33+
resource.TestCheckNoResourceAttr("github_organization_role.test", "permissions"),
3434
resource.TestCheckResourceAttr("github_organization_role.test", "permissions.#", "0"),
3535
),
3636
},
@@ -57,11 +57,11 @@ func TestAccGithubOrganizationRole(t *testing.T) {
5757
Steps: []resource.TestStep{
5858
{
5959
Config: config,
60-
Check: resource.ComposeTestCheckFunc(
60+
Check: resource.ComposeAggregateTestCheckFunc(
6161
resource.TestCheckResourceAttrSet("github_organization_role.test", "id"),
6262
resource.TestCheckResourceAttr("github_organization_role.test", "name", name),
6363
resource.TestCheckResourceAttr("github_organization_role.test", "base_role", baseRole),
64-
resource.TestCheckResourceAttrSet("github_organization_role.test", "permissions.#"),
64+
resource.TestCheckNoResourceAttr("github_organization_role.test", "permissions"),
6565
resource.TestCheckResourceAttr("github_organization_role.test", "permissions.#", "0"),
6666
),
6767
},
@@ -167,16 +167,16 @@ func TestAccGithubOrganizationRole(t *testing.T) {
167167
Steps: []resource.TestStep{
168168
{
169169
Config: config,
170-
Check: resource.ComposeTestCheckFunc(
170+
Check: resource.ComposeAggregateTestCheckFunc(
171171
resource.TestCheckResourceAttrSet("github_organization_role.test", "id"),
172172
resource.TestCheckResourceAttrSet("github_organization_role.test", "role_id"),
173173
resource.TestCheckResourceAttr("github_organization_role.test", "name", name),
174174
resource.TestCheckResourceAttr("github_organization_role.test", "description", description),
175175
resource.TestCheckResourceAttr("github_organization_role.test", "base_role", baseRole),
176176
resource.TestCheckResourceAttrSet("github_organization_role.test", "permissions.#"),
177177
resource.TestCheckResourceAttr("github_organization_role.test", "permissions.#", "2"),
178-
resource.TestCheckResourceAttr("github_organization_role.test", "permissions.0", permission0),
179-
resource.TestCheckResourceAttr("github_organization_role.test", "permissions.1", permission1),
178+
resource.TestCheckTypeSetElemAttr("github_organization_role.test", "permissions.*", permission0),
179+
resource.TestCheckTypeSetElemAttr("github_organization_role.test", "permissions.*", permission1),
180180
),
181181
},
182182
},

‎github/transport.go‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package github
22

33
import (
44
"bytes"
5+
"context"
56
"errors"
67
"io"
78
"log"
@@ -66,7 +67,7 @@ func (rlt *RateLimitTransport) RoundTrip(req *http.Request) (*http.Response, err
6667
// for read and write requests. See isWriteMethod for the distinction between them.
6768
if rlt.nextRequestDelay > 0 {
6869
log.Printf("[DEBUG] Sleeping %s between operations", rlt.nextRequestDelay)
69-
time.Sleep(rlt.nextRequestDelay)
70+
sleep(req.Context(), rlt.nextRequestDelay)
7071
}
7172

7273
rlt.nextRequestDelay = rlt.calculateNextDelay(req.Method)
@@ -82,6 +83,7 @@ func (rlt *RateLimitTransport) RoundTrip(req *http.Request) (*http.Response, err
8283
// See https://github.com/google/go-github/pull/986
8384
r1, r2, err := drainBody(resp.Body)
8485
if err != nil {
86+
rlt.smartLock(false)
8587
return nil, err
8688
}
8789
resp.Body = r1
@@ -95,7 +97,7 @@ func (rlt *RateLimitTransport) RoundTrip(req *http.Request) (*http.Response, err
9597
retryAfter := arlErr.GetRetryAfter()
9698
log.Printf("[WARN] Abuse detection mechanism triggered, sleeping for %s before retrying",
9799
retryAfter)
98-
time.Sleep(retryAfter)
100+
sleep(req.Context(), retryAfter)
99101
rlt.smartLock(false)
100102
return rlt.RoundTrip(req)
101103
}
@@ -106,7 +108,7 @@ func (rlt *RateLimitTransport) RoundTrip(req *http.Request) (*http.Response, err
106108
retryAfter := time.Until(rlErr.Rate.Reset.Time)
107109
log.Printf("[WARN] Rate limit %d reached, sleeping for %s (until %s) before retrying",
108110
rlErr.Rate.Limit, retryAfter, time.Now().Add(retryAfter))
109-
time.Sleep(retryAfter)
111+
sleep(req.Context(), retryAfter)
110112
rlt.smartLock(false)
111113
return rlt.RoundTrip(req)
112114
}
@@ -116,6 +118,17 @@ func (rlt *RateLimitTransport) RoundTrip(req *http.Request) (*http.Response, err
116118
return resp, nil
117119
}
118120

121+
// sleep is used an alternative to time.Sleep that supports cancellation via the passed context.Context.
122+
func sleep(ctx context.Context, dur time.Duration) {
123+
t := time.NewTimer(dur)
124+
defer t.Stop()
125+
126+
select {
127+
case <-t.C:
128+
case <-ctx.Done():
129+
}
130+
}
131+
119132
// smartLock wraps the mutex locking system and performs its operation via a boolean input for locking and unlocking.
120133
// It also skips the locking when parallelRequests is set to true since, in this case, the lock is not needed.
121134
func (rlt *RateLimitTransport) smartLock(lock bool) {

‎github/transport_test.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,42 @@ func TestRateLimitTransport_abuseLimit_get(t *testing.T) {
160160
}
161161
}
162162

163+
func TestRateLimitTransport_abuseLimit_get_cancelled(t *testing.T) {
164+
ts := githubApiMock([]*mockResponse{
165+
{
166+
ExpectedUri: "/repos/test/blah",
167+
ResponseBody: `{
168+
"message": "You have triggered an abuse detection mechanism and have been temporarily blocked from content creation. Please retry your request again later.",
169+
"documentation_url": "https://developer.github.com/v3/#abuse-rate-limits"
170+
}`,
171+
StatusCode: 403,
172+
ResponseHeaders: map[string]string{
173+
"Retry-After": "10",
174+
},
175+
},
176+
})
177+
defer ts.Close()
178+
179+
httpClient := http.DefaultClient
180+
httpClient.Transport = NewRateLimitTransport(http.DefaultTransport)
181+
182+
client := github.NewClient(httpClient)
183+
u, _ := url.Parse(ts.URL + "/")
184+
client.BaseURL = u
185+
186+
ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond)
187+
defer cancel()
188+
189+
start := time.Now()
190+
_, _, err := client.Repositories.Get(ctx, "test", "blah")
191+
if !errors.Is(err, context.DeadlineExceeded) {
192+
t.Fatalf("Expected context deadline exceeded, got: %v", err)
193+
}
194+
if time.Since(start) > time.Second {
195+
t.Fatalf("Waited for longer than expected: %s", time.Since(start))
196+
}
197+
}
198+
163199
func TestRateLimitTransport_abuseLimit_post(t *testing.T) {
164200
ts := githubApiMock([]*mockResponse{
165201
{

0 commit comments

Comments
 (0)