Skip to content

Fixed destroy tests staging databases when a PR is merged #729

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

Merged
rmgpinto merged 1 commit into from
May 26, 2025
Merged

Fixed destroy tests staging databases when a PR is merged #729

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 18 additions & 10 deletions .github/workflows/cicd.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,24 @@ jobs:
- region: europe-west3
region_name: frankfurt
steps:
- name: "Authenticate with GCP (staging envs)"
if: ${{ matrix.region == 'europe-west4' }}
uses: google-github-actions/auth@v2
with:
token_format: access_token
workload_identity_provider: projects/687476608778/locations/global/workloadIdentityPools/github-oidc-activitypub/providers/github-provider-activitypub
service_account: stg-activitypub-cicd-stg-envs@ghost-activitypub.iam.gserviceaccount.com

- name: "Destroy Tests databases"
if: ${{ matrix.region == 'europe-west4' }}
env:
GCP_PROJECT: ghost-activitypub
run: |
TEST_DATABASES=$(gcloud sql databases list --instance=stg-netherlands-activitypub --filter="name~test*" --format="value(name)" --project ${GCP_PROJECT})
for TEST_DATABASE in ${TEST_DATABASES}; do
gcloud sql databases delete ${TEST_DATABASE} --instance=stg-netherlands-activitypub --quiet --project ${GCP_PROJECT}
done
Comment on lines +319 to +327
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 26, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refine database deletion filter to target only ephemeral test databases.

Using --filter="name~test*" risks matching unintended databases. It’s safer to anchor the regex to your ephemeral naming convention (e.g. pr_<number>_test…) and enable strict bash safety flags.

Apply a targeted diff:

- run: |
-   TEST_DATABASES=$(gcloud sql databases list --instance=stg-netherlands-activitypub --filter="name~test*" --format="value(name)" --project ${GCP_PROJECT})
-   for TEST_DATABASE in ${TEST_DATABASES}; do
-     gcloud sql databases delete ${TEST_DATABASE} --instance=stg-netherlands-activitypub --quiet --project ${GCP_PROJECT}
-   done
+ run: |
+   set -euo pipefail
+   TEST_DATABASES=$(gcloud sql databases list \
+     --instance=stg-netherlands-activitypub \
+     --filter="name~'^pr_[0-9]+_test.*$'" \
+     --format="value(name)" \
+     --project="${GCP_PROJECT}")
+   for TEST_DATABASE in ${TEST_DATABASES}; do
+     gcloud sql databases delete "${TEST_DATABASE}" \
+       --instance=stg-netherlands-activitypub \
+       --quiet \
+       --project="${GCP_PROJECT}"
+   done
📝 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: "Destroy Tests databases"
if: ${{ matrix.region == 'europe-west4' }}
env:
GCP_PROJECT: ghost-activitypub
run: |
TEST_DATABASES=$(gcloud sql databases list --instance=stg-netherlands-activitypub --filter="name~test*" --format="value(name)" --project ${GCP_PROJECT})
for TEST_DATABASE in ${TEST_DATABASES}; do
gcloud sql databases delete ${TEST_DATABASE} --instance=stg-netherlands-activitypub --quiet --project ${GCP_PROJECT}
done
- name: "Destroy Tests databases"
if: ${{ matrix.region == 'europe-west4' }}
env:
GCP_PROJECT: ghost-activitypub
run: |
set -euo pipefail
TEST_DATABASES=$(gcloud sql databases list \
--instance=stg-netherlands-activitypub \
--filter="name~'^pr_[0-9]+_test.*$'" \
--format="value(name)" \
--project="${GCP_PROJECT}")
for TEST_DATABASE in ${TEST_DATABASES}; do
gcloud sql databases delete "${TEST_DATABASE}" \
--instance=stg-netherlands-activitypub \
--quiet \
--project="${GCP_PROJECT}"
done
🤖 Prompt for AI Agents 
In .github/workflows/cicd.yml around lines 319 to 327, the database deletion
command uses a loose filter "name~test*" which may match unintended databases.
Refine the filter to match your specific ephemeral test database naming pattern,
such as "pr_<number>_test", and add bash safety flags like 'set -euo pipefail'
at the start of the run block to improve script robustness and prevent
accidental deletions. Ensure the filter accurately targets only the intended
ephemeral test databases.


- name: "Authenticate with GCP"
id: gcp-auth
uses: google-github-actions/auth@v2
Expand Down Expand Up @@ -340,16 +358,6 @@ jobs:
# labels: |-
# commit-sha=${{ github.sha }}

- name: "Destroy Tests databases"
if: ${{ matrix.region == 'europe-west4' }}
env:
GCP_PROJECT: ghost-activitypub
run: |
TEST_DATABASES=$(gcloud sql databases list --instance=stg-netherlands-activitypub --filter="name~test*" --format="value(name)" --project ${GCP_PROJECT})
for TEST_DATABASE in ${TEST_DATABASES}; do
gcloud sql databases delete ${TEST_DATABASE} --instance=stg-netherlands-activitypub --quiet --project ${GCP_PROJECT}
done

- name: "Deploy ActivityPub Queue to Cloud Run"
uses: google-github-actions/deploy-cloudrun@v2
with:
Expand Down