feat(#4477): add caData, skipTLSVerify to OgxEntityProviderConfig - #4574
feat(#4477): add caData, skipTLSVerify to OgxEntityProviderConfig#4574fullsend-ai-coder[bot] wants to merge 7 commits into
Conversation
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
|
Important This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior. Changed Packages
|
|
🤖 Finished Review · ✅ Success · Started 7:38 PM UTC · Completed 7:51 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Cost: $5.53 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4574 +/- ##
==========================================
+ Coverage 62.36% 62.39% +0.02%
==========================================
Files 2607 2607
Lines 104739 104772 +33
Branches 29433 29441 +8
==========================================
+ Hits 65325 65374 +49
+ Misses 37584 37568 -16
Partials 1830 1830
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
ReviewFindingsMedium
Low
Labels: PR adds new TLS configuration feature to the OGX entity provider in the boost workspace. Previous runReview — commentAdds per-provider TLS connection settings ( Four medium-severity findings are worth discussing but none individually block the change. Findings1. ⚠ Medium — Naming inconsistency:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
requires-manual-review
|
🤖 Review · Commit: |
|
🤖 Review · ❌ Terminated · Started 8:13 PM UTC · Ended 8:30 PM UTC Commit: |
|
/fs-fix address all the medium findings from #4574 (comment) ... with the first finding, keep a global cache for the created Agent |
|
🤖 Fix · ❌ Terminated · Started 8:21 PM UTC · Ended 8:39 PM UTC Commit: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
removed
the
requires-manual-review
fullsend-ai-review
Bot
added
the
requires-manual-review
|
🤖 Finished Review · ✅ Success · Started 8:13 PM UTC · Completed 8:30 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Cost: $6.03 |
- 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
|
🤖 Finished Fix · ✅ Success · Started 8:21 PM UTC · Completed 8:39 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Cost: $2.78 |
|
🤖 Review · ❌ Terminated · Started 8:40 PM UTC · Ended 8:59 PM UTC Commit: |
|
| @@ -212,3 +269,19 @@ export class OgxModelEntityProvider implements EntityProvider { | |||
| }; | |||
There was a problem hiding this comment.
[medium] code-duplication
isValidPem() function and PEM_HEADER/PEM_FOOTER constants duplicate logic from boost-connector-utils/src/ca-bundle.ts. The source function has a different signature (Buffer vs string) and is private, so direct import is not possible without refactoring.
Suggested fix: Either extract a shared string-based isValidPem into boost-connector-utils, or expand the comment to document this as an intentional standalone copy for independent deployment isolation.
| @@ -35,6 +35,8 @@ import { | |||
| normalizeAIAssetVersion, | |||
| } from '@red-hat-developer-hub/backstage-plugin-boost-entity-provider-sdk'; | |||
|
|
|||
There was a problem hiding this comment.
[low] HTTP-client-inconsistency
Uses undici.Agent while boost-connector-utils uses node:https Agent. Technically necessary since global fetch() requires an undici dispatcher.
|
|
||
| /** | ||
| * Read OGX entity provider configuration from app-config.yaml. | ||
| * |
There was a problem hiding this comment.
[low] export-scope
readOgxEntityProviderConfig exported for testing with @internal annotation. Not re-exported from index.ts, so the public API is unaffected.
| maxAgentTurns?: number; | ||
| /** Static agent configurations from YAML/admin config. */ | ||
| agents?: OgxAgentConfig[]; | ||
| /** PEM-encoded CA certificate or certificate bundle used to verify OGX. */ |
There was a problem hiding this comment.
[low] naming-convention
Flat TLS fields follow Kubernetes caData/insecureSkipTLSVerify conventions rather than the nested tls sub-object pattern in boost-connector-utils.
| @@ -114,8 +114,10 @@ export const catalogModuleOgxEntityProvider = createBackendModule({ | |||
|
|
|||
| /** | |||
| * Read OGX entity provider configuration from app-config.yaml. | |||
fullsend-ai-review
Bot
added
requires-manual-review
|
🤖 Finished Review · ✅ Success · Started 8:40 PM UTC · Completed 8:59 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Cost: $6.71 |
1 participant
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:
skipTLSVerify takes precedence with warning logged
OgxAgentEntityProvider is unchanged (no outbound requests).
Closes #4477
Post-script verification
agent/4477-ogx-tls-config)193b36a397eeaa92f836ff2085060f645ba81644..HEAD)