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

[gabemontero]

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)

[rhdh-gh-app]

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]

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

[fullsend-ai-review]

Review — kserve-kubeflow-connector-backend introduction

This PR introduces a new kserve-kubeflow-connector-backend Backstage plugin (~2,700 lines) that ports the Go-based sidecar connector for KServe/KubeFlow Model Registry to TypeScript. It also modifies the existing catalog-backend-module-model-catalog and catalog-techdoc-url-reader-backend plugins to communicate via Backstage discovery and auth services instead of direct HTTP calls.

The architectural direction (eliminating sidecars, using Backstage plugin APIs) is sound and well-documented via the included openspec design documents. However, there are several security, correctness, and quality issues that should be addressed before merging.

---

Critical

1. TLS certificate verification globally disabled (Kfmr.ts)

The file creates a module-level undici.Agent with rejectUnauthorized: false, which is then used for all HTTP requests to KubeFlow Model Registry endpoints. This disables TLS certificate verification entirely, enabling man-in-the-middle attacks in production.

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

This should be configurable (opt-in for development only, never the default) or removed entirely. If self-signed certificates are needed, the correct approach is to supply a custom CA bundle.

---

High

2. Static admin token bypass in techdoc URL reader (plugin.ts)

The techdoc URL reader falls back to a static token from process.env.RHDH_TOKEN, bypassing Backstage's service-to-service authentication:

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

The inline TODO comments acknowledge this is a workaround for a 401 issue. Shipping this workaround means any environment variable leak exposes an admin token that bypasses all auth checks. The root cause of the 401 (likely targetPluginId mismatch — the code passes urlReaderFactoriesServiceRef.id instead of the connector's plugin ID) should be debugged and fixed.

3. No unit tests for ~2,700 lines of new plugin code

The entire kserve-kubeflow-connector-backend plugin has zero test files. The services (InformerService.ts — 1,406 lines, KServe.ts — 311 lines, Kfmr.ts — 837 lines) contain complex reconciliation logic, Kubernetes API interactions, and data transformation that are all untested. The design document itself notes "Additional unit tests for the converted golang→typescript code" as a known TODO.

4. Module-level mutable global state without concurrency safety (InformerService.ts)

The modelCatalog and modelCards Maps are module-level globals mutated by concurrent async informer callbacks (add, update, delete events) and a periodic polling timer:

const modelCards = new Map<string, ModelCardMetadata>();
const modelCatalog = new Map<string, ModelCatalogMetadata>();

While JavaScript is single-threaded, the async interleaving of innerStart (which deletes stale keys) with individual reconcile callbacks (which add/update keys) can produce race conditions where keys are deleted and immediately re-added, or where the keys array in innerStart is incomplete because a reconcile is still in progress. These Maps should be managed through a service with controlled mutation points.

---

Medium

5. KFMR code included despite design document calling for its removal

Decision 4 in the design document (design.md) explicitly states: "Remove all KFMR client code (Kfmr.ts, related types, KFMR-specific label handling in InformerService.ts)." Yet the PR includes Kfmr.ts (837 lines) and extensive KFMR processing in InformerService.ts. If the intent is to keep this code temporarily, the design document should be updated to reflect that. If the intent is to follow the design, this code should be removed.

6. console.log/console.error used throughout instead of Backstage LoggerService

The new plugin has 100+ calls to console.log and console.error across all service files. The plugin's init method receives a logger dependency but never passes it to the services. This means log output bypasses Backstage's structured logging, won't include plugin context, and won't respect log-level configuration.

7. Empty namespace passed to listNamespacedCustomObject (Kfmr.ts)

When ns === '', the code calls listNamespacedCustomObject with an empty string as the namespace parameter. This is likely incorrect — for cluster-wide listing, listClusterCustomObject should be used instead.

8. Multiple @ts-ignore directives hiding type issues (Kfmr.ts)

Several @ts-ignore comments suppress TypeScript errors for unused variables and type mismatches (lines with GET_REG_MODEL_URI, GET_MODEL_ARTIFACT_URI, and others that are defined but never used). These mask potential issues and should be cleaned up — unused constants should be removed.

9. Authorization: Bearer sent with empty token (BridgeResourceConnector.ts)

When fetchModelCatalogFromKey is called without a token, the code sends Authorization: Bearer (empty string after Bearer). This should either omit the header entirely or require the token:

let tok = '';
if (token) {
  tok = token?.token;
}

10. Boilerplate README still present

The new plugin's README.md contains template text: "You should replace this text with a description of your plugin backend." This should describe the plugin's purpose, configuration, and usage.

---

Low

11. Duplicated types instead of resolving package dependency (types.ts)

The types file contains a TODO comment: "b GGM TODO so I could not get the @redhat-ai-dev/model-catalog-types node module dependency to reconcile...so duplicating for now." This creates a maintenance burden where the canonical types and duplicated types can drift apart.

12. setupInformer() error handling is fire-and-forget (plugin.ts)

The plugin's init method calls setupInformer().catch(...) without awaiting the result. If the informer fails to start (e.g., no kubeconfig available), the plugin will appear healthy but serve empty data from the router endpoints. At minimum, the error should be surfaced more prominently.

13. Polling timer stored on informer via type assertion

The polling timer is attached to the informer using a type assertion: (config.informer as any).__pollingTimer = setInterval(...). This is fragile and provides no cleanup mechanism. The timer should be managed separately.

14. listInferenceServices fallback calls listNamespacedCustomObject with empty namespace

Similar to finding 7 — the fallback API call in InformerService.ts calls listNamespacedCustomObject with '' as namespace, which should be listClusterCustomObject.

---

Labels: PR introduces a new backend plugin and modifies AI integration plugins with security-relevant auth changes

Previous run

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 comments — Kfmr.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 type — KServe.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 (2)

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 (3)

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 (4)

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 (5)

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 baseurl → baseUrl 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 (6)

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]

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

