Repository navigation
feat(npx-skill-connector): add service auth, integration tests, and catalog provider docs - #5497
fullsend-ai-coder[bot] wants to merge 1 commit into
Conversation
…atalog provider docs Add httpAuth dependency to the npx skill connector plugin to make service-to-service authentication intent explicit. Add per-source diagnostic logging at startup for operational visibility. Add startConnector test helper and NpxConnectorTestConfig type to testUtils following the OCI connector pattern. Add comprehensive integration test suite covering service auth enforcement, unknown source handling, snapshot lifecycle (loading/ready/empty/failed/partial states), source isolation, and scheduled refresh. Expand README with catalog provider integration guide, combined OCI+npx source configuration, and RHESS demo instructions. Ref: RHIDP-18493 Assisted-by: Claude Opus 4.6 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Missing ChangesetsThe following package(s) are changed by this PR but do not have a changeset:
See CONTRIBUTING.md for more information about how to add changesets. Changed Packages
|
PR Summary by QodoDocument and test authenticated npx skill snapshots
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
|
🤖 Review · Commit: |
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
|
Code Review by Qodo 1. Visitors can read skill snapshots without auth
|
|
Important The |
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 10:39 PM UTC · Completed 11:00 PM UTC Commit: Runtime: claude · Model: claude-opus-4-6 · Effort: high · Cost: $6.79 |
|
Risk Assessment: moderate (2/5) DetailsBot-authored PR adding integration tests and service auth docs to a nascent plugin; the sole elevated signal is a large blast radius from the script, but all other Tier 1 and Tier 2 signals are low, yielding a composite of ~1.60 which rounds to moderate (2). |
ReviewFindingsMedium
Low
Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| env.registerInit({ | ||
| deps: { | ||
| httpRouter: coreServices.httpRouter, | ||
| httpAuth: coreServices.httpAuth, |
There was a problem hiding this comment.
[medium] implicit auth enforcement / code clarity
httpAuth is declared in deps but not destructured in init. In Backstage new backend system, declaring httpAuth activates automatic service-to-service auth enforcement (confirmed by integration tests and the identical pattern in kserve-kubeflow-connector-backend). However, since every other dep in this plugin IS destructured and used, the pattern is not self-documenting. An inline comment or explicit destructuring would clarify intent.
Suggested fix: Add an inline comment explaining the implicit auth mechanism, or destructure httpAuth and pass it to createRouter for explicit credential checks.
| } | ||
|
|
||
| for (const source of sources) { | ||
| logger.info( |
There was a problem hiding this comment.
[low] information disclosure
Source configuration including indexUrl and schedule is logged at INFO level on startup. If an indexUrl ever contains query parameters with credentials, these would be written to logs.
Suggested fix: Log only the source id and sanitized URL, or use DEBUG level.
| }); | ||
| } | ||
|
|
||
| for (const source of sources) { |
There was a problem hiding this comment.
[low] Code organization
A second for...of loop over sources was introduced solely for per-source logging. The scheduling loop immediately above already iterates the same array; the log could be inlined there.
Suggested fix: Move the logger.info call inside the existing scheduler.scheduleTask loop.
| * limitations under the License. | ||
| */ | ||
|
|
||
| import type { BackendFeature } from '@backstage/backend-plugin-api'; |
There was a problem hiding this comment.
[low] Naming conventions
BackendFeature and LoggerService are imported from @backstage/backend-plugin-api in two separate import type statements. The codebase convention is to merge imports from the same module.
Suggested fix: Combine into: import type { BackendFeature, LoggerService } from @backstage/backend-plugin-api;
|
/fs-fix address review comments and fix failing ci |
|
🤖 Finished Fix · ❌ Failure (post-script /home/runner/work/rhdh-plugins/rhdh-plugins/.fullsend/.fullsend-cache/resources/sha256/4db593f33ca767a71bc3f0cf32904bd8e701379beb3b8c14973e895cc9e2e3c0/scripts/post-fix.sh failed: exit sta…) · Started 11:18 PM UTC · Completed 11:38 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.82 |
|
The fix agent completed, but the post-fix script failed before finishing. Workflow run: https://github.com/redhat-developer/rhdh-plugins/actions/runs/42719158892 Details: |
Summary
Publish authenticated npx skill snapshots and integrate with the catalog provider (RHIDP-18493).
httpAuthdependency to make service-to-service auth intent explicit on the npx connector plugin, following the OCI connector patternplugin.integration.test.ts) covering auth enforcement, unknown source handling, snapshot lifecycle states (loading/ready/empty/failed/partial), source isolation, and scheduled refresh. AddstartConnectortest helper andNpxConnectorTestConfigtype totestUtils.tsindexUrland schedule details for operational visibilityTest plan
yarn tsc --noEmitpasses cleanlyyarn lint:allpassesyarn prettier:checkpassesrouter.test.ts)report.api.md— no public API changes)Ref: RHIDP-18493
Related to https://redhat.atlassian.net/browse/RHIDP-18493
Post-script verification
agent/RHIDP-18493-npx-auth-integration)b84916817da7d6a6fb4a3dbcc7bf97f38045fd30..HEAD)