Skip to content

Commit 5831ddb

Browse files
Copilotlpcoxgithub-advanced-security[bot]
authored
Centralize provider adapter assembly with buildProviderAdapter and enforce isEnabled contract (#5205)
* Initial plan * Add buildProviderAdapter helper; refactor all four provider adapters to use it * Potential fix for pull request finding 'CodeQL / Unused variable, import, function or class' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * fix(api-proxy): enforce isEnabled in provider adapter builder --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Landon Cox <landon.cox@microsoft.com> Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
1 parent fa7e87a commit 5831ddb

7 files changed

Lines changed: 428 additions & 125 deletions

File tree

containers/api-proxy/adapter-factory.js

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,71 @@ function createAdapterMethods(opts) {
151151
};
152152
}
153153

154+
/**
155+
* Assemble a provider adapter object from its constituent parts.
156+
*
157+
* Every provider adapter returns the same outer object shape:
158+
* { name, port, isManagementPort, alwaysBind, getAuthHeaders, getBodyTransform,
159+
* ...adapterMethods, getUnconfiguredResponse?, getUnconfiguredHealthResponse? }
160+
*
161+
* This helper centralises that boilerplate so each adapter only needs to supply
162+
* its provider-specific values and callbacks. Provider-specific logic (auth
163+
* header construction, OIDC plumbing, body transforms, introspection fields, etc.)
164+
* is passed in via the `getAuthHeaders`, `bodyTransform`, and `extra` parameters.
165+
*
166+
* @param {object} opts
167+
* @param {string} opts.name - Unique provider slug (e.g. 'openai')
168+
* @param {number} opts.port - Port the adapter listens on
169+
* @param {boolean} [opts.isManagementPort=false] - True only for port 10000 (OpenAI)
170+
* @param {boolean} [opts.alwaysBind=true] - Start a stub server even when isEnabled() is false
171+
* @param {object} opts.adapterMethods - Result of createAdapterMethods()
172+
* @param {(req?: import('http').IncomingMessage) => Record<string,string>} opts.getAuthHeaders - Auth header factory
173+
* @param {((body: Buffer) => Buffer|null)|null} [opts.bodyTransform=null] - Body transform wrapped into getBodyTransform()
174+
* @param {(() => boolean)} [opts.isEnabled] - Optional isEnabled override (must be provided either here or via `extra`)
175+
* @param {((url: string) => string)} [opts.transformRequestUrl] - Optional URL transformer
176+
* @param {(() => import('./providers/index').UnconfiguredResponse)} [opts.getUnconfiguredResponse] - Optional not-configured response
177+
* @param {(() => import('./providers/index').UnconfiguredResponse)} [opts.getUnconfiguredHealthResponse] - Optional not-configured /health response
178+
* @param {object} [opts.extra={}] - Extra fields spread into the returned object last (e.g. OIDC runtime methods, introspection fields, overrides)
179+
* @returns {import('./providers/index').ProviderAdapter}
180+
*/
181+
function buildProviderAdapter({
182+
name,
183+
port,
184+
isManagementPort = false,
185+
alwaysBind = true,
186+
adapterMethods,
187+
getAuthHeaders,
188+
bodyTransform = null,
189+
isEnabled,
190+
transformRequestUrl,
191+
getUnconfiguredResponse,
192+
getUnconfiguredHealthResponse,
193+
extra = {},
194+
}) {
195+
const adapter = {
196+
name,
197+
port,
198+
isManagementPort,
199+
alwaysBind,
200+
...(isEnabled !== undefined ? { isEnabled } : {}),
201+
getAuthHeaders,
202+
...(transformRequestUrl !== undefined ? { transformRequestUrl } : {}),
203+
getBodyTransform() { return bodyTransform; },
204+
...adapterMethods,
205+
...(getUnconfiguredResponse !== undefined ? { getUnconfiguredResponse } : {}),
206+
...(getUnconfiguredHealthResponse !== undefined ? { getUnconfiguredHealthResponse } : {}),
207+
...extra,
208+
};
209+
210+
if (typeof adapter.isEnabled !== 'function') {
211+
throw new TypeError(`Provider adapter "${name}" must define an isEnabled() function`);
212+
}
213+
214+
return adapter;
215+
}
216+
154217
module.exports = {
155218
createBaseAdapterConfig,
156219
createAdapterMethods,
220+
buildProviderAdapter,
157221
};
Lines changed: 287 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,287 @@
1+
'use strict';
2+
3+
const { buildProviderAdapter } = require('./adapter-factory');
4+
5+
describe('buildProviderAdapter', () => {
6+
function makeAdapterMethods(overrides = {}) {
7+
return {
8+
getTargetHost() { return 'api.example.com'; },
9+
getBasePath() { return ''; },
10+
participatesInValidation: true,
11+
getValidationProbe() { return null; },
12+
getModelsFetchConfig() { return null; },
13+
getReflectionInfo() { return { provider: 'test', port: 10099 }; },
14+
...overrides,
15+
};
16+
}
17+
18+
describe('required fields', () => {
19+
it('sets name, port, isManagementPort, alwaysBind from opts', () => {
20+
const adapter = buildProviderAdapter({
21+
name: 'test',
22+
port: 10099,
23+
isManagementPort: true,
24+
alwaysBind: false,
25+
adapterMethods: makeAdapterMethods(),
26+
getAuthHeaders() { return {}; },
27+
isEnabled() { return true; },
28+
});
29+
expect(adapter.name).toBe('test');
30+
expect(adapter.port).toBe(10099);
31+
expect(adapter.isManagementPort).toBe(true);
32+
expect(adapter.alwaysBind).toBe(false);
33+
});
34+
35+
it('defaults isManagementPort to false and alwaysBind to true', () => {
36+
const adapter = buildProviderAdapter({
37+
name: 'test',
38+
port: 10099,
39+
adapterMethods: makeAdapterMethods(),
40+
getAuthHeaders() { return {}; },
41+
isEnabled() { return true; },
42+
});
43+
expect(adapter.isManagementPort).toBe(false);
44+
expect(adapter.alwaysBind).toBe(true);
45+
});
46+
47+
it('includes getAuthHeaders from opts', () => {
48+
const getAuthHeaders = () => ({ 'x-test-key': 'val' });
49+
const adapter = buildProviderAdapter({
50+
name: 'test',
51+
port: 10099,
52+
adapterMethods: makeAdapterMethods(),
53+
getAuthHeaders,
54+
isEnabled() { return true; },
55+
});
56+
expect(adapter.getAuthHeaders).toBe(getAuthHeaders);
57+
expect(adapter.getAuthHeaders()).toEqual({ 'x-test-key': 'val' });
58+
});
59+
60+
it('spreads all adapterMethods into the returned object', () => {
61+
const adapterMethods = makeAdapterMethods({ participatesInValidation: false });
62+
const adapter = buildProviderAdapter({
63+
name: 'test',
64+
port: 10099,
65+
adapterMethods,
66+
getAuthHeaders() { return {}; },
67+
isEnabled() { return true; },
68+
});
69+
expect(adapter.getTargetHost).toBe(adapterMethods.getTargetHost);
70+
expect(adapter.getBasePath).toBe(adapterMethods.getBasePath);
71+
expect(adapter.participatesInValidation).toBe(false);
72+
expect(adapter.getValidationProbe).toBe(adapterMethods.getValidationProbe);
73+
expect(adapter.getModelsFetchConfig).toBe(adapterMethods.getModelsFetchConfig);
74+
expect(adapter.getReflectionInfo).toBe(adapterMethods.getReflectionInfo);
75+
});
76+
});
77+
78+
describe('getBodyTransform', () => {
79+
it('wraps bodyTransform in getBodyTransform()', () => {
80+
const transform = (body) => body;
81+
const adapter = buildProviderAdapter({
82+
name: 'test',
83+
port: 10099,
84+
adapterMethods: makeAdapterMethods(),
85+
getAuthHeaders() { return {}; },
86+
isEnabled() { return true; },
87+
bodyTransform: transform,
88+
});
89+
expect(adapter.getBodyTransform()).toBe(transform);
90+
});
91+
92+
it('defaults bodyTransform to null', () => {
93+
const adapter = buildProviderAdapter({
94+
name: 'test',
95+
port: 10099,
96+
adapterMethods: makeAdapterMethods(),
97+
getAuthHeaders() { return {}; },
98+
isEnabled() { return true; },
99+
});
100+
expect(adapter.getBodyTransform()).toBeNull();
101+
});
102+
});
103+
104+
describe('optional methods', () => {
105+
it('throws when isEnabled is not provided', () => {
106+
expect(() => buildProviderAdapter({
107+
name: 'test',
108+
port: 10099,
109+
adapterMethods: makeAdapterMethods(),
110+
getAuthHeaders() { return {}; },
111+
})).toThrow('must define an isEnabled() function');
112+
});
113+
114+
it('includes isEnabled when provided', () => {
115+
const isEnabled = () => true;
116+
const adapter = buildProviderAdapter({
117+
name: 'test',
118+
port: 10099,
119+
adapterMethods: makeAdapterMethods(),
120+
getAuthHeaders() { return {}; },
121+
isEnabled,
122+
});
123+
expect(adapter.isEnabled).toBe(isEnabled);
124+
expect(adapter.isEnabled()).toBe(true);
125+
});
126+
127+
it('accepts isEnabled provided via extra', () => {
128+
const adapter = buildProviderAdapter({
129+
name: 'test',
130+
port: 10099,
131+
adapterMethods: makeAdapterMethods(),
132+
getAuthHeaders() { return {}; },
133+
extra: {
134+
isEnabled() { return true; },
135+
},
136+
});
137+
expect(adapter.isEnabled()).toBe(true);
138+
});
139+
140+
it('omits transformRequestUrl when not provided', () => {
141+
const adapter = buildProviderAdapter({
142+
name: 'test',
143+
port: 10099,
144+
adapterMethods: makeAdapterMethods(),
145+
getAuthHeaders() { return {}; },
146+
isEnabled() { return true; },
147+
});
148+
expect('transformRequestUrl' in adapter).toBe(false);
149+
});
150+
151+
it('includes transformRequestUrl when provided', () => {
152+
const transformRequestUrl = (url) => url + '?transformed=1';
153+
const adapter = buildProviderAdapter({
154+
name: 'test',
155+
port: 10099,
156+
adapterMethods: makeAdapterMethods(),
157+
getAuthHeaders() { return {}; },
158+
isEnabled() { return true; },
159+
transformRequestUrl,
160+
});
161+
expect(adapter.transformRequestUrl).toBe(transformRequestUrl);
162+
});
163+
164+
it('omits getUnconfiguredResponse when not provided', () => {
165+
const adapter = buildProviderAdapter({
166+
name: 'test',
167+
port: 10099,
168+
adapterMethods: makeAdapterMethods(),
169+
getAuthHeaders() { return {}; },
170+
isEnabled() { return true; },
171+
});
172+
expect('getUnconfiguredResponse' in adapter).toBe(false);
173+
});
174+
175+
it('includes getUnconfiguredResponse when provided', () => {
176+
const getUnconfiguredResponse = () => ({ statusCode: 503, body: { error: 'not configured' } });
177+
const adapter = buildProviderAdapter({
178+
name: 'test',
179+
port: 10099,
180+
adapterMethods: makeAdapterMethods(),
181+
getAuthHeaders() { return {}; },
182+
isEnabled() { return true; },
183+
getUnconfiguredResponse,
184+
});
185+
expect(adapter.getUnconfiguredResponse).toBe(getUnconfiguredResponse);
186+
});
187+
188+
it('omits getUnconfiguredHealthResponse when not provided', () => {
189+
const adapter = buildProviderAdapter({
190+
name: 'test',
191+
port: 10099,
192+
adapterMethods: makeAdapterMethods(),
193+
getAuthHeaders() { return {}; },
194+
isEnabled() { return true; },
195+
});
196+
expect('getUnconfiguredHealthResponse' in adapter).toBe(false);
197+
});
198+
199+
it('includes getUnconfiguredHealthResponse when provided', () => {
200+
const getUnconfiguredHealthResponse = () => ({ statusCode: 503, body: { status: 'down' } });
201+
const adapter = buildProviderAdapter({
202+
name: 'test',
203+
port: 10099,
204+
adapterMethods: makeAdapterMethods(),
205+
getAuthHeaders() { return {}; },
206+
isEnabled() { return true; },
207+
getUnconfiguredHealthResponse,
208+
});
209+
expect(adapter.getUnconfiguredHealthResponse).toBe(getUnconfiguredHealthResponse);
210+
});
211+
});
212+
213+
describe('extra fields', () => {
214+
it('spreads extra fields into the returned object after adapterMethods', () => {
215+
const adapter = buildProviderAdapter({
216+
name: 'test',
217+
port: 10099,
218+
adapterMethods: makeAdapterMethods({ participatesInValidation: false }),
219+
getAuthHeaders() { return {}; },
220+
isEnabled() { return true; },
221+
extra: {
222+
participatesInValidation: true, // override from adapterMethods
223+
_customField: 'hello',
224+
getOidcProvider() { return null; },
225+
},
226+
});
227+
expect(adapter.participatesInValidation).toBe(true);
228+
expect(adapter._customField).toBe('hello');
229+
expect(adapter.getOidcProvider()).toBeNull();
230+
});
231+
232+
it('defaults extra to empty object when not provided', () => {
233+
expect(() => buildProviderAdapter({
234+
name: 'test',
235+
port: 10099,
236+
adapterMethods: makeAdapterMethods(),
237+
getAuthHeaders() { return {}; },
238+
isEnabled() { return true; },
239+
})).not.toThrow();
240+
});
241+
});
242+
243+
describe('integration with createAdapterMethods', () => {
244+
it('correctly wires up a complete minimal adapter', () => {
245+
const { createBaseAdapterConfig, createAdapterMethods } = require('./adapter-factory');
246+
const env = { MY_API_KEY: 'test-key' };
247+
const { apiKey, rawTarget, basePath } = createBaseAdapterConfig(env, {
248+
keyEnvVar: 'MY_API_KEY',
249+
targetEnvVar: 'MY_API_TARGET',
250+
basePathEnvVar: 'MY_API_BASE_PATH',
251+
defaultTarget: 'api.example.com',
252+
});
253+
const adapterMethods = createAdapterMethods({
254+
apiKey,
255+
rawTarget,
256+
basePath,
257+
provider: 'test',
258+
port: 10099,
259+
modelsPath: '/v1/models',
260+
validationPath: '/v1/models',
261+
validationHeaders: () => ({ 'Authorization': '******' }),
262+
});
263+
264+
const adapter = buildProviderAdapter({
265+
name: 'test',
266+
port: 10099,
267+
adapterMethods,
268+
getAuthHeaders() { return { 'Authorization': '******' }; },
269+
isEnabled() { return !!apiKey; },
270+
getUnconfiguredResponse() {
271+
return { statusCode: 503, body: { error: 'not configured' } };
272+
},
273+
});
274+
275+
expect(adapter.name).toBe('test');
276+
expect(adapter.port).toBe(10099);
277+
expect(adapter.isManagementPort).toBe(false);
278+
expect(adapter.alwaysBind).toBe(true);
279+
expect(adapter.isEnabled()).toBe(true);
280+
expect(adapter.getAuthHeaders()).toEqual({ 'Authorization': '******' });
281+
expect(adapter.getBodyTransform()).toBeNull();
282+
expect(adapter.getTargetHost()).toBe('api.example.com');
283+
expect(adapter.getBasePath()).toBe('');
284+
expect(adapter.getUnconfiguredResponse()).toEqual({ statusCode: 503, body: { error: 'not configured' } });
285+
});
286+
});
287+
});

0 commit comments

Comments
 (0)