feat(kserve-kubeflow-connector): RHIDP-15199- introduce plugin only OpenShift AI Connector#3705
feat(kserve-kubeflow-connector): RHIDP-15199- introduce plugin only OpenShift AI Connector#3705gabemontero wants to merge 20 commits into
Conversation
…ore secure cause of CSB, laptop)
Add dist-dynamic to .prettierignore (matches other workspaces). Generate missing API report for kserve-kubeflow-connector-backend. Update stale API reports for ai-experience and model-catalog. Assisted-by: Claude Opus 4.6 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Mirror proposal and design from redhat-ai-dev/agentic-feature-refinement openspec/changes/transition-oai-connector-to-kserve-plugin into the local workspace, following the pattern from PR redhat-developer#3692. Tasks and behavioral specs will be added in follow-up branches as implementation stories are picked up. Assisted-by: Claude Opus 4.6 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Important This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior. Unnecessary ChangesetsThe following package(s) are private and do not need a changeset:
Changed Packages
|
|
🤖 Finished Review · ✅ Success · Started 7:29 PM UTC · Completed 7:35 PM UTC |
Review — request-changesThis PR introduces a new The architectural direction is sound and aligns with the included openspec design documents. However, several issues need resolution before merge — particularly around security, scope consistency with the design doc, and production readiness. Findings🔴 High1. TLS certificate verification disabled for all KFMR REST calls ( The const tlsSkipAgent = new Agent({
connect: {
rejectUnauthorized: false,
},
});All Remediation: Remove 🟡 Medium2. The code obtains a proper service-to-service token via let tok = token.token;
if (process.env.RHDH_TOKEN && process.env.RHDH_TOKEN.length > 0) {
tok = process.env.RHDH_TOKEN;
}Additionally, Remediation: Investigate why the proper backend-to-backend token gets 401 and fix the root cause. The 3. KFMR code included despite design Decision 4 to remove it ( Decision 4 in the design doc ( Remediation: Either remove the KFMR code as specified in Decision 4, or update the design doc to reflect the decision to keep it in this initial PR. The current state is contradictory — the design doc says one thing while the code does another. 4. Fire-and-forget informer startup (
async init({ logger, httpRouter }) {
setupInformer().catch(error => {
logger.error('Failed to set up informer:', error);
});
httpRouter.use(await createRouter());
},Remediation: Either 5. The Remediation: Thread the 6. Configuration via env vars instead of app-config.yaml ( The design doc's Decision 2 explicitly calls for migrating from env vars to Backstage's K8s plugin config. However, the code reads Remediation: Accept these values from ⚪ Low7. README is boilerplate — "You should replace this text with a description of your plugin backend." A real description should be provided. 8. Multiple 9. 10. Duplicate 11. No cleanup for polling timer — The Positive aspects
SummaryThe core architectural approach (replacing golang sidecars with a TypeScript backend plugin) is well-motivated. The main blockers are: (1) the TLS skip that must not ship to production, (2) the auth workaround in the techdoc reader, (3) aligning the code scope with the design doc's decisions, and (4) production-readiness concerns (logging, config, startup sequencing). Previous runReview —
|
Minor bump for both packages — new discovery/auth service dependencies in the model-catalog entity provider, and bearer-token auth + path-based URL matching in the techdoc URL reader. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: gabemontero <gmontero@redhat.com> Assisted-by: Claude Opus 4.6
|
🤖 Review · ❌ Terminated · Started 7:36 PM UTC · Ended 7:49 PM UTC |
|
🤖 Finished Review · ✅ Success · Started 7:36 PM UTC · Completed 7:49 PM UTC |
Code Review by Qodo
Context used✅ Compliance rules (platform):
14 rules 1. Overbroad modelcard URL match
|
The backstage-repo-tools --tsc flag forces skipLibCheck=false, which fails on transitive type mismatches in node_modules (@types/request, @octokit, react-use). Replace with tsc:full (skipLibCheck=true) piped into api-reports generation. Also regenerates ai-experience report.api.md. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: gabemontero <gmontero@redhat.com> Assisted-by: Claude Opus 4.6
|
🤖 Review · ❌ Terminated · Started 8:00 PM UTC · Ended 8:06 PM UTC |
PR Summary by Qodofeat: introduce plugin-only kserve-kubeflow-connector-backend (RHIDP-15199)
AI Description
Diagram
High-Level Assessment
Files changed (30)
|
|
🤖 Finished Review · ✅ Success · Started 8:00 PM UTC · Completed 8:06 PM UTC |
- Fix case-sensitive config key: baseurl -> baseUrl (config.ts) - Fix cached backendToken expiry: get fresh token each run() call (ModelCatalogResourceEntityProvider.ts) - Fix inconsistent modelCardKey replacer in InformerService Path 2: use replacer() to strip spaces, matching Path 1 behavior - Fix yarn fix --check CI failure: correct repository.directory and backstage.pluginPackages in kserve-kubeflow-connector-backend package.json - Fix bridgePredicate test: provide config with matching id for path-based URL predicate matching - Add kserve-kubeflow-connector-backend patch to changeset Signed-off-by: gabemontero <gmontero@redhat.com> Assisted-by: Claude Opus 4.6 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Thanks for the thorough review. Here's how we've addressed each finding: Fixed in this PR (commit 4b5ddad)
Additionally fixed
Deferred to follow-up PRsThese are acknowledged and tracked but out of scope for this initial connector introduction:
|
|
🤖 Review · ❌ Terminated · Started 8:29 PM UTC · Ended 8:36 PM UTC |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3705 +/- ##
==========================================
- Coverage 54.55% 54.54% -0.01%
==========================================
Files 2349 2349
Lines 89735 89752 +17
Branches 25135 25134 -1
==========================================
+ Hits 48953 48957 +4
- Misses 40482 40495 +13
Partials 300 300
*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:
|
|
🤖 Finished Review · ❌ Failure · Started 8:29 PM UTC · Completed 8:36 PM UTC |
using Marcel's fullsend skills claude and I were able to analyze the findings from https://github.com/redhat-developer/rhdh-plugins/actions/runs/28896478597/artifacts/8150535056 for that flake post analysis failure with the fullsend review agent no new findinds |
Response to Qodo reviewAlready fixed (qodo confirms with ✓ Resolved):
Already deferred (same findings as fullsend review, see earlier response):
Not actionable for this PR:
New findings — deferring both:
|
Remove Backstage CLI template comments, unused catalog mock entity, and stale 'todos' API references that don't apply to this plugin. Reduces SonarQube duplicate code flagging. Signed-off-by: gabemontero <gmontero@redhat.com> Assisted-by: Claude Opus 4.6 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Response to SonarQube duplicate code findings
All four are extractable into helper functions, but each touches the core reconciliation logic. Refactoring safely wants unit test coverage first (also tracked for follow-up). Deferring both together. |
|
🤖 Review · ❌ Terminated · Started 8:53 PM UTC · Ended 9:03 PM UTC |
|
🤖 Finished Review · ✅ Success · Started 8:53 PM UTC · Completed 9:03 PM UTC |
Replace mutable entityList concat inside Promise.all callbacks with collect-and-flatten pattern to avoid fragile concurrent mutation. Signed-off-by: gabemontero <gmontero@redhat.com> Assisted-by: Claude Opus 4.6 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
|
🤖 Finished Review · ✅ Success · Started 9:25 PM UTC · Completed 9:31 PM UTC |


Hey, I just made a Pull Request!
this rebases and brings in the plugin only form of the connector that I prototyped during 1.10
also brings in openspec designs
I'll create tasks and I continue with the rest of the RHDHPLAN-404 Jiras
For now, tested against the dev cluster, both kserve onlyl and kubeflow model registry integrations (though they will be removed later) and kubeflow model catalog integrations (which will be kept, as well as tweaked to deal with kserve only)
@rajin-kichannagari FYI / PTAL
✔️ Checklist