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

chore: reduce default max payload size in webhooks to 50MB #21101

Merged
Merged
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
2 changes: 1 addition & 1 deletion docs/operator-manual/argocd-cm.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ data:
name: some-cluster
server: https://some-cluster
# The maximum size of the payload that can be sent to the webhook server.
webhook.maxPayloadSizeMB: "1024"
webhook.maxPayloadSizeMB: "50"

# application.sync.impersonation.enabled enables application sync to use a custom service account, via impersonation. This allows decoupling sync from control-plane service account.
application.sync.impersonation.enabled: "false"
2 changes: 1 addition & 1 deletion docs/operator-manual/webhook.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ URL configured in the Git provider should use the `/api/webhook` endpoint of you
(e.g. `https://argocd.example.com/api/webhook`). If you wish to use a shared secret, input an
arbitrary value in the secret. This value will be used when configuring the webhook in the next step.

To prevent DDoS attacks with unauthenticated webhook events (the `/api/webhook` endpoint currently lacks rate limiting protection), it is recommended to limit the payload size. You can achieve this by configuring the `argocd-cm` ConfigMap with the `webhook.maxPayloadSizeMB` attribute. The default value is 1GB.
To prevent DDoS attacks with unauthenticated webhook events (the `/api/webhook` endpoint currently lacks rate limiting protection), it is recommended to limit the payload size. You can achieve this by configuring the `argocd-cm` ConfigMap with the `webhook.maxPayloadSizeMB` attribute. The default value is 50MB.

## Github

Expand Down
4 changes: 2 additions & 2 deletions util/settings/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -537,8 +537,8 @@ const (
)

const (
// default max webhook payload size is 1GB
defaultMaxWebhookPayloadSize = int64(1) * 1024 * 1024 * 1024
// default max webhook payload size is 50MB
defaultMaxWebhookPayloadSize = int64(50) * 1024 * 1024

// application sync with impersonation feature is disabled by default.
defaultImpersonationEnabledFlag = false
Expand Down
4 changes: 2 additions & 2 deletions util/webhook/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ type reactorDef struct {
}

func NewMockHandler(reactor *reactorDef, applicationNamespaces []string, objects ...runtime.Object) *ArgoCDWebhookHandler {
defaultMaxPayloadSize := int64(1) * 1024 * 1024 * 1024
defaultMaxPayloadSize := int64(50) * 1024 * 1024
return NewMockHandlerWithPayloadLimit(reactor, applicationNamespaces, defaultMaxPayloadSize, objects...)
}

Expand Down Expand Up @@ -428,7 +428,7 @@ func TestInvalidEvent(t *testing.T) {
close(h.queue)
h.Wait()
assert.Equal(t, http.StatusBadRequest, w.Code)
expectedLogResult := "Webhook processing failed: The payload is either too large or corrupted. Please check the payload size (must be under 1024 MB) and ensure it is valid JSON"
expectedLogResult := "Webhook processing failed: The payload is either too large or corrupted. Please check the payload size (must be under 50 MB) and ensure it is valid JSON"
assert.Equal(t, expectedLogResult, hook.LastEntry().Message)
assert.Equal(t, expectedLogResult+"\n", w.Body.String())
hook.Reset()
Expand Down
Loading