Conversation
feat: add development frontend origin to CORS configuration
chore: trigger GitOps pipeline
WalkthroughAdds a development origin to the gateway CORS allowed-origins and inserts explicit Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example:
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}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cmd/gateway/main.go (1)
229-229: LGTM! Development origin added correctly.The addition of the development domain follows the existing pattern and uses HTTPS, which is appropriate for security.
Optional: Consider making CORS origins configurable.
While this change is correct, the entire AllowedOrigins list is currently hardcoded. For better flexibility across environments, consider loading these from an environment variable (e.g.,
ALLOWED_ORIGINSas a comma-separated list). This would make it easier to manage different deployments without code changes.Example approach:
// Read from environment variable, fallback to defaults allowedOrigins := []string{ "http://localhost:3000", "http://127.0.0.1:3000", } if envOrigins := os.Getenv("ALLOWED_ORIGINS"); envOrigins != "" { allowedOrigins = strings.Split(envOrigins, ",") } router.Use(cors.New(cors.Options{ AllowedOrigins: allowedOrigins, // ... rest of config }).Handler)This would align with the existing pattern of environment variable overrides used for service URLs (lines 59-86).
| target_url: "http://localhost:8083" | ||
| strip_prefix: "/api/v1" | ||
| auth_required: false # Public endpoint | ||
| env_var: "APPOINTMENTS_SERVICE_URL" |
There was a problem hiding this comment.
Inconsistent env_var handling for appointments services—standardize declaration approach.
The "appointments-availability" and "service-types" services now explicitly declare env_var: "APPOINTMENTS_SERVICE_URL", but the related "appointments" service (line 42–47) omits the env_var field and relies on auto-generation. This creates an inconsistency with the established pattern in the config: services sharing a backend (auth+users, services+projects, payments+invoices) all use explicit env_var declarations.
If auto-generation occurs for "appointments" but explicit declarations are made for "appointments-availability" and "service-types", there's a risk of env_var mismatch or confusion about which variable controls which service.
For consistency and clarity, add env_var: "APPOINTMENTS_SERVICE_URL" to the "appointments" service as well.
# Protected appointment endpoints
- name: "appointments"
path_prefix: "/api/v1/appointments"
target_url: "http://localhost:8083"
strip_prefix: "/api/v1"
auth_required: true
- # env_var not specified - will auto-generate: APPOINTMENTS_SERVICE_URL
+ env_var: "APPOINTMENTS_SERVICE_URL"Also update the comment on line 47 to be consistent with the new explicit declaration.
Also applies to: 55-55
🤖 Prompt for AI Agents
In config.yaml around lines 42 to 47 (and also line 55), the "appointments"
service is missing an explicit env_var while related services use env_var:
"APPOINTMENTS_SERVICE_URL"; add env_var: "APPOINTMENTS_SERVICE_URL" to the
"appointments" service block (same indentation as the other services) and update
the comment on line 47 to reflect the explicit declaration (make it consistent
with the other services' comments); repeat the same explicit env_var insertion
for the related occurrence at line 55.
1 participant
Summary by CodeRabbit