Skip to content

Commit 03af207

Browse files
committed
feat: add retry support on google SDK client requested in discussion #442
refactor: use a simplest http retry library
1 parent aceff47 commit 03af207

File tree

8 files changed

+146
-59
lines changed

8 files changed

+146
-59
lines changed

.vscode/settings.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
"grpr",
4444
"hashcode",
4545
"hashicorp",
46+
"httpretrier",
4647
"idpid",
4748
"idpscim",
4849
"idpscimcli",

cmd/idpscimcli/cmd/aws.go

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ import (
44
"context"
55
"log/slog"
66
"net/http"
7+
"time"
78

9+
"github.com/p2p-b2b/httpretrier"
810
"github.com/slashdevops/idp-scim-sync/internal/version"
911
"github.com/slashdevops/idp-scim-sync/pkg/aws"
1012
"github.com/spf13/cobra"
@@ -90,17 +92,13 @@ func runAWSServiceConfig(_ *cobra.Command, _ []string) error {
9092
ctx, cancel := context.WithTimeout(context.Background(), reqTimeout)
9193
defer cancel()
9294

93-
httpTransport := http.DefaultTransport.(*http.Transport).Clone()
94-
httpTransport.MaxIdleConns = 100
95-
httpTransport.MaxConnsPerHost = 100
96-
httpTransport.MaxIdleConnsPerHost = 100
97-
98-
httpClient := &http.Client{
99-
Transport: httpTransport,
100-
Timeout: maxTimeout,
101-
}
95+
httpRetryClient := httpretrier.NewClient(
96+
3, // Max Retries
97+
httpretrier.ExponentialBackoff(10*time.Millisecond, 100*time.Millisecond),
98+
nil, // Use http.DefaultTransport
99+
)
102100

103-
awsSCIMService, err := aws.NewSCIMService(httpClient, cfg.AWSSCIMEndpoint, cfg.AWSSCIMAccessToken)
101+
awsSCIMService, err := aws.NewSCIMService(httpRetryClient, cfg.AWSSCIMEndpoint, cfg.AWSSCIMAccessToken)
104102
if err != nil {
105103
slog.Error("error creating SCIM service", "error", err.Error())
106104
return err

cmd/idpscimcli/cmd/gws.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,14 @@ package cmd
22

33
import (
44
"context"
5+
"fmt"
56
"log/slog"
67
"os"
8+
"time"
79

10+
"github.com/p2p-b2b/httpretrier"
811
"github.com/slashdevops/idp-scim-sync/internal/config"
12+
"github.com/slashdevops/idp-scim-sync/internal/version"
913
"github.com/slashdevops/idp-scim-sync/pkg/google"
1014
"github.com/spf13/cobra"
1115
admin "google.golang.org/api/admin/directory/v1"
@@ -122,7 +126,21 @@ func getGWSDirectoryService(ctx context.Context) *google.DirectoryService {
122126
"https://www.googleapis.com/auth/admin.directory.user.readonly",
123127
}
124128

125-
gService, err := google.NewService(ctx, cfg.GWSUserEmail, gCreds, gScopes...)
129+
httpRetryClient := httpretrier.NewClient(
130+
3, // Max Retries
131+
httpretrier.ExponentialBackoff(10*time.Millisecond, 100*time.Millisecond),
132+
nil, // Use http.DefaultTransport
133+
)
134+
135+
gServiceConfig := google.DirectoryServiceConfig{
136+
UserEmail: cfg.GWSUserEmail,
137+
ServiceAccount: gCreds,
138+
Scopes: gScopes,
139+
Client: httpRetryClient,
140+
UserAgent: fmt.Sprintf("idp-scim-sync/%s", version.Version),
141+
}
142+
143+
gService, err := google.NewService(ctx, gServiceConfig)
126144
if err != nil {
127145
slog.Error("error creating service", "error", err)
128146
os.Exit(1)

go.mod

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ require (
1010
github.com/aws/aws-sdk-go-v2/service/s3 v1.88.1
1111
github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.39.4
1212
github.com/google/go-cmp v0.7.0
13-
github.com/hashicorp/go-retryablehttp v0.7.8
13+
github.com/p2p-b2b/httpretrier v0.0.3
1414
github.com/pkg/errors v0.9.1
1515
github.com/spf13/cobra v1.10.1
1616
github.com/spf13/viper v1.21.0
@@ -50,7 +50,6 @@ require (
5050
github.com/google/uuid v1.6.0 // indirect
5151
github.com/googleapis/enterprise-certificate-proxy v0.3.6 // indirect
5252
github.com/googleapis/gax-go/v2 v2.15.0 // indirect
53-
github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
5453
github.com/inconshreveable/mousetrap v1.1.0 // indirect
5554
github.com/pelletier/go-toml/v2 v2.2.4 // indirect
5655
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect

go.sum

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,6 @@ github.com/aws/smithy-go v1.23.0/go.mod h1:t1ufH5HMublsJYulve2RKmHDC15xu1f26kHCp
4747
github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
4848
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM=
4949
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
50-
github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM=
51-
github.com/fatih/color v1.16.0/go.mod h1:fL2Sau1YI5c0pdGEVCbKQbLXB6edEj1ZgiY4NijnWvE=
5250
github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg=
5351
github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
5452
github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8=
@@ -74,22 +72,14 @@ github.com/googleapis/enterprise-certificate-proxy v0.3.6 h1:GW/XbdyBFQ8Qe+YAmFU
7472
github.com/googleapis/enterprise-certificate-proxy v0.3.6/go.mod h1:MkHOF77EYAE7qfSuSS9PU6g4Nt4e11cnsDUowfwewLA=
7573
github.com/googleapis/gax-go/v2 v2.15.0 h1:SyjDc1mGgZU5LncH8gimWo9lW1DtIfPibOG81vgd/bo=
7674
github.com/googleapis/gax-go/v2 v2.15.0/go.mod h1:zVVkkxAQHa1RQpg9z2AUCMnKhi0Qld9rcmyfL1OZhoc=
77-
github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ=
78-
github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48=
79-
github.com/hashicorp/go-hclog v1.6.3 h1:Qr2kF+eVWjTiYmU7Y31tYlP1h0q/X3Nl3tPGdaB11/k=
80-
github.com/hashicorp/go-hclog v1.6.3/go.mod h1:W4Qnvbt70Wk/zYJryRzDRU/4r0kIg0PVHBcfoyhpF5M=
81-
github.com/hashicorp/go-retryablehttp v0.7.8 h1:ylXZWnqa7Lhqpk0L1P1LzDtGcCR0rPVUrx/c8Unxc48=
82-
github.com/hashicorp/go-retryablehttp v0.7.8/go.mod h1:rjiScheydd+CxvumBsIrFKlx3iS0jrZ7LvzFGFmuKbw=
8375
github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
8476
github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
8577
github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
8678
github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
8779
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
8880
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
89-
github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA=
90-
github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg=
91-
github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY=
92-
github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
81+
github.com/p2p-b2b/httpretrier v0.0.3 h1:6Udp+5zuzgBZ6Hu5Es4/dKWHv8pzRSvXCHXRjYJTgNU=
82+
github.com/p2p-b2b/httpretrier v0.0.3/go.mod h1:J/aV00SornLs70X/DY3B4IPkDjICynXLfA/TWpAlErw=
9383
github.com/pelletier/go-toml/v2 v2.2.4 h1:mye9XuhQ6gvn5h28+VilKrrPoQVanw5PMw/TB0t5Ec4=
9484
github.com/pelletier/go-toml/v2 v2.2.4/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY=
9585
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=

internal/setup/setup.go

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111

1212
"github.com/aws/aws-sdk-go-v2/service/s3"
1313
"github.com/aws/aws-sdk-go-v2/service/secretsmanager"
14-
"github.com/hashicorp/go-retryablehttp"
14+
"github.com/p2p-b2b/httpretrier"
1515
"github.com/pkg/errors"
1616
"github.com/slashdevops/idp-scim-sync/internal/config"
1717
"github.com/slashdevops/idp-scim-sync/internal/core"
@@ -216,8 +216,24 @@ func SyncService(ctx context.Context, cfg *config.Config) (*core.SyncService, er
216216
gwsServiceAccountContent = gwsServiceAccount
217217
}
218218

219+
httpRetryClient := httpretrier.NewClient(
220+
3, // Max Retries
221+
httpretrier.ExponentialBackoff(10*time.Millisecond, 100*time.Millisecond),
222+
nil, // Use http.DefaultTransport
223+
)
224+
225+
userAgent := fmt.Sprintf("idp-scim-sync/%s", version.Version)
226+
227+
gServiceConfig := google.DirectoryServiceConfig{
228+
UserEmail: cfg.GWSUserEmail,
229+
ServiceAccount: gwsServiceAccountContent,
230+
Scopes: cfg.GWSServiceAccountScopes,
231+
UserAgent: userAgent,
232+
Client: httpRetryClient,
233+
}
234+
219235
// Google Client Service
220-
gwsService, err := google.NewService(ctx, cfg.GWSUserEmail, gwsServiceAccountContent, cfg.GWSServiceAccountScopes...)
236+
gwsService, err := google.NewService(ctx, gServiceConfig)
221237
if err != nil {
222238
return nil, errors.Wrap(err, "cannot create google service")
223239
}
@@ -234,26 +250,12 @@ func SyncService(ctx context.Context, cfg *config.Config) (*core.SyncService, er
234250
return nil, errors.Wrap(err, "cannot create identity provider service")
235251
}
236252

237-
// httpClient
238-
retryClient := retryablehttp.NewClient()
239-
retryClient.RetryMax = 10
240-
retryClient.RetryWaitMin = time.Millisecond * 100
241-
242-
// set the logger only in debug mode
243-
if cfg.Debug {
244-
retryClient.Logger = slog.Default()
245-
} else {
246-
retryClient.Logger = nil
247-
}
248-
249-
httpClient := retryClient.StandardClient()
250-
251253
// AWS SCIM Service
252-
awsSCIM, err := aws.NewSCIMService(httpClient, cfg.AWSSCIMEndpoint, cfg.AWSSCIMAccessToken)
254+
awsSCIM, err := aws.NewSCIMService(httpRetryClient, cfg.AWSSCIMEndpoint, cfg.AWSSCIMAccessToken)
253255
if err != nil {
254256
return nil, errors.Wrap(err, "cannot create aws scim service")
255257
}
256-
awsSCIM.UserAgent = fmt.Sprintf("idp-scim-sync/%s", version.Version)
258+
awsSCIM.UserAgent = userAgent
257259

258260
scimService, err := scim.NewProvider(awsSCIM)
259261
if err != nil {

pkg/google/google.go

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"log/slog"
7+
"net/http"
78

89
"golang.org/x/oauth2/google"
910
admin "google.golang.org/api/admin/directory/v1"
@@ -36,36 +37,74 @@ var (
3637

3738
// ErrGroupIDNil is returned when the group ID is nil.
3839
ErrGroupIDNil = fmt.Errorf("google: group id is required")
40+
41+
// ErrServiceAccountNil is returned when the service account credentials are nil.
42+
ErrServiceAccountNil = fmt.Errorf("google: service account credentials are required")
43+
44+
// ErrUserAgentNil is returned when the user agent is nil.
45+
ErrUserAgentNil = fmt.Errorf("google: user agent is required")
46+
47+
// ErrGoogleClientNil is returned when the google client is nil.
48+
ErrGoogleClientNil = fmt.Errorf("google: google client is required")
3949
)
4050

4151
// DirectoryService represent the Google Directory API client.
4252
type DirectoryService struct {
4353
svc *admin.Service
4454
}
4555

56+
type DirectoryServiceConfig struct {
57+
Client *http.Client
58+
UserEmail string
59+
ServiceAccount []byte
60+
Scopes []string
61+
UserAgent string
62+
}
63+
4664
// NewService create a Google Directory Service.
4765
// References:
4866
// - https://pkg.go.dev/google.golang.org/api/admin/directory/v1
4967
// Examples of scope:
5068
// - "https://www.googleapis.com/auth/admin.directory.group.readonly"
5169
// - "https://www.googleapis.com/auth/admin.directory.group.member.readonly"
5270
// - "https://www.googleapis.com/auth/admin.directory.user.readonly"
53-
func NewService(ctx context.Context, userEmail string, serviceAccount []byte, scope ...string) (*admin.Service, error) {
54-
if len(scope) == 0 {
71+
func NewService(ctx context.Context, config DirectoryServiceConfig) (*admin.Service, error) {
72+
if config.Client == nil {
73+
return nil, ErrGoogleClientNil
74+
}
75+
76+
if config.UserEmail == "" {
77+
return nil, ErrUserIDNil
78+
}
79+
80+
if config.ServiceAccount == nil {
81+
return nil, ErrServiceAccountNil
82+
}
83+
84+
if len(config.Scopes) == 0 {
5585
return nil, ErrGoogleClientScopeNil
5686
}
5787

58-
creds, err := google.CredentialsFromJSONWithParams(ctx, serviceAccount, google.CredentialsParams{
59-
Scopes: scope,
60-
Subject: userEmail,
88+
if config.UserAgent == "" {
89+
return nil, ErrUserAgentNil
90+
}
91+
92+
creds, err := google.CredentialsFromJSONWithParams(ctx, config.ServiceAccount, google.CredentialsParams{
93+
Scopes: config.Scopes,
94+
Subject: config.UserEmail,
6195
})
6296
if err != nil {
63-
return nil, fmt.Errorf("google: error getting config for Service Account: %v", err)
97+
return nil, fmt.Errorf("google: %v", err)
6498
}
6599

66-
svc, err := admin.NewService(ctx, option.WithTokenSource(creds.TokenSource))
100+
svc, err := admin.NewService(
101+
ctx,
102+
option.WithTokenSource(creds.TokenSource),
103+
option.WithUserAgent(config.UserAgent),
104+
option.WithHTTPClient(config.Client),
105+
)
67106
if err != nil {
68-
return nil, fmt.Errorf("google: error creating service: %v", err)
107+
return nil, fmt.Errorf("google: %v", err)
69108
}
70109

71110
return svc, nil

pkg/google/google_test.go

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,33 +18,61 @@ func TestNewService(t *testing.T) {
1818
ctx := context.TODO()
1919
userEmail := "mock-email@mock-project.iam.gserviceaccount.com"
2020
serviceAccountFile := "testdata/service_account.json"
21-
scope := "admin.AdminDirectoryGroupReadonlyScope, admin.AdminDirectoryGroupMemberReadonlyScope, admin.AdminDirectoryUserReadonlyScope"
21+
scopes := []string{
22+
"https://www.googleapis.com/auth/admin.directory.group.readonly",
23+
"https://www.googleapis.com/auth/admin.directory.group.member.readonly",
24+
"https://www.googleapis.com/auth/admin.directory.user.readonly",
25+
}
2226

2327
serviceAccount, err := os.ReadFile(serviceAccountFile)
2428
if err != nil {
2529
t.Fatalf("Error loading golden file: %s", err)
2630
}
2731

28-
svc, err := NewService(ctx, userEmail, serviceAccount, scope)
32+
config := DirectoryServiceConfig{
33+
Client: &http.Client{},
34+
UserEmail: userEmail,
35+
ServiceAccount: serviceAccount,
36+
Scopes: scopes,
37+
UserAgent: "test-agent",
38+
}
39+
40+
svc, err := NewService(ctx, config)
2941
assert.NoError(t, err)
3042
assert.NotNil(t, svc)
3143
})
3244

3345
t.Run("Should return a new Service with empty service account parameter", func(t *testing.T) {
3446
ctx := context.TODO()
3547
userEmail := ""
36-
scope := ""
48+
scopes := []string{}
49+
50+
config := DirectoryServiceConfig{
51+
Client: &http.Client{},
52+
UserEmail: userEmail,
53+
ServiceAccount: nil,
54+
Scopes: scopes,
55+
UserAgent: "test-agent",
56+
}
3757

38-
svc, err := NewService(ctx, userEmail, nil, scope)
58+
svc, err := NewService(ctx, config)
3959
assert.Error(t, err)
4060
assert.Nil(t, svc)
4161
})
4262

4363
t.Run("Should return an error when scope is nil", func(t *testing.T) {
4464
ctx := context.TODO()
45-
userEmail := ""
65+
userEmail := "test@example.com"
66+
67+
config := DirectoryServiceConfig{
68+
Client: &http.Client{},
69+
UserEmail: userEmail,
70+
ServiceAccount: []byte("{}"), // provide minimal valid JSON
71+
Scopes: nil,
72+
UserAgent: "test-agent",
73+
}
4674

47-
svc, err := NewService(ctx, userEmail, nil)
75+
svc, err := NewService(ctx, config)
4876
assert.Error(t, err)
4977
assert.Nil(t, svc)
5078
assert.ErrorIs(t, err, ErrGoogleClientScopeNil)
@@ -56,14 +84,26 @@ func TestNewDirectoryService(t *testing.T) {
5684
ctx := context.TODO()
5785
userEmail := "mock-email@mock-project.iam.gserviceaccount.com"
5886
serviceAccountFile := "testdata/service_account.json"
59-
scope := "admin.AdminDirectoryGroupReadonlyScope, admin.AdminDirectoryGroupMemberReadonlyScope, admin.AdminDirectoryUserReadonlyScope"
87+
scopes := []string{
88+
"https://www.googleapis.com/auth/admin.directory.group.readonly",
89+
"https://www.googleapis.com/auth/admin.directory.group.member.readonly",
90+
"https://www.googleapis.com/auth/admin.directory.user.readonly",
91+
}
6092

6193
serviceAccount, err := os.ReadFile(serviceAccountFile)
6294
if err != nil {
6395
t.Fatalf("Error loading golden file: %s", err)
6496
}
6597

66-
svc, err := NewService(ctx, userEmail, serviceAccount, scope)
98+
config := DirectoryServiceConfig{
99+
Client: &http.Client{},
100+
UserEmail: userEmail,
101+
ServiceAccount: serviceAccount,
102+
Scopes: scopes,
103+
UserAgent: "test-agent",
104+
}
105+
106+
svc, err := NewService(ctx, config)
67107
if err != nil {
68108
t.Fatalf("Error creating a service: %s", err)
69109
}

0 commit comments

Comments
 (0)