Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test Backend api #12

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Test Backend api #12

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
128cc80
Test Backend api
Sep 30, 2024
4811eeb
TEST
Sep 30, 2024
f9a35c6
TEST
Sep 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions .github/workflows/deploy-to-gcp.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,23 @@ jobs:
with:
version: 9

- name: Check for Terraform changes
id: check_changes
run: |
if git diff --name-only -r HEAD^1 HEAD | grep -q '^terraform/.*\.tf$'; then
echo "backend_api_hanges=true" >> $GITHUB_ENV
else
echo "backend_api_changes=false" >> $GITHUB_ENV
fi

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix typo in environment variable name and improve script robustness

There's a typo in the environment variable name, and the script can be improved for better robustness:

  1. Line 39: backend_api_hanges should be backend_api_changes.
  2. Use double quotes around variables to prevent globbing and word splitting.

Apply this diff to fix the issues:

 - name: Check for Terraform changes
   id: check_changes
   run: |
-    if git diff --name-only -r HEAD^1 HEAD | grep -q '^terraform/.*\.tf$'; then
-      echo "backend_api_hanges=true" >> $GITHUB_ENV
+    if git diff --name-only -r HEAD^1 HEAD | grep -q "^terraform/.*\.tf$"; then
+      echo "backend_api_changes=true" >> "$GITHUB_ENV"
     else
-      echo "backend_api_changes=false" >> $GITHUB_ENV
+      echo "backend_api_changes=false" >> "$GITHUB_ENV"
     fi

These changes will fix the typo and improve the script's robustness by preventing potential issues with word splitting and globbing.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Check for Terraform changes
id: check_changes
run: |
if git diff --name-only -r HEAD^1 HEAD | grep -q '^terraform/.*\.tf$'; then
echo "backend_api_hanges=true" >> $GITHUB_ENV
else
echo "backend_api_changes=false" >> $GITHUB_ENV
fi
- name: Check for Terraform changes
id: check_changes
run: |
if git diff --name-only -r HEAD^1 HEAD | grep -q "^terraform/.*\.tf$"; then
echo "backend_api_changes=true" >> "$GITHUB_ENV"
else
echo "backend_api_changes=false" >> "$GITHUB_ENV"
fi
🧰 Tools
🪛 actionlint

37-37: shellcheck reported issue in this script: SC2086:info:2:37: Double quote to prevent globbing and word splitting

(shellcheck)

37-37: shellcheck reported issue in this script: SC2086:info:4:39: Double quote to prevent globbing and word splitting

(shellcheck)

- name: Install taskfile
run: |
sudo snap install task --classic

- name: Terraform Plan
run: task terraform-plan
- name: Backend API Terraform Plan
run: task backend-plan
if: env.backend_api_changes == 'true'

- name: Terraform Apply
if: github.ref == 'refs/heads/"main"' && github.event_name == 'push'
- name: Backend Terraform Apply
if: github.ref == 'refs/heads/"main"' && github.event_name == 'push' && env.backend_api_changes == 'true'
run: task terraform-apply
Comment on lines +56 to 58
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix branch name condition

The step has been correctly renamed and the additional condition for backend_api_changes is properly implemented. However, there's still an issue with the branch name condition that was mentioned in the previous review.

Apply this diff to fix the branch name condition:

- if: github.ref == 'refs/heads/"main"' && github.event_name == 'push' && env.backend_api_changes == 'true'
+ if: github.ref == 'refs/heads/main' && github.event_name == 'push' && env.backend_api_changes == 'true'

This change removes the extra double quotes around "main", which could cause the condition to fail.

The rest of the changes in this step look good. The additional condition for backend_api_changes is correctly implemented, and the step name change provides better clarity.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Backend Terraform Apply
if: github.ref == 'refs/heads/"main"' && github.event_name == 'push' && env.backend_api_changes == 'true'
run: task terraform-apply
- name: Backend Terraform Apply
if: github.ref == 'refs/heads/main' && github.event_name == 'push' && env.backend_api_changes == 'true'
run: task terraform-apply

16 changes: 8 additions & 8 deletions Taskfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,21 +84,21 @@ tasks:
terraform workspace select ${TERRAFORM_WORKSPACE}
echo -e "Successfully switched to terraform workspace: ${GREEN}${TERRAFORM_WORKSPACE}${NC}"

terraform-providers-lock:
backend-providers-lock:
dir: terraform
desc: Lock terraform providers for all platforms
cmds:
- |
terraform providers lock -platform=windows_amd64 -platform=darwin_amd64 -platform=linux_amd64

terraform-fmt:
backend-fmt:
dir: terraform
desc: Format terraform files
cmds:
- |
terraform fmt

terraform-init:
backend-init:
dir: terraform
desc: Initialize terraform backend, providers, plugins and modules
deps:
Expand All @@ -109,7 +109,7 @@ tasks:
-backend-config="bucket=${ARTIFACTS_BUCKET_NAME}" \
-backend-config="prefix=${ARTIFACTS_BUCKET_TERRAFORM_PREFIX}"

terraform-plan:
backend-plan:
dir: terraform
desc: Creates an execution plan, which lets you preview the changes that Terraform plans to make to your infrastructure.
cmds:
Expand All @@ -124,7 +124,7 @@ tasks:
-var "region=${GOOGLE_CLOUD_PROJECT_REGION}" \
-var "docker_image_tag={{.DOCKER_IMAGE_TAG}}"

terraform-apply:
backend-apply:
dir: terraform
desc: Apply the planned changes to the target infrastructure
cmds:
Expand All @@ -141,7 +141,7 @@ tasks:
-var "region=${GOOGLE_CLOUD_PROJECT_REGION}" \
-var "docker_image_tag={{.DOCKER_IMAGE_TAG}}"

terraform-plan-destroy:
backend-plan-destroy:
dir: terraform
cmds:
- task: switch-to-terraform-workspace
Expand All @@ -153,7 +153,7 @@ tasks:
-var "region=${GOOGLE_CLOUD_PROJECT_REGION}" \
-var "docker_image_tag={{.DOCKER_IMAGE_TAG}}"

terraform-destroy:
backend-destroy:
dir: terraform
desc: Delete all resources created by terraform
cmds:
Expand All @@ -166,7 +166,7 @@ tasks:
-var "region=${GOOGLE_CLOUD_PROJECT_REGION}" \
-var "docker_image_tag={{.DOCKER_IMAGE_TAG}}"

terraform-upgrade-providers:
backend-upgrade-providers:
dir: terraform
desc: Upgrade terraform providers
cmds:
Expand Down
Loading