Skip to content

Commit 7e8bbe4

Browse files
committed
Only schema changes, no runtime behavior
Signed-off-by: Sanskarzz <sanskar.gur@gmail.com>
1 parent 4329062 commit 7e8bbe4

21 files changed

Lines changed: 277 additions & 937 deletions

File tree

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

Lines changed: 10 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ 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-
rlconfig "github.com/stacklok/toolhive/pkg/ratelimit/config"
1210
)
1311

1412
// Condition types for MCPServer
@@ -513,26 +511,26 @@ type SessionStorageConfig struct {
513511
type RateLimitConfig struct {
514512
// Global is a token bucket shared across all users for the entire server.
515513
// +optional
516-
Global *RateLimitBucket `json:"global,omitempty"`
514+
Global *RateLimitBucket `json:"global,omitempty" yaml:"global,omitempty"`
517515

518516
// Shared is a deprecated alias for Global. Use global instead.
519517
// +optional
520-
Shared *RateLimitBucket `json:"shared,omitempty"`
518+
Shared *RateLimitBucket `json:"shared,omitempty" yaml:"shared,omitempty"`
521519

522520
// PerUser is a token bucket applied independently to each authenticated user
523521
// at the server level. Requires authentication to be enabled.
524522
// Each unique userID creates Redis keys that expire after 2x refillPeriod.
525523
// Memory formula: unique_users_per_TTL_window * (1 + num_tools_with_per_user_limits) keys.
526524
// +optional
527-
PerUser *RateLimitBucket `json:"perUser,omitempty"`
525+
PerUser *RateLimitBucket `json:"perUser,omitempty" yaml:"perUser,omitempty"`
528526

529527
// Tools defines per-tool rate limit overrides.
530528
// Each entry applies additional rate limits to calls targeting a specific tool name.
531529
// A request must pass both the server-level limit and the per-tool limit.
532530
// +listType=map
533531
// +listMapKey=name
534532
// +optional
535-
Tools []ToolRateLimitConfig `json:"tools,omitempty"`
533+
Tools []ToolRateLimitConfig `json:"tools,omitempty" yaml:"tools,omitempty"`
536534
}
537535

538536
// RateLimitBucket defines a token bucket configuration with a maximum capacity
@@ -543,13 +541,13 @@ type RateLimitBucket struct {
543541
// instantaneously before the bucket is depleted.
544542
// +kubebuilder:validation:Required
545543
// +kubebuilder:validation:Minimum=1
546-
MaxTokens int32 `json:"maxTokens"`
544+
MaxTokens int32 `json:"maxTokens" yaml:"maxTokens"`
547545

548546
// RefillPeriod is the duration to fully refill the bucket from zero to maxTokens.
549547
// The effective refill rate is maxTokens / refillPeriod tokens per second.
550548
// Format: Go duration string (e.g., "1m0s", "30s", "1h0m0s").
551549
// +kubebuilder:validation:Required
552-
RefillPeriod metav1.Duration `json:"refillPeriod"`
550+
RefillPeriod metav1.Duration `json:"refillPeriod" yaml:"refillPeriod"`
553551
}
554552

555553
// ToolRateLimitConfig defines rate limits for a specific tool.
@@ -562,53 +560,19 @@ type ToolRateLimitConfig struct {
562560
// Name is the MCP tool name this limit applies to.
563561
// +kubebuilder:validation:Required
564562
// +kubebuilder:validation:MinLength=1
565-
Name string `json:"name"`
563+
Name string `json:"name" yaml:"name"`
566564

567565
// Global token bucket for this specific tool.
568566
// +optional
569-
Global *RateLimitBucket `json:"global,omitempty"`
567+
Global *RateLimitBucket `json:"global,omitempty" yaml:"global,omitempty"`
570568

571569
// Shared is a deprecated alias for Global. Use global instead.
572570
// +optional
573-
Shared *RateLimitBucket `json:"shared,omitempty"`
571+
Shared *RateLimitBucket `json:"shared,omitempty" yaml:"shared,omitempty"`
574572

575573
// PerUser token bucket configuration for this tool.
576574
// +optional
577-
PerUser *RateLimitBucket `json:"perUser,omitempty"`
578-
}
579-
580-
// ToInternal converts the Kubernetes API rate limit config to the runtime-neutral representation.
581-
func (in *RateLimitConfig) ToInternal() *rlconfig.Config {
582-
if in == nil {
583-
return nil
584-
}
585-
out := &rlconfig.Config{
586-
Global: rateLimitBucketToInternal(in.Global),
587-
Shared: rateLimitBucketToInternal(in.Shared),
588-
PerUser: rateLimitBucketToInternal(in.PerUser),
589-
}
590-
if len(in.Tools) > 0 {
591-
out.Tools = make([]rlconfig.ToolConfig, 0, len(in.Tools))
592-
for _, tool := range in.Tools {
593-
out.Tools = append(out.Tools, rlconfig.ToolConfig{
594-
Name: tool.Name,
595-
Global: rateLimitBucketToInternal(tool.Global),
596-
Shared: rateLimitBucketToInternal(tool.Shared),
597-
PerUser: rateLimitBucketToInternal(tool.PerUser),
598-
})
599-
}
600-
}
601-
return out
602-
}
603-
604-
func rateLimitBucketToInternal(in *RateLimitBucket) *rlconfig.Bucket {
605-
if in == nil {
606-
return nil
607-
}
608-
return &rlconfig.Bucket{
609-
MaxTokens: in.MaxTokens,
610-
RefillPeriod: in.RefillPeriod,
611-
}
575+
PerUser *RateLimitBucket `json:"perUser,omitempty" yaml:"perUser,omitempty"`
612576
}
613577

614578
// Permission profile types

cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig_test.go

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -530,12 +530,20 @@ func TestEnsureVmcpConfigConfigMap(t *testing.T) {
530530
var cfg vmcpconfig.Config
531531
require.NoError(t, yaml.Unmarshal([]byte(cm.Data["config.yaml"]), &cfg))
532532
require.NotNil(t, cfg.RateLimiting, "runtime config must include spec.rateLimiting")
533-
require.NotNil(t, cfg.RateLimiting.PerUser)
534-
assert.Equal(t, int32(2), cfg.RateLimiting.PerUser.MaxTokens)
535-
require.Len(t, cfg.RateLimiting.Tools, 1)
536-
assert.Equal(t, "backend_a_echo", cfg.RateLimiting.Tools[0].Name)
537-
require.NotNil(t, cfg.RateLimiting.Tools[0].Global)
538-
assert.Equal(t, int32(5), cfg.RateLimiting.Tools[0].Global.MaxTokens)
533+
534+
rateLimiting := cfg.RateLimiting.Get()
535+
perUser, ok := rateLimiting["perUser"].(map[string]any)
536+
require.True(t, ok)
537+
assert.EqualValues(t, 2, perUser["maxTokens"])
538+
tools, ok := rateLimiting["tools"].([]any)
539+
require.True(t, ok)
540+
require.Len(t, tools, 1)
541+
tool, ok := tools[0].(map[string]any)
542+
require.True(t, ok)
543+
assert.Equal(t, "backend_a_echo", tool["name"])
544+
global, ok := tool["global"].(map[string]any)
545+
require.True(t, ok)
546+
assert.EqualValues(t, 5, global["maxTokens"])
539547
}
540548

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

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

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package vmcpconfig
66

77
import (
88
"context"
9+
"encoding/json"
910
"fmt"
1011

1112
"github.com/go-logr/logr"
@@ -19,6 +20,7 @@ import (
1920
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/oidc"
2021
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/spectoconfig"
2122
"github.com/stacklok/toolhive/pkg/authserver"
23+
thvjson "github.com/stacklok/toolhive/pkg/json"
2224
"github.com/stacklok/toolhive/pkg/telemetry"
2325
"github.com/stacklok/toolhive/pkg/vmcp/auth/converters"
2426
authtypes "github.com/stacklok/toolhive/pkg/vmcp/auth/types"
@@ -140,7 +142,13 @@ func (c *Converter) Convert(
140142
}
141143

142144
config.SessionStorage = convertSessionStorage(vmcp)
143-
config.RateLimiting = vmcp.Spec.RateLimiting.ToInternal()
145+
if vmcp.Spec.RateLimiting != nil {
146+
rateLimiting, err := convertRateLimiting(vmcp.Spec.RateLimiting)
147+
if err != nil {
148+
return nil, nil, fmt.Errorf("failed to convert rate limiting: %w", err)
149+
}
150+
config.RateLimiting = rateLimiting
151+
}
144152

145153
// Apply operational defaults (fills missing values)
146154
config.EnsureOperationalDefaults()
@@ -158,6 +166,22 @@ func (c *Converter) Convert(
158166
return config, authServerRC, nil
159167
}
160168

169+
func convertRateLimiting(rateLimiting *mcpv1beta1.RateLimitConfig) (*thvjson.Map, error) {
170+
if rateLimiting == nil {
171+
return nil, nil
172+
}
173+
raw, err := json.Marshal(rateLimiting)
174+
if err != nil {
175+
return nil, err
176+
}
177+
var value map[string]any
178+
if err := json.Unmarshal(raw, &value); err != nil {
179+
return nil, err
180+
}
181+
converted := thvjson.NewMap(value)
182+
return &converted, nil
183+
}
184+
161185
// convertIncomingAuth converts IncomingAuthConfig from CRD to vmcp config.
162186
func (c *Converter) convertIncomingAuth(
163187
ctx context.Context,

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

Lines changed: 59 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,10 +127,65 @@ func TestConverter_RateLimitingFromTopLevelSpec(t *testing.T) {
127127
cfg, _, err := converter.Convert(context.Background(), vmcp, nil)
128128
require.NoError(t, err)
129129
require.NotNil(t, cfg.RateLimiting)
130-
require.NotNil(t, cfg.RateLimiting.Global)
131-
assert.Equal(t, int32(10), cfg.RateLimiting.Global.MaxTokens)
132-
require.Len(t, cfg.RateLimiting.Tools, 1)
133-
assert.Equal(t, "backend_a_echo", cfg.RateLimiting.Tools[0].Name)
130+
131+
rateLimiting := cfg.RateLimiting.Get()
132+
global, ok := rateLimiting["global"].(map[string]any)
133+
require.True(t, ok)
134+
assert.EqualValues(t, 10, global["maxTokens"])
135+
tools, ok := rateLimiting["tools"].([]any)
136+
require.True(t, ok)
137+
require.Len(t, tools, 1)
138+
tool, ok := tools[0].(map[string]any)
139+
require.True(t, ok)
140+
assert.Equal(t, "backend_a_echo", tool["name"])
141+
}
142+
143+
func TestConverter_RateLimitingPreservesSharedAliasAndCopiesInput(t *testing.T) {
144+
t.Parallel()
145+
146+
vmcp := &mcpv1beta1.VirtualMCPServer{
147+
ObjectMeta: metav1.ObjectMeta{Name: "test-vmcp", Namespace: "default"},
148+
Spec: mcpv1beta1.VirtualMCPServerSpec{
149+
GroupRef: &mcpv1beta1.MCPGroupRef{Name: "test-group"},
150+
IncomingAuth: &mcpv1beta1.IncomingAuthConfig{Type: "anonymous"},
151+
RateLimiting: &mcpv1beta1.RateLimitConfig{
152+
Shared: &mcpv1beta1.RateLimitBucket{
153+
MaxTokens: 10,
154+
RefillPeriod: metav1.Duration{Duration: time.Minute},
155+
},
156+
Tools: []mcpv1beta1.ToolRateLimitConfig{
157+
{
158+
Name: "backend_a_echo",
159+
Shared: &mcpv1beta1.RateLimitBucket{
160+
MaxTokens: 1,
161+
RefillPeriod: metav1.Duration{Duration: time.Second},
162+
},
163+
},
164+
},
165+
},
166+
},
167+
}
168+
169+
converter := newTestConverter(t, newNoOpMockResolver(t))
170+
cfg, _, err := converter.Convert(context.Background(), vmcp, nil)
171+
require.NoError(t, err)
172+
require.NotNil(t, cfg.RateLimiting)
173+
174+
vmcp.Spec.RateLimiting.Shared.MaxTokens = 99
175+
vmcp.Spec.RateLimiting.Tools[0].Shared.MaxTokens = 88
176+
177+
rateLimiting := cfg.RateLimiting.Get()
178+
shared, ok := rateLimiting["shared"].(map[string]any)
179+
require.True(t, ok)
180+
assert.EqualValues(t, 10, shared["maxTokens"])
181+
tools, ok := rateLimiting["tools"].([]any)
182+
require.True(t, ok)
183+
require.Len(t, tools, 1)
184+
tool, ok := tools[0].(map[string]any)
185+
require.True(t, ok)
186+
toolShared, ok := tool["shared"].(map[string]any)
187+
require.True(t, ok)
188+
assert.EqualValues(t, 1, toolShared["maxTokens"])
134189
}
135190

136191
func TestConverter_OIDCResolution(t *testing.T) {

cmd/thv-operator/test-integration/virtualmcp/virtualmcpserver_sessionstorage_cel_test.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,11 @@ var _ = Describe("CEL Validation for SessionStorageConfig on VirtualMCPServer",
147147
Address: "redis:6379",
148148
})
149149
vmcp.Spec.IncomingAuth = &mcpv1beta1.IncomingAuthConfig{
150-
Type: "oidc",
151-
OIDCConfigRef: &mcpv1beta1.MCPOIDCConfigReference{Name: "oidc"},
150+
Type: "oidc",
151+
OIDCConfigRef: &mcpv1beta1.MCPOIDCConfigReference{
152+
Name: "oidc",
153+
Audience: "test-audience",
154+
},
152155
}
153156
vmcp.Spec.RateLimiting = &mcpv1beta1.RateLimitConfig{
154157
PerUser: &mcpv1beta1.RateLimitBucket{

docs/operator/crd-api.md

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

0 commit comments

Comments
 (0)