Skip to content

Commit 88667f7

Browse files
authored
Configure rate limits on VirtualMCPServer PR A (#5079)
* Configure rate limits on VirtualMCPServer Signed-off-by: Sanskarzz <sanskar.gur@gmail.com> * Only schema changes, no runtime behavior Signed-off-by: Sanskarzz <sanskar.gur@gmail.com> * Address vMCP rate limit review feedback Signed-off-by: Sanskarzz <sanskar.gur@gmail.com> * removed wrapper and reverted path marshall directly Signed-off-by: Sanskarzz <sanskar.gur@gmail.com> * moved onto config.Config as spec.config.rateLimiting Signed-off-by: Sanskarzz <sanskar.gur@gmail.com> * removed the duplicate rate-limit structs from pkg/vmcp/config Signed-off-by: Sanskarzz <sanskar.gur@gmail.com> --------- Signed-off-by: Sanskarzz <sanskar.gur@gmail.com>
1 parent 8fd704e commit 88667f7

23 files changed

Lines changed: 1122 additions & 289 deletions

cmd/thv-operator/Taskfile.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ tasks:
168168
platforms: [windows]
169169
ignore_error: true # Windows has no mkdir -p, so just ignore error if it exists
170170
- go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.17.3
171-
- $(go env GOPATH)/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./cmd/thv-operator/..." paths="./pkg/json/..." paths="./pkg/vmcp/config/..." paths="./pkg/vmcp/auth/types/..." paths="./pkg/telemetry/..." paths="./pkg/audit/..."
171+
- $(go env GOPATH)/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./cmd/thv-operator/..." paths="./pkg/json/..." paths="./pkg/ratelimit/types/..." paths="./pkg/vmcp/config/..." paths="./pkg/vmcp/auth/types/..." paths="./pkg/telemetry/..." paths="./pkg/audit/..."
172172

173173
operator-manifests:
174174
desc: Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects
@@ -286,6 +286,7 @@ tasks:
286286
sources:
287287
- '{{.ROOT_DIR}}/cmd/thv-operator/config/crd/bases/**/*.yaml'
288288
- '{{.ROOT_DIR}}/cmd/thv-operator/api/**/*.go'
289+
- '{{.ROOT_DIR}}/pkg/ratelimit/types/*.go'
289290
- '{{.ROOT_DIR}}/pkg/vmcp/config/*.go'
290291
- '{{.ROOT_DIR}}/pkg/vmcp/auth/types/*.go'
291292
- '{{.ROOT_DIR}}/pkg/telemetry/*.go'

cmd/thv-operator/api/v1beta1/mcpserver_types.go

Lines changed: 10 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
corev1 "k8s.io/api/core/v1"
88
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
99
"k8s.io/apimachinery/pkg/runtime"
10+
11+
ratelimittypes "github.com/stacklok/toolhive/pkg/ratelimit/types"
1012
)
1113

1214
// Condition types for MCPServer
@@ -373,7 +375,7 @@ type MCPServerSpec struct {
373375
// RateLimiting defines rate limiting configuration for the MCP server.
374376
// Requires Redis session storage to be configured for distributed rate limiting.
375377
// +optional
376-
RateLimiting *RateLimitConfig `json:"rateLimiting,omitempty"`
378+
RateLimiting *ratelimittypes.RateLimitConfig `json:"rateLimiting,omitempty"`
377379
}
378380

379381
// ResourceOverrides defines overrides for annotations and labels on created resources
@@ -525,69 +527,17 @@ type SessionStorageConfig struct {
525527
}
526528

527529
// RateLimitConfig defines rate limiting configuration for an MCP server.
528-
// At least one of shared, perUser, or tools must be configured.
529-
//
530-
// +kubebuilder:validation:XValidation:rule="has(self.shared) || has(self.perUser) || (has(self.tools) && size(self.tools) > 0)",message="at least one of shared, perUser, or tools must be configured"
531-
//
532-
//nolint:lll // CEL validation rules exceed line length limit
533-
type RateLimitConfig struct {
534-
// Shared is a token bucket shared across all users for the entire server.
535-
// +optional
536-
Shared *RateLimitBucket `json:"shared,omitempty"`
537-
538-
// PerUser is a token bucket applied independently to each authenticated user
539-
// at the server level. Requires authentication to be enabled.
540-
// Each unique userID creates Redis keys that expire after 2x refillPeriod.
541-
// Memory formula: unique_users_per_TTL_window * (1 + num_tools_with_per_user_limits) keys.
542-
// +optional
543-
PerUser *RateLimitBucket `json:"perUser,omitempty"`
544-
545-
// Tools defines per-tool rate limit overrides.
546-
// Each entry applies additional rate limits to calls targeting a specific tool name.
547-
// A request must pass both the server-level limit and the per-tool limit.
548-
// +listType=map
549-
// +listMapKey=name
550-
// +optional
551-
Tools []ToolRateLimitConfig `json:"tools,omitempty"`
552-
}
530+
// +gendoc
531+
type RateLimitConfig = ratelimittypes.RateLimitConfig
553532

