Skip to content

feat(kserve-kubeflow-connector): RHIDP-15199- introduce plugin only OpenShift AI Connector#3705

Open
gabemontero wants to merge 20 commits into
redhat-developer:mainfrom
gabemontero:kserve-kubeflow-connector
Open

feat(kserve-kubeflow-connector): RHIDP-15199- introduce plugin only OpenShift AI Connector#3705
gabemontero wants to merge 20 commits into
redhat-developer:mainfrom
gabemontero:kserve-kubeflow-connector

Conversation

@gabemontero

Copy link
Copy Markdown
Contributor

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

  • [/] A changeset describing the change and affected packages. (more info)
  • [/] Added or Updated documentation
  • [/] Tests for new functionality and regression tests for bug fixes
  • [n/a] Screenshots attached (for UI changes)

gabemontero and others added 14 commits July 7, 2026 14:45
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>
@rhdh-gh-app

rhdh-gh-app Bot commented Jul 7, 2026

Copy link
Copy Markdown

Important

This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior.

Unnecessary Changesets

The following package(s) are private and do not need a changeset:

  • @red-hat-developer-hub/backstage-plugin-kserve-kubeflow-connector-backend

Changed Packages

Package Name Package Path Changeset Bump Current Version
backend workspaces/ai-integrations/packages/backend none v0.0.13
@red-hat-developer-hub/backstage-plugin-catalog-backend-module-model-catalog workspaces/ai-integrations/plugins/catalog-backend-module-model-catalog minor v0.9.1
@red-hat-developer-hub/backstage-plugin-catalog-techdoc-url-reader-backend workspaces/ai-integrations/plugins/catalog-techdoc-url-reader-backend minor v0.5.1
@red-hat-developer-hub/backstage-plugin-kserve-kubeflow-connector-backend workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend patch v0.1.0

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 7, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:29 PM UTC · Completed 7:35 PM UTC
Commit: 26407b3 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 7, 2026

Copy link
Copy Markdown

Review — request-changes

This PR introduces a new kserve-kubeflow-connector-backend plugin (~3,100 lines) that replaces the golang sidecar architecture with a TypeScript backend plugin using K8s Informers. It also modifies the existing catalog-backend-module-model-catalog and catalog-techdoc-url-reader-backend plugins to use Backstage discovery + auth services for inter-plugin communication.

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

🔴 High

1. TLS certificate verification disabled for all KFMR REST calls (Kfmr.ts)

The undici.Agent is created with rejectUnauthorized: false, disabling TLS certificate validation for every HTTP call to the KubeFlow Model Registry. This makes the connector vulnerable to man-in-the-middle attacks in production environments.

const tlsSkipAgent = new Agent({
  connect: {
    rejectUnauthorized: false,
  },
});

All getFromModelRegistry() calls use this agent via dispatcher: tlsSkipAgent. If custom CA bundles are needed for internal clusters, they should be configured properly rather than disabling verification entirely.

Remediation: Remove rejectUnauthorized: false. If custom CAs are needed, accept a CA certificate path from configuration and pass it to the agent. At minimum, gate this behavior behind an explicit opt-in configuration flag (e.g., kfmr.tls.skipVerify: true) with a warning log.


🟡 Medium

2. RHDH_TOKEN env var fallback bypasses Backstage auth (catalog-techdoc-url-reader-backend/src/plugin.ts)

The code obtains a proper service-to-service token via auth.getPluginRequestToken(), but then unconditionally overrides it with process.env.RHDH_TOKEN when set. The inline TODO comment acknowledges the proper token "got 401 / unauthorized" — this static admin token workaround circumvents Backstage's auth model and introduces a credential that must be managed outside Backstage.

let tok = token.token;
if (process.env.RHDH_TOKEN && process.env.RHDH_TOKEN.length > 0) {
  tok = process.env.RHDH_TOKEN;
}

Additionally, discovery and auth are stored as static class properties on ModeCatalogBridgeTechdocUrlReader, which is fragile and acknowledged as a workaround in the code comments.

Remediation: Investigate why the proper backend-to-backend token gets 401 and fix the root cause. The targetPluginId passed to getPluginRequestToken() is urlReaderFactoriesServiceRef.id — it should likely be 'kserve-kubeflow-connector' to match the target plugin.


3. KFMR code included despite design Decision 4 to remove it (Kfmr.ts, InformerService.ts)

Decision 4 in the design doc (design.md) explicitly states: "Remove all KFMR client code (Kfmr.ts, related types, KFMR-specific label handling in InformerService.ts)." Yet this PR includes all 837 lines of Kfmr.ts, extensive KFMR processing in InformerService.ts (processKFMR, innerStart, KFMR route setup), and related types in types.ts. The design doc notes this "significantly reduces the connector's surface area (~837 lines in Kfmr.ts alone, plus cross-references in InformerService.ts)."

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 (plugin.ts)

setupInformer() is called without await, meaning the plugin's init() completes and the HTTP router is registered before the informer is ready. Early requests to /list or /models/* will return empty results with no indication that the system is still initializing.

async init({ logger, httpRouter }) {
  setupInformer().catch(error => {
    logger.error('Failed to set up informer:', error);
  });
  httpRouter.use(await createRouter());
},

Remediation: Either await setupInformer() in init() (which may delay startup), or add a readiness check to the router that returns 503 until the informer has completed its initial list operation.


5. console.log used pervasively instead of Backstage LoggerService (all new plugin files)

The logger service is available in plugin.ts but is never passed to InformerService.ts, KServe.ts, Kfmr.ts, or router.ts. These files use console.log and console.error extensively (~100+ instances). Backstage's logger service provides structured logging, log levels, and integration with observability tooling. Using console.log bypasses all of this.

Remediation: Thread the LoggerService through to all service functions. Replace console.log with logger.info/logger.debug and console.error with logger.error.


6. Configuration via env vars instead of app-config.yaml (InformerService.ts, Kfmr.ts)

The design doc's Decision 2 explicitly calls for migrating from env vars to Backstage's K8s plugin config. However, the code reads K8S_TOKEN, LIFECYCLE, OWNER, POLLING_INTERVAL, MODEL_REGISTRY_ROUTE, and MODEL_REGISTRY_TOKEN from process.env. The plugin does not receive coreServices.rootConfig as a dependency.

Remediation: Accept these values from app-config.yaml via coreServices.rootConfig, following the pattern already used by the model-catalog entity provider. Document the config schema in config.d.ts.


⚪ Low

7. README is boilerplate — "You should replace this text with a description of your plugin backend." A real description should be provided.

8. Multiple @ts-ignore commentsKfmr.ts has several @ts-ignore for URI format strings that are never used (Go-style %s format strings assigned to constants), and for the dispatcher option on fetch(). The unused URI constants should be removed.

9. httpAuth declared but unused — In plugin.ts, httpAuth is declared in deps but never used or passed to createRouter(). The unused dependency should be cleaned up.

10. Duplicate KServeInferenceService typeKServe.ts and types.ts both define KServeInferenceService with incompatible status.url types ({ toString(): string } vs string). This could cause runtime issues when casting between them.

11. No cleanup for polling timer — The setInterval timer in setupInformer() is stored on (config.informer as any).__pollingTimer with no shutdown cleanup, preventing graceful process termination.


Positive aspects

  • The discovery + auth integration in ModelCatalogResourceEntityProvider is well-implemented, following Backstage patterns correctly
  • The design documents (proposal.md, design.md) are thorough and well-structured
  • Test updates for the entity provider properly mock the new discovery/auth dependencies
  • The changeset file accurately describes the changes

Summary

The 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 run

Review — request-changes

This PR introduces the new kserve-kubeflow-connector-backend plugin (~3,000 lines of TypeScript ported from a Go prototype), integrates Backstage discovery/auth services into the existing catalog-backend-module-model-catalog and catalog-techdoc-url-reader-backend plugins, and adds OpenSpec design documents. The overall direction is sound and the architecture (separate connector plugin + entity provider) aligns well with the design doc. However, there are several high-severity issues — primarily around authentication, credential handling, and correctness — that need to be resolved before this can be merged.

High-severity findings

1. Unauthenticated router endpoints

router.ts exposes /list, /models/:model/:version, and /modelcard/:sourceId/:modelName with no authentication. httpAuth is declared as a dependency in plugin.ts but never used. Any caller that can reach the Backstage backend can read all model data.

2. TLS verification disabled for all KFMR API calls

Kfmr.ts creates a global undici.Agent with rejectUnauthorized: false. All Model Registry HTTP calls bypass certificate verification, enabling man-in-the-middle attacks in production.

3. RHDH_TOKEN env var overrides proper auth tokens

In catalog-techdoc-url-reader-backend/src/plugin.ts, the RHDH_TOKEN environment variable overrides the Backstage service token. The root cause appears to be that targetPluginId is set to urlReaderFactoriesServiceRef.id instead of 'kserve-kubeflow-connector', causing 401s. The env var workaround ships a static credential bypass.

4. Race condition in entityList concatenation

In ModelCatalogResourceEntityProvider.ts, entityList = entityList.concat(catalogEntities) inside Promise.all callbacks creates a race — concurrent async callbacks read a stale reference and overwrite each other's results, losing catalog entries.

5. Invalid K8s API call with empty namespace

InformerService.ts calls listNamespacedCustomObject() with empty string '' for namespace. This produces a malformed API URL. The intent is cluster-wide listing, which requires listClusterCustomObject(). Same issue in Kfmr.ts for Route resources.

Medium-severity findings

6. K8s token reused for KFMR authentication

