-
Notifications
You must be signed in to change notification settings - Fork 59
config to throttle all Scan endpoints, all IPs #2646
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
config to throttle all Scan endpoints, all IPs #2646
Conversation
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
- see https://cloud.google.com/armor/docs/rules-language-reference Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements throttling for all endpoints and all IPs by refactoring the Cloud Armor configuration to support a new throttling mechanism. The changes move away from per-endpoint, per-IP throttling to a global throttling approach across all endpoints for all IPs.
Key changes:
- Updated configuration schema to support new throttling parameters with hostname, path prefix, and throttling limits
- Refactored Cloud Armor policy implementation to use the new configuration structure
- Added example configuration for the scan service with throttling disabled (maxRequestsBeforeHttp429: 0)
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cluster/pulumi/infra/src/config.ts | Updated CloudArmorConfigSchema to define new throttling configuration structure with hostname, path prefix, and throttling parameters |
| cluster/pulumi/infra/src/cloudArmor.ts | Refactored throttling logic to use new configuration, simplified rule creation, and removed per-IP throttling support |
| cluster/deployment/config.yaml | Added example configuration for scan service with throttling disabled |
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
…[skip ci] Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
… [ci] Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
…-throttle-all-endpoints-all-ips [ci]
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
|
Test enable 5667b0a on scratchd (NB: unattached so even with preview off this does nothing): cloudArmor:
enabled: true
publicEndpoints:
publicScan:
hostname: scan.sv-2.scratchd.global.canton.network.digitalasset.com
pathPrefix: /api/scan
throttleAcrossAllEndpointsAllIps:
maxRequestsBeforeHttp429: 42enabled preview security policyPulumi preview & diff, enabled=true preview=true
enabled non-preview security policycloudArmor:
allRulesPreviewOnly: falsePulumi preview & diff, after disabling Cloud Armor preview
disabling after enabling tears downPulumi preview & diff, teardown with enabled=false (i.e. this PR) |
Signed-off-by: Stephen Compall <stephen.compall@digitalasset.com>
julientinguely-da
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks




Fixes DACH-NY/canton-network-internal#2152. As
cloudArmor.enabled: false, this still does nothing (yet) as with #2582.This also fixes up the default rule and actually applies successfully to a GCP cluster; see comments below for example.
Pull Request Checklist
Cluster Testing
/cluster_teston this PR to request it, and ping someone with access to the DA-internal system to approve it./hdm_teston this PR to request it, and ping someone with access to the DA-internal system to approve it.PR Guidelines
Fixes #n, and mention issues worked on using#nMerge Guidelines