554533
// RateLimitBucket defines a token bucket configuration with a maximum capacity
555-
// and a refill period. Used by both shared (global) and per-user rate limits.
556-
type RateLimitBucket struct {
557-
// MaxTokens is the maximum number of tokens (bucket capacity).
558-
// This is also the burst size: the maximum number of requests that can be served
559-
// instantaneously before the bucket is depleted.
560-
// +kubebuilder:validation:Required
561-
// +kubebuilder:validation:Minimum=1
562-
MaxTokens int32 `json:"maxTokens"`
563-
564-
// RefillPeriod is the duration to fully refill the bucket from zero to maxTokens.
565-
// The effective refill rate is maxTokens / refillPeriod tokens per second.
566-
// Format: Go duration string (e.g., "1m0s", "30s", "1h0m0s").
567-
// +kubebuilder:validation:Required
568-
RefillPeriod metav1.Duration `json:"refillPeriod"`
569-
}
534+
// and a refill period. Used by both shared and per-user rate limits.
535+
// +gendoc
536+
type RateLimitBucket = ratelimittypes.RateLimitBucket
570537

571538
// ToolRateLimitConfig defines rate limits for a specific tool.
572-
// At least one of shared or perUser must be configured.
573-
//
574-
// +kubebuilder:validation:XValidation:rule="has(self.shared) || has(self.perUser)",message="at least one of shared or perUser must be configured"
575-
//
576-
//nolint:lll // kubebuilder marker exceeds line length
577-
type ToolRateLimitConfig struct {
578-
// Name is the MCP tool name this limit applies to.
579-
// +kubebuilder:validation:Required
580-
// +kubebuilder:validation:MinLength=1
581-
Name string `json:"name"`
582-
583-
// Shared token bucket for this specific tool.
584-
// +optional
585-
Shared *RateLimitBucket `json:"shared,omitempty"`
586-
587-
// PerUser token bucket configuration for this tool.
588-
// +optional
589-
PerUser *RateLimitBucket `json:"perUser,omitempty"`
590-
}
539+
// +gendoc
540+
type ToolRateLimitConfig = ratelimittypes.ToolRateLimitConfig
591541