When MODEL_REGISTRY_TOKEN is unset, Kfmr.ts falls back to config.k8sToken — the K8s cluster token — to authenticate against the Model Registry REST API. This is a credential scope violation: a cluster token is leaked to a separate service.

7. Empty bearer token sent as fail-open

BridgeResourceConnector.ts sends Authorization: Bearer (empty) when no token is provided, instead of failing. Token should be required or the header omitted.

8. Fire-and-forget informer with no health indication

setupInformer().catch() in plugin.ts starts the K8s informer without awaiting. If it fails, the router silently serves empty data. No health endpoint or readiness probe surfaces this failure.

9. Zero test coverage for ~3,000 lines of new code

The new plugin has no test files. InformerService.ts (1,406 lines), Kfmr.ts (837 lines), KServe.ts (311 lines) — all untested. The reconciliation logic with global mutable state is particularly important to test.

10. Concurrent event handler races on global state

InformerService.ts uses module-level Maps for modelCards and modelCatalog. The async informer event handlers (add/update/delete) mutate these concurrently. The delete handler calls innerStart() for full reconciliation, which can conflict with concurrent add/update handlers.

11. console.log instead of LoggerService (~100+ instances)

The entire new plugin uses console.log/console.error instead of the Backstage LoggerService. This bypasses structured logging, log levels, and may expose sensitive K8s resource data (names, namespaces, labels, URLs) that cannot be suppressed in production.

12. Design doc says remove KFMR, PR includes 837 lines of KFMR code

Decision 4 in the design doc explicitly states "Remove all KFMR client code (Kfmr.ts, related types, KFMR-specific label handling in InformerService.ts)". The PR includes the full 837-line Kfmr.ts and extensive KFMR handling in InformerService.ts. This design-implementation gap should be clarified.

13. New plugin README is template boilerplate

The README says "You should replace this text with a description of your plugin backend." It documents none of the environment variables (K8S_TOKEN, LIFECYCLE, OWNER, POLLING_INTERVAL, MODEL_REGISTRY_ROUTE, MODEL_REGISTRY_TOKEN, RHDH_TOKEN) or configuration requirements.

14. Background polling timer resource leak

setInterval in setupInformer is stored as (config.informer as any).__pollingTimer but never cleared. No shutdown hook is registered.

15. Static mutable auth/discovery on techdoc reader

ModeCatalogBridgeTechdocUrlReader.discovery and .auth are static class properties set during factory init. This creates a timing dependency and shared mutable state across instances.

Low-severity findings

16. Wrong targetPluginId for service token

In catalog-techdoc-url-reader-backend/src/plugin.ts, targetPluginId is set to urlReaderFactoriesServiceRef.id instead of 'kserve-kubeflow-connector'. This is likely the root cause of the 401 errors that led to the RHDH_TOKEN workaround.

17. bridgePredicate changed to overly broad matching

The predicate changed from hostname+port matching to url.pathname.includes('modelcard') && url.pathname.includes(bc.id) — this could match unintended URLs.

18. Discovery URL fallback logic

In ModelCatalogResourceEntityProvider.ts, this.name is config.id (the config provider key), not the plugin ID. Discovery lookup via this.discovery.getBaseUrl(this.name) only works if the config key matches the plugin ID.

19. Commented-out code and TODO comments

router.ts has leftover debug middleware and commented-out imports. Multiple TODO/FIXME comments indicate unfinished work.

20. Changeset version bump

The changeset marks the new kserve-kubeflow-connector-backend as patch. For a brand-new plugin, this should be at least minor (first published version).


Reviewed dimensions: correctness, security, intent/coherence, style/conventions, documentation currency.

Previous run (2)

Review — kserve-kubeflow-connector plugin introduction

PR: #3705 | Base: main | Head: 4b5ddad4

Summary

This PR introduces a new kserve-kubeflow-connector-backend plugin that replaces a golang-based sidecar architecture with a TypeScript Backstage backend plugin. It also modifies the catalog-backend-module-model-catalog entity provider and catalog-techdoc-url-reader-backend to use Backstage discovery and auth services for plugin-to-plugin communication. Additionally, it includes openspec design/proposal documents.

The architectural direction is sound — replacing sidecar containers with a native Backstage plugin is a significant improvement. However, the implementation has several security gaps and prototype-quality patterns that need resolution before merging.


Findings

🔴 High

[H1] Missing authentication on connector REST endpoints
plugins/kserve-kubeflow-connector-backend/src/plugin.ts (line 36), src/router.ts

The plugin declares httpAuth as a dependency but does not destructure or use it in the init() method. The router has no authentication middleware — the /list, /models/:model/:version, and /modelcard/:sourceId/:modelName endpoints are accessible to any caller within the backend network without credential verification. Meanwhile, the entity provider in ModelCatalogResourceEntityProvider.ts obtains service tokens via auth.getPluginRequestToken() and sends them as Bearer tokens, but the receiver never validates them. This defeats the purpose of the auth integration described in the changeset.

Remediation: Pass httpAuth to createRouter(), and add await httpAuth.credentials(req, { allow: ['service'] }) to each route handler (or as middleware). This ensures only authenticated backend plugins can access the connector's data.


[H2] TLS certificate verification disabled unconditionally
plugins/kserve-kubeflow-connector-backend/src/services/Kfmr.ts (line 18–22)

A global tlsSkipAgent is created with rejectUnauthorized: false, hardcoded at module scope. This disables TLS certificate validation for all KFMR REST client calls, making the connection vulnerable to MITM attacks. There is no configuration option to enable TLS verification.

Remediation: Make TLS verification configurable (e.g., via app-config.yaml). Default to rejectUnauthorized: true and only allow disabling it through explicit configuration, ideally with a logged warning.


[H3] RHDH_TOKEN env var bypasses Backstage service-to-service auth
plugins/catalog-techdoc-url-reader-backend/src/plugin.ts (lines 182–184 in the diff)

When process.env.RHDH_TOKEN is set, it unconditionally overrides the properly-obtained service token with a static admin token. The TODO comment acknowledges this is a workaround for 401 errors, but shipping it as-is means any deployment with RHDH_TOKEN set bypasses Backstage's auth mechanism entirely. Combined with finding H1 (no auth validation on the connector), this creates an unauthenticated data path.

Remediation: Fix the root cause (finding H1 — missing auth validation on the connector) rather than working around it with a static token. Remove the RHDH_TOKEN override or, at minimum, log a security warning when it's active and document it as a temporary development-only option.


[H4] setupInformer() errors swallowed — plugin appears healthy while serving empty data
plugins/kserve-kubeflow-connector-backend/src/plugin.ts (lines 41–43 in the diff)

setupInformer() is called fire-and-forget with .catch() that only logs the error. If the K8s informer fails to start (no cluster access, invalid credentials, CRD not installed), the plugin registers its router and appears healthy, but all endpoints return empty/404 responses. The entity provider will then receive empty catalog key lists and issue a full mutation with zero entities — deleting all previously-ingested model catalog entities from the Backstage catalog on every 30-second polling cycle.

Remediation: Either (a) await the informer setup and fail the plugin init if it can't connect, or (b) set a readiness flag that the router checks before serving data, returning 503 when the informer isn't ready. Option (b) prevents the entity provider from interpreting "no data" as "all entities deleted."


🟡 Medium

[M1] Zero test coverage for 2,600+ lines of new plugin code
plugins/kserve-kubeflow-connector-backend/

The new plugin has no test files at all — no unit tests for InformerService.ts (1,406 lines), KServe.ts (311 lines), Kfmr.ts (837 lines), router.ts, or plugin.ts. The design document itself notes "Additional unit tests for the converted golang→typescript code" as a known TODO.

Remediation: Add tests, at minimum for: (1) router.ts route handlers, (2) KServe.callBackstagePrinters entity generation, (3) getDiscoveryUris / getModelCatalog / getModelCard public API, and (4) sanitizeName / buildImportKeyAndURI helper functions.


[M2] KFMR code retained despite design document stating removal
plugins/kserve-kubeflow-connector-backend/src/services/Kfmr.ts, InformerService.ts

Design Decision 4 explicitly states: "Remove all KFMR client code (Kfmr.ts, related types, KFMR-specific label handling in InformerService.ts)." However, the PR includes the full 837-line Kfmr.ts and all KFMR integration code in InformerService.ts. The PR description mentions "kubeflow model registry integrations (though they will be removed later)" — if this is intentional for this phase, the design doc should be updated to reflect the phased approach. As-is, the code and the design document contradict each other.


[M3] Extensive console.log usage instead of Backstage structured logger
plugins/kserve-kubeflow-connector-backend/src/services/InformerService.ts, Kfmr.ts, KServe.ts; plugins/catalog-backend-module-model-catalog/src/clients/BridgeResourceConnector.ts

There are 80+ console.log/console.error calls throughout the new plugin code and one console.log in the modified BridgeResourceConnector.ts. These bypass Backstage's structured LoggerService, preventing log level filtering, structured metadata, and redaction. Some log statements include URLs that may contain tokens or internal network addresses.

Remediation: Inject the LoggerService into setupInformer(), createRouter(), and service modules. Replace console.log with logger.info/logger.debug and console.error with logger.error.


[M4] Empty Bearer token sent when token is undefined
plugins/catalog-backend-module-model-catalog/src/clients/BridgeResourceConnector.ts (lines 57–63 in the diff)

In fetchModelCatalogFromKey, when token is undefined, the code sets tok = '' and still sends Authorization: Bearer (with empty string). This sends a malformed auth header rather than omitting it, which may cause 401 errors on services that reject invalid Authorization headers.

Remediation: Conditionally include the Authorization header only when a valid token is available:

const headers: Record<string, string> = { 'Content-Type': 'application/json' };
if (token?.token) {
  headers.Authorization = `Bearer ${token.token}`;
}

