Tech debt: updating the project structure #102
Conversation
… of obsolete files
tmikula-dev
requested review from
lsulak,
oto-macenauer-absa and
petr-pokorny-absa
as code owners
WalkthroughRemoved an obsolete Terraform CI workflow, relocated API and topic schema files (api.yaml moved to project root; topic schemas moved to Changes
Sequence Diagram(s)(omitted — changes do not introduce a new multi-component control-flow requiring visualization) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
AquaSec has completed a full security repository scan ✅ You can find the analysis results for this PR branch on this overview.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
116-116: Fix typo in script filename reference.Line 116 references
scripts/prepare.deplyoment.sh— should bescripts/prepare.deployment.sh.
🤖 Fix all issues with AI agents
In @api.yaml:
- Around line 38-39: The description for HTTP response code '303' contains a
typo ("Loing service"); update the string value to read "Login service" so the
description becomes "Redirect to actual address of Login service which performs
auth up to its capabilities".
- Around line 142-144: The OpenAPI spec returns a '200' success code for the
POST publish endpoint but the README documents a 202 Accepted for successful
publishes; update the responses block for the POST operation to use '202' as the
success status (replace the '200' response key with '202' and keep/adjust the
description to "Accepted (all writers succeeded)" to match the README) so the
API spec and README are consistent.
In @terraform_examples/api_gateway.tf:
- Around line 9-24: The Terraform JSON block inside the policy (jsonencode) has
a missing comma after the Principal = "*" entry which causes a parse error;
update the policy statement by adding a trailing comma after Principal = "*"
(i.e., between the Principal line and the Condition block) so the Statement
object is valid and the Condition referencing var.vpc_endpoint remains correctly
parsed.
In @terraform_examples/lambda.tf:
- Around line 20-51: The aws_lambda_function resource
(aws_lambda_function.event_gate_lambda) needs conditional inclusion of
code_signing_config_arn only when var.lambda_package_type == "Zip" because code
signing is not supported for Image package types; update the resource to set
code_signing_config_arn = var.lambda_code_signing_config_arn (or null) based on
that condition, ensuring the variable var.lambda_code_signing_config_arn exists
and is used only for Zip deployments so the Image path remains unaffected.
🧹 Nitpick comments (3)
terraform_examples/api_gateway.tf (1)
166-177: Inconsistent indentation: mixed tabs and spaces.Lines 170, 172-175 (and line 17 earlier) use tabs while the rest of the file uses spaces. Consider normalizing to spaces for consistent formatting.
♻️ Proposed fix
triggers = { redeployment = sha1(jsonencode([ - aws_api_gateway_integration.event_gate_api_api_get_integration, + aws_api_gateway_integration.event_gate_api_api_get_integration, aws_api_gateway_integration.event_gate_api_token_get_integration, - aws_api_gateway_integration.event_gate_api_topics_get_integration, - aws_api_gateway_integration.event_gate_api_topic_name_get_integration, - aws_api_gateway_integration.event_gate_api_topic_name_post_integration, - aws_api_gateway_integration.event_gate_api_terminate_post_integration + aws_api_gateway_integration.event_gate_api_topics_get_integration, + aws_api_gateway_integration.event_gate_api_topic_name_get_integration, + aws_api_gateway_integration.event_gate_api_topic_name_post_integration, + aws_api_gateway_integration.event_gate_api_terminate_post_integration ])) }api.yaml (1)
1-202: Consider static analysis feedback on security definitions.Checkov flags that no global security field is defined (CKV_OPENAPI_4). Since only
POST /topics/{topicName}requires authentication, the current per-operation security is valid. However, you could add an empty global security array to explicitly document that most endpoints are public:security: [] # Most endpoints are public; auth required only where specifiedThis silences the linter while documenting intent. This is optional since the current approach is functionally correct.
terraform_examples/lambda.tf (1)
8-12: Permissive egress rule allows all outbound traffic.The egress rule permits all protocols (
-1) to all destinations (0.0.0.0/0). While this is common for Lambda functions requiring external API access, consider restricting to specific CIDR ranges or protocols if the Lambda only needs connectivity to known endpoints (e.g., Kafka brokers, EventBridge, RDS).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.github/workflows/check_terraform.yml.gitignoreDockerfileREADME.mdapi.yamlconf/topic_schemas/dlchange.jsonconf/topic_schemas/runs.jsonconf/topic_schemas/test.jsonsrc/event_gate_lambda.pysrc/handlers/handler_topic.pyterraform_examples/api_gateway.tfterraform_examples/lambda.tfterraform_examples/provider.tfterraform_examples/variables.tftests/handlers/test_handler_topic.py
💤 Files with no reviewable changes (2)
- .github/workflows/check_terraform.yml
- .gitignore
🧰 Additional context used
🪛 Checkov (3.2.334)
terraform_examples/lambda.tf
[high] 20-51: Ensure AWS Lambda function is configured to validate code-signing
(CKV_AWS_272)
api.yaml
[high] 1-203: Ensure that the global security field has rules defined
(CKV_OPENAPI_4)
[high] 1-203: Ensure that security operations is not empty.
(CKV_OPENAPI_5)
[medium] 88-93: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🔇 Additional comments (9)
Dockerfile (1)
76-76: LGTM!The addition correctly copies
api.yamlto the Lambda task root, aligning with the project restructuring where the OpenAPI spec is now loaded fromPROJECT_ROOT/api.yamlinstead of the conf directory.terraform_examples/variables.tf (1)
1-10: LGTM!Formatting-only changes with no functional impact. The variable declarations are preserved correctly.
README.md (1)
36-36: LGTM!Documentation updates correctly reflect the project restructuring:
- Terraform examples directory reference
- Configuration and topic schema paths
- Health endpoint and status codes
- Troubleshooting section additions
Also applies to: 49-51, 61-61, 68-68, 75-75, 106-106, 112-112, 181-181
api.yaml (1)
197-202: LGTM!Adding
bearerFormat: JWTto the bearer auth scheme improves API documentation clarity by explicitly indicating the expected token format.terraform_examples/provider.tf (1)
1-3: LGTM!Standard AWS provider configuration with region parameterized via variable.
src/event_gate_lambda.py (2)
35-35: LGTM!The
PROJECT_ROOTcomputation correctly resolves to the repository root from thesrc/directory, aligning with the PR objective to relocateapi.yamlto the project root.
53-56: API definition now loaded from project root.This change separates the OpenAPI spec (
api.yaml) from the runtime configuration directory (CONF_DIR). The Dockerfile correctly copiesapi.yamlto the Lambda task root at deployment time.tests/handlers/test_handler_topic.py (1)
31-51: LGTM!Test mock filenames correctly updated to match the new schema file naming convention (
runs.json,dlchange.json,test.json). Themock_open_side_effectlogic properly handles the subdirectory path structure by checking if the filename substring exists in the full path.src/handlers/handler_topic.py (1)
55-63: Topic schemas successfully reorganized into dedicated subdirectory.The relocation to
topic_schemas/with simplified filenames (runs.json, dlchange.json, test.json) is correctly implemented. Schema files exist at the expected locations and are properly loaded by the handler.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @api.yaml:
- Line 39: Fix the typo in the API description string: change "Loging" to
"Login" in the description field (the description value shown in api.yaml) and
scan other description/summary fields in api.yaml for the same misspelling to
update them as well.
- Around line 182-196: Add an authentication requirement to the sensitive
/terminate operation by adding an operation-level security entry using the
bearerAuth scheme (e.g., add "security: - bearerAuth: []" under the /terminate
POST), and ensure a corresponding bearerAuth definition exists in
components.securitySchemes (type: http, scheme: bearer, bearerFormat: JWT or
appropriate) so the OpenAPI spec enforces token-based auth for this endpoint.
🧹 Nitpick comments (2)
api.yaml (2)
88-90: Consider addingmaxItemsconstraint to the array schema.Per static analysis (CKV_OPENAPI_21), arrays without bounds can lead to unexpectedly large responses. Adding
maxItemsdocuments the expected limit and improves API contract clarity.Proposed fix
schema: type: array + maxItems: 1000 items: type: string
197-202: Consider adding global security with explicit overrides for public endpoints.Static analysis (CKV_OPENAPI_4, CKV_OPENAPI_5) flags the absence of a global
securityfield. Adding global security as the default, with explicitsecurity: []overrides on public endpoints (/api,/token,/health), makes the security model explicit and reduces the risk of accidentally exposing new endpoints without authentication.Example structure
Add at the root level (after
pathsorcomponents):security: - bearerAuth: []Then override on public endpoints:
/health: get: security: [] # Explicitly public ...
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
api.yamlterraform_examples/api_gateway.tf
🚧 Files skipped from review as they are similar to previous changes (1)
- terraform_examples/api_gateway.tf
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-12T14:50:03.518Z
Learnt from: tmikula-dev
Repo: AbsaOSS/EventGate PR: 102
File: terraform_examples/lambda.tf:20-51
Timestamp: 2026-01-12T14:50:03.518Z
Learning: The terraform_examples/ directory in the EventGate repository contains example Terraform configurations for reference purposes only, not production-ready configurations. Security checks and hardening recommendations may be appropriately omitted in these example files.
Applied to files:
api.yaml
🪛 Checkov (3.2.334)
api.yaml
[high] 1-203: Ensure that the global security field has rules defined
(CKV_OPENAPI_4)
[high] 1-203: Ensure that security operations is not empty.
(CKV_OPENAPI_5)
[medium] 88-93: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🔇 Additional comments (1)
api.yaml (1)
197-202: LGTM onbearerFormat: JWTaddition.This improves the API documentation by explicitly indicating the expected token format.
Overview
This PR implements several changes, that are making this project up to date. The changes are required, since there is continuous development. It updates the configuration folder, naming of the terraform files, README file, separation of concerns.
Release Notes
Related
Closes #101
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.