Add Secret Management UI Implementation - #3129
Conversation
|
Naduni Pamudika seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
Warning Review limit reached
Next review available in: 44 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (32)
📝 WalkthroughWalkthroughThe change adds revision-aware secret lifecycle events, gateway synchronization, secret handle validation, upstream authentication serialization, portal secret management pages, SQL Server locking support, and integration coverage for live rotation and deprecation. ChangesSecret lifecycle backend
Deployment authentication
Secret management portal
Integration coverage
Persistence compatibility
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Portal
participant PlatformAPI
participant GatewayEventsService
participant GatewayController
participant SecretStore
Portal->>PlatformAPI: Rotate or delete secret
PlatformAPI->>GatewayEventsService: Broadcast lifecycle event
GatewayEventsService->>GatewayController: Publish WebSocket event
GatewayController->>PlatformAPI: Fetch rotated value
GatewayController->>SecretStore: Upsert or delete local secret
Portal->>PlatformAPI: Read secret state
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@platform-api/internal/model/secret.go`:
- Around line 72-90: Add a revision or generation field to SecretUpdatedEvent
and SecretDeprecatedEvent, then update gateway lifecycle handling to ignore
events older than the locally recorded revision while preserving existing
idempotency and delivery coverage. Ensure deletion followed by reactivation
cannot be undone by a stale event, and add a test covering that sequence.
In `@portals/ai-workspace/src/App.tsx`:
- Around line 591-622: Update portals/ai-workspace/src/App.tsx lines 591-622 to
guard the entire secrets route branch, including nested routes, with
SCOPES.SECRET_READ and replace the hard-coded settings index redirect with a
scope-aware destination. In
portals/ai-workspace/src/pages/appShell/appShellPages/settings/Main.tsx lines
79-84, authorize the matched settings route using its requiredScope rather than
visibleNavItems, preserving access only for users with the route’s required
scope.
In
`@portals/ai-workspace/src/pages/appShell/appShellPages/secret/SecretOverview.tsx`:
- Around line 59-76: Bind secret fetch results to the active organization and
handle across SecretOverview.tsx lines 59-76, RotateSecret.tsx lines 68-89, and
SecretsList.tsx lines 77-94. Update the request flows using request IDs or
cancellation, and reject responses that are stale after the organization or
handle changes; ensure SecretOverview cannot replace the current secret
metadata, RotateSecret cannot submit stale metadata, and SecretsList cannot
replace the current organization’s list.
In
`@portals/ai-workspace/src/pages/appShell/appShellPages/secret/SecretsList.tsx`:
- Around line 252-257: Update the row keyboard handler in SecretsList so
keyboard events originating from child controls such as the delete IconButton
are ignored, while Enter and Space pressed directly on the row still prevent
default behavior and navigate to the secret. Use the event target/currentTarget
relationship to distinguish the row from its descendants.
In `@tests/integration-e2e/ingress_helpers_test.go`:
- Around line 68-78: Introduce one shared delay helper that validates a positive
interval and a non-zero interval/2 before adding bounded random jitter, then use
it before every polling fetch. Update
tests/integration-e2e/ingress_helpers_test.go:68-78 in waitIngressWithHeaders,
tests/integration-e2e/secret_helpers_test.go:213-240 before each secret-value
fetch, and tests/integration-e2e/secret_helpers_test.go:246-270 before each
deletion-status fetch; replace the fixed two-second sleeps while preserving the
existing polling behavior.
In `@tests/integration-e2e/README.md`:
- Around line 255-257: Update the “Deletion” scenario in the README to document
the explicit deleteSecret operation: state that DELETE /secrets/:handle triggers
the original secret’s deprecation event, rather than attributing deprecation to
cleanupRotatedSecret. Preserve the existing description of redeployment and
gateway cache eviction.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 600a8b72-f6f9-4d74-b6e9-34bdeb670754
📒 Files selected for processing (31)
gateway/gateway-controller/pkg/controlplane/client.gogateway/gateway-controller/pkg/controlplane/events.gogateway/gateway-controller/pkg/controlplane/sync_secrets_test.goplatform-api/internal/constants/constants.goplatform-api/internal/dto/api.goplatform-api/internal/model/secret.goplatform-api/internal/server/server.goplatform-api/internal/service/gateway_events.goplatform-api/internal/service/secret_service.goplatform-api/internal/service/secret_service_broadcast_test.goplatform-api/internal/service/secret_service_test.goplatform-api/internal/utils/api.goplatform-api/internal/utils/api_test.goportals/ai-workspace/src/App.tsxportals/ai-workspace/src/apis/secretApis.tsportals/ai-workspace/src/clients/choreoApiClient.tsportals/ai-workspace/src/hooks/useIsMounted.tsportals/ai-workspace/src/pages/appShell/appShellPages/secret/CreateSecret.tsxportals/ai-workspace/src/pages/appShell/appShellPages/secret/DeleteSecretDialog.tsxportals/ai-workspace/src/pages/appShell/appShellPages/secret/RotateSecret.tsxportals/ai-workspace/src/pages/appShell/appShellPages/secret/SecretOverview.tsxportals/ai-workspace/src/pages/appShell/appShellPages/secret/SecretsList.tsxportals/ai-workspace/src/pages/appShell/appShellPages/settings/Main.tsxtests/integration-e2e/README.mdtests/integration-e2e/features/secret_lifecycle.featuretests/integration-e2e/features/secured-api-invocation.featuretests/integration-e2e/ingress_helpers_test.gotests/integration-e2e/secret_helpers_test.gotests/integration-e2e/secret_lifecycle_steps_test.gotests/integration-e2e/steps_secured_test.gotests/integration-e2e/steps_test.go
💤 Files with no reviewable changes (2)
- tests/integration-e2e/features/secured-api-invocation.feature
- tests/integration-e2e/steps_secured_test.go
d00f44d to
b6cbfc3
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/integration-e2e/secret_helpers_test.go`:
- Around line 217-236: Add randomized positive pre-request jitter to both
polling loops around the visible HTTP fetches, using a non-zero half-interval
rather than a fixed two-second retry cadence. Apply the same jittered delay
before every request in the loops containing httpClient.Do, while preserving the
existing deadline and retry behavior.
- Line 162: Update the response-body reads in the relevant secret helper test
paths around the existing io.ReadAll calls to wrap resp.Body with io.LimitReader
before reading. Use the configured response-size limit when available, with a
safe default fallback, and apply the same bounded-reader pattern to all three
occurrences.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3402839c-59e4-48ab-9753-061df24b30f4
📒 Files selected for processing (28)
gateway/gateway-controller/pkg/controlplane/client.gogateway/gateway-controller/pkg/controlplane/events.gogateway/gateway-controller/pkg/controlplane/sync_secrets_test.goplatform-api/internal/constants/constants.goplatform-api/internal/dto/api.goplatform-api/internal/model/secret.goplatform-api/internal/server/server.goplatform-api/internal/service/gateway_events.goplatform-api/internal/service/secret_service.goplatform-api/internal/service/secret_service_broadcast_test.goplatform-api/internal/service/secret_service_test.goplatform-api/internal/utils/api.goplatform-api/internal/utils/api_test.goportals/ai-workspace/src/App.tsxportals/ai-workspace/src/apis/secretApis.tsportals/ai-workspace/src/clients/choreoApiClient.tsportals/ai-workspace/src/hooks/useIsMounted.tsportals/ai-workspace/src/pages/appShell/appShellPages/secret/CreateSecret.tsxportals/ai-workspace/src/pages/appShell/appShellPages/secret/DeleteSecretDialog.tsxportals/ai-workspace/src/pages/appShell/appShellPages/secret/RotateSecret.tsxportals/ai-workspace/src/pages/appShell/appShellPages/secret/SecretOverview.tsxportals/ai-workspace/src/pages/appShell/appShellPages/secret/SecretsList.tsxportals/ai-workspace/src/pages/appShell/appShellPages/settings/Main.tsxtests/integration-e2e/README.mdtests/integration-e2e/features/secret_lifecycle.featuretests/integration-e2e/secret_helpers_test.gotests/integration-e2e/secret_lifecycle_steps_test.gotests/integration-e2e/steps_test.go
🚧 Files skipped from review as they are similar to previous changes (21)
- portals/ai-workspace/src/hooks/useIsMounted.ts
- portals/ai-workspace/src/apis/secretApis.ts
- platform-api/internal/model/secret.go
- tests/integration-e2e/features/secret_lifecycle.feature
- platform-api/internal/server/server.go
- gateway/gateway-controller/pkg/controlplane/sync_secrets_test.go
- platform-api/internal/service/secret_service_test.go
- platform-api/internal/utils/api_test.go
- platform-api/internal/dto/api.go
- platform-api/internal/utils/api.go
- portals/ai-workspace/src/pages/appShell/appShellPages/secret/CreateSecret.tsx
- portals/ai-workspace/src/pages/appShell/appShellPages/secret/RotateSecret.tsx
- gateway/gateway-controller/pkg/controlplane/events.go
- portals/ai-workspace/src/pages/appShell/appShellPages/secret/SecretOverview.tsx
- tests/integration-e2e/secret_lifecycle_steps_test.go
- portals/ai-workspace/src/pages/appShell/appShellPages/secret/DeleteSecretDialog.tsx
- platform-api/internal/service/secret_service_broadcast_test.go
- platform-api/internal/service/secret_service.go
- portals/ai-workspace/src/App.tsx
- portals/ai-workspace/src/pages/appShell/appShellPages/secret/SecretsList.tsx
- gateway/gateway-controller/pkg/controlplane/client.go
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gateway/gateway-controller/pkg/controlplane/client.go`:
- Around line 3810-3827: Update handleMessage and the revision-bearing event
decode path to preserve revision values as full int64 numbers instead of default
float64 values before isStaleSecretEvent compares them. Ensure equal revisions
remain idempotent while adjacent epoch-nanosecond revisions remain
distinguishable, and add a regression test covering adjacent revision values and
reordered events.
In `@platform-api/internal/model/secret.go`:
- Around line 79-85: Replace the time-based revision described in the Secret
lifecycle event flow with a database-backed per-secret version. Add an atomic
data_version/revision increment to both SecretService.Update and
SecretService.Delete, ensure the version is read or returned only after the
corresponding row write commits, and use that committed value in emitted events
so concurrent update and soft-delete operations preserve commit order.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 28faceb6-1ae3-4e64-b38c-13f7f0827ddd
📒 Files selected for processing (13)
gateway/gateway-controller/pkg/controlplane/client.gogateway/gateway-controller/pkg/controlplane/events.gogateway/gateway-controller/pkg/controlplane/sync_secrets_test.goplatform-api/internal/model/secret.goplatform-api/internal/service/secret_service.goportals/ai-workspace/cypress/e2e/001-providers/002-provider-secret-management.cy.jsportals/ai-workspace/cypress/e2e/001-providers/003-llm-proxy-secret-management.cy.jsportals/ai-workspace/src/App.tsxportals/ai-workspace/src/pages/appShell/appShellPages/secret/RotateSecret.tsxportals/ai-workspace/src/pages/appShell/appShellPages/secret/SecretOverview.tsxportals/ai-workspace/src/pages/appShell/appShellPages/secret/SecretsList.tsxportals/ai-workspace/src/pages/appShell/appShellPages/settings/Main.tsxtests/integration-e2e/secret_helpers_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
- portals/ai-workspace/src/pages/appShell/appShellPages/secret/RotateSecret.tsx
- portals/ai-workspace/src/pages/appShell/appShellPages/secret/SecretOverview.tsx
- portals/ai-workspace/src/pages/appShell/appShellPages/secret/SecretsList.tsx
- gateway/gateway-controller/pkg/controlplane/events.go
- platform-api/internal/service/secret_service.go
f2bfd50 to
15a248d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@platform-api/internal/repository/secret.go`:
- Around line 247-251: Update the SQL Server branch in FindRefsAndSoftDelete so
the secret lookup acquires the same blocking lock semantics as the PostgreSQL
path instead of only switching to SELECT TOP (1). Apply a SQL Server row-locking
equivalent on the secrets read, and make sure the later refsQuery and UPDATE
still run under that contract so concurrent artifact_secret_refs writes cannot
slip past the empty-reference check. If needed, align the reference write path
with the same locking behavior, and add a SQL Server concurrency test that
exercises secret deprecation against concurrent reference insertion or update.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7f8c2da6-333e-4a66-9f64-b4f231e49064
📒 Files selected for processing (2)
gateway/gateway-controller/pkg/controlplane/client.goplatform-api/internal/repository/secret.go
🚧 Files skipped from review as they are similar to previous changes (1)
- gateway/gateway-controller/pkg/controlplane/client.go
15a248d to
830bd59
Compare
830bd59 to
9facac3
Compare
Purpose
Platform API already supports encrypting upstream credentials for LLM providers, LLM proxies, MCP proxies, and REST APIs via {{ secret "handle" }} placeholders (added in #2149, but two gaps made that feature incomplete in practice:
Resolves:
#2149
Goals
Approach
Backend (Go, platform-api):
dto.UpstreamTarget gained an Auth field, and BuildAPIDeploymentYAML now copies it through — the deployment-YAML fix.
Backend (Go, gateway-controller):
Frontend (AI Workspace):
UIs
User Stories
Documentation
https://github.com/wso2/api-platform/blob/main/docs/rest-apis/platform-api/secrets.md
Automation Tests
Related PRs
Refer the PRs mentioned in #2149
Test Environment