[M5] Static class properties for auth/discovery — initialization race risk
plugins/catalog-techdoc-url-reader-backend/src/plugin.ts (lines 119–120 in the diff)

ModeCatalogBridgeTechdocUrlReader.discovery and .auth are set as static class properties, populated during service factory initialization. The readUrl method accesses these statics at call time. If a URL read is triggered before the factory runs (e.g., during backend bootstrap ordering), accessing .auth throws a TypeError. The TODO comment acknowledges this is a workaround for the ReaderFactory interface limitations.


[M6] Informer error handler creates infinite restart loop
plugins/kserve-kubeflow-connector-backend/src/services/InformerService.ts (lines 2593–2599 in the diff)

The informer.on('error', ...) handler restarts the informer after a 5-second setTimeout. If the error is persistent (e.g., permission denied, missing CRD), this creates an infinite restart loop that floods logs and makes repeated failing API calls every 5 seconds.

Remediation: Implement exponential backoff with a maximum retry count or maximum delay (e.g., cap at 5 minutes). After exceeding max retries, log a fatal error and stop retrying.


[M7] K8s credentials read from environment variables (prototype pattern)
plugins/kserve-kubeflow-connector-backend/src/services/InformerService.ts (lines 2490–2500 in the diff)

K8S_TOKEN, LIFECYCLE, OWNER, and POLLING_INTERVAL are read from process.env. The design document (Decision 2) explicitly states these should migrate to Backstage's K8s plugin configuration. If this migration is deferred, it should be documented as a known limitation.


🟢 Low

[L1] Boilerplate/template content in README and dev setup
plugins/kserve-kubeflow-connector-backend/README.md (line 3), dev/index.ts

The README says "This plugin backend was templated using the Backstage CLI. You should replace this text with a description of your plugin backend." The dev/index.ts references a "todos" API from the template scaffold that doesn't exist in this plugin.


Verdict

Request changes — Four high-severity findings require resolution before merge. The most critical are the missing auth on the connector's REST endpoints (H1) and the fire-and-forget informer setup that can silently delete all catalog entities (H4). The auth bypass via RHDH_TOKEN (H3) and unconditional TLS skip (H2) are also security concerns that should not ship as-is.

Previous run (3)

Review — request-changes

PR: #3705 — feat(kserve-kubeflow-connector): RHIDP-15199 — introduce plugin-only OpenShift AI Connector
Scope: New kserve-kubeflow-connector-backend plugin replacing golang sidecar architecture, modifications to catalog-backend-module-model-catalog and catalog-techdoc-url-reader-backend for discovery/auth integration.

This is a large PR (~7200 additions) that introduces the prototype connector plugin and integrates it with existing entity provider and techdocs plugins. The architectural direction is sound — replacing sidecars with a native TypeScript plugin is a meaningful improvement. However, several security and correctness issues need resolution before merge.


Findings

🔴 High — Unauthenticated router endpoints

File: plugins/kserve-kubeflow-connector-backend/src/router.ts (lines 61–83) / src/plugin.ts (line 37)

The plugin declares httpAuth as a dependency but never uses it. The /list, /models/:model/:version, and /modelcard/:sourceId/:modelName endpoints have no authentication middleware. Any caller with network access to the Backstage backend can read model catalog data and model cards without credentials.

Remediation: Pass httpAuth into createRouter, and call httpAuth.credentials(req, { allow: ['service'] }) (or appropriate policy) in each route handler to enforce backend-to-backend auth. This is the standard Backstage pattern for internal plugin APIs.


🔴 High — Config key case mismatch (baseurl vs baseUrl)

File: plugins/catalog-backend-module-model-catalog/src/providers/config.ts (lines 46–49)

The code checks config.has('baseurl') and reads config.getString('baseurl') (lowercase u), but config.d.ts declares the key as baseUrl (camelCase). Backstage config keys are case-sensitive. Any user who configures baseUrl: in their app-config.yaml (as the config schema declares) will always get an empty string, silently breaking the fallback path.

Remediation: Change 'baseurl''baseUrl' on both the has() and getString() calls.


🔴 High — TLS certificate verification disabled globally

File: plugins/kserve-kubeflow-connector-backend/src/services/Kfmr.ts (lines 28–32)

A global undici.Agent with rejectUnauthorized: false is created and used for all KFMR REST calls. This disables TLS certificate verification for every outbound connection through this agent, making all KFMR communication susceptible to man-in-the-middle attacks.

Remediation: Remove the global TLS skip. If self-signed certificates are needed for specific environments, expose a config option (e.g., kfmr.tls.skipVerify: true) and document the security implications. Do not ship with TLS disabled by default.


🟠 Medium — RHDH_TOKEN environment variable bypasses service auth

File: plugins/catalog-techdoc-url-reader-backend/src/plugin.ts (lines 181–189)

The code obtains a proper service-to-service token via auth.getPluginRequestToken(), then immediately overrides it with process.env.RHDH_TOKEN if set. The TODO comment above acknowledges this is a workaround for a 401 issue. Shipping an env-var backdoor that bypasses Backstage's auth system is a security risk — any token in RHDH_TOKEN will be sent as a bearer token to the connector, regardless of its validity or scope.

Remediation: Investigate and fix the root cause of the 401 (likely the missing httpAuth middleware in the router — see first finding). Remove the RHDH_TOKEN workaround entirely.


🟠 Medium — Backend token cached indefinitely (stale token risk)

File: plugins/catalog-backend-module-model-catalog/src/providers/ModelCatalogResourceEntityProvider.ts (lines 170–175)

The service token is obtained once (this.backendToken = await this.auth.getPluginRequestToken(...)) and cached for the lifetime of the provider. Backstage service tokens have a limited validity window. Once the token expires, all subsequent fetchModelCatalogKeys and fetchModelCatalogFromKey calls will fail with 401.

Remediation: Obtain a fresh token on each run() invocation, or at minimum before each set of API calls. The Backstage auth service handles caching internally — repeated calls are inexpensive.


🟠 Medium — No unit tests for ~2500 lines of new service code

Files: plugins/kserve-kubeflow-connector-backend/src/services/InformerService.ts, KServe.ts, Kfmr.ts, router.ts

The new plugin contains ~2500 lines of business logic (informer reconciliation, KFMR client, KServe catalog generation, REST routes) with zero unit tests. The AGENTS.md file in this workspace emphasizes testing. The reconciliation logic is complex (multiple code paths for KFMR vs KServe-only, label matching, status checks) and is a direct Go→TypeScript port — exactly the kind of code most likely to harbor subtle bugs.

Remediation: Add tests for at minimum: reconcileInferenceService (both KFMR and KServe-only paths), getDiscoveryUris/getModelCatalog/getModelCard (route handlers), isInferenceServiceReady (status checks), and callBackstagePrinters from KServe.ts.


🟠 Medium — KFMR code included despite design decision to remove it

Files: plugins/kserve-kubeflow-connector-backend/src/services/Kfmr.ts (837 lines), InformerService.ts (KFMR references throughout)

The design document (design.md, Decision 4) explicitly says "Remove all KFMR client code (Kfmr.ts, related types, KFMR-specific label handling in InformerService.ts)." However, the full Kfmr.ts file and all KFMR processing logic in InformerService.ts are included. This contradicts the stated design intent and substantially inflates the PR's surface area (~837 lines of code that the design says should not exist).

Remediation: Either remove the KFMR code per the design decision, or update the design document to reflect that KFMR is being kept for the initial release. The current state — code and design contradicting each other — creates confusion.


🟡 Low — console.log/console.error used instead of Backstage logger

Files: InformerService.ts (throughout), Kfmr.ts (throughout), KServe.ts, router.ts, BridgeResourceConnector.ts (line 63)

The new plugin uses console.log and console.error extensively (~60+ occurrences) instead of the Backstage LoggerService. This bypasses structured logging, log level filtering, and any log aggregation configuration. In production, this will flood stdout with debug-level reconciliation traces that cannot be silenced.

