Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for TLSConfig in webhook receiver #247

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions receivers/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,29 @@ func (cfg *TLSConfig) ToCryptoTLSConfig() (*tls.Config, error) {
return tlsCfg, nil
}

// NewTLSClient creates a new HTTP client with the provided TLS configuration or with default settings.
func NewTLSClient(tlsConfig *tls.Config) *http.Client {
nc := func(tlsConfig *tls.Config) *http.Client {
return &http.Client{
Timeout: time.Second * 30,
Transport: &http.Transport{
TLSClientConfig: tlsConfig,
Proxy: http.ProxyFromEnvironment,
Dial: (&net.Dialer{
Timeout: 30 * time.Second,
}).Dial,
TLSHandshakeTimeout: 5 * time.Second,
},
}
}

if tlsConfig == nil {
return nc(&tls.Config{Renegotiation: tls.RenegotiateFreelyAsClient})
}

return nc(tlsConfig)
}

// SendHTTPRequest sends an HTTP request.
// Stubbable by tests.
//
Expand Down
27 changes: 27 additions & 0 deletions receivers/util_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package receivers

import (
"crypto/tls"
"net/http"
"testing"

"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -106,3 +108,28 @@ func TestNewTLSConfig(t *testing.T) {
})
}
}

func Test_NewTLSClient(t *testing.T) {
tc := []struct {
name string
cfg *tls.Config
expCfg *tls.Config
}{
{
name: "empty TLSConfig",
expCfg: &tls.Config{Renegotiation: tls.RenegotiateFreelyAsClient},
},
{
name: "valid TLSConfig",
cfg: &tls.Config{InsecureSkipVerify: true},
expCfg: &tls.Config{InsecureSkipVerify: true},
},
}

for _, tt := range tc {
t.Run(tt.name, func(t *testing.T) {
c := NewTLSClient(tt.cfg)
require.Equal(t, tt.expCfg, c.Transport.(*http.Transport).TLSClientConfig)
})
}
}
6 changes: 5 additions & 1 deletion receivers/webhook.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package receivers

import "context"
import (
"context"
"crypto/tls"
)

