Skip to content

Commit 8d7ff32

Browse files
ChrisJBurnsclaude
andcommitted
Thread factory through buildFullAuthzConfigJSON, drop redundant lookups
buildFullAuthzConfigJSON now returns the resolved AuthorizerFactory alongside the envelope JSON, so validateAuthzConfig can dispatch ValidateConfig directly without a second GetFactory call and without re-Unmarshalling the JSON it just built. Before: Validate() ran IsRegistered, buildFullAuthzConfigJSON ran GetFactory, validateAuthzConfig re-Unmarshalled the envelope and ran GetFactory again — three near-identical lookups for the same key on every reconcile, with three slightly different error messages. The authzConfigEnvelope struct is removed; nothing else consumed it. Tests updated to assert the returned factory's ConfigKey matches the expected nested envelope key, locking in the bidirectional contract. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent e6a8e16 commit 8d7ff32

2 files changed

Lines changed: 26 additions & 39 deletions

File tree

cmd/thv-operator/controllers/mcpauthzconfig_controller.go

Lines changed: 19 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -183,71 +183,53 @@ func (*MCPAuthzConfigReconciler) validateAuthzConfig(authzConfig *mcpv1beta1.MCP
183183
return err
184184
}
185185

186-
fullConfigJSON, err := buildFullAuthzConfigJSON(authzConfig.Spec)
186+
// buildFullAuthzConfigJSON returns the registered factory alongside the
187+
// envelope, so we don't have to re-Unmarshal the JSON or re-look-up the
188+
// factory just to dispatch ValidateConfig.
189+
fullConfigJSON, factory, err := buildFullAuthzConfigJSON(authzConfig.Spec)
187190
if err != nil {
188191
return err
189192
}
190-
191-
// Parse and validate via the authorizer factory
192-
var cfg authzConfigEnvelope
193-
if err := json.Unmarshal(fullConfigJSON, &cfg); err != nil {
194-
return fmt.Errorf("failed to parse reconstructed authz config: %w", err)
195-
}
196-
if cfg.Version == "" || cfg.Type == "" {
197-
return fmt.Errorf("reconstructed config missing version or type")
198-
}
199-
200-
factory := authorizers.GetFactory(cfg.Type)
201-
if factory == nil {
202-
return fmt.Errorf("unsupported authorizer type: %s (registered types: %v)",
203-
cfg.Type, authorizers.RegisteredTypes())
204-
}
205-
206193
return factory.ValidateConfig(fullConfigJSON)
207194
}
208195

209-
// authzConfigEnvelope is a minimal struct for extracting version and type from reconstructed JSON.
210-
type authzConfigEnvelope struct {
211-
Version string `json:"version"`
212-
Type string `json:"type"`
213-
}
214-
215196
// buildFullAuthzConfigJSON reconstructs the full authorizer config JSON from a
216-
// MCPAuthzConfig spec. The result is the same format accepted by authorizers.Config
217-
// and used in ConfigMaps: {"version": "1.0", "type": "<type>", "<configKey>": {<config>}}.
218-
func buildFullAuthzConfigJSON(spec mcpv1beta1.MCPAuthzConfigSpec) ([]byte, error) {
197+
// MCPAuthzConfig spec and returns it alongside the resolved factory. The JSON
198+
// shape is the same one accepted by authorizers.Config and stored in
199+
// ConfigMaps: {"version": "1.0", "type": "<type>", "<configKey>": {<config>}}.
200+
// Returning the factory together with the JSON lets callers (notably
201+
// validateAuthzConfig) skip a second registry lookup.
202+
func buildFullAuthzConfigJSON(spec mcpv1beta1.MCPAuthzConfigSpec) ([]byte, authorizers.AuthorizerFactory, error) {
219203
factory := authorizers.GetFactory(spec.Type)
220204
if factory == nil {
221-
return nil, fmt.Errorf("unsupported authorizer type: %s (registered types: %v)",
205+
return nil, nil, fmt.Errorf("unsupported authorizer type: %s (registered types: %v)",
222206
spec.Type, authorizers.RegisteredTypes())
223207
}
224208

225-
configKey := factory.ConfigKey()
226-
227209
if len(spec.Config.Raw) == 0 {
228-
return nil, fmt.Errorf("config field is empty")
210+
return nil, nil, fmt.Errorf("config field is empty")
229211
}
230212

231213
versionJSON, err := marshalJSONString(authzConfigVersion)
232214
if err != nil {
233-
return nil, fmt.Errorf("failed to marshal version: %w", err)
215+
return nil, nil, fmt.Errorf("failed to marshal version: %w", err)
234216
}
235217
typeJSON, err := marshalJSONString(spec.Type)
236218
if err != nil {
237-
return nil, fmt.Errorf("failed to marshal type: %w", err)
219+
return nil, nil, fmt.Errorf("failed to marshal type: %w", err)
238220
}
239221

240222
fullConfig := map[string]json.RawMessage{
241-
"version": versionJSON,
242-
"type": typeJSON,
243-
configKey: spec.Config.Raw,
223+
"version": versionJSON,
224+
"type": typeJSON,
225+
factory.ConfigKey(): spec.Config.Raw,
244226
}
245227

246228
result, err := json.Marshal(fullConfig)
247229
if err != nil {
248-
return nil, fmt.Errorf("failed to marshal full authz config: %w", err)
230+
return nil, nil, fmt.Errorf("failed to marshal full authz config: %w", err)
249231
}
250-
return result, nil
232+
return result, factory, nil
251233
}
252234

253235
// marshalJSONString marshals a string value to JSON, returning an error instead of panicking.

cmd/thv-operator/controllers/mcpauthzconfig_controller_test.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,14 +101,18 @@ func Test_buildFullAuthzConfigJSON(t *testing.T) {
101101
t.Run(tt.name, func(t *testing.T) {
102102
t.Parallel()
103103

104-
result, err := buildFullAuthzConfigJSON(tt.spec)
104+
result, factory, err := buildFullAuthzConfigJSON(tt.spec)
105105

106106
if tt.expectError {
107107
require.Error(t, err)
108+
assert.Nil(t, factory, "factory must be nil when buildFullAuthzConfigJSON errors")
108109
return
109110
}
110111

111112
require.NoError(t, err)
113+
require.NotNil(t, factory, "factory must accompany a successful build")
114+
assert.Equal(t, tt.expectKey, factory.ConfigKey(),
115+
"returned factory's ConfigKey must match the nested envelope key")
112116

113117
var parsed map[string]json.RawMessage
114118
require.NoError(t, json.Unmarshal(result, &parsed), "Output must be valid JSON")
@@ -135,9 +139,10 @@ func Test_buildFullAuthzConfigJSON_EmptyConfigRaw(t *testing.T) {
135139
Config: runtime.RawExtension{Raw: []byte{}},
136140
}
137141

138-
result, err := buildFullAuthzConfigJSON(spec)
142+
result, factory, err := buildFullAuthzConfigJSON(spec)
139143
require.Error(t, err)
140144
assert.Nil(t, result)
145+
assert.Nil(t, factory, "factory must be nil when buildFullAuthzConfigJSON errors")
141146
assert.Contains(t, err.Error(), "config field is empty")
142147
}
143148

0 commit comments

Comments
 (0)