Skip to content

Commit 783b97d

Browse files
afzal442bmbferreirapasha-codefreshlrochetteblakepettersson
authored
fix(http): instantiate the http.Transport for connection limits (#376)
* feat: add `github.pullRequestComment.commentTag` to upsert github comments (#358) * add comment tag to update existing comments Signed-off-by: Bruno Ferreira <[email protected]> * make Send function testable Signed-off-by: Bruno Ferreira <[email protected]> * test if comment gets updated Signed-off-by: Bruno Ferreira <[email protected]> * add docs Signed-off-by: Bruno Ferreira <[email protected]> * use string format and add constants to reuse pattern Signed-off-by: Bruno Ferreira <[email protected]> * fixes after github lib update Signed-off-by: Bruno Ferreira <[email protected]> * fix linting Signed-off-by: Bruno Ferreira <[email protected]> --------- Signed-off-by: Bruno Ferreira <[email protected]> Co-authored-by: Pasha Kostohrys <[email protected]> Signed-off-by: Afzal Ansari <[email protected]> * fix: instantiate the http.Transport for conn limits Signed-off-by: Afzal Ansari <[email protected]> * transfer to *Options struct Signed-off-by: Afzal Ansari <[email protected]> * modift the transport layer for svcs Signed-off-by: Afzal Ansari <[email protected]> * refactor the transport connection into the svc Signed-off-by: Afzal Ansari <[email protected]> * minor typo fix Signed-off-by: Afzal Ansari <[email protected]> * parse the idleConnTimeout Signed-off-by: Afzal Ansari <[email protected]> * modify to use `IdleConnTimeout` as a string Signed-off-by: Afzal Ansari <[email protected]> * parse the conn timeouts if not empty Signed-off-by: Afzal Ansari <[email protected]> * add err var to ret type error Signed-off-by: Afzal Ansari <[email protected]> * add the info about the new params to docs Signed-off-by: Afzal Ansari <[email protected]> * add extra error field Signed-off-by: Afzal Ansari <[email protected]> * rmvs the err extra field Signed-off-by: Afzal Ansari <[email protected]> * update the err field Signed-off-by: Afzal Ansari <[email protected]> * fix the linting issue Signed-off-by: Afzal Ansari <[email protected]> * docs: Adding info about template for Grafana service (#386) * Adding info about template Signed-off-by: lrochette <[email protected]> * signing off Signed-off-by: lrochette <[email protected]> --------- Signed-off-by: lrochette <[email protected]> Signed-off-by: Afzal Ansari <[email protected]> * chore: bump golanglint-ci (#383) There's a major upgrade of it. This is an initial upgrade to make the changes as minimal as possible. Signed-off-by: Blake Pettersson <[email protected]> Co-authored-by: Michael Crenshaw <[email protected]> Signed-off-by: Afzal Ansari <[email protected]> * chore: more lint (#387) Try to slowly converge towards what exists on argo-cd. Signed-off-by: Blake Pettersson <[email protected]> Signed-off-by: Afzal Ansari <[email protected]> * added a module to refactor the method Signed-off-by: Afzal Ansari <[email protected]> * refactor services to use newService HttpClient Signed-off-by: Afzal Ansari <[email protected]> * restructure into tpOptions Signed-off-by: Afzal Ansari <[email protected]> * refactor: simplify HTTP client initialization by using TransportOptions directly Signed-off-by: Afzal Ansari <[email protected]> --------- Signed-off-by: Bruno Ferreira <[email protected]> Signed-off-by: Afzal Ansari <[email protected]> Signed-off-by: lrochette <[email protected]> Signed-off-by: Blake Pettersson <[email protected]> Co-authored-by: Bruno Ferreira <[email protected]> Co-authored-by: Pasha Kostohrys <[email protected]> Co-authored-by: Laurent Rochette <[email protected]> Co-authored-by: Blake Pettersson <[email protected]> Co-authored-by: Michael Crenshaw <[email protected]>
1 parent 58cdc54 commit 783b97d

File tree

20 files changed

+161
-87
lines changed

20 files changed

+161
-87
lines changed

‎docs/services/alertmanager.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ The notification service is used to push events to [Alertmanager](https://github
1111
* `basicAuth` - optional, server auth
1212
* `bearerToken` - optional, server auth
1313
* `timeout` - optional, the timeout in seconds used when sending alerts, default is "3 seconds"
14+
* `maxIdleConns` - optional, maximum number of idle (keep-alive) connections across all hosts.
15+
* `maxIdleConnsPerHost` - optional, maximum number of idle (keep-alive) connections per host.
16+
* `maxConnsPerHost` - optional, maximum total connections per host.
17+
* `idleConnTimeout` - optional, maximum amount of time an idle (keep-alive) connection will remain open before closing.
1418

1519
`basicAuth` or `bearerToken` is used for authentication, you can choose one. If the two are set at the same time, `basicAuth` takes precedence over `bearerToken`.
1620

‎docs/services/github.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ The GitHub notification service changes commit status using [GitHub Apps](https:
88
- `installationID` - the app installation id
99
- `privateKey` - the app private key
1010
- `enterpriseBaseURL` - optional URL, e.g. https://git.example.com/api/v3
11+
- `maxIdleConns` - optional, maximum number of idle (keep-alive) connections across all hosts.
12+
- `maxIdleConnsPerHost` - optional, maximum number of idle (keep-alive) connections per host.
13+
- `maxConnsPerHost` - optional, maximum total connections per host.
14+
- `idleConnTimeout` - optional, maximum amount of time an idle (keep-alive) connection will remain open before closing.
1115

1216
> ⚠️ _NOTE:_ Specifying `/api/v3` in the `enterpriseBaseURL` is required until [argoproj/notifications-engine#205](https://github.com/argoproj/notifications-engine/issues/205) is resolved.
1317

‎docs/services/grafana.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ Available parameters :
99
* `apiURL` - the server url, e.g. https://grafana.example.com
1010
* `apiKey` - the API key for the serviceaccount
1111
* `insecureSkipVerify` - optional bool, true or false
12+
* `maxIdleConns` - optional, maximum number of idle (keep-alive) connections across all hosts.
13+
* `maxIdleConnsPerHost` - optional, maximum number of idle (keep-alive) connections per host.
14+
* `maxConnsPerHost` - optional, maximum total connections per host.
15+
* `idleConnTimeout` - optional, maximum amount of time an idle (keep-alive) connection will remain open before closing.
1216

1317
1. Login to your Grafana instance as `admin`
1418
2. On the left menu, go to Configuration / API Keys

‎docs/services/mattermost.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55
* `apiURL` - the server url, e.g. https://mattermost.example.com
66
* `token` - the bot token
77
* `insecureSkipVerify` - optional bool, true or false
8+
* `maxIdleConns` - optional, maximum number of idle (keep-alive) connections across all hosts.
9+
* `maxIdleConnsPerHost` - optional, maximum number of idle (keep-alive) connections per host.
10+
* `maxConnsPerHost` - optional, maximum total connections per host.
11+
* `idleConnTimeout` - optional, maximum amount of time an idle (keep-alive) connection will remain open before closing, e.g. '90s'.
812

913
## Configuration
1014

‎docs/services/newrelic.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44

55
* `apiURL` - the api server url, e.g. https://api.newrelic.com
66
* `apiKey` - a [NewRelic ApiKey](https://docs.newrelic.com/docs/apis/rest-api-v2/get-started/introduction-new-relic-rest-api-v2/#api_key)
7+
* `maxIdleConns` - optional, maximum number of idle (keep-alive) connections across all hosts.
8+
* `maxIdleConnsPerHost` - optional, maximum number of idle (keep-alive) connections per host.
9+
* `maxConnsPerHost` - optional, maximum total connections per host.
10+
* `idleConnTimeout` - optional, maximum amount of time an idle (keep-alive) connection will remain open before closing, e.g. '90s'.
711

812
## Configuration
913

‎docs/services/slack.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ The Slack notification service configuration includes following settings:
1616
| `token` | **True** | `string` | The app's OAuth access token. | `xoxb-1234567890-1234567890123-5n38u5ed63fgzqlvuyxvxcx6` |
1717
| `username` | False | `string` | The app username. | `argocd` |
1818
| `disableUnfurl` | False | `bool` | Disable slack unfurling links in messages | `true` |
19+
| `maxIdleConns` | False | `int` | Maximum number of idle (keep-alive) connections across all hosts. | — |
20+
| `maxIdleConnsPerHost` | False | `int` | Maximum number of idle (keep-alive) connections per host. | — |
21+
| `maxConnsPerHost` | False | `int` | Maximum total connections per host. | — |
22+
| `idleConnTimeout` | False | `string` | Maximum amount of time an idle (keep-alive) connection will remain open before closing (e.g., `90s`). | — |
23+
1924

2025
## Configuration
2126

‎docs/services/webhook.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ The Webhook notification service configuration includes following settings:
1414
- `retryWaitMin` - Optional, the minimum wait time between retries. Default value: 1s.
1515
- `retryWaitMax` - Optional, the maximum wait time between retries. Default value: 5s.
1616
- `retryMax` - Optional, the maximum number of retries. Default value: 3.
17+
- `maxIdleConns` - optional, maximum number of idle (keep-alive) connections across all hosts.
18+
- `maxIdleConnsPerHost` - optional, maximum number of idle (keep-alive) connections per host.
19+
- `maxConnsPerHost` - optional, maximum total connections per host.
20+
- `idleConnTimeout` - optional, maximum amount of time an idle (keep-alive) connection will remain open before closing, e.g. '90s'.
1721

1822
## Retry Behavior
1923

‎pkg/services/alertmanager.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,9 @@ type AlertmanagerOptions struct {
3838
APIPath string `json:"apiPath"`
3939
BasicAuth *BasicAuth `json:"basicAuth"`
4040
BearerToken string `json:"bearerToken"`
41-
InsecureSkipVerify bool `json:"insecureSkipVerify"`
4241
Timeout int `json:"timeout"`
42+
InsecureSkipVerify bool `json:"insecureSkipVerify"`
43+
httputil.TransportOptions
4344
}
4445

4546
// NewAlertmanagerService new service
@@ -204,14 +205,13 @@ func (s alertmanagerService) Send(notification Notification, dest Destination) e
204205
return nil
205206
}
206207

207-
func (s alertmanagerService) sendOneTarget(ctx context.Context, target string, rawBody []byte) error {
208+
func (s alertmanagerService) sendOneTarget(ctx context.Context, target string, rawBody []byte) (err error) {
208209
rawURL := fmt.Sprintf("%v://%v%v", s.opts.Scheme, target, s.opts.APIPath)
209210

210-
transport := httputil.NewTransport(rawURL, s.opts.InsecureSkipVerify)
211-
client := &http.Client{
212-
Transport: httputil.NewLoggingRoundTripper(transport, s.entry),
211+
client, err := httputil.NewServiceHTTPClient(s.opts.TransportOptions, s.opts.InsecureSkipVerify, rawURL, "alertmanager")
212+
if err != nil {
213+
return err
213214
}
214-
215215
req, err := http.NewRequestWithContext(ctx, http.MethodPost, rawURL, bytes.NewReader(rawBody))
216216
if err != nil {
217217
return err

‎pkg/services/github.go‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"github.com/bradleyfalzon/ghinstallation/v2"
1515
giturls "github.com/chainguard-dev/git-urls"
1616
"github.com/google/go-github/v69/github"
17-
log "github.com/sirupsen/logrus"
1817
"github.com/spf13/cast"
1918

2019
httputil "github.com/argoproj/notifications-engine/pkg/util/http"
@@ -26,10 +25,12 @@ var (
2625
)
2726

2827
type GitHubOptions struct {
29-
AppID interface{} `json:"appID"`
30-
InstallationID interface{} `json:"installationID"`
31-
PrivateKey string `json:"privateKey"`
32-
EnterpriseBaseURL string `json:"enterpriseBaseURL"`
28+
AppID interface{} `json:"appID"`
29+
InstallationID interface{} `json:"installationID"`
30+
PrivateKey string `json:"privateKey"`
31+
EnterpriseBaseURL string `json:"enterpriseBaseURL"`
32+
InsecureSkipVerify bool `json:"insecureSkipVerify"`
33+
httputil.TransportOptions
3334
}
3435

3536
type GitHubNotification struct {
@@ -395,26 +396,28 @@ func NewGitHubService(opts GitHubOptions) (*gitHubService, error) {
395396
return nil, err
396397
}
397398

398-
tr := httputil.NewLoggingRoundTripper(
399-
httputil.NewTransport(url, false), log.WithField("service", "github"))
400-
itr, err := ghinstallation.New(tr, appID, installationID, []byte(opts.PrivateKey))
399+
client, err := httputil.NewServiceHTTPClient(opts.TransportOptions, opts.InsecureSkipVerify, url, "github")
400+
if err != nil {
401+
return nil, err
402+
}
403+
itr, err := ghinstallation.New(client.Transport, appID, installationID, []byte(opts.PrivateKey))
401404
if err != nil {
402405
return nil, err
403406
}
404407

405-
var client *github.Client
408+
var ghclient *github.Client
406409
if opts.EnterpriseBaseURL == "" {
407-
client = github.NewClient(&http.Client{Transport: itr})
410+
ghclient = github.NewClient(&http.Client{Transport: itr})
408411
} else {
409412
itr.BaseURL = opts.EnterpriseBaseURL
410-
client, err = github.NewClient(&http.Client{Transport: itr}).WithEnterpriseURLs(opts.EnterpriseBaseURL, "")
413+
ghclient, err = github.NewClient(&http.Client{Transport: itr}).WithEnterpriseURLs(opts.EnterpriseBaseURL, "")
411414
if err != nil {
412415
return nil, err
413416
}
414417
}
415418

416419
return &gitHubService{
417-
client: &githubClientAdapter{client: client},
420+
client: &githubClientAdapter{client: ghclient},
418421
}, nil
419422
}
420423

‎pkg/services/googlechat.go‎

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

1212
"github.com/google/uuid"
1313

14-
log "github.com/sirupsen/logrus"
1514
"sigs.k8s.io/yaml"
1615

1716
"google.golang.org/api/chat/v1"
@@ -80,7 +79,9 @@ func (n *GoogleChatNotification) GetTemplater(name string, f texttemplate.FuncMa
8079
}
8180

8281
type GoogleChatOptions struct {
83-
WebhookUrls map[string]string `json:"webhooks"`
82+
WebhookUrls map[string]string `json:"webhooks"`
83+
InsecureSkipVerify bool `json:"insecureSkipVerify"`
84+
httputil.TransportOptions
8485
}
8586

8687
type googleChatService struct {
@@ -101,14 +102,15 @@ type webhookError struct {
101102
Status string `json:"status"`
102103
}
103104

104-
func (s googleChatService) getClient(recipient string) (*googlechatClient, error) {
105+
func (s googleChatService) getClient(recipient string) (googlechatclient *googlechatClient, err error) {
105106
webhookUrl, ok := s.opts.WebhookUrls[recipient]
106107
if !ok {
107108
return nil, fmt.Errorf("no Google chat webhook configured for recipient %s", recipient)
108109
}
109-
transport := httputil.NewTransport(webhookUrl, false)
110-
client := &http.Client{
111-
Transport: httputil.NewLoggingRoundTripper(transport, log.WithField("service", "googlechat")),
110+
111+
client, err := httputil.NewServiceHTTPClient(s.opts.TransportOptions, s.opts.InsecureSkipVerify, webhookUrl, "googlechat")
112+
if err != nil {
113+
return nil, err
112114
}
113115
return &googlechatClient{httpClient: client, url: webhookUrl}, nil
114116
}

0 commit comments

Comments
 (0)