type SendWebhookSettings struct {
URL string
Expand All @@ -11,6 +14,7 @@ type SendWebhookSettings struct {
HTTPHeader map[string]string
ContentType string
Validation func(body []byte, statusCode int) error
TLSConfig *tls.Config
}

type WebhookSender interface {
Expand Down
16 changes: 14 additions & 2 deletions receivers/webhook/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ type Config struct {
User string
Password string

Title string
Message string
Title string
Message string
TLSConfig *receivers.TLSConfig
}

func NewConfig(jsonData json.RawMessage, decryptFn receivers.DecryptFunc) (Config, error) {
Expand All @@ -38,6 +39,7 @@ func NewConfig(jsonData json.RawMessage, decryptFn receivers.DecryptFunc) (Confi
Password string `json:"password,omitempty" yaml:"password,omitempty"`
Title string `json:"title,omitempty" yaml:"title,omitempty"`
Message string `json:"message,omitempty" yaml:"message,omitempty"`
TLSConfig *receivers.TLSConfig `json:"tlsConfig,omitempty" yaml:"tlsConfig,omitempty"`
}{}

err := json.Unmarshal(jsonData, &rawSettings)
Expand Down Expand Up @@ -77,5 +79,15 @@ func NewConfig(jsonData json.RawMessage, decryptFn receivers.DecryptFunc) (Confi
if settings.Message == "" {
settings.Message = templates.DefaultMessageEmbed
}

if tlsConfig := rawSettings.TLSConfig; tlsConfig != nil {
settings.TLSConfig = &receivers.TLSConfig{
InsecureSkipVerify: tlsConfig.InsecureSkipVerify,
CACertificate: decryptFn("tlsConfig.caCertificate", tlsConfig.CACertificate),
ClientCertificate: decryptFn("tlsConfig.clientCertificate", tlsConfig.ClientCertificate),
ClientKey: decryptFn("tlsConfig.clientKey", tlsConfig.ClientKey),
}
}

return settings, err
}
13 changes: 13 additions & 0 deletions receivers/webhook/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/stretchr/testify/require"

"github.com/grafana/alerting/receivers"
receiversTesting "github.com/grafana/alerting/receivers/testing"
"github.com/grafana/alerting/templates"
)
Expand Down Expand Up @@ -82,6 +83,12 @@ func TestNewConfig(t *testing.T) {
Password: "test-pass",
Title: "test-title",
Message: "test-message",
TLSConfig: &receivers.TLSConfig{
InsecureSkipVerify: false,
ClientCertificate: "test-client-certificate",
ClientKey: "test-client-key",
CACertificate: "test-ca-certificate",
},
},
},
{
Expand All @@ -98,6 +105,12 @@ func TestNewConfig(t *testing.T) {
Password: "test-secret-pass",
Title: "test-title",
Message: "test-message",
TLSConfig: &receivers.TLSConfig{
InsecureSkipVerify: false,
ClientCertificate: "test-client-certificate",
ClientKey: "test-client-key",
CACertificate: "test-ca-certificate",
},
},
},
{
Expand Down
19 changes: 19 additions & 0 deletions receivers/webhook/fixtures/ca.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
-----BEGIN CERTIFICATE-----
MIIDATCCAemgAwIBAgIUOOYoK5DrzPFD4hTL/51+zmOqukwwDQYJKoZIhvcNAQEL
BQAwEDEOMAwGA1UEAwwFTXkgQ0EwHhcNMjQwOTE5MjIzNzAyWhcNMzQwOTE3MjIz
NzAyWjAQMQ4wDAYDVQQDDAVNeSBDQTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCC
AQoCggEBAMaFRXzqRGf/lcvFh9zxzgLCxz8kywr/hJA0hG35gZ4lfOw63HqoMm5Y
poy1BaPB131vqC8AYfdwSshrxT2FKu3exM2Q5ksjBNUWnIJx6uE3fWg5B54q5K0K
47AbOk8HV0NIIuOCjc6XvGeE51W2yujDi8Rn32BvvYpNHGgfrVtsyLl0poatA5me
Tl0rn9gMXY633OlhW6ZYGUnTZZs5WJpVoA5wWdWzQu1u+8oxKGQRdHjN2m/a8nHP
IprLnBXIO2Kyvzz22WUrANhNhQyKT6aoGpi5k3dCrHofmGQKopqQI6fwck2iGJ+H
pa+lCGCHfFHkFSahMg+fheuy1t7LCqsCAwEAAaNTMFEwHQYDVR0OBBYEFJlHFD5L
B/rts+UjQXhIoVovpyu8MB8GA1UdIwQYMBaAFJlHFD5LB/rts+UjQXhIoVovpyu8
MA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBADvLNe+3kjQDYG/h
U1gWhSyV0iOsZ3BbC2mcw1FAPxX0WGaZS6j0mzUQiyBo31kkOyjoJ7PShi7KkxTS
x8+WtMKnfdGsAh1lWeryhRVfh1VTbLd1KQiiAPfaa7ARpw2Ko0gMaKNk2+H8cVQL
gugekoH+naeJHDU1bn4+hsvPN27iBCnJfxXUurTX+JdRgpzEusG+leaPvPifh4mZ
HAfisYtVep05/kFQ8SUSMer8jEUA7Z6Ed5L09/OyFVJKNtEEjCS4tBrib0ahE9lv
38SXrsQqQ/dXHQLoyomAKMCtvVfmqq+roEsLI4pirIHBOWfLXFuOjx/vmLzn14Hm
Xltugek=
-----END CERTIFICATE-----
28 changes: 28 additions & 0 deletions receivers/webhook/fixtures/client.key
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
-----BEGIN PRIVATE KEY-----
MIIEvAIBADANBgkqhkiG9w0BAQEFAASCBKYwggSiAgEAAoIBAQDudpiC2Sur9HuD
33M5C4DbJ1mh+5v0mUPpiAGpOTxW8YL4WF4F2mh7nFQScm2VA/EfSZHa1uxxpOnD
xmeuQgtCIQKSUBvykNkaeDUuZJreEebrdEJI0KAvJ3DtiRjHE+NeSFk+fIRnp1w5
wwXsr59kcNduAetOLusmXAGRs/5VAiMUHlBc5S6taEFy1755a8r93/eB5x6ORSy1
Vl0ThKSJvu92zCKd6FP4zv22YT1iu0Ag5X7kmJhyDTUybBLPblOZPjETGJIpf+k6
MUqfniA9V5B4sZ91gQaFPFfM15EO3ixJ1t7/NAtb8bAmhWQ0BowKkdGB/GVR77x4
74ZQv9H1AgMBAAECggEAGJImHH7FwLX0JNOZOvID/iY7q/SnOQNdMOAfklvAQLx+
6xmpNSowOjhn80SmJO90i22YYU0PAkC6COmrKj9KNgSnuq1qy8oqt08BOn1e9cMw
y5G9o2TCdJ64RJ7WlkC8MgKmFh8HbOoc7Hi7Zgwh54a3CUdFAy0oyVmRRyhIQbg/
5oHrkLNiGpYlqbspKRdH6A/7g5tGAL21ZBtRDDGGcJSdQkCYT3vMHAu+8sf07bcI
kI8pct8ngIDRIly+DylT/UBxuA7t2w8oHpyQFbd0i65eGSs09osYH7AZNZAsbxGW
orq2YHI3JeVnBa9uZJMqANNpVxbP1m8p8Ozf0hAbSwKBgQD3daq1hvzVjJQeddh4
ChBCIrmCLvJAPW3adjkgI1ddFSx/RNFV6ReaCpxJ2mmkJtA6rg5v5vS4qAtFf6JF
lWNbtX9fLajoeqdmGNISLaTl/z7/Em+tkBpKOrt4YuMUywkfNIQA5ZHtINA0nqq8
yiLluZXS/6WI8muOhzkTlAI3swKBgQD2sXHqQkAEIpg3fVRJP+8xtpPdGwwcA4c7
OsJjHPo4z9AklRt2iknyQ62QFXR6lWf0rRyfhXycSmjJjD0NE8BueBiaxMcEXtlN
MxNJHXIBVAFC/M4+r5x0OD9ktKeityqRf0tVBJB/vnL8YI5LPdRP5p5P7voxTRkr
qO61wH17twKBgCmXXkzKVl5yFUcVWX+7eUYUXgeSamb/rCIGss1DA2ECuQre/ywo
VC4w0FndWtobJ+5k6MpIECOlItfYfuahGCUG2VSe8P+59b7ENzKU910szGLEr36F
dNW7D11JiY8qHOtgwTsBWsh3NmdRr7KxarIfwh3HmAPvcsJu2dn/i2vfAoGAe71r
/241wB0+Lu89nPRl3ZDQQGL1wwN+DJaftKlvVZnftMnaGa6qAswxvgkVAPv6Rf+j
UNzbMWYdg6NFaE7VAlRZOyCjFy0gYJnS/aS4b7QcYeZ+6XCa0Kz6F2CKiZLI1mx8
c+uzM3BrKu6f0Vv1KBLsq/maI6qhioXVHvOrvacCgYBIfWZImz9+2HgUgtinSZ2Z
kvMKmzW08jTUp48rijB0c1Q6CkgJusUVT+kuECqTvwks5xIUv2FZbc6mcL2CuEsZ
rrUiOAom5RB4GJb9GViLc5Qoyfi8c2ljSovMPxqoHGdyMHzaiDD+PrWP+Fz/wPFk
Ig+WGGmTJHhiLepdLNGtww==
-----END PRIVATE KEY-----
19 changes: 19 additions & 0 deletions receivers/webhook/fixtures/client.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
-----BEGIN CERTIFICATE-----
MIIDHzCCAgegAwIBAgIUavnhHqP+vrimj0rDqeMrppf8h9QwDQYJKoZIhvcNAQEL
BQAwEDEOMAwGA1UEAwwFTXkgQ0EwHhcNMjQwOTE5MjIzNzAyWhcNMzQwOTE3MjIz
NzAyWjARMQ8wDQYDVQQDDAZDbGllbnQwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAw
ggEKAoIBAQDudpiC2Sur9HuD33M5C4DbJ1mh+5v0mUPpiAGpOTxW8YL4WF4F2mh7
nFQScm2VA/EfSZHa1uxxpOnDxmeuQgtCIQKSUBvykNkaeDUuZJreEebrdEJI0KAv
J3DtiRjHE+NeSFk+fIRnp1w5wwXsr59kcNduAetOLusmXAGRs/5VAiMUHlBc5S6t
aEFy1755a8r93/eB5x6ORSy1Vl0ThKSJvu92zCKd6FP4zv22YT1iu0Ag5X7kmJhy
DTUybBLPblOZPjETGJIpf+k6MUqfniA9V5B4sZ91gQaFPFfM15EO3ixJ1t7/NAtb
8bAmhWQ0BowKkdGB/GVR77x474ZQv9H1AgMBAAGjcDBuMCwGA1UdEQQlMCOCCWxv
Y2FsaG9zdIcEfwAAAYcQAAAAAAAAAAAAAAAAAAAAATAdBgNVHQ4EFgQUB3OcrZze
PWSes0IFrhrvBtAcjo0wHwYDVR0jBBgwFoAUmUcUPksH+u2z5SNBeEihWi+nK7ww
DQYJKoZIhvcNAQELBQADggEBAIu3cbWKsh1/oJcniPuDsSrjUWeB7cqj9NQhUHpi
7NkdtEUlfrWwxgN3RdOErLjzxx0Dv9qgHfg+D6x7YAy51L3ihQX6ajXwdz1OSzLO
guHN487tUXIj6fwNINcJ6JgRbZjxDOFjdTdANxVzJ8Ax/QsexATpG9y2kIc5Slsb
Z83O/SZgNfOL1ijpaOaFw5OVkgQHWJvgS2LMRLEOf6xmw2edIIidRF52VCPEyfNw
7/pjRT6TppfaAxH/HxWtQESnWvGGjQJk8tPSpcs52IJQYOiks/fRWG76MDtnFV/z
ln5hDNMtBgC+JZ57Fe2OmuAH8iswTQWXqJz+aZyBFDpa6Eo=
-----END CERTIFICATE-----
13 changes: 11 additions & 2 deletions receivers/webhook/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,20 @@ const FullValidConfigForTesting = `{
"username": "test-user",
"password": "test-pass",
"title": "test-title",
"message": "test-message"
"message": "test-message",
"tlsConfig": {
"insecureSkipVerify": false,
"clientCertificate": "test-client-certificate",
"clientKey": "test-client-key",
"caCertificate": "test-ca-certificate"
}
}`

// FullValidSecretsForTesting is a string representation of JSON object that contains all fields that can be overridden from secrets
const FullValidSecretsForTesting = `{
"username": "test-secret-user",
"password": "test-secret-pass"
"password": "test-secret-pass",
"clientCertificate": "test-client-certificate",
"clientKey": "test-client-key",
"caCertificate": "test-ca-certificate"
}`
9 changes: 9 additions & 0 deletions receivers/webhook/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package webhook

import (
"context"
"crypto/tls"
"encoding/json"
"fmt"

Expand Down Expand Up @@ -111,13 +112,21 @@ func (wn *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error
return false, tmplErr
}

var tlsConfig *tls.Config
if wn.settings.TLSConfig != nil {
if tlsConfig, err = wn.settings.TLSConfig.ToCryptoTLSConfig(); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slightly unrelated to this PR, but we are pretty much redefining the library here: https://github.com/grafana/dskit/blob/main/crypto/tls/tls.go#L87

It would need to be updated to take the certs/keys as a string though (currently only takes filepaths)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, that's a good point. You'd mention that library but I just went with the same setup that we had for mqtt and forgot about it.
The dskit is more complete also. I could adapt both to use that instead or create another issue to standardize it after this is merged - wdyt?

return false, err
}
}

cmd := &receivers.SendWebhookSettings{
URL: parsedURL,
User: wn.settings.User,
Password: wn.settings.Password,
Body: string(body),
HTTPMethod: wn.settings.HTTPMethod,
HTTPHeader: headers,
TLSConfig: tlsConfig,
}

if err := wn.ns.SendWebhook(ctx, cmd); err != nil {
Expand Down
Loading