Upgrade ThunderID v0.34 → v0.45 and fix breaking auth changes#1123
Open
yashed wants to merge 8 commits into
Open
Upgrade ThunderID v0.34 → v0.45 and fix breaking auth changes#1123yashed wants to merge 8 commits into
yashed wants to merge 8 commits into
Conversation
|
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR upgrades the Thunder IDP dependency from v0.34.0 (Asgardeo) to v0.45.0 (ThunderID), renames all OAuth permission scopes to use an Changesamp:-prefixed OAuth scopes and ThunderID v0.45.0 upgrade
Sequence DiagramssequenceDiagram
participant Caller
participant InviteUser as thunderClient.InviteUser
participant ThunderAPI as Thunder API
rect rgba(70, 130, 180, 0.5)
Note over Caller,ThunderAPI: Adaptive InviteUser flow (ThunderID v0.45.0)
Caller->>InviteUser: InviteUser(ouID, email)
InviteUser->>ThunderAPI: POST /flows/USER_ONBOARDING (start)
ThunderAPI-->>InviteUser: challengeToken + actions[]
InviteUser->>ThunderAPI: submit user type (action_usertype)
ThunderAPI-->>InviteUser: challengeToken + actions[]
alt action_ou_selection present in actions[]
InviteUser->>ThunderAPI: submit OU selection (action_ou_selection)
ThunderAPI-->>InviteUser: challengeToken + actions[]
end
InviteUser->>ThunderAPI: select invite mode (action_invite_user)
ThunderAPI-->>InviteUser: challengeToken + actions[]
InviteUser->>ThunderAPI: submit email (action_submit_email)
ThunderAPI-->>InviteUser: challengeToken + actions[]
InviteUser->>ThunderAPI: request share-link (action_share_manually)
ThunderAPI-->>InviteUser: response body with invite link
InviteUser-->>Caller: extractInviteLink(response)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
agent-manager-service/clients/thundersvc/identity_client.go (1)
937-955: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider defensive validation when
action_ou_selectionis present butouIDis empty.The flow relies on Thunder not presenting
action_ou_selectionfor single-OU deployments. However, if this action is present andouIDis empty (as set by the controller for on-prem), the request sends{"ouId": ""}which may cause unexpected behavior.Consider adding a guard or logging:
🛡️ Suggested defensive check
if hasAction("action_ou_selection") { + if ouID == "" { + return "", fmt.Errorf("thunder invite user: OU selection required but ouID is empty") + } body, err = c.doRequest(ctx, http.MethodPost, c.baseURL+"/flow/execute", token,🤖 Prompt for 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. In `@agent-manager-service/clients/thundersvc/identity_client.go` around lines 937 - 955, The code in the hasAction("action_ou_selection") block sends a request with ouID as an empty string when ouID is not provided in on-prem deployments, which may cause unexpected behavior in Thunder. Add defensive validation before the doRequest call within the hasAction("action_ou_selection") conditional to check if ouID is empty and either return an error with a descriptive message, log a warning about the empty ouID, or skip this step entirely to prevent sending invalid data to the Thunder API.deployments/scripts/setup-openchoreo.sh (1)
320-351: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider edge case: pods may not exist or be in non-Running state.
The version detection relies on pods existing with the matching label. If the release exists but pods are pending, crashlooping, or not yet created,
kubectl get podsmay return no image, triggering the abort path at line 337-338.This is likely acceptable for a development/setup script, but worth noting that re-running the script during a transient pod state could fail unnecessarily.
💡 Optional: Add brief wait or check deployment replicas
if helm status amp-thunder-extension -n amp-thunder &>/dev/null; then + # Give pods a moment to schedule if release just deployed + kubectl wait -n amp-thunder --for=condition=ready pod -l "$selector" --timeout=30s 2>/dev/null || true local current_image current_image=$(kubectl get pods -n amp-thunder -l "$selector" \🤖 Prompt for 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. In `@deployments/scripts/setup-openchoreo.sh` around lines 320 - 351, The version detection logic queries pod images directly using kubectl get pods, but fails if the Helm release exists while pods are still pending, crashlooping, or not yet scheduled. Instead of immediately checking pods for the image when the selector matches, add a brief wait for deployment readiness (or check the deployment replicas to ensure pods are expected to exist) before querying the current_image. This prevents unnecessary aborts when re-running the script during transient pod states where the release exists but pods haven't fully initialized yet.
🤖 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.
Nitpick comments:
In `@agent-manager-service/clients/thundersvc/identity_client.go`:
- Around line 937-955: The code in the hasAction("action_ou_selection") block
sends a request with ouID as an empty string when ouID is not provided in
on-prem deployments, which may cause unexpected behavior in Thunder. Add
defensive validation before the doRequest call within the
hasAction("action_ou_selection") conditional to check if ouID is empty and
either return an error with a descriptive message, log a warning about the empty
ouID, or skip this step entirely to prevent sending invalid data to the Thunder
API.
In `@deployments/scripts/setup-openchoreo.sh`:
- Around line 320-351: The version detection logic queries pod images directly
using kubectl get pods, but fails if the Helm release exists while pods are
still pending, crashlooping, or not yet scheduled. Instead of immediately
checking pods for the image when the selector matches, add a brief wait for
deployment readiness (or check the deployment replicas to ensure pods are
expected to exist) before querying the current_image. This prevents unnecessary
aborts when re-running the script during transient pod states where the release
exists but pods haven't fully initialized yet.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 453b14c0-f1b3-4537-bbfe-24a114118ed4
⛔ Files ignored due to path filters (1)
deployments/helm-charts/wso2-amp-thunder-extension/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
agent-manager-service/clients/openchoreosvc/auth/auth.goagent-manager-service/clients/openchoreosvc/client/authz.goagent-manager-service/clients/thundersvc/identity_client.goagent-manager-service/main.goagent-manager-service/rbac/permissions.goconsole/apps/web-ui/public/config.jsconsole/workspaces/core-ui/src/Layouts/OxygenLayout/navigationItems.tsxconsole/workspaces/libs/auth/src/asgardio/hooks/authHooks.tsdeployments/docker-compose.ymldeployments/helm-charts/wso2-agent-manager/templates/console/configmap.yamldeployments/helm-charts/wso2-agent-manager/values.yamldeployments/helm-charts/wso2-amp-api-platform-gateway-extension/templates/gateway-bootstrap-job.yamldeployments/helm-charts/wso2-amp-platform-resources-extension/templates/authz-cluster-role-binding.yamldeployments/helm-charts/wso2-amp-thunder-extension/Chart.yamldeployments/helm-charts/wso2-amp-thunder-extension/templates/amp-thunder-bootstrap.yamldeployments/helm-charts/wso2-amp-thunder-extension/values.yamldeployments/scripts/setup-openchoreo.sh
Contributor
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 `@deployments/setup/setup-openchoreo.sh`:
- Around line 343-346: The PVC deletion command uses || true which silently
ignores failures, allowing the script to proceed even if deletion fails,
potentially causing the helm install to reuse a stale PVC. Replace the silent
failure handling (|| true) with explicit verification that checks if the PVCs
were actually deleted after the kubectl delete pvc command executes. If PVCs
still exist after deletion, the script should exit with an error or retry
deletion before proceeding to the subsequent helm install command. Update or add
logic after the kubectl delete pvc line and before the "✅ Existing Thunder
release removed" echo statement to validate that the PVC deletion was
successful.
🪄 Autofix (Beta)
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
Run ID: 36c09c1b-3c29-48b8-a07b-4a9d92d85a6b
⛔ Files ignored due to path filters (1)
deployments/helm-charts/wso2-amp-thunder-extension/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
agent-manager-service/clients/openchoreosvc/auth/auth.goagent-manager-service/clients/openchoreosvc/client/authz.goagent-manager-service/clients/thundersvc/identity_client.goagent-manager-service/main.goagent-manager-service/rbac/permissions.goconsole/apps/web-ui/public/config.jsconsole/workspaces/core-ui/src/Layouts/OxygenLayout/navigationItems.tsxconsole/workspaces/libs/auth/src/asgardio/hooks/authHooks.tsdeployments/docker-compose.ymldeployments/helm-charts/wso2-agent-manager/templates/console/configmap.yamldeployments/helm-charts/wso2-agent-manager/values.yamldeployments/helm-charts/wso2-amp-api-platform-gateway-extension/templates/gateway-bootstrap-job.yamldeployments/helm-charts/wso2-amp-platform-resources-extension/templates/authz-cluster-role-binding.yamldeployments/helm-charts/wso2-amp-thunder-extension/Chart.yamldeployments/helm-charts/wso2-amp-thunder-extension/templates/amp-thunder-bootstrap.yamldeployments/helm-charts/wso2-amp-thunder-extension/values.yamldeployments/setup/setup-openchoreo.sh
✅ Files skipped from review due to trivial changes (1)
- deployments/helm-charts/wso2-amp-thunder-extension/Chart.yaml
🚧 Files skipped from review as they are similar to previous changes (15)
- deployments/helm-charts/wso2-agent-manager/templates/console/configmap.yaml
- agent-manager-service/clients/openchoreosvc/client/authz.go
- deployments/helm-charts/wso2-amp-platform-resources-extension/templates/authz-cluster-role-binding.yaml
- deployments/helm-charts/wso2-amp-api-platform-gateway-extension/templates/gateway-bootstrap-job.yaml
- agent-manager-service/clients/openchoreosvc/auth/auth.go
- agent-manager-service/rbac/permissions.go
- agent-manager-service/clients/thundersvc/identity_client.go
- console/apps/web-ui/public/config.js
- deployments/helm-charts/wso2-agent-manager/values.yaml
- deployments/helm-charts/wso2-amp-thunder-extension/values.yaml
- agent-manager-service/main.go
- deployments/docker-compose.yml
- console/workspaces/libs/auth/src/asgardio/hooks/authHooks.ts
- console/workspaces/core-ui/src/Layouts/OxygenLayout/navigationItems.tsx
- deployments/helm-charts/wso2-amp-thunder-extension/templates/amp-thunder-bootstrap.yaml
yashed
changed the title
Contributor
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 `@deployments/helm-charts/wso2-amp-thunder-extension/values.yaml`:
- Around line 84-103: The consent database path configuration is still using the
old path structure while the config, runtime, and user databases have been
migrated to the new database directory structure. Locate the
configuration.consent.database.sqlitePath setting (currently pointing to
repository/database/consentdb.db) and update it to use the new path structure
(database/consentdb.db) to match the migrated paths for the config, runtime, and
user database entries, ensuring all database paths reference the same location
directory.
🪄 Autofix (Beta)
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
Run ID: 97faaac2-69bc-4fc4-9109-e3de40deac4f
⛔ Files ignored due to path filters (1)
deployments/helm-charts/wso2-amp-thunder-extension/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
deployments/helm-charts/wso2-amp-thunder-extension/Chart.yamldeployments/helm-charts/wso2-amp-thunder-extension/templates/amp-thunder-bootstrap.yamldeployments/helm-charts/wso2-amp-thunder-extension/values.yamldeployments/setup/setup-openchoreo.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- deployments/helm-charts/wso2-amp-thunder-extension/Chart.yaml
- deployments/helm-charts/wso2-amp-thunder-extension/templates/amp-thunder-bootstrap.yaml
- Prefix scopes with amp: resource server handle - Switch OpenChoreo entitlement claim from sub to client_id - Refactor InviteUser to 6-step flow with challenge token threading - Auto-detect and reinstall stale ThunderID on version mismatch
- Scope Thunder pod/PVC ops to release label; guard empty image - Disable Thunder persistence to avoid PVC conflicts on reinstall - Fix openchoreo-api-config patch to avoid Helm field manager conflicts - Reset invite flow step before each unmarshal to prevent stale carry-over - Clear stale token payload on empty token or fetch failure - Add missing read scopes for resources and infrastructure nav sections
-Migrate built-in ClusterAuthzRoleBindings sub→client_id and patch -observer-auth-config in setup-openchoreo.sh so build logs and agent -build/workload-publish stop 403ing after the ThunderID upgrade; bump version comments to v0.45.
Contributor
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
`@deployments/helm-charts/wso2-amp-thunder-extension/templates/amp-thunder-bootstrap.yaml`:
- Around line 161-164: The grep condition that checks for the presence of an
email attribute is searching the entire BODY response instead of checking only
the matched schema identified by SCHEMA_ID. This means if any other user type in
the response contains an email attribute, the patch operation will be skipped
even if the target schema is missing email. Instead of grepping the entire BODY
output, extract and check the email attribute only from the specific schema that
matches the target schema name, ensuring the patch is applied only when the
target schema itself lacks the email attribute.
- Around line 286-309: When a layout with matching handle is found and
EXISTING_LAYOUT_ID is discovered from the API response, the LAYOUT_PAYLOAD
variable still contains the static ID from the Helm values through the toJson
filter. Before calling the api_call PUT function with EXISTING_LAYOUT_ID in the
URL path, update the LAYOUT_PAYLOAD to replace its ID field with the actual
EXISTING_LAYOUT_ID value discovered from the database to ensure consistency
between the URL path parameter and the request body, preventing data integrity
issues from mismatched IDs.
In `@deployments/helm-charts/wso2-amp-thunder-extension/values.yaml`:
- Around line 20-22: The setup script comments reference ThunderID version
0.45.0, but all actual version pins are set to 0.44.0 across configuration files
including the tag field in values.yaml, the version field in Chart.yaml, and the
target_image reference in setup-openchoreo.sh. Choose one approach to resolve
this inconsistency: either update all version pin references (the tag, version
fields, and target_image values) to 0.45.0 to match the comments, or update the
setup script comments to reference 0.44.0 to match the current configuration.
Ensure consistency across all version references before finalizing the release.
- Around line 165-257: Update all scope requests in test/e2e/framework/auth.go
(lines 42-61) to include the "amp:" prefix to match the allowlist defined in
values.yaml. The current requests use unprefixed scopes like
"agent-kind:create", "org:view", "agent:build", etc., but the allowlist only
grants "amp:"-prefixed versions (e.g., "amp:agent-kind:create", "amp:org:view").
Add the "amp:" prefix to every scope string in that scope request list to ensure
they intersect with the allowlist and prevent 403 RBAC failures.
In `@deployments/setup/setup-openchoreo.sh`:
- Around line 388-389: The helm uninstall command for amp-thunder-extension at
line 388 uses || true which silently ignores all failures, allowing the script
to proceed to PVC deletion and reinstall even when the uninstall fails, risking
stale resources. Remove the || true operator from the helm uninstall
amp-thunder-extension command and add explicit error checking that either exits
the script with a clear error message or logs a failure and prevents further
execution if the uninstall fails. This ensures the script only continues with
PVC operations after confirming a successful uninstall.
- Around line 103-105: The kubectl patch command for
clusterauthzrolebinding.openchoreo.dev is suppressing all error output with
>/dev/null 2>&1 and only logging success with &&, which means patch failures go
unnoticed while the script continues. Remove the error suppression redirection
and add explicit error handling by checking the patch command's exit status with
a conditional statement that exits the script with an error message if the patch
fails, ensuring stale bindings with the old claim value cannot silently remain.
🪄 Autofix (Beta)
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
Run ID: b3db2f98-a0d1-4c4d-932e-406723c3c6f2
⛔ Files ignored due to path filters (1)
deployments/helm-charts/wso2-amp-thunder-extension/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
agent-manager-service/clients/openchoreosvc/auth/auth.goagent-manager-service/clients/openchoreosvc/client/authz.goagent-manager-service/clients/thundersvc/identity_client.goagent-manager-service/main.goagent-manager-service/rbac/permissions.goconsole/apps/web-ui/public/config.jsconsole/workspaces/core-ui/src/Layouts/OxygenLayout/navigationItems.tsxconsole/workspaces/libs/auth/src/asgardio/hooks/authHooks.tsdeployments/docker-compose.ymldeployments/helm-charts/wso2-agent-manager/templates/console/configmap.yamldeployments/helm-charts/wso2-agent-manager/values.yamldeployments/helm-charts/wso2-amp-api-platform-gateway-extension/templates/gateway-bootstrap-job.yamldeployments/helm-charts/wso2-amp-platform-resources-extension/templates/authz-cluster-role-binding.yamldeployments/helm-charts/wso2-amp-thunder-extension/Chart.yamldeployments/helm-charts/wso2-amp-thunder-extension/templates/amp-thunder-bootstrap.yamldeployments/helm-charts/wso2-amp-thunder-extension/values.yamldeployments/setup/setup-openchoreo.sh
🚧 Files skipped from review as they are similar to previous changes (14)
- deployments/helm-charts/wso2-amp-thunder-extension/Chart.yaml
- agent-manager-service/clients/openchoreosvc/client/authz.go
- deployments/helm-charts/wso2-amp-api-platform-gateway-extension/templates/gateway-bootstrap-job.yaml
- deployments/helm-charts/wso2-amp-platform-resources-extension/templates/authz-cluster-role-binding.yaml
- deployments/helm-charts/wso2-agent-manager/templates/console/configmap.yaml
- agent-manager-service/clients/openchoreosvc/auth/auth.go
- console/workspaces/core-ui/src/Layouts/OxygenLayout/navigationItems.tsx
- deployments/helm-charts/wso2-agent-manager/values.yaml
- agent-manager-service/rbac/permissions.go
- console/apps/web-ui/public/config.js
- console/workspaces/libs/auth/src/asgardio/hooks/authHooks.ts
- agent-manager-service/main.go
- agent-manager-service/clients/thundersvc/identity_client.go
- deployments/docker-compose.yml
Comment on lines
+20
to
+22
| registry: ghcr.io/thunder-id | ||
| repository: thunderid | ||
| tag: "0.44.0" |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify all ThunderID version/image references agree on one target.
# Expect: one intended ThunderID version across chart values, chart dependencies, and setup logic.
rg -n 'thunderid:0\.(44|45)\.0|version: 0\.(44|45)\.0|0\.(44|45)\.0|ghcr\.io/thunder-id/thunderid' \
deployments/helm-charts/wso2-amp-thunder-extension deployments/setup/setup-openchoreo.shRepository: wso2/agent-manager
Length of output: 768
Resolve version mismatch: setup comments reference v0.45.0 but all version pins are 0.44.0.
The setup script comments at lines 69 and 259 reference ThunderID v0.45.0, but the actual version pins across all configuration files are set to 0.44.0:
values.yamlline 22:tag: "0.44.0"Chart.yamlline 24:version: "0.44.0"setup-openchoreo.shline 374:target_image="ghcr.io/thunder-id/thunderid:0.44.0"
Either update the comments to reflect 0.44.0 or upgrade all version pins to 0.45.0 to ensure consistency before release.
🤖 Prompt for 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.
In `@deployments/helm-charts/wso2-amp-thunder-extension/values.yaml` around lines
20 - 22, The setup script comments reference ThunderID version 0.45.0, but all
actual version pins are set to 0.44.0 across configuration files including the
tag field in values.yaml, the version field in Chart.yaml, and the target_image
reference in setup-openchoreo.sh. Choose one approach to resolve this
inconsistency: either update all version pin references (the tag, version
fields, and target_image values) to 0.45.0 to match the comments, or update the
setup script comments to reference 0.44.0 to match the current configuration.
Ensure consistency across all version references before finalizing the release.
- Bump thunder subchart to 0.45.0 (Chart.yaml, Chart.lock, image tag) and move config paths to the v0.45 layout (config/certs/*, database/*.db) - amp:-prefix the e2e auth client_credentials scopes so RBAC routes no longer 403 - Abort the Thunder reinstall in setup-openchoreo.sh if `helm uninstall` fails (don't delete PVCs on a half-removed release); simplify migration comments
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
deployments/helm-charts/wso2-amp-thunder-extension/values.yaml (1)
83-138: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winConsent DB path not migrated alongside config/runtime/user paths.
The config, runtime, and user SQLite paths were migrated to
database/*.db(lines 87, 95, 103), butconfiguration.consent.database.sqlitePathat line 138 still points torepository/database/consentdb.db. This splits persisted state across the old and new base directories.Proposed fix
database: type: sqlite - sqlitePath: "repository/database/consentdb.db" + sqlitePath: "database/consentdb.db"🤖 Prompt for 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. In `@deployments/helm-charts/wso2-amp-thunder-extension/values.yaml` around lines 83 - 138, The consent database path is inconsistent with the other database paths in the values file. The config, runtime, and user database paths under the database section use the pattern database/*.db, but the sqlitePath field under consent.database still references the old repository/database/ directory structure. Update the sqlitePath value under the consent database configuration to follow the same pattern as the other databases by changing it to point to database/consentdb.db instead of repository/database/consentdb.db to maintain consistency across all database paths.
🤖 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.
Duplicate comments:
In `@deployments/helm-charts/wso2-amp-thunder-extension/values.yaml`:
- Around line 83-138: The consent database path is inconsistent with the other
database paths in the values file. The config, runtime, and user database paths
under the database section use the pattern database/*.db, but the sqlitePath
field under consent.database still references the old repository/database/
directory structure. Update the sqlitePath value under the consent database
configuration to follow the same pattern as the other databases by changing it
to point to database/consentdb.db instead of repository/database/consentdb.db to
maintain consistency across all database paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7c704565-709e-48c5-8b5e-5136329d2c56
⛔ Files ignored due to path filters (1)
deployments/helm-charts/wso2-amp-thunder-extension/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
deployments/helm-charts/wso2-amp-thunder-extension/Chart.yamldeployments/helm-charts/wso2-amp-thunder-extension/templates/amp-thunder-bootstrap.yamldeployments/helm-charts/wso2-amp-thunder-extension/values.yamldeployments/setup/setup-openchoreo.shtest/e2e/framework/auth.go
🚧 Files skipped from review as they are similar to previous changes (3)
- deployments/helm-charts/wso2-amp-thunder-extension/Chart.yaml
- deployments/setup/setup-openchoreo.sh
- deployments/helm-charts/wso2-amp-thunder-extension/templates/amp-thunder-bootstrap.yaml
- setup-openchoreo.sh: abort the Thunder reinstall if `helm uninstall` fails (don't delete PVCs on a half-removed release), and fail fast if a ClusterAuthzRoleBinding sub→client_id patch fails instead of silently skipping - amp-thunder-bootstrap.yaml: align the layout payload id with the existing layout's id before PUT, and scope the email-attribute check to the matched user schema instead of grepping the whole /user-types response
lashinijay
approved these changes
Jun 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Goals
Approach
What changed in ThunderID v0.44/v0.45 and the fixes made:
Backend -
agent-manager-servicerbac/permissions.go- Thunder now prefixes every permission scope with the resource-serverhandle (
org:view→amp:org:view).Permission.Scope()now returns theamp:-prefixed form soRBAC checks match the scopes the new tokens carry.
clients/openchoreosvc/client/authz.go-client_credentialstokens now carry the clientidentifier in the
client_idclaim, notsub(subis now a random UUID).EnsureClusterRoleBindingnow uses
client_idin both the create path and the idempotency check.clients/openchoreosvc/auth/auth.go+main.go- added aScopefield to the OpenChoreo M2Mauth config so client-credentials token requests include the required
amp:*scopes.clients/thundersvc/identity_client.go- the invitation flow went from 4 to 6 steps, withchallenge tokens mandatory across steps and OU selection becoming action-driven.
InviteUserrefactored to the new flow.
Console
public/config.js,wso2-agent-manager/values.yaml,docker-compose.yml-AUTH_SCOPES/OAUTH_SCOPES_SUPPORTEDupdated to theamp:-prefixed format.navigationItems.tsx- sidebar items gated on the newamp:*read scopes.authHooks.ts- clear stale token payload on empty token / fetch failure; reset invite stepbefore each unmarshal to avoid stale carry-over.
RBAC_ENABLEDmade overridable (console configmap + docker-compose) for zero-downtime rollout.ThunderID extension chart (
wso2-amp-thunder-extension)Chart.yaml/values.yaml- bump to v0.45.0 and updated OAuth config. Theamp:prefix isonly valid on scope strings; removed it from non-scope OAuth app fields (
grantTypes,allowedUserTypes,redirectUris, CORSallowedOrigins,userAttributes), which v0.44+validates strictly and otherwise rejects at startup.
amp-thunder-bootstrap.yaml- scoped pod/PVC ops to the release label, guard empty image,disable persistence to avoid PVC conflicts on reinstall.
API Platform Gateway extension chart (
wso2-amp-api-platform-gateway-extension)gateway-bootstrap-job.yaml- the bootstrap job requested un-prefixed scopes(
gateway:read …); v0.45 grants nothing for those, so the minted token had no scopes and gatewayregistration failed with
403 insufficient permissions. Updated to requestamp:-prefixed scopes.authz-cluster-role-binding.yaml- theamp-api-clientbinding switchedsub→client_id.OpenChoreo setup -
deployments/setup/setup-openchoreo.shhelm upgradedoesn't re-run ThunderID's pre-install bootstrap job, leaving the old scopeformat in the DB. Detects a version mismatch, then uninstalls + deletes the PVC + reinstalls so the
bootstrap re-seeds with the new format.
openchoreo-api-configso the control plane resolves the service-account subject byclient_id(the chart schema doesn't expose this).ClusterAuthzRoleBindings fromsub→client_id(
workload-publisher,observer-resource-reader,backstage,finops,rca,mcp-tryout).After the global claim switch these stale
subbindings stop matching, so agent build(workload publish) and build-log/observability reads returned 403.
observer-auth-config(observability plane)sub→client_id- the observer keepsits own subject-resolution config; without this, build logs returned
403 Access denied.User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit
Release Notes
New Features
Improvements
amp:-prefixed permission identifiers.client_id(instead ofsub), with automated, idempotent migration during setup/upgrade.Configuration
amp:scheme.