[OSDEV-1886] Fix script for delete Lamda@Edge functions#597
Conversation
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
|
Warning Rate limit exceeded@roninzp has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 23 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis update modifies several components related to Lambda@Edge function deletion and workflow management. The Changes
Sequence Diagram(s)sequenceDiagram
participant Workflow
participant DeleteLambdaScript
participant AWS Lambda
participant AWS CloudFront
Workflow->>DeleteLambdaScript: Trigger destroy workflow
DeleteLambdaScript->>AWS CloudFront: Get distribution config
DeleteLambdaScript->>DeleteLambdaScript: Remove Lambda@Edge associations
DeleteLambdaScript->>AWS CloudFront: Update distribution config
loop For each Lambda version (excluding "$LATEST")
DeleteLambdaScript->>AWS Lambda: Attempt to delete version
alt Replication error
DeleteLambdaScript->>DeleteLambdaScript: Wait 5 minutes, retry
else Success
DeleteLambdaScript->>Workflow: Exit on success
else Other error
DeleteLambdaScript->>Workflow: Print error, exit with failure
end
end
Possibly related PRs
Suggested reviewers
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. 🪧 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 (
|
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
React App | Jest test suite - Code coverage reportTotal: 33.99%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
deployment/environments/terraform-preprod.tfvars (1)
31-32: Reduced CLI compute resourcesThe CLI task CPU and memory have been significantly reduced. The memory allocation of 124MB seems particularly low and might be a typo (common values are 128, 256, 512, etc.).
- cli_fargate_memory = "124" + cli_fargate_memory = "128"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignore(1 hunks)deployment/delete_lambda(3 hunks)deployment/environments/terraform-preprod.tfvars(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: run-flake8-linter
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: run-django-code-quality
- GitHub Check: get-base-branch-django-cov
🔇 Additional comments (6)
.gitignore (1)
97-98: LGTM: Standard ignore patterns for backup files addedThese patterns will prevent backup and temporary files created by various text editors from being committed to the repository, which is a good housekeeping practice.
deployment/delete_lambda (2)
1-1: Added debugging flags to shell executionThe
-xvflags will enable verbose logging and command tracing, which is helpful for troubleshooting script execution issues. This is appropriate for a maintenance script dealing with infrastructure changes.
22-22: Improved version selection logicThe updated JQ expression now properly filters out the literal "$LATEST" version before selecting the maximum numeric version. This is more robust than the previous approach, especially if versions aren't returned in numeric order.
deployment/environments/terraform-preprod.tfvars (3)
19-19: Significant downgrade of RDS instance typeChanged from a higher tier instance to
db.t3.micro, which is a burstable instance with limited CPU and memory. Ensure this is sufficient for your preprod database workload.
25-25: Reduced app ECS resourcesApp container count reduced to 1 and compute resources scaled down. This is appropriate for cost optimization in preprod, but verify that it meets your testing requirements.
Also applies to: 28-29
54-55: Adjusted resource allocations for additional servicesResources for data deduplication, OpenSearch, and Logstash components have been adjusted for the preprod environment. These changes align with the overall strategy to optimize resource usage in the preprod environment.
Also applies to: 58-58, 61-61
Dedupe Hub App | Unittest test suite - Code coverage reportTotal: 56.14%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Countries App | Unittest test suite - Code coverage reportTotal: 100%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
Contricleaner App | Unittest test suite - Code coverage reportTotal: 98.91%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
had a problem deploying
to
Quality Environment
GitHub Actions
Failure
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
roninzp
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
deployment/delete_lambda (1)
42-49: Use temporary files and automatic cleanup for distribution config artefactsPersisting
full-config.json,dist-config.json, andnew-config.jsonin the working directory may leak sensitive data and pollute the repo when the script is run locally.
Leveragemktempand atrapto remove the files on exit.-aws cloudfront get-distribution-config --id $DISTRIBUTION_ID > full-config.json -ETAG=$(cat full-config.json | jq -r '.ETag') -... -cat full-config.json | jq '.DistributionConfig' > dist-config.json +FULL_CFG="$(mktemp)" +DIST_CFG="$(mktemp)" +NEW_CFG="$(mktemp)" +trap 'rm -f "$FULL_CFG" "$DIST_CFG" "$NEW_CFG"' EXIT + +aws cloudfront get-distribution-config --id "$DISTRIBUTION_ID" > "$FULL_CFG" +ETAG=$(jq -r '.ETag' < "$FULL_CFG") +jq '.DistributionConfig' "$FULL_CFG" > "$DIST_CFG"Remember to replace later occurrences of
dist-config.json/new-config.jsonwith the new variables.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
deployment/delete_lambda(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-flake8-linter
- GitHub Check: run-fe-code-quality
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-django-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-django-cov
There was a problem hiding this comment.
- I have a question: why did you test the deletion of only the AddSecurityHeaders function, and not both functions(AddSecurityHeaders and RedirectToS3origin), is it enough in this case? https://github.com/opensupplyhub/open-supply-hub/actions/runs/14594317960/job/40940180830
- I have left a few comments.
The first function was removed by the same script during local tests. Since the script removes only one function at a time, I decided that testing the removal of just one function in the pipeline would be sufficient, without redeploying the environment. |
|
3 participants
Fixed the script to run within the Destroy Environment GitHub workflow to delete the Lambda@Edge functions before destroying the infrastructure.