Add CORS configuration section to deployment drawer#959
Conversation
|
Warning Review limit reached
More reviews will be available in 13 minutes and 32 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds TypeScript type definitions for CORS configuration and extends the DeploymentConfig React component with a new CORS configuration UI section for API agents. Users can enable CORS, configure allowed origins/methods/headers, and manage credential permissions via multi-value inputs. ChangesFrontend CORS Configuration UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
AnoshanJ
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
agent-manager-service/services/agent_manager.go (1)
362-369:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHonor the configured
allowCredentialsdefault at create time.This hardcodes
false, but the rest of the flow persists and returnsconfig.GetAgentWorkloadConfig().CORS.AllowCredentials. If the platform default istrue, a newly created API agent will report/persistallowCredentials=truewhile the attachedapi-configurationtrait is provisioned withfalseuntil the first deploy.Proposed fix
createPolicies := []map[string]interface{}{ client.CORSPolicy( strings.Split(corsConfig.AllowOrigin, ","), strings.Split(corsConfig.AllowMethods, ","), strings.Split(corsConfig.AllowHeaders, ","), - false, // allowCredentials defaults to false at agent creation + corsConfig.AllowCredentials, ), client.APIKeyAuthPolicy(), }🤖 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/services/agent_manager.go` around lines 362 - 369, The create-time CORS policy hardcodes allowCredentials to false in the client.CORSPolicy call (inside agent_manager.go where corsConfig := config.GetAgentWorkloadConfig().CORS is read), causing a mismatch with the persisted/returned config; change that literal false to use the configured value corsConfig.AllowCredentials (i.e., pass config.GetAgentWorkloadConfig().CORS.AllowCredentials / corsConfig.AllowCredentials into client.CORSPolicy) so the created agent's policy honors the platform default.
🤖 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 `@agent-manager-service/services/agent_manager.go`:
- Around line 2039-2045: The validation block that errors when
corsAllowCredentials is true and corsAllowOrigins contains "*" should be skipped
when CORS is disabled; update the logic in agent_manager.go so the existing
check (iterating corsAllowOrigins and comparing to "*") is only executed if
corsEnabled is true (i.e., wrap or gate the corsAllowCredentials /
corsAllowOrigins validation behind corsEnabled), leaving the rest of DeployAgent
behavior unchanged and ensuring stale allowOrigin=["*"] + allowCredentials=true
values do not block disabling CORS.
In
`@console/workspaces/libs/shared-component/src/components/DeploymentConfig.tsx`:
- Around line 507-510: When toggling "Allow all origins" off (handler using
setCorsAllowAll), ensure you remove the wildcard entry from corsOrigins and
update the effective origins used in the payload so credentials cannot be
re-enabled with "*" present; specifically, in the onChange where you call
setCorsAllowAll(checked) and setCorsAllowCredentials(false) when checked===true,
add logic for the unchecked case to filter out the "*" from corsOrigins (and
update the same logic in the other toggle handlers around the Cors UI mentioned)
and ensure payload assembly uses the filtered/effective origins array (not the
raw state that might still contain "*") before validating/submitting.
---
Outside diff comments:
In `@agent-manager-service/services/agent_manager.go`:
- Around line 362-369: The create-time CORS policy hardcodes allowCredentials to
false in the client.CORSPolicy call (inside agent_manager.go where corsConfig :=
config.GetAgentWorkloadConfig().CORS is read), causing a mismatch with the
persisted/returned config; change that literal false to use the configured value
corsConfig.AllowCredentials (i.e., pass
config.GetAgentWorkloadConfig().CORS.AllowCredentials /
corsConfig.AllowCredentials into client.CORSPolicy) so the created agent's
policy honors the platform default.
🪄 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: 08a4c7d2-6f71-4536-9528-43cf6c2dd59d
📒 Files selected for processing (19)
agent-manager-service/clients/openchoreosvc/client/components.goagent-manager-service/config/config.goagent-manager-service/config/config_loader.goagent-manager-service/db_migrations/021_add_cors_allow_origins.goagent-manager-service/db_migrations/migration_list.goagent-manager-service/docs/api_v1_openapi.yamlagent-manager-service/models/agent.goagent-manager-service/models/agent_config.goagent-manager-service/repositories/agent_config_repository.goagent-manager-service/services/agent_manager.goagent-manager-service/spec/model_configurations.goagent-manager-service/spec/model_cors_config.goagent-manager-service/spec/model_deploy_agent_request.goagent-manager-service/utils/makeresults.goconsole/workspaces/libs/shared-component/src/components/DeploymentConfig.tsxconsole/workspaces/libs/types/src/api/common.tsconsole/workspaces/libs/types/src/api/deployments.tsdeployments/helm-charts/wso2-agent-manager/templates/agent-manager-service/configmap.yamldeployments/helm-charts/wso2-agent-manager/values.yaml
Signed-off-by: Anoshan Jayahanthan <101160077+AnoshanJ@users.noreply.github.com>
Signed-off-by: Anoshan Jayahanthan <101160077+AnoshanJ@users.noreply.github.com>
…ced config Signed-off-by: Anoshan Jayahanthan <101160077+AnoshanJ@users.noreply.github.com>
Signed-off-by: Anoshan Jayahanthan <101160077+AnoshanJ@users.noreply.github.com>
Signed-off-by: Anoshan Jayahanthan <101160077+AnoshanJ@users.noreply.github.com>
Signed-off-by: Anoshan Jayahanthan <101160077+AnoshanJ@users.noreply.github.com>
Signed-off-by: Anoshan Jayahanthan <101160077+AnoshanJ@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
console/workspaces/libs/shared-component/src/components/DeploymentConfig.tsx (1)
525-533: 💤 Low valueRedundant
keyprop onChipcomponents.The
getTagProps({ index })spread already provides akeyprop. Providing an explicitkeyafter the spread overrides it, which is fine but redundant and may cause React warnings in some configurations if the spread's key is applied first.Suggested simplification
renderTags={(tagValues, getTagProps) => tagValues.map((option, index) => ( <Chip label={option as string} size="small" + key={option as string} {...getTagProps({ index })} - key={option as string} /> )) }Move
keybefore the spread or remove it entirely sincegetTagPropsprovides it.Also applies to: 549-557, 572-580
🤖 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 `@console/workspaces/libs/shared-component/src/components/DeploymentConfig.tsx` around lines 525 - 533, In the renderTags implementation where tagValues.map returns a Chip, remove the explicit key prop that follows the spread of getTagProps({ index }) (or move it before the spread) to avoid overriding/duplicating the key provided by getTagProps; update the Chip usages inside renderTags (and the other similar blocks at the noted locations) so they rely on getTagProps for the key instead of supplying a redundant key prop.
🤖 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
`@console/workspaces/libs/shared-component/src/components/DeploymentConfig.tsx`:
- Around line 525-533: In the renderTags implementation where tagValues.map
returns a Chip, remove the explicit key prop that follows the spread of
getTagProps({ index }) (or move it before the spread) to avoid
overriding/duplicating the key provided by getTagProps; update the Chip usages
inside renderTags (and the other similar blocks at the noted locations) so they
rely on getTagProps for the key instead of supplying a redundant key prop.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 97a9045b-70aa-4705-966b-4c91e8deda19
📒 Files selected for processing (3)
console/workspaces/libs/shared-component/src/components/DeploymentConfig.tsxconsole/workspaces/libs/types/src/api/common.tsconsole/workspaces/libs/types/src/api/deployments.ts
Signed-off-by: Anoshan Jayahanthan <101160077+AnoshanJ@users.noreply.github.com>
1 participant
Purpose
The deployment drawer for API agents had no way to configure CORS settings, users had no visibility into or control over which origins, methods, and headers the agent endpoint would accept. This PR surfaces the CORS configuration introduced in #955 in the UI.
Goals
Approach
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
#955 (backend CORS config API — must merge first)
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit
Release Notes