Skip to content

chore: add webhook tests for notification subsystem #13942

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jul 19, 2024
Merged

chore: add webhook tests for notification subsystem #13942

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5db00ce
Extend wait period for test; drive-by fix for possibly flaky test
dannykopping Jul 18, 2024
07b4a69
chore: add webhook tests
dannykopping Jul 19, 2024
6dbd9f4
assert
dannykopping Jul 19, 2024
18009b1
Provoke timeout immediately
dannykopping Jul 19, 2024
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
11 changes: 8 additions & 3 deletions coderd/notifications/dispatch/webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,16 @@ func (w *WebhookHandler) dispatch(msgPayload types.MessagePayload, title, body,
return false, xerrors.Errorf("create HTTP request: %v", err)
}
req.Header.Set("Content-Type", "application/json")
req.Header.Set("X-Message-Id", msgID.String())

// Send request.
resp, err := w.cl.Do(req)
if err != nil {
return true, xerrors.Errorf("failed to send HTTP request: %v", err)
if errors.Is(err, context.DeadlineExceeded) {
return true, xerrors.Errorf("request timeout: %w", err)
}

return true, xerrors.Errorf("request failed: %w", err)
}
defer resp.Body.Close()

Expand All @@ -93,11 +98,11 @@ func (w *WebhookHandler) dispatch(msgPayload types.MessagePayload, title, body,
lr := io.LimitReader(resp.Body, int64(len(respBody)))
n, err := lr.Read(respBody)
if err != nil && !errors.Is(err, io.EOF) {
return true, xerrors.Errorf("non-200 response (%d), read body: %w", resp.StatusCode, err)
return true, xerrors.Errorf("non-2xx response (%d), read body: %w", resp.StatusCode, err)
}
w.log.Warn(ctx, "unsuccessful delivery", slog.F("status_code", resp.StatusCode),
slog.F("response", respBody[:n]), slog.F("msg_id", msgID))
return true, xerrors.Errorf("non-200 response (%d)", resp.StatusCode)
return true, xerrors.Errorf("non-2xx response (%d)", resp.StatusCode)
}

return false, nil
Expand Down
144 changes: 144 additions & 0 deletions coderd/notifications/dispatch/webhook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
package dispatch_test

import (
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"net/url"
"testing"
"time"

"github.com/google/uuid"
"github.com/stretchr/testify/require"

"cdr.dev/slog"
"cdr.dev/slog/sloggers/slogtest"
"github.com/coder/serpent"

"github.com/coder/coder/v2/coderd/notifications/dispatch"
"github.com/coder/coder/v2/coderd/notifications/types"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/testutil"
)

func TestWebhook(t *testing.T) {
t.Parallel()

const (
titleTemplate = "this is the title ({{.Labels.foo}})"
bodyTemplate = "this is the body ({{.Labels.baz}})"
)

msgPayload := types.MessagePayload{
Version: "1.0",
NotificationName: "test",
Labels: map[string]string{
"foo": "bar",
"baz": "quux",
},
}

tests := []struct {
name string
serverURL string
serverTimeout time.Duration
serverFn func(uuid.UUID, http.ResponseWriter, *http.Request)

expectSuccess bool
expectRetryable bool
expectErr string
}{
{
name: "successful",
serverFn: func(msgID uuid.UUID, w http.ResponseWriter, r *http.Request) {
var payload dispatch.WebhookPayload
err := json.NewDecoder(r.Body).Decode(&payload)
require.NoError(t, err)
require.Equal(t, "application/json", r.Header.Get("Content-Type"))
require.Equal(t, msgID, payload.MsgID)
require.Equal(t, msgID.String(), r.Header.Get("X-Message-Id"))

w.WriteHeader(http.StatusOK)
_, err = w.Write([]byte(fmt.Sprintf("received %s", payload.MsgID)))
require.NoError(t, err)
},
expectSuccess: true,
},
{
name: "invalid endpoint",
// Build a deliberately invalid URL to fail validation.
serverURL: "invalid .com",
expectSuccess: false,
expectErr: "invalid URL escape",
expectRetryable: false,
},
{
name: "timeout",
serverTimeout: time.Millisecond,
expectSuccess: false,
expectRetryable: true,
expectErr: "request timeout",
},
{
name: "non-200 response",
serverFn: func(_ uuid.UUID, w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusInternalServerError)
},
expectSuccess: false,
expectRetryable: true,
expectErr: "non-2xx response (500)",
},
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we could add a test that sends bad data (wrong format) too, just to have the case covered?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not actually possible, the webhook dispatch sends a well-formed request.

See func (w *WebhookHandler) dispatch

}

logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)

// nolint:paralleltest // Irrelevant as of Go v1.22
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

timeout := testutil.WaitLong
if tc.serverTimeout > 0 {
timeout = tc.serverTimeout
}

var (
err error
ctx = testutil.Context(t, timeout)
msgID = uuid.New()
)

var endpoint *url.URL
if tc.serverURL != "" {
endpoint = &url.URL{Host: tc.serverURL}
} else {
// Mock server to simulate webhook endpoint.
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
tc.serverFn(msgID, w, r)
}))
defer server.Close()

endpoint, err = url.Parse(server.URL)
require.NoError(t, err)
}

cfg := codersdk.NotificationsWebhookConfig{
Endpoint: *serpent.URLOf(endpoint),
}
handler := dispatch.NewWebhookHandler(cfg, logger.With(slog.F("test", tc.name)))
deliveryFn, err := handler.Dispatcher(msgPayload, titleTemplate, bodyTemplate)
require.NoError(t, err)

retryable, err := deliveryFn(ctx, msgID)
if tc.expectSuccess {
require.NoError(t, err)
require.False(t, retryable)
return
}

require.ErrorContains(t, err, tc.expectErr)
require.Equal(t, tc.expectRetryable, retryable)
})
}
}
Loading