feat: add CORS support for API Gateway and clean up configuration #24
Conversation
WalkthroughThe changes in the pull request involve modifications to the Terraform configuration for an AWS API Gateway, specifically to implement CORS (Cross-Origin Resource Sharing) support. New resources are added to handle OPTIONS requests, including a method for CORS, an integration with a MOCK type, and corresponding method and integration responses. Minor formatting adjustments are also made, along with comments on the deployment stage and resource count, indicating potential management changes. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Terraform Cloud Plan Output |
![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
🧹 Outside diff range and nitpick comments (2)
modules/rest_api/main.tf (2)
86-86: Verify stage management approachThe stage will now be created unconditionally (count = 0 removed), and the stage name is hardcoded to "dev". Consider using variables for the stage name to support multiple environments.
- stage_name = "dev"#aws_api_gateway_deployment.dev.stage_name + stage_name = var.environmentAlso applies to: 88-88
121-177: CORS implementation structure looks goodThe CORS implementation includes all necessary components (method, integration, responses) and follows AWS best practices. Consider moving the CORS configuration to a separate file for better organization.
Consider creating a separate
cors.tffile in the same module to improve code organization.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
modules/rest_api/main.tf(3 hunks)
🔇 Additional comments (2)
modules/rest_api/main.tf (2)
46-47: LGTM: Formatting changes are consistent
The spacing adjustments follow Terraform style conventions and improve readability.
Also applies to: 51-51, 54-54
162-166:
Remove hard-coded dependencies
The integration response has hard-coded dependencies on specific method responses ("production", "orders", "payment"). This makes the module less reusable and maintainable.
- depends_on = [
- aws_api_gateway_method_response.cors_method_response["production"],
- aws_api_gateway_method_response.cors_method_response["orders"],
- aws_api_gateway_method_response.cors_method_response["payment"]
- ]
+ depends_on = [aws_api_gateway_method_response.cors_method_response]Likely invalid or redundant comment.
| @@ -72,20 +72,20 @@ resource "aws_api_gateway_vpc_link" "vpc_link" { | |||
| resource "aws_api_gateway_deployment" "dev" { | |||
| depends_on = [aws_api_gateway_integration.integrations] | |||
| rest_api_id = aws_api_gateway_rest_api.api_gtw.id | |||
| stage_name = "dev" | |||
| # stage_name = "dev" | |||
There was a problem hiding this comment.
💡 Codebase verification
Stage configuration needs attention - potential deployment issues
The verification reveals inconsistencies in the stage configuration:
- The stage resource has a commented-out count = 0 which could prevent stage creation
- The stage_name reference contains a mix of hardcoded "dev" and a commented reference to the deployment's stage_name
- The deployment resource has its stage_name commented out, but there's no clear indication of whether this is intentional
🔗 Analysis chain
Verify deployment strategy changes
The stage name has been commented out in the deployment resource and moved to the stage resource. The triggers configuration ensures proper redeployment when the API changes.
Also applies to: 80-82
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any other deployment resources or stage configurations
rg -A 5 "aws_api_gateway_deployment|aws_api_gateway_stage" --type tf
Length of output: 1133
| "method.response.header.Access-Control-Allow-Headers" = "'Content-Type,X-Amz-Date,Authorization,X-Api-Key,X-Amz-Security-Token,X-Amz-User-Agent,Cpf'" | ||
| "method.response.header.Access-Control-Allow-Methods" = "'GET,PUT,POST,PATCH,OPTIONS'" | ||
| "method.response.header.Access-Control-Allow-Origin" = "'*'" | ||
| } |
There was a problem hiding this comment.
Review CORS security configuration
Several security considerations in the CORS configuration:
- Using
'*'for Allow-Origin is overly permissive. Consider restricting to specific domains. - The Allow-Methods header should match your actual API methods.
- Consider adding security headers like CORS-Safelisted-Request-Headers.
response_parameters = {
"method.response.header.Access-Control-Allow-Headers" = "'Content-Type,X-Amz-Date,Authorization,X-Api-Key,X-Amz-Security-Token,X-Amz-User-Agent,Cpf'"
- "method.response.header.Access-Control-Allow-Methods" = "'GET,PUT,POST,PATCH,OPTIONS'"
- "method.response.header.Access-Control-Allow-Origin" = "'*'"
+ "method.response.header.Access-Control-Allow-Methods" = "'GET,POST,OPTIONS'"
+ "method.response.header.Access-Control-Allow-Origin" = "var.allowed_origins"
+ "method.response.header.Access-Control-Max-Age" = "'3600'"
}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
@CodeRabbit create a issue for it
There was a problem hiding this comment.
@italopessoa Issue criada para acompanhar essa melhoria: #25
🔗 Analysis chain
@italopessoa Entendido, vou criar uma issue para acompanhar essa melhoria.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
gh issue create \
--title "Rever configuração de segurança CORS" \
--body $'Conforme discutido no PR #24 (https://github.com/soat-fiap/bmb.authenticator/pull/24), no comentário (https://github.com/soat-fiap/bmb.authenticator/pull/24#discussion_r1857103500), há considerações de segurança na configuração do CORS que precisam ser abordadas.\n\n- Uso de \'*\' em Allow-Origin é excessivamente permissivo. Considere restringir a domínios específicos.\n- O header Allow-Methods deve corresponder aos métodos reais da API.\n- Considere adicionar headers de segurança como CORS-Safelisted-Request-Headers.\n\nSolicitado por @italopessoa.' \
--assignee "italopessoa"
Length of output: 707
No description provided.