From 51201df4def012bd63f1cd22da6dcf156fefe627 Mon Sep 17 00:00:00 2001 From: fullsend-code <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Thu, 3 Sep 2026 19:33:06 +0000 Subject: [PATCH 1/7] feat(#4477): add caData, skipTLSVerify to OgxEntityProviderConfig Add per-provider TLS connection settings to the OGX catalog entity provider so OgxModelEntityProvider can fetch /v1/models from OGX endpoints that use a private CA or self-signed certificates. Changes: - types.ts: add optional caData and skipTLSVerify fields - module.ts: read both fields from both config paths - OgxModelEntityProvider.ts: use undici Agent dispatcher for TLS; skipTLSVerify takes precedence with warning logged - package.json: add undici dep, @backstage/config devDep - module.test.ts: new config parsing tests - OgxModelEntityProvider.test.ts: TLS test suite - app-config.yaml: commented TLS examples - changeset: minor bump for ogx-entity-provider OgxAgentEntityProvider is unchanged (no outbound requests). Closes #4477 --- workspaces/boost/.changeset/ogx-tls-config.md | 5 + workspaces/boost/app-config.yaml | 5 + .../plugins/ogx-entity-provider/package.json | 6 +- .../ogx-entity-provider/src/module.test.ts | 116 ++++++++++ .../plugins/ogx-entity-provider/src/module.ts | 8 +- .../providers/OgxModelEntityProvider.test.ts | 200 ++++++++++++++++++ .../src/providers/OgxModelEntityProvider.ts | 40 +++- .../plugins/ogx-entity-provider/src/types.ts | 4 + workspaces/boost/yarn.lock | 6 +- 9 files changed, 384 insertions(+), 6 deletions(-) create mode 100644 workspaces/boost/.changeset/ogx-tls-config.md create mode 100644 workspaces/boost/plugins/ogx-entity-provider/src/module.test.ts diff --git a/workspaces/boost/.changeset/ogx-tls-config.md b/workspaces/boost/.changeset/ogx-tls-config.md new file mode 100644 index 00000000000..cf553b96dc9 --- /dev/null +++ b/workspaces/boost/.changeset/ogx-tls-config.md @@ -0,0 +1,5 @@ +--- +'@red-hat-developer-hub/backstage-plugin-ogx-entity-provider': minor +--- + +Add per-provider TLS connection settings (`caData` and `skipTLSVerify`) to `OgxEntityProviderConfig` so `OgxModelEntityProvider` can fetch `/v1/models` from OGX endpoints that use a private CA or self-signed certificates. diff --git a/workspaces/boost/app-config.yaml b/workspaces/boost/app-config.yaml index 9a2e29808e3..07c614a8df4 100644 --- a/workspaces/boost/app-config.yaml +++ b/workspaces/boost/app-config.yaml @@ -75,6 +75,11 @@ boost: baseUrl: ${BOOST_OGX_URL:-http://localhost:8321} defaultAgent: router maxAgentTurns: 10 + # caData: | + # -----BEGIN CERTIFICATE----- + # + # -----END CERTIFICATE----- + # skipTLSVerify: false # Set to true only for development agents: - id: router name: FantaCo Router diff --git a/workspaces/boost/plugins/ogx-entity-provider/package.json b/workspaces/boost/plugins/ogx-entity-provider/package.json index ce0eec3104f..a9950e88abc 100644 --- a/workspaces/boost/plugins/ogx-entity-provider/package.json +++ b/workspaces/boost/plugins/ogx-entity-provider/package.json @@ -31,11 +31,13 @@ "@backstage/backend-plugin-api": "^1.9.2", "@backstage/catalog-model": "^1.9.0", "@backstage/plugin-catalog-node": "^2.2.2", - "@red-hat-developer-hub/backstage-plugin-boost-entity-provider-sdk": "workspace:^" + "@red-hat-developer-hub/backstage-plugin-boost-entity-provider-sdk": "workspace:^", + "undici": "^6.21.1" }, "devDependencies": { "@backstage/backend-test-utils": "^1.11.4", - "@backstage/cli": "^0.36.3" + "@backstage/cli": "^0.36.3", + "@backstage/config": "^1.3.2" }, "sideEffects": false, "scripts": { diff --git a/workspaces/boost/plugins/ogx-entity-provider/src/module.test.ts b/workspaces/boost/plugins/ogx-entity-provider/src/module.test.ts new file mode 100644 index 00000000000..e44f100fa1a --- /dev/null +++ b/workspaces/boost/plugins/ogx-entity-provider/src/module.test.ts @@ -0,0 +1,116 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { ConfigReader } from '@backstage/config'; + +import { readOgxEntityProviderConfig } from './module'; + +describe('readOgxEntityProviderConfig', () => { + it('reads caData and skipTLSVerify from boost.entityProviders.ogx', () => { + const config = new ConfigReader({ + boost: { + entityProviders: { + ogx: { + baseUrl: 'https://ogx.example.com', + caData: + '-----BEGIN CERTIFICATE-----\nMIIBxTCC...\n-----END CERTIFICATE-----', + skipTLSVerify: true, + }, + }, + }, + }); + + const result = readOgxEntityProviderConfig(config); + + expect(result.baseUrl).toBe('https://ogx.example.com'); + expect(result.caData).toBe( + '-----BEGIN CERTIFICATE-----\nMIIBxTCC...\n-----END CERTIFICATE-----', + ); + expect(result.skipTLSVerify).toBe(true); + }); + + it('reads caData and skipTLSVerify from fallback boost.providers.ogx', () => { + const config = new ConfigReader({ + boost: { + providers: { + ogx: { + baseUrl: 'https://ogx-fallback.example.com', + caData: 'PEM-CERT-DATA', + skipTLSVerify: false, + }, + }, + }, + }); + + const result = readOgxEntityProviderConfig(config); + + expect(result.baseUrl).toBe('https://ogx-fallback.example.com'); + expect(result.caData).toBe('PEM-CERT-DATA'); + expect(result.skipTLSVerify).toBe(false); + }); + + it('returns undefined for caData and skipTLSVerify when not configured', () => { + const config = new ConfigReader({ + boost: { + entityProviders: { + ogx: { + baseUrl: 'http://localhost:8321', + }, + }, + }, + }); + + const result = readOgxEntityProviderConfig(config); + + expect(result.baseUrl).toBe('http://localhost:8321'); + expect(result.caData).toBeUndefined(); + expect(result.skipTLSVerify).toBeUndefined(); + }); + + it('falls back to localhost when no OGX config is present', () => { + const config = new ConfigReader({}); + + const result = readOgxEntityProviderConfig(config); + + expect(result.baseUrl).toBe('http://localhost:8321'); + expect(result.caData).toBeUndefined(); + expect(result.skipTLSVerify).toBeUndefined(); + }); + + it('prefers entityProviders.ogx over providers.ogx', () => { + const config = new ConfigReader({ + boost: { + entityProviders: { + ogx: { + baseUrl: 'https://primary.example.com', + caData: 'PRIMARY-CA', + }, + }, + providers: { + ogx: { + baseUrl: 'https://fallback.example.com', + caData: 'FALLBACK-CA', + }, + }, + }, + }); + + const result = readOgxEntityProviderConfig(config); + + expect(result.baseUrl).toBe('https://primary.example.com'); + expect(result.caData).toBe('PRIMARY-CA'); + }); +}); diff --git a/workspaces/boost/plugins/ogx-entity-provider/src/module.ts b/workspaces/boost/plugins/ogx-entity-provider/src/module.ts index 3d9bbf56ed9..73d2b921623 100644 --- a/workspaces/boost/plugins/ogx-entity-provider/src/module.ts +++ b/workspaces/boost/plugins/ogx-entity-provider/src/module.ts @@ -114,8 +114,10 @@ export const catalogModuleOgxEntityProvider = createBackendModule({ /** * Read OGX entity provider configuration from app-config.yaml. + * + * @internal Exported for testing only. */ -function readOgxEntityProviderConfig( +export function readOgxEntityProviderConfig( config: typeof coreServices.rootConfig extends { T: infer T } ? T : never, ): OgxEntityProviderConfig { // Try the entity-provider-specific config first @@ -134,6 +136,8 @@ function readOgxEntityProviderConfig( defaultAgent: epConfig.getOptionalString('defaultAgent'), maxAgentTurns: epConfig.getOptionalNumber('maxAgentTurns'), agents: readAgentConfigs(epConfig), + caData: epConfig.getOptionalString('caData'), + skipTLSVerify: epConfig.getOptionalBoolean('skipTLSVerify'), }; } @@ -147,6 +151,8 @@ function readOgxEntityProviderConfig( defaultAgent: providerConfig.getOptionalString('defaultAgent'), maxAgentTurns: providerConfig.getOptionalNumber('maxAgentTurns'), agents: readAgentConfigs(providerConfig), + caData: providerConfig.getOptionalString('caData'), + skipTLSVerify: providerConfig.getOptionalBoolean('skipTLSVerify'), }; } diff --git a/workspaces/boost/plugins/ogx-entity-provider/src/providers/OgxModelEntityProvider.test.ts b/workspaces/boost/plugins/ogx-entity-provider/src/providers/OgxModelEntityProvider.test.ts index e52d84299fa..6e57563da4c 100644 --- a/workspaces/boost/plugins/ogx-entity-provider/src/providers/OgxModelEntityProvider.test.ts +++ b/workspaces/boost/plugins/ogx-entity-provider/src/providers/OgxModelEntityProvider.test.ts @@ -27,9 +27,20 @@ import { AI_ASSET_VERSION_ANNOTATION, } from '@red-hat-developer-hub/backstage-plugin-boost-entity-provider-sdk'; +import { Agent } from 'undici'; + import { OgxModelEntityProvider } from './OgxModelEntityProvider'; import type { OgxEntityProviderConfig } from '../types'; +jest.mock('undici', () => { + const mockAgentInstance = { mocked: true }; + return { + Agent: jest.fn(() => mockAgentInstance), + }; +}); + +const MockAgent = Agent as jest.MockedClass; + const mockFetch = jest.fn() as jest.MockedFunction; global.fetch = mockFetch; @@ -62,6 +73,7 @@ describe('OgxModelEntityProvider', () => { beforeEach(() => { jest.clearAllMocks(); + MockAgent.mockClear(); taskRunner = new TaskRunnerMock(); }); @@ -246,4 +258,192 @@ describe('OgxModelEntityProvider', () => { expect(mutation.entities).toHaveLength(1); expect(mutation.entities[0].entity.spec.models.available).toEqual([]); }); + + describe('TLS configuration', () => { + it('should not create a dispatcher when neither caData nor skipTLSVerify is set', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ data: [] }), + } as Response); + + const provider = new OgxModelEntityProvider({ + config: defaultConfig, + logger: mockServices.logger.mock(), + taskRunner, + }); + + await provider.connect(mockConnection); + await taskRunner.runAll(); + + expect(MockAgent).not.toHaveBeenCalled(); + expect(mockFetch).toHaveBeenCalledWith( + 'http://localhost:8321/v1/models', + expect.not.objectContaining({ dispatcher: expect.anything() }), + ); + }); + + it('should configure HTTPS request with custom CA when caData is set', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ data: [{ id: 'model-1' }] }), + } as Response); + + const provider = new OgxModelEntityProvider({ + config: { + ...defaultConfig, + baseUrl: 'https://ogx.example.com', + caData: + '-----BEGIN CERTIFICATE-----\nTEST\n-----END CERTIFICATE-----', + }, + logger: mockServices.logger.mock(), + taskRunner, + }); + + await provider.connect(mockConnection); + await taskRunner.runAll(); + + expect(MockAgent).toHaveBeenCalledWith({ + connect: { + ca: '-----BEGIN CERTIFICATE-----\nTEST\n-----END CERTIFICATE-----', + rejectUnauthorized: true, + }, + }); + expect(mockFetch).toHaveBeenCalledWith( + 'https://ogx.example.com/v1/models', + expect.objectContaining({ + dispatcher: expect.anything(), + }), + ); + }); + + it('should disable certificate verification when skipTLSVerify is true and logs a warning', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ data: [{ id: 'model-1' }] }), + } as Response); + + const childWarn = jest.fn(); + const mockLogger = { + ...mockServices.logger.mock(), + child: jest.fn().mockReturnValue({ + info: jest.fn(), + warn: childWarn, + error: jest.fn(), + debug: jest.fn(), + child: jest.fn(), + }), + }; + + const provider = new OgxModelEntityProvider({ + config: { ...defaultConfig, skipTLSVerify: true }, + logger: mockLogger, + taskRunner, + }); + + await provider.connect(mockConnection); + await taskRunner.runAll(); + + expect(MockAgent).toHaveBeenCalledWith({ + connect: { rejectUnauthorized: false }, + }); + expect(mockFetch).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + dispatcher: expect.anything(), + }), + ); + expect(childWarn).toHaveBeenCalledWith( + expect.stringContaining('TLS certificate verification is disabled'), + ); + }); + + it('should give skipTLSVerify precedence when both fields are set', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ data: [] }), + } as Response); + + const childWarn = jest.fn(); + const mockLogger = { + ...mockServices.logger.mock(), + child: jest.fn().mockReturnValue({ + info: jest.fn(), + warn: childWarn, + error: jest.fn(), + debug: jest.fn(), + child: jest.fn(), + }), + }; + + const provider = new OgxModelEntityProvider({ + config: { + ...defaultConfig, + caData: '-----BEGIN CERTIFICATE-----\nCA\n-----END CERTIFICATE-----', + skipTLSVerify: true, + }, + logger: mockLogger, + taskRunner, + }); + + await provider.connect(mockConnection); + await taskRunner.runAll(); + + expect(MockAgent).toHaveBeenCalledWith({ + connect: { rejectUnauthorized: false }, + }); + expect(childWarn).toHaveBeenCalledWith( + expect.stringContaining('TLS certificate verification is disabled'), + ); + }); + + it('should preserve Authorization header when TLS settings are used', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ data: [] }), + } as Response); + + const provider = new OgxModelEntityProvider({ + config: { + ...defaultConfig, + apiKey: 'secret-key', + caData: 'PEM-CERT', + }, + logger: mockServices.logger.mock(), + taskRunner, + }); + + await provider.connect(mockConnection); + await taskRunner.runAll(); + + expect(mockFetch).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + headers: expect.objectContaining({ + Authorization: 'Bearer secret-key', + }), + dispatcher: expect.anything(), + }), + ); + }); + + it('should retain non-2xx error handling with TLS settings', async () => { + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 502, + } as Response); + + const provider = new OgxModelEntityProvider({ + config: { ...defaultConfig, skipTLSVerify: true }, + logger: mockServices.logger.mock(), + taskRunner, + }); + + await provider.connect(mockConnection); + await taskRunner.runAll(); + + const mutation = (mockConnection.applyMutation as jest.Mock).mock + .calls[0][0]; + expect(mutation.entities).toHaveLength(0); + }); + }); }); diff --git a/workspaces/boost/plugins/ogx-entity-provider/src/providers/OgxModelEntityProvider.ts b/workspaces/boost/plugins/ogx-entity-provider/src/providers/OgxModelEntityProvider.ts index 552499fde82..99ec5cf9b97 100644 --- a/workspaces/boost/plugins/ogx-entity-provider/src/providers/OgxModelEntityProvider.ts +++ b/workspaces/boost/plugins/ogx-entity-provider/src/providers/OgxModelEntityProvider.ts @@ -35,6 +35,8 @@ import { normalizeAIAssetVersion, } from '@red-hat-developer-hub/backstage-plugin-boost-entity-provider-sdk'; +import { Agent } from 'undici'; + import type { OgxEntityProviderConfig, OgxModelListResponse, @@ -128,7 +130,14 @@ export class OgxModelEntityProvider implements EntityProvider { headers.Authorization = `Bearer ${this.config.apiKey}`; } - const response = await fetch(url, { headers }); + const fetchOptions: RequestInit & { dispatcher?: Agent } = { headers }; + + const dispatcher = this.createTlsDispatcher(); + if (dispatcher) { + fetchOptions.dispatcher = dispatcher; + } + + const response = await fetch(url, fetchOptions); if (!response.ok) { throw new Error(`OGX API returned ${response.status} from ${url}`); @@ -148,6 +157,35 @@ export class OgxModelEntityProvider implements EntityProvider { return []; } + /** + * Create a TLS-aware Undici dispatcher when caData or skipTLSVerify is set. + * + * - skipTLSVerify takes precedence: sets rejectUnauthorized=false and logs a warning. + * - caData alone: sets the custom CA with rejectUnauthorized=true. + * - Neither: returns undefined (use default fetch behavior). + */ + private createTlsDispatcher(): Agent | undefined { + const { caData, skipTLSVerify } = this.config; + + if (!caData && !skipTLSVerify) { + return undefined; + } + + if (skipTLSVerify) { + this.logger.warn( + 'TLS certificate verification is disabled for OGX endpoint — this should only be used in development environments', + ); + return new Agent({ + connect: { rejectUnauthorized: false }, + }); + } + + // caData only — custom CA with verification enabled + return new Agent({ + connect: { ca: caData, rejectUnauthorized: true }, + }); + } + /** * Convert the OGX server + its models into a single AiModelServerAPI entity. */ diff --git a/workspaces/boost/plugins/ogx-entity-provider/src/types.ts b/workspaces/boost/plugins/ogx-entity-provider/src/types.ts index 40bf7fe38c7..c43d74814a2 100644 --- a/workspaces/boost/plugins/ogx-entity-provider/src/types.ts +++ b/workspaces/boost/plugins/ogx-entity-provider/src/types.ts @@ -112,4 +112,8 @@ export interface OgxEntityProviderConfig { maxAgentTurns?: number; /** Static agent configurations from YAML/admin config. */ agents?: OgxAgentConfig[]; + /** PEM-encoded CA certificate or certificate bundle used to verify OGX. */ + caData?: string; + /** Disable TLS certificate verification. Development use only. */ + skipTLSVerify?: boolean; } diff --git a/workspaces/boost/yarn.lock b/workspaces/boost/yarn.lock index 280051bd8a2..350fd4359aa 100644 --- a/workspaces/boost/yarn.lock +++ b/workspaces/boost/yarn.lock @@ -3064,7 +3064,7 @@ __metadata: languageName: node linkType: hard -"@backstage/config@npm:^1.3.6, @backstage/config@npm:^1.3.8": +"@backstage/config@npm:^1.3.2, @backstage/config@npm:^1.3.6, @backstage/config@npm:^1.3.8": version: 1.3.8 resolution: "@backstage/config@npm:1.3.8" dependencies: @@ -9191,8 +9191,10 @@ __metadata: "@backstage/backend-test-utils": "npm:^1.11.4" "@backstage/catalog-model": "npm:^1.9.0" "@backstage/cli": "npm:^0.36.3" + "@backstage/config": "npm:^1.3.2" "@backstage/plugin-catalog-node": "npm:^2.2.2" "@red-hat-developer-hub/backstage-plugin-boost-entity-provider-sdk": "workspace:^" + undici: "npm:^6.21.1" languageName: unknown linkType: soft @@ -30091,7 +30093,7 @@ __metadata: languageName: node linkType: hard -"undici@npm:^6.25.0": +"undici@npm:^6.21.1, undici@npm:^6.25.0": version: 6.28.0 resolution: "undici@npm:6.28.0" checksum: 10c0/3029a70df06b38b5b2f30732932a1e92544c753cd82c8abdf0d35afad48e0ba91612e79fe3a442dbbb9434d6a9eba2b714d5ea28984c903dda2b5d5444f38354 From 879903d3cac6a05fa13b6f122b582d59d4bfdd0f Mon Sep 17 00:00:00 2001 From: Gabe Montero Date: Thu, 3 Sep 2026 16:10:37 -0400 Subject: [PATCH 2/7] Update workspaces/boost/app-config.yaml --- workspaces/boost/app-config.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/workspaces/boost/app-config.yaml b/workspaces/boost/app-config.yaml index 07c614a8df4..5e2186589cd 100644 --- a/workspaces/boost/app-config.yaml +++ b/workspaces/boost/app-config.yaml @@ -75,7 +75,6 @@ boost: baseUrl: ${BOOST_OGX_URL:-http://localhost:8321} defaultAgent: router maxAgentTurns: 10 - # caData: | # -----BEGIN CERTIFICATE----- # # -----END CERTIFICATE----- From 1d2bdb4ad094a2338831174c9a12238efaee4f9b Mon Sep 17 00:00:00 2001 From: Gabe Montero Date: Thu, 3 Sep 2026 16:10:46 -0400 Subject: [PATCH 3/7] Update workspaces/boost/app-config.yaml --- workspaces/boost/app-config.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/workspaces/boost/app-config.yaml b/workspaces/boost/app-config.yaml index 5e2186589cd..0be3e14f0d4 100644 --- a/workspaces/boost/app-config.yaml +++ b/workspaces/boost/app-config.yaml @@ -75,7 +75,6 @@ boost: baseUrl: ${BOOST_OGX_URL:-http://localhost:8321} defaultAgent: router maxAgentTurns: 10 - # -----BEGIN CERTIFICATE----- # # -----END CERTIFICATE----- # skipTLSVerify: false # Set to true only for development From c8339ffe8b8e0dd288dcdaebc5fc8e4fd608895a Mon Sep 17 00:00:00 2001 From: Gabe Montero Date: Thu, 3 Sep 2026 16:10:55 -0400 Subject: [PATCH 4/7] Update workspaces/boost/app-config.yaml --- workspaces/boost/app-config.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/workspaces/boost/app-config.yaml b/workspaces/boost/app-config.yaml index 0be3e14f0d4..7b91d8d0c3f 100644 --- a/workspaces/boost/app-config.yaml +++ b/workspaces/boost/app-config.yaml @@ -75,7 +75,6 @@ boost: baseUrl: ${BOOST_OGX_URL:-http://localhost:8321} defaultAgent: router maxAgentTurns: 10 - # # -----END CERTIFICATE----- # skipTLSVerify: false # Set to true only for development agents: From 1483dd1088ebecfb34b97a2e144821417151a07e Mon Sep 17 00:00:00 2001 From: Gabe Montero Date: Thu, 3 Sep 2026 16:11:03 -0400 Subject: [PATCH 5/7] Update workspaces/boost/app-config.yaml --- workspaces/boost/app-config.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/workspaces/boost/app-config.yaml b/workspaces/boost/app-config.yaml index 7b91d8d0c3f..beeeabd5b0f 100644 --- a/workspaces/boost/app-config.yaml +++ b/workspaces/boost/app-config.yaml @@ -75,7 +75,6 @@ boost: baseUrl: ${BOOST_OGX_URL:-http://localhost:8321} defaultAgent: router maxAgentTurns: 10 - # -----END CERTIFICATE----- # skipTLSVerify: false # Set to true only for development agents: - id: router From 14be65f646f5b49a8bffc28c1feaf02cf53e0ff7 Mon Sep 17 00:00:00 2001 From: Gabe Montero Date: Thu, 3 Sep 2026 16:11:11 -0400 Subject: [PATCH 6/7] Update workspaces/boost/app-config.yaml --- workspaces/boost/app-config.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/workspaces/boost/app-config.yaml b/workspaces/boost/app-config.yaml index beeeabd5b0f..9a2e29808e3 100644 --- a/workspaces/boost/app-config.yaml +++ b/workspaces/boost/app-config.yaml @@ -75,7 +75,6 @@ boost: baseUrl: ${BOOST_OGX_URL:-http://localhost:8321} defaultAgent: router maxAgentTurns: 10 - # skipTLSVerify: false # Set to true only for development agents: - id: router name: FantaCo Router From e02480b133484fe0fd7810aaec575018307e38da Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Thu, 3 Sep 2026 20:34:18 +0000 Subject: [PATCH 7/7] fix(ogx-entity-provider): address review feedback on PR #4574 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Cache undici Agent as class field to prevent resource leak from creating a new Agent with its own connection pool on every refresh - Warning for skipTLSVerify now fires only once (on first dispatcher creation) instead of every ~60s refresh cycle - Add PEM validation for caData — logs clear error when certificate markers are missing instead of failing with opaque TLS handshake errors - Create config.d.ts with @visibility annotations for apiKey (secret) and caData (backend) to prevent exposure via /api/config endpoint - Add tests for Agent caching, single-warning, and PEM validation Addresses review feedback on #4574 --- .../plugins/ogx-entity-provider/config.d.ts | 80 +++++++++ .../plugins/ogx-entity-provider/package.json | 1 + .../providers/OgxModelEntityProvider.test.ts | 159 +++++++++++++++++- .../src/providers/OgxModelEntityProvider.ts | 51 +++++- 4 files changed, 277 insertions(+), 14 deletions(-) create mode 100644 workspaces/boost/plugins/ogx-entity-provider/config.d.ts diff --git a/workspaces/boost/plugins/ogx-entity-provider/config.d.ts b/workspaces/boost/plugins/ogx-entity-provider/config.d.ts new file mode 100644 index 00000000000..3e3fb03c0ed --- /dev/null +++ b/workspaces/boost/plugins/ogx-entity-provider/config.d.ts @@ -0,0 +1,80 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Configuration schema for the OGX entity provider module. + * + * Declares the config paths read by readOgxEntityProviderConfig so that + * Backstage validates and enforces visibility on these keys even if + * the module is loaded independently of boost-backend. + */ +export interface Config { + boost?: { + /** Entity-provider-specific config (standalone deployment). */ + entityProviders?: { + /** OGX entity provider connection. */ + ogx?: { + /** + * Base URL of the OGX API endpoint. + * @configScope yaml-only + */ + baseUrl?: string; + /** + * API key for authenticated endpoints. + * @visibility secret + */ + apiKey?: string; + /** + * PEM-encoded CA certificate or certificate bundle used to verify the OGX endpoint. + * @visibility backend + */ + caData?: string; + /** + * Disable TLS certificate verification. Development use only. + * @configScope yaml-only + */ + skipTLSVerify?: boolean; + }; + }; + + /** Provider module config (composed deployment). */ + providers?: { + /** OGX provider connection. */ + ogx?: { + /** + * Base URL of the OGX API endpoint. + * @configScope yaml-only + */ + baseUrl?: string; + /** + * API key for authenticated endpoints. + * @visibility secret + */ + apiKey?: string; + /** + * PEM-encoded CA certificate or certificate bundle used to verify the OGX endpoint. + * @visibility backend + */ + caData?: string; + /** + * Disable TLS certificate verification. Development use only. + * @configScope yaml-only + */ + skipTLSVerify?: boolean; + }; + }; + }; +} diff --git a/workspaces/boost/plugins/ogx-entity-provider/package.json b/workspaces/boost/plugins/ogx-entity-provider/package.json index a9950e88abc..08bc9e740e2 100644 --- a/workspaces/boost/plugins/ogx-entity-provider/package.json +++ b/workspaces/boost/plugins/ogx-entity-provider/package.json @@ -2,6 +2,7 @@ "name": "@red-hat-developer-hub/backstage-plugin-ogx-entity-provider", "version": "0.4.0", "license": "Apache-2.0", + "configSchema": "config.d.ts", "description": "OGX entity provider for the Backstage catalog — emits AI models and agents as catalog entities", "main": "src/index.ts", "types": "src/index.ts", diff --git a/workspaces/boost/plugins/ogx-entity-provider/src/providers/OgxModelEntityProvider.test.ts b/workspaces/boost/plugins/ogx-entity-provider/src/providers/OgxModelEntityProvider.test.ts index 6e57563da4c..1fef0a1c126 100644 --- a/workspaces/boost/plugins/ogx-entity-provider/src/providers/OgxModelEntityProvider.test.ts +++ b/workspaces/boost/plugins/ogx-entity-provider/src/providers/OgxModelEntityProvider.test.ts @@ -32,12 +32,9 @@ import { Agent } from 'undici'; import { OgxModelEntityProvider } from './OgxModelEntityProvider'; import type { OgxEntityProviderConfig } from '../types'; -jest.mock('undici', () => { - const mockAgentInstance = { mocked: true }; - return { - Agent: jest.fn(() => mockAgentInstance), - }; -}); +jest.mock('undici', () => ({ + Agent: jest.fn(() => ({ mocked: true })), +})); const MockAgent = Agent as jest.MockedClass; @@ -445,5 +442,155 @@ describe('OgxModelEntityProvider', () => { .calls[0][0]; expect(mutation.entities).toHaveLength(0); }); + + it('should reuse the cached Agent across multiple refresh cycles', async () => { + mockFetch + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ data: [{ id: 'model-1' }] }), + } as Response) + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ data: [{ id: 'model-2' }] }), + } as Response); + + const provider = new OgxModelEntityProvider({ + config: { + ...defaultConfig, + caData: + '-----BEGIN CERTIFICATE-----\nTEST\n-----END CERTIFICATE-----', + }, + logger: mockServices.logger.mock(), + taskRunner, + }); + + await provider.connect(mockConnection); + await taskRunner.runAll(); + + // First refresh creates the Agent + expect(MockAgent).toHaveBeenCalledTimes(1); + + // Simulate a second refresh cycle + await provider.run(); + + // Agent is reused — still only one instance created + expect(MockAgent).toHaveBeenCalledTimes(1); + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('should log skipTLSVerify warning only once across multiple refresh cycles', async () => { + mockFetch + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ data: [] }), + } as Response) + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ data: [] }), + } as Response); + + const childWarn = jest.fn(); + const mockLogger = { + ...mockServices.logger.mock(), + child: jest.fn().mockReturnValue({ + info: jest.fn(), + warn: childWarn, + error: jest.fn(), + debug: jest.fn(), + child: jest.fn(), + }), + }; + + const provider = new OgxModelEntityProvider({ + config: { ...defaultConfig, skipTLSVerify: true }, + logger: mockLogger, + taskRunner, + }); + + await provider.connect(mockConnection); + await taskRunner.runAll(); + + // Second refresh + await provider.run(); + + // Warning emitted only once despite two refresh cycles + const tlsWarnings = childWarn.mock.calls.filter((call: string[]) => + call[0].includes('TLS certificate verification is disabled'), + ); + expect(tlsWarnings).toHaveLength(1); + }); + + it('should log an error when caData is not valid PEM', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ data: [] }), + } as Response); + + const childError = jest.fn(); + const mockLogger = { + ...mockServices.logger.mock(), + child: jest.fn().mockReturnValue({ + info: jest.fn(), + warn: jest.fn(), + error: childError, + debug: jest.fn(), + child: jest.fn(), + }), + }; + + const provider = new OgxModelEntityProvider({ + config: { + ...defaultConfig, + caData: 'not-a-valid-pem-string', + }, + logger: mockLogger, + taskRunner, + }); + + await provider.connect(mockConnection); + await taskRunner.runAll(); + + expect(childError).toHaveBeenCalledWith( + expect.stringContaining( + 'does not contain valid PEM certificate markers', + ), + ); + // Agent is still created — invalid PEM is a warning, not a hard stop + expect(MockAgent).toHaveBeenCalledTimes(1); + }); + + it('should not log PEM error when caData has valid PEM markers', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ data: [] }), + } as Response); + + const childError = jest.fn(); + const mockLogger = { + ...mockServices.logger.mock(), + child: jest.fn().mockReturnValue({ + info: jest.fn(), + warn: jest.fn(), + error: childError, + debug: jest.fn(), + child: jest.fn(), + }), + }; + + const provider = new OgxModelEntityProvider({ + config: { + ...defaultConfig, + caData: + '-----BEGIN CERTIFICATE-----\nMIIBxTCC...\n-----END CERTIFICATE-----', + }, + logger: mockLogger, + taskRunner, + }); + + await provider.connect(mockConnection); + await taskRunner.runAll(); + + expect(childError).not.toHaveBeenCalled(); + }); }); }); diff --git a/workspaces/boost/plugins/ogx-entity-provider/src/providers/OgxModelEntityProvider.ts b/workspaces/boost/plugins/ogx-entity-provider/src/providers/OgxModelEntityProvider.ts index 99ec5cf9b97..a062300b5bc 100644 --- a/workspaces/boost/plugins/ogx-entity-provider/src/providers/OgxModelEntityProvider.ts +++ b/workspaces/boost/plugins/ogx-entity-provider/src/providers/OgxModelEntityProvider.ts @@ -63,6 +63,8 @@ export class OgxModelEntityProvider implements EntityProvider { private readonly scheduleFn: () => Promise; private connection?: EntityProviderConnection; private cachedEntity: Entity | undefined; + /** Lazily-initialized TLS dispatcher — created once and reused across refresh cycles. */ + private cachedTlsDispatcher: Agent | null | undefined; constructor(options: { config: OgxEntityProviderConfig; @@ -132,7 +134,7 @@ export class OgxModelEntityProvider implements EntityProvider { const fetchOptions: RequestInit & { dispatcher?: Agent } = { headers }; - const dispatcher = this.createTlsDispatcher(); + const dispatcher = this.getTlsDispatcher(); if (dispatcher) { fetchOptions.dispatcher = dispatcher; } @@ -158,16 +160,25 @@ export class OgxModelEntityProvider implements EntityProvider { } /** - * Create a TLS-aware Undici dispatcher when caData or skipTLSVerify is set. + * Return the cached TLS dispatcher, creating it on first call. * - * - skipTLSVerify takes precedence: sets rejectUnauthorized=false and logs a warning. - * - caData alone: sets the custom CA with rejectUnauthorized=true. + * The dispatcher is created once and reused across refresh cycles to avoid + * accumulating orphaned Agents with their own connection pools. + * + * - skipTLSVerify takes precedence: sets rejectUnauthorized=false and logs a warning (once). + * - caData alone: validates PEM markers, then sets the custom CA with rejectUnauthorized=true. * - Neither: returns undefined (use default fetch behavior). */ - private createTlsDispatcher(): Agent | undefined { + private getTlsDispatcher(): Agent | undefined { + // undefined = not yet initialized; null = initialized, no TLS needed + if (this.cachedTlsDispatcher !== undefined) { + return this.cachedTlsDispatcher ?? undefined; + } + const { caData, skipTLSVerify } = this.config; if (!caData && !skipTLSVerify) { + this.cachedTlsDispatcher = null; return undefined; } @@ -175,15 +186,23 @@ export class OgxModelEntityProvider implements EntityProvider { this.logger.warn( 'TLS certificate verification is disabled for OGX endpoint — this should only be used in development environments', ); - return new Agent({ + this.cachedTlsDispatcher = new Agent({ connect: { rejectUnauthorized: false }, }); + return this.cachedTlsDispatcher; + } + + // caData only — validate PEM markers before passing to undici + if (caData && !isValidPem(caData)) { + this.logger.error( + 'caData does not contain valid PEM certificate markers (expected -----BEGIN CERTIFICATE----- / -----END CERTIFICATE-----) — TLS connections to the OGX endpoint may fail', + ); } - // caData only — custom CA with verification enabled - return new Agent({ + this.cachedTlsDispatcher = new Agent({ connect: { ca: caData, rejectUnauthorized: true }, }); + return this.cachedTlsDispatcher; } /** @@ -250,3 +269,19 @@ export class OgxModelEntityProvider implements EntityProvider { }; } } + +const PEM_HEADER = '-----BEGIN CERTIFICATE-----'; +const PEM_FOOTER = '-----END CERTIFICATE-----'; + +/** + * Check whether a string contains at least one complete PEM certificate block. + * Mirrors the logic in boost-connector-utils/src/ca-bundle.ts. + */ +function isValidPem(content: string): boolean { + if (!content.includes(PEM_HEADER) || !content.includes(PEM_FOOTER)) { + return false; + } + const beginCount = content.split(PEM_HEADER).length - 1; + const endCount = content.split(PEM_FOOTER).length - 1; + return beginCount > 0 && beginCount === endCount; +}