Remediation: Thread the logger from plugin.ts through to setupInformer, createRouter, and all service functions. Replace console.log with logger.debug/logger.info and console.error with logger.error. Also remove the console.log in BridgeResourceConnector.ts line 63 (console.log(\Fetching model catalog key ...``).


🟡 Low — Static mutable state for discovery/auth services

File: plugins/catalog-techdoc-url-reader-backend/src/plugin.ts (lines 120–121, 287–288)

DiscoveryService and AuthService are stored as static class properties on ModeCatalogBridgeTechdocUrlReader. The TODO comment acknowledges this is a workaround for the ReaderFactory interface limitation. While functional for a single instance, static mutable state is fragile — it couples service lifecycle to class loading order and makes testing harder.


🟡 Low — Dev boilerplate not cleaned up

File: plugins/kserve-kubeflow-connector-backend/dev/index.ts

The dev server setup still contains the Backstage CLI template's TODO comments referencing "todos" (curl examples for a TODO list app). The router.ts also has commented-out template code. This should be cleaned up to match the actual plugin functionality.


🟡 Low — package.json metadata inconsistencies

File: plugins/kserve-kubeflow-connector-backend/package.json

  • backstage.pluginPackages references @red-hat-developer-hub/backstage-plugin-backend-kserve-kubeflow-connector which doesn't match the actual package name (@red-hat-developer-hub/backstage-plugin-kserve-kubeflow-connector-backend)
  • repository.directory points to workspaces/ai-integrations/plugins/kserve-kubeflow-connector (missing -backend suffix)

Summary

The architectural direction (replacing sidecars with a native plugin, integrating via Backstage discovery/auth) is well-motivated and the design documents are thorough. The core issues are:

  1. Security gaps — unauthenticated endpoints, TLS disabled, env-var credential bypass
  2. Config bug — case mismatch silently breaks the baseUrl fallback
  3. Token caching — will cause runtime failures when tokens expire
  4. No tests — large, complex Go→TypeScript port with zero test coverage

The KFMR inclusion vs. design doc contradiction should also be resolved to avoid confusion for reviewers and future contributors.

Previous run (4)

Review — request-changes

This PR introduces the kserve-kubeflow-connector-backend plugin to replace a golang sidecar architecture with a TypeScript backend plugin. The approach is sound and the design documents are thorough, but the implementation has several high-severity issues that must be resolved before merge — primarily around authentication, TLS, and a config key casing bug that will silently break the integration at runtime.

Critical Path Issues

1. Config key case mismatch breaks baseUrl reading — config.ts (High)

In catalog-backend-module-model-catalog/src/providers/config.ts, the PR changes config.getString('baseUrl') to config.has('baseurl') / config.getString('baseurl') (lowercase u). Backstage's Config API is case-sensitive, and config.d.ts defines the key as baseUrl (camelCase). This means config.has('baseurl') will always return false for users who configure baseUrl: in their app-config, silently defaulting to an empty string. The entity provider then falls through to discovery.getBaseUrl(this.name), which may or may not resolve correctly depending on the config key name.

2. Router endpoints have no authentication — plugin.ts / router.ts (High)

httpAuth is declared as a dependency in plugin.ts but is never destructured or passed to the router. The three GET endpoints (/list, /models/:model/:version, /modelcard/:sourceId/:modelName) have zero authentication middleware. Any network caller can enumerate models, read model cards, and retrieve model catalog data. This contradicts the PR's stated goal of using auth services for plugin-to-plugin communication.

3. TLS certificate verification unconditionally disabled — Kfmr.ts (High)

A global undici.Agent with rejectUnauthorized: false is hardcoded and used for all HTTP requests to the KubeFlow Model Registry. This disables TLS verification entirely with no configuration option to enable it, making all KFMR API calls (which include Bearer tokens in Authorization headers) vulnerable to man-in-the-middle attacks.

4. Static admin token bypass — catalog-techdoc-url-reader-backend/plugin.ts (High)

The techdoc URL reader obtains a proper service-to-service token via auth.getPluginRequestToken() but immediately overrides it with process.env.RHDH_TOKEN if set. The TODO comment acknowledges this is a workaround for 401 errors. This bypasses Backstage's scoped auth model with a long-lived static admin token. The root cause is that targetPluginId is set to urlReaderFactoriesServiceRef.id instead of 'kserve-kubeflow-connector' — fixing the plugin ID should resolve the 401.

Medium Severity Issues

5. Backend token cached indefinitely without refresh

In ModelCatalogResourceEntityProvider.ts, this.backendToken is cached once and never refreshed. Backstage service tokens expire (typically ~1 hour). After expiry, all model catalog sync calls will fail with 401 until restart.

6. Informer/router race condition — plugin.ts

setupInformer() is fire-and-forget (.catch()) while the router registers immediately. The router reads from global Maps that are populated asynchronously. Until the informer completes initial sync, /list returns empty results and /models/* returns 404s. No readiness signal exists.

7. modelCardKey inconsistency between processKFMR paths — InformerService.ts

Path 1 (line ~430) applies replacer (strips spaces) to build modelCardKey. Path 2 (line ~589) does NOT apply replacer. The same model processed through different paths gets different keys, causing cache misses and orphaned entries.

8. listNamespacedCustomObject with empty namespace — InformerService.ts

listInferenceServices passes '' as namespace, producing an invalid K8s API path (/namespaces//inferenceservices). For cluster-wide listing, listClusterCustomObject should be used instead.

9. Cross-registry accumulation bug — InformerService.ts

kfmrRMs and kfmrISs arrays in processKFMR are declared before the KFMR clients loop and accumulate across iterations. Registered models from registry A can be incorrectly matched with inference services from registry B.

10. No tests for new plugin

The new kserve-kubeflow-connector-backend plugin has zero test files covering 2,500+ lines of complex reconciliation logic, K8s API interactions, and global mutable state.

11. Breaking public API changes — report.api.md

fetchModelCatalogKeys now requires a mandatory token parameter, and ModelCatalogResourceEntityProvider.fromConfig now requires discovery and auth deps. These are @public APIs — external consumers will get compile errors on upgrade.

12. Kfmr.ts included despite design doc Decision 4

The design doc (included in this PR) explicitly states "Remove KubeFlow Model Registry (KFMR) interaction" as Decision 4, yet 837 lines of KFMR client code are included. This adds significant surface area that the design says should be removed.

Low Severity Issues

  • console.log/error throughout new plugin instead of Backstage's LoggerService. The existing codebase uses injected loggers.
  • Informer error handler restarts on a fixed 5-second timer with no backoff, retry limit, or .catch() on the start() promise.
  • Commented-out code and template boilerplate in router.ts and dev/index.ts.
  • Boilerplate README still says "This plugin backend was templated using the Backstage CLI. You should replace this text."
  • @ts-ignore suppressions for unused URI constants in Kfmr.ts.
  • Empty Bearer token sent when token parameter is undefined in fetchModelCatalogFromKey.

Recommendation

The architecture (standalone plugin replacing sidecar) is well-motivated and the design docs are valuable. However, the implementation has auth gaps (unauthenticated endpoints, TLS disabled, admin token bypass), a config bug that will silently break the integration, and zero test coverage on the new plugin. These must be addressed before merge.

Prioritized fix order:

  1. Fix baseurlbaseUrl case in config.ts
  2. Wire up httpAuth in the router
  3. Make TLS verification configurable (default to enabled)
  4. Fix targetPluginId in techdoc reader and remove RHDH_TOKEN fallback
  5. Remove token caching or add TTL-based refresh
  6. Add unit tests for InformerService, KServe, and router
  7. Either remove Kfmr.ts or update the design doc to reflect the plan
Previous run (5)

Review — kserve-kubeflow-connector plugin introduction

This PR introduces the kserve-kubeflow-connector-backend plugin — a TypeScript backend plugin that replaces the golang-based sidecar architecture for connecting RHDH to KServe InferenceServices and KubeFlow Model Registry/Catalog APIs. It also modifies existing plugins (catalog-backend-module-model-catalog, catalog-techdoc-url-reader-backend) to integrate via Backstage service discovery and auth rather than direct baseUrl HTTP calls.

The design documents and prototype code are substantial (~7k additions). The architectural direction is sound — eliminating sidecars in favor of a Backstage backend plugin is a clear improvement. However, several correctness and security issues need resolution before merge.


Critical / High Findings

1. Config key casing mismatch — baseurl vs baseUrl (high)
In config.ts, the PR changes the config read from config.getString('baseUrl') to config.has('baseurl') / config.getString('baseurl') (all lowercase). However, the TypeScript config schema in config.d.ts still declares the field as baseUrl (camelCase), and app-config.yaml also uses baseUrl. Backstage's Config API is case-sensitive — config.has('baseurl') will never match a baseUrl key, so baseUrl will always be empty string, silently breaking the fallback path for users who explicitly configure a baseUrl.

2. Stale backend token cached indefinitely (high)
In ModelCatalogResourceEntityProvider.ts, the backendToken is obtained once and cached on the instance (this.backendToken). Backstage service-to-service tokens are short-lived (typically 1 hour). The entity provider polls every 30 seconds — after the token expires, all fetchModelCatalogKeys and fetchModelCatalogFromKey calls will fail with 401 errors. The token should be refreshed on each run() invocation.

3. Static mutable state on class — ModeCatalogBridgeTechdocUrlReader.discovery/.auth (high)
The ModeCatalogBridgeTechdocUrlReader class stores discovery and auth services as static properties. These are set during factory initialization but are shared across all instances. If multiple factory instantiations occur (e.g., in tests or multi-backend setups), the last writer wins, creating a race condition. The TODO comment acknowledges this is a workaround — it should be documented as a known limitation.

4. TLS verification globally disabled for all KFMR requests (medium)
Kfmr.ts creates a global undici.Agent with rejectUnauthorized: false and uses it for all model registry REST calls. This disables certificate validation, making all KFMR API communication vulnerable to MITM attacks. While this may be needed for dev/test with self-signed certs, it should be configurable (e.g., via app-config.yaml) rather than unconditionally disabled.

5. Environment variable RHDH_TOKEN overrides service auth token (medium)
In catalog-techdoc-url-reader-backend/src/plugin.ts, if process.env.RHDH_TOKEN is set, it overrides the Backstage-obtained service token. This is a security concern — environment variables containing static tokens bypass Backstage's auth model and could be leaked. The TODO comments suggest this is a workaround for a 401 issue that should be investigated rather than shipped.


Medium Findings

6. No unit tests for the new kserve-kubeflow-connector-backend plugin (medium)
The new plugin adds ~2700 lines of service code (InformerService.ts, KServe.ts, Kfmr.ts, types.ts) with zero test files. The design document itself acknowledges "Additional unit tests for the converted golang→typescript code" as a known TODO. At minimum, the router endpoints and the callBackstagePrinters / generateModelCatalog functions should have test coverage before merge.

7. console.log/console.error used throughout instead of logger service (medium)
The entire kserve-kubeflow-connector-backend plugin uses raw console.log and console.error for logging instead of the Backstage LoggerService. The plugin's init method receives a logger but doesn't pass it to setupInformer(), createRouter(), or any of the service modules. This means log output won't follow the backend's log configuration (level filtering, structured output, correlation IDs). Additionally, console.error(error) on line 516 of module.ts in the entity provider module should use the injected logger.

8. httpAuth declared as dependency but never used (medium)
In plugin.ts, httpAuth is declared as a dependency via deps: { httpAuth: coreServices.httpAuth, ... } but is not destructured in the init function nor passed to the router. This means the router endpoints (/list, /models/:model/:version, /modelcard/:sourceId/:modelName) are completely unauthenticated — any caller can access them without a valid Backstage service token. The httpAuth middleware should be applied to the router or individual routes.

9. KFMR code included despite design decision to remove it (medium)
Decision 4 in the design document explicitly states: "Remove all KFMR client code (Kfmr.ts, related types, KFMR-specific label handling in InformerService.ts)." Yet the PR includes the full Kfmr.ts (~837 lines) and extensive KFMR integration code in InformerService.ts. This contradicts the stated design and significantly increases the surface area. Either update the design doc to reflect the current approach, or remove the KFMR code as planned.

10. setupInformer() fire-and-forget with no lifecycle management (medium)
In plugin.ts, setupInformer() is called with .catch() but its result is not awaited or tracked. The informer starts a K8s watch + background polling interval, but there's no cleanup when the plugin shuts down (no AbortController, no clearInterval). The background polling timer is stored on the informer as (config.informer as any).__pollingTimer — a fragile pattern with no shutdown hook. This can cause resource leaks and orphaned timers.

11. Race condition in entityList concatenation (medium)
In ModelCatalogResourceEntityProvider.ts, entityList is a let variable modified inside Promise.all(catalogKeys.map(async key => { ... entityList = entityList.concat(...) })). While concat returns a new array (so the mutation pattern works), this is error-prone — if any future change introduces intermediate push calls, concurrency bugs will appear. Consider collecting results from Promise.all and flattening.


Low Findings

12. Boilerplate dev/index.ts references TODO template (low)
The dev/index.ts file contains template comments referencing "todos" endpoints (curl http://localhost:7007/api/kserve-kubeflow-connector/todos) from the Backstage plugin template. These should be updated or removed to avoid confusion.

13. package.json repository directory path mismatch (low)
The repository.directory in kserve-kubeflow-connector-backend/package.json is set to workspaces/ai-integrations/plugins/kserve-kubeflow-connector (missing -backend suffix). The actual directory is kserve-kubeflow-connector-backend.

14. Unused @ts-ignore comments and dead variables (low)
Kfmr.ts has multiple // @ts-ignore annotations on URI template constants (GET_REG_MODEL_URI, GET_SERVING_ENV_URI, etc.) that are declared but never used. These should be removed or the constants should be used in the actual URL construction instead of inline string replacement.

15. name field removed from config.d.ts but used in app-config.yaml (low)
The config.d.ts diff removes the name?: string field from the schema, but app-config.yaml adds name: my-k8s-cluster. This won't cause a runtime error (Backstage ignores unknown config keys by default), but it's misleading — the config schema should document what fields are actually consumed.


Verdict

request-changes — The config casing bug (#1) will prevent the baseUrl fallback from working. The uncached token (#2) will cause silent auth failures after ~1 hour. The unauthenticated router endpoints (#8) expose internal model data without authorization. These need to be resolved before merge.


Labels: PR introduces new kserve-kubeflow-connector-backend plugin and modifies ai-integrations workspace plugins

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added ai-integrations enhancement New feature or request labels Jul 7, 2026
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
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 7, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 7:36 PM UTC · Ended 7:49 PM UTC
Commit: 26407b3 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:36 PM UTC · Completed 7:49 PM UTC
Commit: 26407b3 · View workflow run →

@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Jul 7, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (5) 📘 Rule violations (1) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 14 rules

Grey Divider


Action required

1. Overbroad modelcard URL match 🐞 Bug ⛨ Security
Description
bridgePredicate matches URLs using substring checks on the pathname, and readUrl forwards a
bearer token (or RHDH_TOKEN) to any matched URL; if an untrusted party can influence the
TechDocs/modelcard URL, this can leak credentials and enable SSRF. This change also diverges from
existing unit tests that expect host/port-based matching semantics.
Code

workspaces/ai-integrations/plugins/catalog-techdoc-url-reader-backend/src/plugin.ts[R145-154]

  bridgePredicate = (url: URL): boolean => {
    for (let index = 0; index < this.bridgeConfigs.length; index++) {
      const bc = this.bridgeConfigs[index];
-      const bcUrl = new URL(bc.baseUrl);
-      if (
-        url.hostname === bcUrl.hostname &&
-        url.port === bcUrl.port &&
-        url.pathname.startsWith('/modelcard')
-      ) {
+      if (url.pathname.includes('modelcard') && url.pathname.includes(bc.id)) {
        return true;
      }
    }
-    if (
-      url.hostname === 'localhost' &&
-      url.port === '9090' &&
-      url.pathname.startsWith('/modelcard')
-    ) {
-      return true;
-    }
    return false;
  };
Relevance

⭐⭐⭐ High

Team previously accepted SSRF/URL-validation hardening when forwarding bearer tokens to
user-provided URLs.

PR-#2581

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The predicate no longer validates URL origin and the implementation forwards bearer tokens
(including an env override) to the provided URL; the unit tests still assert the old
localhost/configured-host behavior, showing the semantic mismatch.

workspaces/ai-integrations/plugins/catalog-techdoc-url-reader-backend/src/plugin.ts[145-187]
workspaces/ai-integrations/plugins/catalog-techdoc-url-reader-backend/src/plugin.test.ts[137-168]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The TechDocs URL reader predicate is too permissive and the reader unconditionally attaches an Authorization header to the requested URL (with an env var override). This can cause sensitive tokens to be sent to unintended hosts if the URL source is not fully trusted.

### Issue Context
Previously, matching was constrained by comparing host/port and a fixed path prefix; now it only checks `pathname.includes('modelcard') && pathname.includes(bc.id)`.

### Fix Focus Areas
- workspaces/ai-integrations/plugins/catalog-techdoc-url-reader-backend/src/plugin.ts[145-205]
- workspaces/ai-integrations/plugins/catalog-techdoc-url-reader-backend/src/plugin.test.ts[137-170]

### Suggested fix
- Tighten `bridgePredicate` to only match URLs whose origin (scheme/host/port) matches one of the configured `catalog.providers.modelCatalog.*.baseUrl` origins (or a discovered base URL), and whose pathname matches an expected prefix.
- Only attach Authorization headers when the destination origin is trusted (matches configured/discovered service origins).
- Remove `process.env.RHDH_TOKEN` override (or gate it behind an explicit `dangerouslyAllowStaticTokenForDev` config flag).
- Update unit tests to reflect the intended matching rules.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Modelcard route mismatch 🐞 Bug ≡ Correctness
Description
KFMR-generated TechDocs URLs use /modelcard/<singleKey>, but the connector router only serves
/modelcard/:sourceId/:modelName and looks up content using a two-segment key, so generated
model-card links will 404. This prevents TechDocs/model card rendering for KFMR-derived entities.
Code

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/services/Kfmr.ts[R619-631]

+  let techdocsUrl = getStringPropVal(PropertyKeys.TechDocsKey, mv, rm);
+  if (techdocsUrl === undefined) {
+    const replacer = (str: string) => str.replace(/ /g, '');
+    let modelCardKey = '';
+    for (const ma of mas) {
+      if (ma.modelSourceClass && ma.modelSourceName) {
+        modelCardKey =
+          replacer(ma.modelSourceClass) + replacer(ma.modelSourceName);
+        break;
+      }
+    }
+    techdocsUrl = `/modelcard/${modelCardKey}`;
+  }
Relevance

⭐⭐⭐ High

Team typically fixes guaranteed-404 correctness mismatches; no contrary historical evidence found.

PR-#1256

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The KFMR normalizer generates a single-segment /modelcard/<key> URL, but the router requires two
segments and composes a different lookup key, making successful retrieval impossible with the
generated URL.

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/services/Kfmr.ts[619-631]
workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/router.ts[51-56]
workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/services/InformerService.ts[418-433]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The model-card URL produced in KFMR normalization does not match the router path shape or the key used for lookup.

### Issue Context
- KFMR sets `techdocsUrl = /modelcard/${modelCardKey}` where `modelCardKey` is a single concatenated string.
- The router expects `/modelcard/:sourceId/:modelName` and calls `getModelCard(`${sourceId}/${modelName}`)`.

### Fix Focus Areas
- workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/services/Kfmr.ts[619-635]
- workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/router.ts[51-66]
- workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/services/InformerService.ts[418-452]

### Suggested fix
Choose one consistent scheme:
1) **Single-segment key**: Change the router to `GET /modelcard/:id` and look up `getModelCard(req.params.id)`.
OR
2) **Two-segment key**: Store keys as `${sourceId}/${modelName}` (preserve separation) and generate TechDocs URL as `/modelcard/${encodeURIComponent(sourceId)}/${encodeURIComponent(modelName)}`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Connector endpoints unauthenticated 🐞 Bug ⛨ Security
Description
The connector exposes /list, /models/*, and /modelcard/* without any auth enforcement, even
though upstream callers obtain/forward bearer tokens. If these endpoints are reachable beyond
trusted internal callers, this becomes an unintended data exposure surface.
Code

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/router.ts[R40-81]

+  // List all model catalog URIs (matching Go handleCatalogDiscoveryGet, server.go lines 162-182)
+  router.get('/list', async (_req, res) => {
+    try {
+      const discoveryResponse = getDiscoveryUris();
+      res.status(200).json(discoveryResponse);
+    } catch (error) {
+      console.error('Error getting discovery URIs:', error);
+      res.status(500).json({ error: 'Internal server error' });
+    }
+  });
+
+  router.get('/modelcard/:sourceId/:modelName', async (req, res) => {
+    try {
+      const sourceId = req.params.sourceId;
+      const modelName = req.params.modelName;
+      const modelCard = getModelCard(`${sourceId}/${modelName}`);
+      if (modelCard) {
+        res.setHeader('Content-Type', 'text/markdown');
+        res.status(200).send(modelCard);
+      } else {
+        res.status(404).json({ error: 'Not Found' });
+      }
+    } catch (error) {
+      console.error('Error getting model card:', error);
+      res.status(500).json({ error: 'Internal server error' });
+    }
+  });
+
+  router.get('/models/:model/:version', async (req, res) => {
+    try {
+      const key = `${req.params.model}/${req.params.version}`;
+      const modelCatalog = getModelCatalog(key);
+      if (modelCatalog) {
+        res.status(200).json(modelCatalog);
+      } else {
+        res.status(404).json({ error: 'Not Found' });
+      }
+    } catch (error) {
+      console.error('Error getting model catalog:', error);
+      res.status(500).json({ error: 'Internal server error' });
+    }
+  });
Relevance

⭐⭐⭐ High

Team has accepted multiple access-control hardening changes (auth/ownership enforcement) for backend
routes.

PR-#3559

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The router handlers return data directly and the plugin does not apply any auth middleware, despite
depending on httpAuth.

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/router.ts[40-81]
workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/plugin.ts[28-42]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The connector router does not validate incoming requests (no httpAuth / service auth checks), making all endpoints effectively public to any caller that can reach the backend.

### Issue Context
The plugin registers `httpAuth` as a dependency but does not use it in router setup.

### Fix Focus Areas
- workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/plugin.ts[28-43]
- workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/router.ts[27-83]

### Suggested fix
- Add auth middleware to validate credentials for all routes (e.g., require Backstage service-to-service auth).
- Plumb `httpAuth` into `createRouter` and call it per-request (or use a shared middleware).
- Decide and document whether endpoints are intended for internal-only calls; if so, still enforce auth to avoid accidental exposure.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (4)
4. TLS verification disabled 🐞 Bug ⛨ Security
Description
The KFMR client disables TLS certificate verification (rejectUnauthorized: false) for all
registry/catalog HTTP calls, which enables MITM attacks and can expose bearer tokens and model
metadata. This is unsafe for any non-dev environment.
Code

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/services/Kfmr.ts[R29-34]

+// Agent to skip TLS verification
+const tlsSkipAgent = new Agent({
+  connect: {
+    rejectUnauthorized: false,
+  },
+});
Relevance

⭐⭐ Medium

Repo shows mixed stance; TLS-skip hardening suggestions were rejected in boost dev scripts/config.

PR-#3648
PR-#3669

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The code explicitly disables TLS verification and uses that agent for model registry requests that
include an Authorization header.

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/services/Kfmr.ts[29-35]
workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/services/Kfmr.ts[88-100]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The KFMR REST client uses an HTTP agent that disables TLS certificate verification and applies it to fetches that include bearer tokens.

### Issue Context
`tlsSkipAgent` is configured with `rejectUnauthorized: false` and is passed as the fetch dispatcher.

### Fix Focus Areas
- workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/services/Kfmr.ts[29-35]
- workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/services/Kfmr.ts[88-110]

### Suggested fix
- Remove the custom agent and rely on default TLS verification.
- If skipping TLS is required for local dev, make it an explicit opt-in config flag (default false) and clearly document it; ensure production defaults remain secure.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Empty namespace list API 🐞 Bug ☼ Reliability
Description
The connector uses listNamespacedCustomObject with an empty namespace string to perform what
appears to be a cluster-wide InferenceService list, which is non-standard and can return empty
results or fail depending on the client/server. This can break KFMR↔KServe matching when the
informer cache is empty and the code falls back to an API list.
Code

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/services/InformerService.ts[R224-234]

+    const response = await client.listNamespacedCustomObject(
+      inference_service_group,
+      inference_service_version,
+      '',
+      inference_service_plural,
+      undefined,
+      undefined,
+      undefined,
+      undefined,
+      labelSelector,
+    );
Relevance

⭐⭐ Medium

No historical evidence about Kubernetes empty-namespace listNamespacedCustomObject vs
listClusterCustomObject usage.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The fallback path explicitly calls the namespaced list API with '' namespace, while the informer
itself is set up using a cluster list API, implying the fallback is inconsistent with intended
behavior.

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/services/InformerService.ts[173-236]
workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/services/InformerService.ts[1296-1307]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`listInferenceServices` falls back to `listNamespacedCustomObject(..., '', ...)` when the informer cache is empty, which is a fragile way to do cluster-wide listing.

### Issue Context
Elsewhere, the informer is correctly created using `listClusterCustomObject`, indicating the intent is cluster-wide scope.

### Fix Focus Areas
- workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/services/InformerService.ts[173-246]

### Suggested fix
- Replace the fallback call with `client.listClusterCustomObject(...)` and pass the label selector appropriately.
- Add an integration/unit test (or mock) ensuring the fallback path returns items when cache is empty.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Wrong baseUrl config key ✓ Resolved 🐞 Bug ≡ Correctness
Description
readModelCatalogApiEntityConfig checks for baseurl instead of baseUrl, so configured URLs are
silently ignored and the provider can end up using an empty URL or unintended discovery fallback.
This breaks connectivity for users who configure catalog.providers.modelCatalog.*.baseUrl.
Code

workspaces/ai-integrations/plugins/catalog-backend-module-model-catalog/src/providers/config.ts[R46-49]

+  let baseUrl = '';
+  if (config.has('baseurl')) {
+    baseUrl = config.getString('baseurl');
+  }
Relevance

⭐⭐ Medium

No clear evidence on baseUrl key casing; other casing-alignment suggestions have been rejected.

PR-#2742

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The provider config reader only looks for baseurl, while the example configuration uses baseUrl,
so the configured value will not be loaded.

workspaces/ai-integrations/plugins/catalog-backend-module-model-catalog/src/providers/config.ts[42-50]
workspaces/ai-integrations/app-config.yaml[94-109]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The model-catalog provider config reader uses the wrong key casing (`baseurl`), causing `baseUrl` to never be read from `app-config.yaml`.

### Issue Context
`app-config.yaml` uses `baseUrl`, and the generated `config.d.ts` also documents `baseUrl`.

### Fix Focus Areas
- workspaces/ai-integrations/plugins/catalog-backend-module-model-catalog/src/providers/config.ts[42-61]

### Suggested fix
- Change `config.has('baseurl')/getString('baseurl')` to `config.has('baseUrl')/getString('baseUrl')`.
- (Optional for backwards compatibility) Support both `baseUrl` and legacy `baseurl`, preferring `baseUrl`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. kserve-kubeflow-connector lacks frontend/common 📘 Rule violation ⌂ Architecture
Description
The PR introduces @red-hat-developer-hub/backstage-plugin-kserve-kubeflow-connector-backend as a
backend-only plugin without corresponding frontend and common packages for the same feature. This
violates the required Backstage plugin package split and risks forcing shared types/contracts into
non-common locations later.
Code

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/package.json[R18-25]

+  "backstage": {
+    "role": "backend-plugin",
+    "pluginId": "kserve-kubeflow-connector",
+    "pluginPackage": "@backstage/plugin-backend",
+    "pluginPackages": [
+      "@red-hat-developer-hub/backstage-plugin-backend-kserve-kubeflow-connector"
+    ]
+  },
Relevance

⭐⭐ Medium

No historical evidence enforcing required frontend/common split for backend-only plugins in this
repo.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2531 requires a plugin to have distinct frontend, backend, and common packages. The
added kserve-kubeflow-connector-backend package explicitly declares itself as a backend-plugin
for pluginId kserve-kubeflow-connector, and it is wired into the running backend, but no
corresponding frontend/common package is introduced for this plugin feature.

Rule 2531: Backstage plugins must be split into frontend, backend, and common packages
workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/package.json[18-25]
workspaces/ai-integrations/packages/backend/src/index.ts[85-89]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `kserve-kubeflow-connector` feature is introduced only as a backend plugin package, but compliance requires plugins to be split into frontend, backend, and common packages.

## Issue Context
The new package declares `backstage.role: backend-plugin` for `pluginId: kserve-kubeflow-connector`, and it is added to the app backend. A corresponding frontend package and a framework-agnostic common package should be created (even if the frontend is minimal) and any shared contracts/types should live in the common package.

## Fix Focus Areas
- workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/package.json[18-25]
- workspaces/ai-integrations/packages/backend/src/index.ts[85-89]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

8. Stale cached request token ✓ Resolved 🐞 Bug ☼ Reliability
Description
ModelCatalogResourceEntityProvider caches the plugin request token in backendToken and never
refreshes it; if tokens are rotated/expire, scheduled syncs can start failing until restart.
Refreshing per run or on 401 makes the provider more resilient.
Code

workspaces/ai-integrations/plugins/catalog-backend-module-model-catalog/src/providers/ModelCatalogResourceEntityProvider.ts[R176-181]

+    if (this.backendToken === undefined) {
+      this.backendToken = await this.auth.getPluginRequestToken({
+        onBehalfOf: await this.auth.getOwnServiceCredentials(),
+        targetPluginId: this.name,
+      });
+    }
Relevance

⭐ Low

Similar “refresh cached token / retry on 401” suggestion was rejected in Kagenti providers review.

PR-#3648

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The provider only calls getPluginRequestToken when backendToken is undefined, whereas other code
(ModelService) fetches a token for each request, indicating caching may be inconsistent with
expected token lifetimes.

workspaces/ai-integrations/plugins/catalog-backend-module-model-catalog/src/providers/ModelCatalogResourceEntityProvider.ts[169-182]
workspaces/ai-integrations/plugins/ai-experience-backend/src/services/ModelService/ModelService.ts[46-54]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The entity provider requests a plugin token once and reuses it indefinitely, which is fragile if the auth implementation returns time-limited tokens.

### Issue Context
Other code in this workspace retrieves a fresh plugin token per operation.

### Fix Focus Areas
- workspaces/ai-integrations/plugins/catalog-backend-module-model-catalog/src/providers/ModelCatalogResourceEntityProvider.ts[169-199]

### Suggested fix
- Fetch a fresh token on each `run()` invocation, or
- Retry token acquisition when requests fail with 401/403 and update `this.backendToken`.
- Ensure logs do not include token values.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

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
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 7, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 8:00 PM UTC · Ended 8:06 PM UTC
Commit: 26407b3 · View workflow run →

@rhdh-qodo-merge

Copy link
Copy Markdown

PR Summary by Qodo

feat: introduce plugin-only kserve-kubeflow-connector-backend (RHIDP-15199)

✨ Enhancement 📝 Documentation 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Adds new kserve-kubeflow-connector-backend plugin: TypeScript K8s Informer on KServe
 InferenceService plus a KubeFlow Model Registry/Catalog REST client.
• Refactors model-catalog entity provider to use Backstage discovery + service-to-service auth
 tokens.
• Updates TechDocs URL reader to authenticate model-card reads and loosen URL matching.
• Mirrors OpenSpec proposal/design for transitioning from sidecar connector to plugin-only
 connector.
• Regenerates API reports and tweaks dev config/prettier ignores for local workflows.
Diagram

graph TD
  K8S[("KServe CRs")] --> INFORMER["InformerService"] --> ROUTER["Connector Router"]
  KFMR{{"KubeFlow APIs"}} --> INFORMER
  ENTPROV["ModelCatalog EntityProvider"] -->|"discovery + auth"| ROUTER
  ENTPROV --> CATALOG[("Backstage Catalog")]
  TECHDOC["TechDocs URL Reader"] -->|"Bearer token"| ROUTER
  subgraph Legend
    direction LR
    _db[(Database/CR)] ~~~ _svc([Service]) ~~~ _ext{{External}}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Ship KServe-only first; defer KFMR code
  • ➕ Reduces review surface and risk (large KFMR port is slated for removal per design)
  • ➕ Lets reviewers focus on core informer + connector REST contract and auth/discovery wiring
  • ➖ Requires splitting PR/commits and follow-up work to reintroduce validated Model Catalog support
2. Merge as-is (current approach)
  • ➕ Keeps the prototype behavior intact for demo/validation
  • ➕ All moving parts (router, entity provider, techdocs reader) land together
  • ➖ Large amount of code to review including code intended to be removed
  • ➖ Known auth workaround (RHDH_TOKEN fallback) and console.log-heavy logging may ship temporarily

Recommendation: The discovery + service-auth based integration is the right direction and aligns with Backstage patterns, but consider reducing scope by landing the KServe-only path first and gating KFMR-related code to a follow-up once RHOAI 2.25+ Model Catalog independence is validated. This would materially lower review complexity and avoid merging code explicitly marked for deletion.

Files changed (30) +3686 / -57

Enhancement (11) +2860 / -31
ModelCatalogGenerator.tsSupport service URL prefix for TechDocs refs +12/-1

Support service URL prefix for TechDocs refs

• Optionally prefixes TechDocs URLs with the discovered connector base URL when generating entities.

workspaces/ai-integrations/plugins/catalog-backend-module-model-catalog/src/clients/ModelCatalogGenerator.ts

module.tsInject discovery/auth into provider module init +12/-6

Inject discovery/auth into provider module init

• Adds discovery and auth core services to module deps and passes them into provider construction.

workspaces/ai-integrations/plugins/catalog-backend-module-model-catalog/src/module.ts

ModelCatalogResourceEntityProvider.tsDiscover connector URL and mint plugin request token +38/-9

Discover connector URL and mint plugin request token

• Resolves runtime service URL via discovery per refresh, mints/caches a plugin request token via AuthService, and uses these for fetching model catalogs and generating entities.

workspaces/ai-integrations/plugins/catalog-backend-module-model-catalog/src/providers/ModelCatalogResourceEntityProvider.ts

plugin.tsAuthenticate model-card reads and adjust bridge predicate +43/-15

Authenticate model-card reads and adjust bridge predicate

• Adds auth/discovery deps and sends bearer auth when fetching model card content; bridge matching now keys off path/id, with an env-token fallback for authorization.

workspaces/ai-integrations/plugins/catalog-techdoc-url-reader-backend/src/plugin.ts

package.jsonCreate new backend plugin package manifest +58/-0

Create new backend plugin package manifest

• Defines the new plugin package, pluginId, scripts, and dependencies including @kubernetes/client-node and undici.

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/package.json

index.tsExport plugin entrypoint +16/-0

Export plugin entrypoint

• Re-exports the plugin definition as the package default export.

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/index.ts

plugin.tsRegister backend plugin and start informer/router +45/-0

Register backend plugin and start informer/router

• Creates the backend plugin feature, starts the informer on init, and mounts the REST router.

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/plugin.ts

router.tsExpose REST endpoints for list/models/modelcard +84/-0

Expose REST endpoints for list/models/modelcard

• Implements /list, /models/:model/:version, and /modelcard endpoints backed by in-memory state maintained by the informer service.

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/router.ts

InformerService.tsImplement K8s informer-based reconciliation and in-memory stores +1404/-0

Implement K8s informer-based reconciliation and in-memory stores

• Large port of the original controller logic: watches InferenceServices, optionally correlates with KFMR data, generates ModelCatalog payloads and model cards, and maintains them in memory for router reads.

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/services/InformerService.ts

KServe.tsGenerate ModelCatalog output from KServe InferenceServices +311/-0

Generate ModelCatalog output from KServe InferenceServices

• Converts KServe InferenceService specs/annotations into Model and ModelServer objects with tags, URLs, and optional TechDocs refs.

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/services/KServe.ts

Kfmr.tsAdd KFMR client + normalization logic (prototype carryover) +837/-0

Add KFMR client + normalization logic (prototype carryover)

• Implements a KFMR REST client and model catalog/model card helpers ported from the bridge; design docs indicate this will be removed or reduced later.

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/services/Kfmr.ts

Bug fix (2) +21 / -3
BridgeResourceConnector.tsAdd bearer auth + /models endpoint to bridge client +17/-2

Add bearer auth + /models endpoint to bridge client

• Adds Authorization headers to fetch calls and updates model fetch URL shape; allows optional token for model fetch.

workspaces/ai-integrations/plugins/catalog-backend-module-model-catalog/src/clients/BridgeResourceConnector.ts

config.tsRelax baseUrl parsing +4/-1

Relax baseUrl parsing

• Makes baseUrl optional in config parsing (but currently checks a lowercase key), enabling discovery-driven mode.

workspaces/ai-integrations/plugins/catalog-backend-module-model-catalog/src/providers/config.ts

Refactor (1) +425 / -0
types.tsAdd shared type definitions and constants +425/-0

Add shared type definitions and constants

• Provides shared Route/KFMR/KServe/ModelCatalog types to avoid circular imports between services.

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/src/services/types.ts

Tests (3) +104 / -5
BridgeResourceConnector.test.tsUpdate client tests for /models path and auth token +9/-5

Update client tests for /models path and auth token

• Adjusts mocked URLs and call signatures to pass bearer tokens and use /models endpoints.

workspaces/ai-integrations/plugins/catalog-backend-module-model-catalog/src/clients/BridgeResourceConnector.test.ts

ModelCatalogResourceEntityProvider.test.tsUpdate provider tests for discovery/auth deps +20/-0

Update provider tests for discovery/auth deps

• Adds mocks for discovery.getBaseUrl and auth token minting used during refresh.

workspaces/ai-integrations/plugins/catalog-backend-module-model-catalog/src/providers/ModelCatalogResourceEntityProvider.test.ts

index.tsAdd standalone dev backend harness +75/-0

Add standalone dev backend harness

• Adds a minimal dev backend wiring with mocked auth/httpAuth and a mock catalog for standalone development.

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/dev/index.ts

Documentation (7) +246 / -12
.openspec.yamlAdd OpenSpec change metadata +3/-0

Add OpenSpec change metadata

• Introduces OpenSpec metadata for the transition to a plugin-only connector.

workspaces/ai-integrations/openspec/changes/transition-oai-connector-to-kserve-plugin/.openspec.yaml

design.mdAdd design for sidecar→plugin connector transition +117/-0

Add design for sidecar→plugin connector transition

• Documents goals, key architectural decisions (discovery/auth, two-plugin split, entity modeling), and risks/migration plan.

workspaces/ai-integrations/openspec/changes/transition-oai-connector-to-kserve-plugin/design.md

proposal.mdAdd proposal for plugin-only connector +61/-0

Add proposal for plugin-only connector

• Captures motivation, starting point, intended scope, and impacted packages for the transition work.

workspaces/ai-integrations/openspec/changes/transition-oai-connector-to-kserve-plugin/proposal.md

report.api.mdRefresh ai-experience API report +11/-11

Refresh ai-experience API report

• Updates the generated API report due to translation ref key changes/reordering.

workspaces/ai-integrations/plugins/ai-experience/report.api.md

report.api.mdRefresh API report for tokenized fetch functions +9/-1

Refresh API report for tokenized fetch functions

• Reflects new token parameters and signature changes in client functions.

workspaces/ai-integrations/plugins/catalog-backend-module-model-catalog/report.api.md

README.mdAdd connector backend README +32/-0

Add connector backend README

• Provides templated installation and development instructions for the new plugin package.

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/README.md

report.api.mdAdd generated API report for connector backend +13/-0

Add generated API report for connector backend

• API Extractor output exposing the BackendFeature default export.

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/report.api.md

Other (6) +30 / -6
.prettierignoreIgnore dist-dynamic directory +1/-0

Ignore dist-dynamic directory

• Adds dist-dynamic to prettier ignore list to avoid formatting generated bundles.

workspaces/ai-integrations/.prettierignore

app-config.yamlAdjust local TechDocs and model catalog provider config +6/-4

Adjust local TechDocs and model catalog provider config

• Runs TechDocs generator locally (not Docker) for dev workflows and updates model catalog provider configuration to reference the connector plugin id with a name field rather than a fixed baseUrl.

workspaces/ai-integrations/app-config.yaml

package.jsonDepend on new connector backend plugin +1/-0

Depend on new connector backend plugin

• Adds the kserve-kubeflow connector backend package as a workspace dependency.

workspaces/ai-integrations/packages/backend/package.json

index.tsRegister connector backend feature +5/-0

Register connector backend feature

• Wires the connector backend plugin into the backend app via dynamic import registration.

workspaces/ai-integrations/packages/backend/src/index.ts

config.d.tsMake model catalog baseUrl optional +1/-2

Make model catalog baseUrl optional

• Updates config typing to allow discovery-based routing without requiring a configured baseUrl.

workspaces/ai-integrations/plugins/catalog-backend-module-model-catalog/config.d.ts

.eslintrc.jsAdd ESLint config for new plugin +16/-0

Add ESLint config for new plugin

• Backstage CLI eslint factory config for the connector backend package.

workspaces/ai-integrations/plugins/kserve-kubeflow-connector-backend/.eslintrc.js

@rhdh-qodo-merge rhdh-qodo-merge Bot added documentation Improvements or additions to documentation Tests Other labels Jul 7, 2026
fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:00 PM UTC · Completed 8:06 PM UTC
Commit: 26407b3 · View workflow run →

- 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>
@gabemontero

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review. Here's how we've addressed each finding:

Fixed in this PR (commit 4b5ddad)

# Finding Resolution
1 Config key case mismatch (baseurl vs baseUrl) Fixed — changed to baseUrl to match the camelCase key in config.d.ts.
2 Backend token cached indefinitely Fixed — removed the backendToken instance field; now calls auth.getPluginRequestToken() fresh each run() invocation.
13 package.json repository directory path mismatch Fixed — corrected repository.directory and backstage.pluginPackages via yarn backstage-cli repo fix --publish.

Additionally fixed

  • Inconsistent modelCardKey replacer (InformerService.ts Path 2, line 589) — now uses replacer() to strip spaces, matching Path 1 behavior.
  • Failing bridgePredicate test — updated test to provide a config entry with a matching id for the path-based URL predicate.

Deferred to follow-up PRs

These are acknowledged and tracked but out of scope for this initial connector introduction:

# Finding Rationale
3 TLS verification disabled (rejectUnauthorized: false) Intentional for dev/internal cluster use with self-signed certs in OpenShift. Will make configurable.
4 RHDH_TOKEN env var bypass Documented workaround for backend-to-backend auth in the current deployment model.
5 No unit tests for InformerService Test coverage for informer and KFMR processing logic planned as a separate effort.
6 KFMR code vs design Decision 4 Design doc describes the target architecture; KFMR is still needed for existing deployments. Phased removal is on the roadmap.
7 console.log instead of Backstage logger Will thread the LoggerService through all service functions in a follow-up.
8 Unauthenticated router endpoints / httpAuth unused Authentication for connector REST endpoints planned for follow-up. Current deployment relies on network-level access control.
9 Static mutable state for discovery/auth Refactoring to instance-level injection planned as part of techdoc-url-reader cleanup.
10 setupInformer() lifecycle / readiness race Adding proper lifecycle management and readiness gate deferred to follow-up.
11 entityList concatenation in Promise.all Acknowledged — works correctly today but will refactor to collect from Promise.all results.
12 Boilerplate dev/index.ts TODO references Will clean up template comments.
14 Unused @ts-ignore and dead variables in Kfmr.ts Will clean up when KFMR code is refactored.
15 name field in app-config.yaml vs config.d.ts Config schema will be updated to document consumed fields.

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 7, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 8:29 PM UTC · Ended 8:36 PM UTC
Commit: 26407b3 · View workflow run →

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 65.21739% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.54%. Comparing base (26407b3) to head (4bccdc9).
✅ All tests successful. No failed tests found.

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              
Flag Coverage Δ *Carryforward flag
adoption-insights 83.70% <ø> (ø) Carriedforward from 0fd2799
ai-integrations 67.24% <65.21%> (-1.11%) ⬇️
app-defaults 69.79% <ø> (ø) Carriedforward from 0fd2799
augment 46.39% <ø> (ø) Carriedforward from 0fd2799
boost 74.68% <ø> (ø) Carriedforward from 0fd2799
bulk-import 72.46% <ø> (ø) Carriedforward from 0fd2799
cost-management 14.10% <ø> (ø) Carriedforward from 0fd2799
dcm 61.81% <ø> (ø) Carriedforward from 0fd2799
extensions 61.53% <ø> (ø) Carriedforward from 0fd2799
global-floating-action-button 71.18% <ø> (ø) Carriedforward from 0fd2799
global-header 59.71% <ø> (ø) Carriedforward from 0fd2799
homepage 50.23% <ø> (ø) Carriedforward from 0fd2799
install-dynamic-plugins 56.77% <ø> (ø) Carriedforward from 0fd2799
konflux 91.49% <ø> (ø) Carriedforward from 0fd2799
lightspeed 68.81% <ø> (ø) Carriedforward from 0fd2799
mcp-integrations 85.46% <ø> (ø) Carriedforward from 0fd2799
orchestrator 42.97% <ø> (ø) Carriedforward from 0fd2799
quickstart 65.63% <ø> (ø) Carriedforward from 0fd2799
sandbox 79.56% <ø> (ø) Carriedforward from 0fd2799
scorecard 82.67% <ø> (ø) Carriedforward from 0fd2799
theme 61.26% <ø> (ø) Carriedforward from 0fd2799
translations 7.25% <ø> (ø) Carriedforward from 0fd2799
x2a 78.68% <ø> (ø) Carriedforward from 0fd2799

*This pull request uses carry forward flags. Click here to find out more.


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 26407b3...4bccdc9. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure · Started 8:29 PM UTC · Completed 8:36 PM UTC
Commit: 26407b3 · View workflow run →

@gabemontero

Copy link
Copy Markdown
Contributor Author

🤖 Finished Review · ❌ Failure · Started 8:29 PM UTC · Completed 8:36 PM UTC Commit: 26407b3 · View workflow run →

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

@gabemontero

Copy link
Copy Markdown
Contributor Author

Response to Qodo review

Already fixed (qodo confirms with ✓ Resolved):

Already deferred (same findings as fullsend review, see earlier response):

Not actionable for this PR:

  • Version Packages (test-workflows) #7 Missing frontend/common packages — this is a backend-only connector plugin by design. The existing ai-experience and ai-experience-common packages already serve as the frontend and shared types for this feature. A dedicated frontend/common split for the connector isn't needed.

New findings — deferring both:

  • build(deps): bump micromatch from 4.0.7 to 4.0.8 #1 Overbroad modelcard URL match — the switch from host+port matching to path-based matching was intentional because discovery URLs aren't known at predicate construction time. The SSRF/credential-leak framing is overblown since these URLs originate from Backstage's internal catalog annotations, not user input. Tightening the predicate requires a design change and is tracked for follow-up.
  • build(deps): bump express from 4.19.2 to 4.21.1 #2 Modelcard route mismatch (KFMR generates /modelcard/<singleKey> but router expects /modelcard/:sourceId/:modelName) — this is a good catch but only affects the KFMR code path, which is slated for removal per design Decision 4. Integration testing on the KServe-only path (the target architecture) passes. Will be moot once KFMR is removed.

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>
@gabemontero

Copy link
Copy Markdown
Contributor Author

Response to SonarQube duplicate code findings

dev/index.ts (46.1%, 35 lines) — Fixed in 0fd2799
Stripped the Backstage CLI template boilerplate: removed scaffold comments referencing a non-existent "todos" API, removed the unused mock catalog entity. Down to just the minimal dev backend setup.

InformerService.ts (10.3%, 145 lines) — Deferring to follow-up
The duplication is real and comes from four patterns in the reconciliation logic (ported from Go):

Pattern Occurrences ~Lines
Model card retrieval (loop artifacts, get card, build key) ~78
Timestamp computation (lastUpdateTimeSinceEpoch max) ~21
callBackstagePrinters + result assembly (Path 1 vs Path 2) ~65
Metadata storage create-or-update (catalog vs card) ~64

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.

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 7, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 8:53 PM UTC · Ended 9:03 PM UTC
Commit: 26407b3 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:53 PM UTC · Completed 9:03 PM UTC
Commit: 26407b3 · View workflow run →

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>
@sonarqubecloud

sonarqubecloud Bot commented Jul 7, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
4.3% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 7, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:25 PM UTC · Completed 9:31 PM UTC
Commit: 26407b3 · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-integrations documentation Improvements or additions to documentation enhancement New feature or request Other Tests workspace/ai-integrations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant