Automated testing for AWS infrastructure #6
Conversation
- add testing env vars
- remove auto trigger from action - run tests only if apply was successful - always run destroy step - add falkordb python test file
- Store state and vars as artifacts - Wait for manual approval before applying infra
- separate pipeline for aws deploy after approval
- apply and test k8s module
- delete and create comment - fix cluster name
- Join all artifacts in one folder - Add scripts to readme - Caching Tofu init - hash key is lock - Replan k8s module after aws
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (21)
- .github/workflows/testing-apply-aws-infra.yaml (1 hunks)
- .github/workflows/testing-plan-aws-infra.yaml (1 hunks)
- .gitignore (1 hunks)
- README.md (1 hunks)
- requirements.txt (1 hunks)
- scripts/aws_update_kubeconfig.sh (1 hunks)
- scripts/create_tfvars_from_env.sh (1 hunks)
- scripts/tofu_apply_aws.sh (1 hunks)
- scripts/tofu_apply_k8s.sh (1 hunks)
- scripts/tofu_plan_aws.sh (1 hunks)
- scripts/tofu_plan_k8s.sh (1 hunks)
- scripts/tofu_show.sh (1 hunks)
- scripts/tofu_test.sh (1 hunks)
- tofu/tests/test_falkordb_connection.py (1 hunks)
- tofu/aws/.terraform.lock.hcl (1 hunks)
- tofu/aws/main.tf (8 hunks)
- tofu/aws/outputs.tf (1 hunks)
- tofu/aws/variables.tf (3 hunks)
- tofu/k8s/.terraform.lock.hcl (1 hunks)
- tofu/k8s/main.tf (7 hunks)
- tofu/k8s/variables.tf (2 hunks)
Files skipped from review due to trivial changes (1)
- .gitignore
Additional comments: 16
scripts/tofu_test.sh (1)
- 1-1: Ensure the working directory for the script execution is correctly set to avoid unexpected behavior. Consider verifying the existence of the
../tofudirectory before attempting to change to it and executetofu test.Verification successful
The verification script confirmed that the
../tofudirectory does not exist, which aligns with the initial review comment suggesting to verify the existence of the../tofudirectory before attempting to change to it and executetofu test. This finding supports the suggestion to ensure the script checks for the directory's existence to avoid unexpected behavior.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the existence of the ../tofu directory if [ ! -d "../tofu" ]; then echo "The directory ../tofu does not exist." exit 1 fiLength of output: 118
requirements.txt (1)
- 1-2: Dependencies listed are correct and up-to-date as per the PR objectives. Ensure that the specified versions of
FalkorDBandpytestare compatible with the project's current Python environment and other dependencies.scripts/aws_update_kubeconfig.sh (1)
- 1-1: The script correctly includes the
--role-arnoption for specifying a role ARN when updating the kubeconfig for AWS EKS. Ensure that the script is executed with the necessary permissions to assume the specified role.scripts/tofu_plan_aws.sh (1)
- 1-1: The script correctly navigates to the
../tofudirectory and executes thetofu plancommand with specified parameters for the AWS module. Verify that thelocal-awsoutput file path and the-target=module.awsparameter align with the project's directory structure and Terraform configuration.scripts/tofu_plan_k8s.sh (1)
- 1-1: Similar to the AWS plan script, this script correctly navigates to the
../tofudirectory and executes thetofu plancommand for the Kubernetes module. Ensure thelocal-k8soutput file path and the-target=module.k8sparameter are correctly set according to the project's directory structure and Terraform configuration..github/workflows/testing-apply-aws-infra.yaml (6)
- 3-11: The trigger mechanism for this workflow is well-defined, utilizing both
pull_request_reviewandworkflow_dispatchevents. This setup ensures flexibility in triggering the workflow either through PR reviews or manually, which aligns with the PR objectives.- 22-24: The condition for running the
apply-test-aws-k8s-infrajob ensures that it only executes when a PR review is approved or when manually triggered viaworkflow_dispatch. This is a good practice for preventing unintended infrastructure changes.- 35-40: Using
aws-actions/configure-aws-credentialsfor setting up AWS credentials is a secure and recommended approach. However, ensure that theaws-regionis correctly set by verifying thatvars.TF_VAR_REGIONis defined and populated correctly, as this could lead to issues if the region is not correctly specified or if the variable is not set.- 57-65: Downloading artifacts from a previous workflow run using
dawidd6/action-download-artifactis a clever way to reuse outputs from the planning phase. Ensure that theworkflowandnamepatterns correctly match the expected outputs from the planning workflow to avoid issues with missing or incorrect artifacts.- 160-168: The inclusion of infrastructure destruction steps (
Destroy K8S infrastructureandDestroy AWS infrastructure) at the end of the workflow is a good practice for ensuring that resources are not left running unnecessarily, potentially incurring costs. However, ensure that these steps are thoroughly tested to prevent accidental deletion of resources.- 170-176: The final step to check the outcome of the Python tests and potentially fail the workflow if the tests did not pass is a critical quality gate. This ensures that the workflow only succeeds if all components, including the application functionality verified by the tests, are working as expected.
tofu/k8s/main.tf (5)
- 1-25: The configuration for the Kubernetes and Helm providers uses variables for the cluster endpoint and certificate authority, which is a good practice for flexibility and maintainability. Additionally, the use of the AWS CLI to obtain tokens for Kubernetes authentication is a secure and efficient method. Ensure that the role ARN provided has the necessary permissions for the operations intended to be performed by Terraform.
- 82-101: The
aws_s3_bucket_lifecycle_configurationresource is correctly configured to manage the lifecycle of S3 bucket objects, including setting expiration and aborting incomplete multipart uploads. Ensure that thebucketvariable andtenant_nameare correctly set and that thebackup_retention_periodaligns with your data retention policy.- 120-136: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [102-133]
The
kubernetes_cron_job_v1resource for database backups is well-configured, including a detailed command to perform the backup operation. The addition of adepends_onblock ensures that the backup namespace exists before the cron job is created. However, ensure that the command's steps, especially the use ofkubectland AWS CLI commands, are tested and have the necessary permissions to execute successfully.
- 155-166: The use of local variables to simplify the configuration and the addition of a
random_stringresource for generating a KMS alias are good practices for maintainability and security. Ensure that the length and character set of therandom_stringresource meet your security requirements.- 146-228: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [207-374]
The Helm release configurations for FalkorDB and its monitoring are comprehensive, covering various settings from resource requests to external DNS configuration. Ensure that all variables, such as
falkordb_password,falkordb_cpu,falkordb_memory, and others, are correctly set and that the Helm chart versions used are compatible with your Kubernetes cluster version. Additionally, verify that the Grafana configuration, especially the additional data sources and plugins, meets your monitoring requirements.
| env: | ||
| PR_NUMBER: ${{github.event.pull_request.number }} | ||
| PLAN_WORKFLOW_NAME: testing-plan-aws-infra.yaml | ||
| # TF_CACHE_DIR: ${{ github.workspace }}/tofu/.terraform | ||
|
|
There was a problem hiding this comment.
The dynamic setting of PR_NUMBER using GitHub context is a smart approach to ensure that the workflow can access the PR number across different jobs and steps. However, the commented-out TF_CACHE_DIR environment variable suggests there was an intention to use a Terraform plugin cache directory, which is not utilized in the current workflow. Consider either implementing caching for Terraform plugins to improve performance or removing this commented-out variable to clean up the code.
| - name: Apply AWS infrastructure | ||
| id: apply_infrastructure | ||
| timeout-minutes: 20 | ||
| continue-on-error: true | ||
| working-directory: ./tofu/aws | ||
| run: | | ||
| tofu init | ||
| tofu apply -auto-approve -state-out ../../state/state-aws plan-aws | ||
|
|
||
| - name: Set AWS infra output variables | ||
| continue-on-error: true | ||
| working-directory: ./tofu/aws | ||
| run: | | ||
| echo "TF_VAR_falkordb_eks_cluster_name=$(tofu output -state=../../state/state-aws -raw falkordb_eks_cluster_name)" >> $GITHUB_ENV | ||
| echo "TF_VAR_falkordb_s3_backup_name=$(tofu output -state=../../state/state-aws -raw falkordb_s3_backup_name)" >> $GITHUB_ENV | ||
| echo "TF_VAR_falkordb_eks_cluster_oidc_issuer_url=$(tofu output -state=../../state/state-aws -raw falkordb_eks_cluster_oidc_issuer_url)" >> $GITHUB_ENV | ||
| echo "TF_VAR_falkordb_eks_cluster_oidc_issuer_arn=$(tofu output -state=../../state/state-aws -raw falkordb_eks_cluster_oidc_issuer_arn)" >> $GITHUB_ENV | ||
| echo "TF_VAR_falkordb_eks_cluster_role_arn=$(tofu output -state=../../state/state-aws -raw falkordb_eks_cluster_role_arn)" >> $GITHUB_ENV | ||
| echo "TF_VAR_falkordb_eks_cluster_endpoint=$(tofu output -state=../../state/state-aws -raw falkordb_eks_cluster_endpoint)" >> $GITHUB_ENV | ||
| echo "TF_VAR_falkordb_eks_cluster_certificate_autority=$(tofu output -state=../../state/state-aws -raw falkordb_eks_cluster_certificate_autority)" >> $GITHUB_ENV |
There was a problem hiding this comment.
The steps for applying AWS infrastructure and setting output variables are well-structured. However, using continue-on-error: true might mask failures that should halt the workflow. Consider carefully whether you want the workflow to continue in the face of errors during these steps, as this could lead to partial or incorrect infrastructure deployment.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- .github/workflows/testing-apply-aws-infra.yaml (1 hunks)
- tofu/tests/test_falkordb_connection.py (1 hunks)
- tofu/k8s/main.tf (7 hunks)
Files skipped from review as they are similar to previous changes (3)
- .github/workflows/testing-apply-aws-infra.yaml
- tofu/tests/test_falkordb_connection.py
- tofu/k8s/main.tf
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- .github/workflows/testing-apply-aws-infra.yaml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/testing-apply-aws-infra.yaml
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- .github/workflows/testing-apply-aws-infra.yaml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/testing-apply-aws-infra.yaml
Summary by CodeRabbit
.gitignoreto enhance project cleanliness.requirements.txtfor improved project setup.