592542
// Permission profile types
593543
const (

cmd/thv-operator/api/v1beta1/mcpserver_types_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import (
1111
"github.com/stretchr/testify/assert"
1212
"github.com/stretchr/testify/require"
1313
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
14+
15+
vmcpconfig "github.com/stacklok/toolhive/pkg/vmcp/config"
1416
)
1517

1618
func TestSessionStorageConfigJSONRoundtrip(t *testing.T) {
@@ -116,6 +118,46 @@ func TestRateLimitConfigJSONRoundtrip(t *testing.T) {
116118
}
117119
}
118120

121+
func TestVirtualMCPServerSpecRateLimitingJSONRoundtrip(t *testing.T) {
122+
t.Parallel()
123+
124+
spec := VirtualMCPServerSpec{
125+
IncomingAuth: &IncomingAuthConfig{Type: "oidc"},
126+
GroupRef: &MCPGroupRef{Name: "group-a"},
127+
SessionStorage: &SessionStorageConfig{
128+
Provider: "redis",
129+
Address: "redis.default.svc.cluster.local:6379",
130+
},
131+
Config: vmcpconfig.Config{
132+
RateLimiting: &RateLimitConfig{
133+
Shared: &RateLimitBucket{MaxTokens: 10, RefillPeriod: metav1.Duration{Duration: time.Minute}},
134+
PerUser: &RateLimitBucket{
135+
MaxTokens: 2,
136+
RefillPeriod: metav1.Duration{Duration: time.Minute},
137+
},
138+
Tools: []ToolRateLimitConfig{
139+
{
140+
Name: "backend_a_echo",
141+
Shared: &RateLimitBucket{
142+
MaxTokens: 5,
143+
RefillPeriod: metav1.Duration{Duration: 30 * time.Second},
144+
},
145+
},
146+
},
147+
},
148+
},
149+
}
150+
151+
b, err := json.Marshal(spec)
152+
require.NoError(t, err)
153+
out := string(b)
154+
assert.Contains(t, out, `"rateLimiting"`)
155+
assert.Contains(t, out, `"shared"`)
156+
assert.Contains(t, out, `"perUser"`)
157+
assert.Contains(t, out, `"backend_a_echo"`)
158+
assert.Contains(t, out, `"config":{"rateLimiting"`)
159+
}
160+
119161
func TestMCPServerSpecScalingFieldsJSONRoundtrip(t *testing.T) {
120162
t.Parallel()
121163

cmd/thv-operator/api/v1beta1/virtualmcpserver_types.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ import (
1616

1717
// VirtualMCPServerSpec defines the desired state of VirtualMCPServer
1818
//
19+
// +kubebuilder:validation:XValidation:rule="!has(self.config) || !has(self.config.rateLimiting) || (has(self.sessionStorage) && self.sessionStorage.provider == 'redis')",message="config.rateLimiting requires sessionStorage with provider 'redis'"
20+
// +kubebuilder:validation:XValidation:rule="!(has(self.config) && has(self.config.rateLimiting) && has(self.config.rateLimiting.perUser)) || (has(self.incomingAuth) && self.incomingAuth.type == 'oidc')",message="config.rateLimiting.perUser requires incomingAuth.type oidc"
21+
// +kubebuilder:validation:XValidation:rule="!has(self.config) || !has(self.config.rateLimiting) || !has(self.config.rateLimiting.tools) || self.config.rateLimiting.tools.all(t, !has(t.perUser)) || (has(self.incomingAuth) && self.incomingAuth.type == 'oidc')",message="per-tool perUser rate limiting requires incomingAuth.type oidc"
22+
//
1923
//nolint:lll // CEL validation rules exceed line length limit
2024
type VirtualMCPServerSpec struct {
2125
// IncomingAuth configures authentication for clients connecting to the Virtual MCP server.

cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go

Lines changed: 2 additions & 74 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,11 @@ func TestEnsureVmcpConfigConfigMap(t *testing.T) {
507507
assert.Equal(t, "test-vmcp-vmcp-config", cm.Name)
508508
assert.Contains(t, cm.Data, "config.yaml")
509509
assert.NotEmpty(t, cm.Annotations["toolhive.stacklok.dev/content-checksum"])
510+
511+
var cfg vmcpconfig.Config
512+
require.NoError(t, yaml.Unmarshal([]byte(cm.Data["config.yaml"]), &cfg))
513+
assert.Equal(t, "test-vmcp", cfg.Name)
514+
assert.Equal(t, "test-group", cfg.Group)
510515
}
511516

512517
// TestSetAuthConfigConditions tests that auth config conditions reflect the current state

cmd/thv-operator/pkg/vmcpconfig/converter_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1601,6 +1601,51 @@ func TestConverter_SessionStorage(t *testing.T) {
16011601
}
16021602
}
16031603

1604+
func TestConverter_RateLimitingPassThrough(t *testing.T) {
1605+
t.Parallel()
1606+
1607+
vmcpServer := &mcpv1beta1.VirtualMCPServer{
1608+
ObjectMeta: metav1.ObjectMeta{
1609+
Name: "test-vmcp",
1610+
Namespace: "default",
1611+
},
1612+
Spec: mcpv1beta1.VirtualMCPServerSpec{
1613+
GroupRef: &mcpv1beta1.MCPGroupRef{Name: "test-group"},
1614+
Config: vmcpconfig.Config{
1615+
RateLimiting: &mcpv1beta1.RateLimitConfig{
1616+
PerUser: &mcpv1beta1.RateLimitBucket{
1617+
MaxTokens: 2,
1618+
RefillPeriod: metav1.Duration{Duration: time.Minute},
1619+
},
1620+
Tools: []mcpv1beta1.ToolRateLimitConfig{
1621+
{
1622+
Name: "backend_a_echo",
1623+
Shared: &mcpv1beta1.RateLimitBucket{
1624+
MaxTokens: 5,
1625+
RefillPeriod: metav1.Duration{Duration: 30 * time.Second},
1626+
},
1627+
},
1628+
},
1629+
},
1630+
},
1631+
},
1632+
}
1633+
1634+
converter := newTestConverter(t, newNoOpMockResolver(t))
1635+
ctx := log.IntoContext(context.Background(), logr.Discard())
1636+
1637+
config, _, err := converter.Convert(ctx, vmcpServer, nil)
1638+
require.NoError(t, err)
1639+
require.NotNil(t, config)
1640+
require.NotNil(t, config.RateLimiting)
1641+
1642+
assert.EqualValues(t, 2, config.RateLimiting.PerUser.MaxTokens)
1643+
require.Len(t, config.RateLimiting.Tools, 1)
1644+
assert.Equal(t, "backend_a_echo", config.RateLimiting.Tools[0].Name)
1645+
require.NotNil(t, config.RateLimiting.Tools[0].Shared)
1646+
assert.EqualValues(t, 5, config.RateLimiting.Tools[0].Shared.MaxTokens)
1647+
}
1648+
16041649
func TestDeriveAllowedAudiences(t *testing.T) {
16051650
t.Parallel()
16061651

cmd/thv-operator/test-integration/embedding-server/embeddingserver_update_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,7 @@ var _ = Describe("EmbeddingServer Controller Update Tests", func() {
466466
Expect(k8sClient.Create(ctx, embeddingServer)).To(Succeed())
467467
Eventually(func(g Gomega) {
468468
g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(embeddingServer), &appsv1.StatefulSet{})).To(Succeed())
469+
g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(embeddingServer), &corev1.Service{})).To(Succeed())
469470
}, timeout, interval).Should(Succeed())
470471
})
471472

0 commit comments

Comments
 (0)