[gabemontero]

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]

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

[codecov]

Codecov Report

❌ Patch coverage is 67.39130% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.54%. Comparing base (26407b3) to head (d1a51cc).
⚠️ Report is 12 commits behind head on main.
✅ 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    25132       -3     
==========================================
+ Hits        48953    48959       +6     
- Misses      40482    40503      +21     
+ Partials      300      290      -10     

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

*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...d1a51cc. 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]

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

[gabemontero]

🤖 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]

Response to Qodo review

Already fixed (qodo confirms with ✓ Resolved):

• Add a test workspace to check if CI/CD workflows are working as expected #6 baseurl → baseUrl casing — fixed in 4b5ddad
• Migrate openshift image registry #8 Stale cached request token — fixed in 4b5ddad (fresh token each run())

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

• build(deps): bump elliptic from 6.5.5 to 6.5.7 #3 Unauthenticated connector endpoints (httpAuth unused)
• build(deps): bump webpack from 5.91.0 to 5.95.0 #4 TLS verification disabled (rejectUnauthorized: false)
• build(deps): bump rollup from 4.14.1 to 4.24.0 #5 Empty namespace listNamespacedCustomObject — tracked for follow-up

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.

[gabemontero]

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)	3×	~78
Timestamp computation (lastUpdateTimeSinceEpoch max)	3×	~21
callBackstagePrinters + result assembly (Path 1 vs Path 2)	2×	~65
Metadata storage create-or-update (catalog vs card)	2×	~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]

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

[fullsend-ai-review]

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

[fullsend-ai-review]

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

[johnmcollier]

Didn't see any red flags that weren't already raised by fullsend's review, so approving from my end

[rajin-kichannagari]

Fullsend's review covers the main issues I saw. One small bug I noticed was that the fetchModelCatalogFromKey error test in BridgeResourceConnector.test.ts actually calls fetchModelCatalogKeys. Looks like it was pre-existing but otherwise lgtm.

[gabemontero]

@rajin-kichannagari Good catch! You're right — the error test in the fetchModelCatalogFromKey describe block was calling fetchModelCatalogKeys instead. Fixed in d1a51cc: changed the test to call fetchModelCatalogFromKey('errorTest', 'some-key', ...) and extended the fetch mock to return a 401 for errorTest/models/* URLs so the error path is properly exercised. All 19 tests pass.

[fullsend-ai-review]

🤖 Review · ❌ Terminated · Started 3:44 PM UTC · Ended 3:50 PM UTC
Commit: f91c274 · View workflow run →

[sonarqubecloud]

Quality Gate failed

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

See analysis details on SonarQube Cloud

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:44 PM UTC · Completed 3:50 PM UTC
Commit: f91c274 · View workflow run →