[SECURESIGN-3115] feat: Add HAProxy rate-limiting support for Ingress resources - #2154
[SECURESIGN-3115] feat: Add HAProxy rate-limiting support for Ingress resources#2154jourdee-lab wants to merge 1 commit into
Conversation
24e0f3e to
0bfb90a
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2154 +/- ##
==========================================
- Coverage 57.13% 56.99% -0.14%
==========================================
Files 284 285 +1
Lines 15991 16134 +143
==========================================
+ Hits 9136 9196 +60
- Misses 5907 5986 +79
- Partials 948 952 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
- Introduced a new `throttling` field in the Console, Fulcio, Rekor, and TSA CRDs to configure HAProxy rate-limiting annotations. - Implemented default throttling values for each component, with the ability to override via the CRD. - Added logic to ensure appropriate annotations are set on Ingress resources based on the throttling configuration. - Created unit tests to validate the behavior of the new throttling feature, including default values, custom overrides, and disabling throttling. - Updated the Ingress utility functions to handle the new throttling settings and annotations.
0bfb90a to
adb0825
Compare
jourdee-lab
changed the title
PR Summary by QodoAdd configurable HAProxy throttling to OpenShift ingress routes
AI Description
Diagram
High-Level Assessment
Files changed (33)
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTo customize comments, go to the Qodo configuration screen, or learn more in the docs. |
osmman
left a comment
osmman
left a comment
There was a problem hiding this comment.
Design concern: HAProxy-specific API surface in the CRD
The IngressThrottling type is a 1:1 mapping of haproxy.router.openshift.io/rate-limit-connections.* annotations, which ties the CRD permanently to one specific ingress controller implementation. On OpenShift clusters running a different ingress controller (nginx, traefik, etc.) these fields are silently ignored — no error, no warning, no condition — and on non-OpenShift deployments (EKS, GKE, vanilla k8s) the entire struct is completely inert. Since CRD fields in v1 are permanent, we'd be locked into this HAProxy-specific surface even if we later need to support other rate-limiting backends.
Proposed alternative
Add an Annotations field to the existing Ingress type — the same pattern already used for Labels:
type Ingress struct {
Enabled *bool `json:"enabled,omitempty"`
Host string `json:"host,omitempty"`
Labels map[string]string `json:"labels,omitempty"`
Annotations map[string]string `json:"annotations,omitempty"` // new
}This gives users full control over any ingress/route annotations their environment needs — HAProxy throttling, nginx rate-limiting, custom headers — without baking vendor-specific semantics into the API. For our managed default deployments, the operator would internally apply sensible HAProxy throttling annotations when running on OpenShift (just like we already set TLS annotations), without requiring any CRD configuration. Users who want to override or disable defaults can do so via the annotations map.
This keeps the CRD vendor-neutral and portable, still gives us throttling-by-default on OCP, lets users on any platform configure their own ingress annotations, and doesn't pollute the API surface with implementation-specific fields.
Rate-limit values concern
Where do the proposed default values come from? They look too restrictive for API endpoints. HAProxy's rate-http uses a hardcoded 10-second sliding window, so rateHTTP=50 means only 5 req/sec per source IP — automated signing clients, CI pipelines, or frontends making parallel requests will hit this very quickly. Similarly rateTCP=100 uses a 3-second window (~33 new connections/sec), which is tight. Only concurrentTCP=100 looks reasonable for most workloads.
These are per-source-IP limits enforced by HAProxy's stick-table, not global. There's also a known issue where TCP-based limits may not be reliably enforced for parallel connections, so these shouldn't be treated as a hard security boundary. I'd suggest bumping rateHTTP to at least 100-200 for API endpoints (Fulcio, Rekor, TSA) and being more conservative only for the Console UI.
3 participants
Summary
OpenShift Routes auto-generated from Ingress resources had no rate-limiting, leaving Fulcio, Rekor, TSA, and Console endpoints exposed to traffic spikes. This PR adds opt-in (enabled
by default) HAProxy throttling annotations that the OpenShift router enforces natively.
throttlingCRD field to Fulcio, Rekor, TSA, and Console specs for configuring per-source-IP connection and request rate limitsthrottlingis omitted (e.g. Rekor gets higher limits since every sign/verify hits it)throttling.enabled: falseWhat changed
api/v1/): newIngressThrottlingstruct, addedThrottlingfield to Fulcio, Rekor, TSA, Console specsinternal/controller/*/actions/): each controller's ingress action wiresEnsureIngressHAProxyThrottlingwith component-specific defaultsinternal/utils/kubernetes/ingress.go):EnsureIngressHAProxyThrottling()handles annotation set/remove logicinternal/testing/action/haproxy_throttling.go) validates all 4 controllers with a single parameterized harnessdocs/ingress-throttling.mddocuments defaults, customization, and disabling