Skip to content

Commit eceb293

Browse files
authored
Refactor auth resolution logic to config struct (#2478)
Move authentication resolution from CLI backend discoverer to OutgoingAuthConfig.ResolveForBackend() method for better separation of concerns and reusability. The resolveAuthConfig method in cli_discoverer.go contained logic that belongs in the config struct. This refactoring allows the auth resolution logic to be reused by any component that needs to resolve backend authentication, not just the CLI discoverer. Changes: - Add ResolveForBackend() method to OutgoingAuthConfig - Update CLI discoverer to use new method - Add comprehensive test coverage for auth resolution logic - Remove duplicate resolveAuthConfig() method from cli_discoverer Fixes #2465
1 parent 630707b commit eceb293

File tree

3 files changed

+173
-30
lines changed

3 files changed

+173
-30
lines changed

pkg/vmcp/aggregator/cli_discoverer.go

Lines changed: 5 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -103,13 +103,11 @@ func (d *cliBackendDiscoverer) Discover(ctx context.Context, groupRef string) ([
103103
}
104104

105105
// Apply authentication configuration if provided
106-
if d.authConfig != nil {
107-
authStrategy, authMetadata := d.resolveAuthConfig(name)
108-
backend.AuthStrategy = authStrategy
109-
backend.AuthMetadata = authMetadata
110-
if authStrategy != "" {
111-
logger.Debugf("Backend %s configured with auth strategy: %s", name, authStrategy)
112-
}
106+
authStrategy, authMetadata := d.authConfig.ResolveForBackend(name)
107+
backend.AuthStrategy = authStrategy
108+
backend.AuthMetadata = authMetadata
109+
if authStrategy != "" {
110+
logger.Debugf("Backend %s configured with auth strategy: %s", name, authStrategy)
113111
}
114112

115113
// Copy user labels to metadata first
@@ -136,29 +134,6 @@ func (d *cliBackendDiscoverer) Discover(ctx context.Context, groupRef string) ([
136134
return backends, nil
137135
}
138136

139-
// resolveAuthConfig determines the authentication strategy and metadata for a backend.
140-
// It checks for backend-specific configuration first, then falls back to default.
141-
func (d *cliBackendDiscoverer) resolveAuthConfig(backendID string) (string, map[string]any) {
142-
if d.authConfig == nil {
143-
return "", nil
144-
}
145-
146-
// Check for backend-specific configuration
147-
if strategy, exists := d.authConfig.Backends[backendID]; exists && strategy != nil {
148-
logger.Debugf("Using backend-specific auth strategy for %s: %s", backendID, strategy.Type)
149-
return strategy.Type, strategy.Metadata
150-
}
151-
152-
// Fall back to default configuration
153-
if d.authConfig.Default != nil {
154-
logger.Debugf("Using default auth strategy for %s: %s", backendID, d.authConfig.Default.Type)
155-
return d.authConfig.Default.Type, d.authConfig.Default.Metadata
156-
}
157-
158-
// No authentication configured
159-
return "", nil
160-
}
161-
162137
// mapWorkloadStatusToHealth converts a workload status to a backend health status.
163138
func mapWorkloadStatusToHealth(status rt.WorkloadStatus) vmcp.BackendHealthStatus {
164139
switch status {

pkg/vmcp/config/config.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,28 @@ type BackendAuthStrategy struct {
125125
Metadata map[string]any
126126
}
127127

128+
// ResolveForBackend returns the auth strategy and metadata for a given backend ID.
129+
// It checks for backend-specific config first, then falls back to default.
130+
// Returns empty string and nil if no authentication is configured.
131+
func (c *OutgoingAuthConfig) ResolveForBackend(backendID string) (string, map[string]any) {
132+
if c == nil {
133+
return "", nil
134+
}
135+
136+
// Check for backend-specific configuration
137+
if strategy, exists := c.Backends[backendID]; exists && strategy != nil {
138+
return strategy.Type, strategy.Metadata
139+
}
140+
141+
// Fall back to default configuration
142+
if c.Default != nil {
143+
return c.Default.Type, c.Default.Metadata
144+
}
145+
146+
// No authentication configured
147+
return "", nil
148+
}
149+
128150
// AggregationConfig configures capability aggregation.
129151
type AggregationConfig struct {
130152
// ConflictResolution is the strategy: "prefix", "priority", "manual"

pkg/vmcp/config/config_test.go

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
package config
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
)
8+
9+
func TestOutgoingAuthConfig_ResolveForBackend(t *testing.T) {
10+
t.Parallel()
11+
12+
tests := []struct {
13+
name string
14+
config *OutgoingAuthConfig
15+
backendID string
16+
wantStrategy string
17+
wantMetadata map[string]any
18+
description string
19+
}{
20+
{
21+
name: "nil config returns empty",
22+
config: nil,
23+
backendID: "backend1",
24+
wantStrategy: "",
25+
wantMetadata: nil,
26+
description: "When config is nil, should return empty values",
27+
},
28+
{
29+
name: "backend-specific config takes precedence",
30+
config: &OutgoingAuthConfig{
31+
Default: &BackendAuthStrategy{
32+
Type: "default-strategy",
33+
Metadata: map[string]any{"key": "default-value"},
34+
},
35+
Backends: map[string]*BackendAuthStrategy{
36+
"backend1": {
37+
Type: "backend-specific-strategy",
38+
Metadata: map[string]any{"key": "backend-value"},
39+
},
40+
},
41+
},
42+
backendID: "backend1",
43+
wantStrategy: "backend-specific-strategy",
44+
wantMetadata: map[string]any{"key": "backend-value"},
45+
description: "Backend-specific config should override default",
46+
},
47+
{
48+
name: "falls back to default when backend not configured",
49+
config: &OutgoingAuthConfig{
50+
Default: &BackendAuthStrategy{
51+
Type: "default-strategy",
52+
Metadata: map[string]any{"key": "default-value"},
53+
},
54+
Backends: map[string]*BackendAuthStrategy{
55+
"backend1": {
56+
Type: "backend-specific-strategy",
57+
Metadata: map[string]any{"key": "backend-value"},
58+
},
59+
},
60+
},
61+
backendID: "backend2",
62+
wantStrategy: "default-strategy",
63+
wantMetadata: map[string]any{"key": "default-value"},
64+
description: "Should use default when specific backend not configured",
65+
},
66+
{
67+
name: "returns empty when no default and backend not configured",
68+
config: &OutgoingAuthConfig{
69+
Backends: map[string]*BackendAuthStrategy{
70+
"backend1": {
71+
Type: "backend-specific-strategy",
72+
Metadata: map[string]any{"key": "backend-value"},
73+
},
74+
},
75+
},
76+
backendID: "backend2",
77+
wantStrategy: "",
78+
wantMetadata: nil,
79+
description: "Should return empty when no default and backend not in map",
80+
},
81+
{
82+
name: "handles nil backend strategy in map",
83+
config: &OutgoingAuthConfig{
84+
Default: &BackendAuthStrategy{
85+
Type: "default-strategy",
86+
Metadata: map[string]any{"key": "default-value"},
87+
},
88+
Backends: map[string]*BackendAuthStrategy{
89+
"backend1": nil,
90+
},
91+
},
92+
backendID: "backend1",
93+
wantStrategy: "default-strategy",
94+
wantMetadata: map[string]any{"key": "default-value"},
95+
description: "Should fall back to default when backend strategy is nil",
96+
},
97+
{
98+
name: "returns empty when only default is nil",
99+
config: &OutgoingAuthConfig{
100+
Default: nil,
101+
Backends: map[string]*BackendAuthStrategy{},
102+
},
103+
backendID: "backend1",
104+
wantStrategy: "",
105+
wantMetadata: nil,
106+
description: "Should return empty when default is nil and backend not found",
107+
},
108+
{
109+
name: "handles strategy with nil metadata",
110+
config: &OutgoingAuthConfig{
111+
Default: &BackendAuthStrategy{
112+
Type: "default-strategy",
113+
Metadata: nil,
114+
},
115+
},
116+
backendID: "backend1",
117+
wantStrategy: "default-strategy",
118+
wantMetadata: nil,
119+
description: "Should handle nil metadata correctly",
120+
},
121+
{
122+
name: "handles strategy with empty metadata",
123+
config: &OutgoingAuthConfig{
124+
Default: &BackendAuthStrategy{
125+
Type: "default-strategy",
126+
Metadata: map[string]any{},
127+
},
128+
},
129+
backendID: "backend1",
130+
wantStrategy: "default-strategy",
131+
wantMetadata: map[string]any{},
132+
description: "Should return empty map when metadata is empty",
133+
},
134+
}
135+
136+
for _, tt := range tests {
137+
t.Run(tt.name, func(t *testing.T) {
138+
t.Parallel()
139+
140+
gotStrategy, gotMetadata := tt.config.ResolveForBackend(tt.backendID)
141+
142+
assert.Equal(t, tt.wantStrategy, gotStrategy, "Strategy mismatch: %s", tt.description)
143+
assert.Equal(t, tt.wantMetadata, gotMetadata, "Metadata mismatch: %s", tt.description)
144+
})
145+
}
146+
}

0 commit comments

Comments
 (0)