Added ephemeral staging environments #432
Conversation
WalkthroughThis pull request updates the GitHub Actions workflow configuration file by renaming the workflow from "CI" to "CICD" and refining the pull_request trigger to respond only to specific actions: opened, synchronize, reopened, labeled, and unlabeled. Permissions are extended to include read access for pull-requests. The build-test-push job's environment is renamed from "staging" to "build", and steps involving GCP authentication, Artifact Registry login, and Docker image pushing are conditionally executed only for pull_request events with the specified actions. A new deploy-pr job is added to deploy ephemeral staging environments when a pull request label matches the pattern "*.ghost.is". This job performs label checking, checks out two repositories, sets up Terraform with a version read from a file, modifies Terraform module URLs and backend prefixes, authenticates with GCP, runs Terraform init and apply, deploys migrations to Cloud Run, and updates a GCP load balancer URL map to route traffic to the ephemeral environment. The deploy-staging and deploy-production jobs remain but have their environment declarations removed. Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (5)
🪧 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 (
|
rmgpinto
force-pushed
the
ephemeral-staging-envs
branch
from
45dd712 to
aafd700
Compare
rmgpinto
force-pushed
the
ephemeral-staging-envs
branch
2 times, most recently
from
57a967b to
037b8fc
Compare
rmgpinto
force-pushed
the
ephemeral-staging-envs
branch
from
037b8fc to
07388f2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
.github/workflows/deploy.yml (1)
97-103:⚠️ Potential issueCritical Issue: Undefined Output in Deploy-Old for ActivityPub API
Finally, the "Deploy ActivityPub API to Cloud Run" step in the deploy-old job references the same undefined output. To ensure proper image deployment, update this reference to correctly obtain the intended version information from the CI workflow.
🧰 Tools
🪛 actionlint (1.7.4)
100-100: property "build-test-push" is not defined in object type {}
(expression)
🧹 Nitpick comments (1)
.github/workflows/deploy.yml (1)
10-33: Deploy Ephemeral Staging: Variable Usage & Environment NamingThe ephemeral staging job properly handles Google Cloud authentication and checks if the infrastructure is already provisioned. Two points to consider:
- Variable Availability: The command uses
GITHUB_HEAD_REFto filter services. Sinceworkflow_runcontexts may not always populate this variable (which is more typical in pull request events), please verify its availability or consider an alternative identifier.- Environment Naming: The job is configured with
environment: build. In the context of ephemeral staging, a more descriptive name (e.g.,ephemeral-staging) might improve clarity in your deployment logs and reports.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build.yml(1 hunks).github/workflows/deploy.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/deploy.yml
52-52: property "build-test-push" is not defined in object type {}
(expression)
60-60: property "build-test-push" is not defined in object type {}
(expression)
67-67: property "build-test-push" is not defined in object type {}
(expression)
85-85: property "build-test-push" is not defined in object type {}
(expression)
93-93: property "build-test-push" is not defined in object type {}
(expression)
100-100: property "build-test-push" is not defined in object type {}
(expression)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build, Test and Push
🔇 Additional comments (2)
.github/workflows/build.yml (1)
6-11: Enhanced Pull Request Event TriggersThe addition of specific pull request event types (
opened,synchronize,reopened,labeled, andunlabeled) improves the granularity of workflow triggers for PR events. This should help ensure that changes are properly tested during development..github/workflows/deploy.yml (1)
1-8: Solid Setup for the New Deployment WorkflowThe workflow is clearly set up to trigger on
workflow_runfor the "CI" workflow with thecompletedtype. This ensures that the deployment process only begins after the CI process finishes. It would be beneficial to document in your repository how the outputs from the CI workflow (like the image version tags) are expected to be passed along, as they are referenced later in the file.
rmgpinto
force-pushed
the
ephemeral-staging-envs
branch
from
07388f2 to
285f845
Compare
rmgpinto
force-pushed
the
ephemeral-staging-envs
branch
from
285f845 to
d6970c0
Compare
rmgpinto
force-pushed
the
ephemeral-staging-envs
branch
from
d6970c0 to
623a891
Compare
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
.github/workflows/deploy.yml (1)
85-117:⚠️ Potential issueDeploy-Old Job: Missing Dependency Declaration
Similarly, the "deploy-old" job uses outputs from theci-docker-tagsjob without declaring a dependency on it. This can result in undefined outputs during execution. To ensure that the necessary data is available, add the dependency like so:- runs-on: ubuntu-latest + needs: [ci-docker-tags] + runs-on: ubuntu-latest🧰 Tools
🪛 actionlint (1.7.4)
99-99: property "ci-docker-tags" is not defined in object type {}
(expression)
107-107: property "ci-docker-tags" is not defined in object type {}
(expression)
114-114: property "ci-docker-tags" is not defined in object type {}
(expression)
🧹 Nitpick comments (1)
.github/workflows/deploy.yml (1)
9-23: CI Docker Tags Job: Output Extraction
Theci-docker-tagsjob correctly downloads thedocker_tagsartifact and extracts the Docker tag values foractivitypubandmigrations. Consider adding error handling or a check to ensure thatdocker_tags.txtexists and contains the expected content; this would help avoid unexpected failures in subsequent deployment jobs if the artifact is missing or malformed.
🛑 Comments failed to post (1)
.github/workflows/deploy.yml (1)
48-84:
⚠️ Potential issueDeploy-Staging Job: Missing Dependency Declaration
This job references outputs from theci-docker-tagsjob using expressions such as${{ needs.ci-docker-tags.outputs.migrations_docker_tags }}and${{ needs.ci-docker-tags.outputs.activitypub_docker_tags }}. However, there is no dependency declared using theneeds:keyword, which means these outputs may not be available and could lead to runtime errors. To fix this, add the dependency to ensure that theci-docker-tagsjob completes before this job starts. For example:- name: (staging) Deploy + needs: [ci-docker-tags] + name: (staging) Deploy📝 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.deploy-staging: if: github.ref == 'refs/heads/main' needs: [ci-docker-tags] name: (staging) Deploy environment: build runs-on: ubuntu-latest strategy: matrix: region: [europe-west4, europe-west3] steps: - name: "Auth with Google Cloud" uses: 'google-github-actions/auth@v2' with: credentials_json: ${{ secrets.SERVICE_ACCOUNT_KEY }} - name: "Deploy Migrations to Cloud Run" if: ${{ matrix.region == 'europe-west4' }} uses: 'google-github-actions/deploy-cloudrun@v2' with: image: europe-docker.pkg.dev/ghost-activitypub/activitypub/migrations:${{ needs.ci-docker-tags.outputs.migrations_docker_tags }} region: ${{ matrix.region }} job: stg-${{ matrix.region }}-activitypub-migrations flags: '--wait --execute-now --set-cloudsql-instances=ghost-activitypub:${{ matrix.region }}:stg-${{ matrix.region }}-activitypub-primary' - name: "Deploy ActivityPub Queue to Cloud Run" uses: 'google-github-actions/deploy-cloudrun@v2' with: image: europe-docker.pkg.dev/ghost-activitypub/activitypub/activitypub:${{ needs.ci-docker-tags.outputs.activitypub_docker_tags }} region: ${{ matrix.region }} service: stg-${{ matrix.region }}-activitypub-queue - name: "Deploy ActivityPub API to Cloud Run" uses: 'google-github-actions/deploy-cloudrun@v2' with: image: europe-docker.pkg.dev/ghost-activitypub/activitypub/activitypub:${{ needs.ci-docker-tags.outputs.activitypub_docker_tags }} region: ${{ matrix.region }} service: stg-${{ matrix.region }}-activitypub-api🧰 Tools
🪛 actionlint (1.7.4)
66-66: property "ci-docker-tags" is not defined in object type {}
(expression)
74-74: property "ci-docker-tags" is not defined in object type {}
(expression)
81-81: property "ci-docker-tags" is not defined in object type {}
(expression)
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
.github/workflows/deploy.yml (1)
85-117:⚠️ Potential issueDeploy-Old Job – Ensure Consistent Image Tag References
Similarly, thedeploy-oldjob references outputs from theci-docker-tagsjob (e.g.,${{ needs.ci-docker-tags.outputs.migrations_docker_tags }}). This setup is dependent on the proper declaration of job outputs in theci-docker-tagsjob. Once the outputs are defined at the job level, these references should resolve correctly.🧰 Tools
🪛 actionlint (1.7.4)
99-99: property "ci-docker-tags" is not defined in object type {}
(expression)
107-107: property "ci-docker-tags" is not defined in object type {}
(expression)
114-114: property "ci-docker-tags" is not defined in object type {}
(expression)
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
99-101: Commented Out Test Step – Verify Intentional Disablement
The "Run Tests" step has been commented out. If this is an intentional decision (perhaps because tests are handled elsewhere or temporarily disabled during staging deployments), please add an inline comment explaining the rationale to avoid confusion in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build.yml(3 hunks).github/workflows/deploy.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/deploy.yml
66-66: property "ci-docker-tags" is not defined in object type {}
(expression)
74-74: property "ci-docker-tags" is not defined in object type {}
(expression)
81-81: property "ci-docker-tags" is not defined in object type {}
(expression)
99-99: property "ci-docker-tags" is not defined in object type {}
(expression)
107-107: property "ci-docker-tags" is not defined in object type {}
(expression)
114-114: property "ci-docker-tags" is not defined in object type {}
(expression)
🔇 Additional comments (3)
.github/workflows/build.yml (2)
6-11: Enhanced Pull Request Trigger Events
The addition of the extra pull_request event types (opened, synchronize, reopened, labeled, unlabeled) broadens the scope of events triggering this workflow. This ensures that changes affecting PR metadata are caught early.
130-139: Output and Upload Docker Tags Steps Added
The new steps to output Docker tags to a file and subsequently upload them as an artifact correctly support the new CD workflow. Ensure that the file path and artifact name remain consistent with what the deployment workflow expects..github/workflows/deploy.yml (1)
1-8: Continuous Deployment Workflow Trigger Configuration
The CD workflow is now configured to trigger on the completion of the CI workflow viaworkflow_run. This setup is well suited to ensure that deployment steps run only after CI completes successfully.
rmgpinto
force-pushed
the
ephemeral-staging-envs
branch
from
623a891 to
05f4435
Compare
rmgpinto
force-pushed
the
ephemeral-staging-envs
branch
from
97d8821 to
bb6739c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/cicd.yml (1)
163-163:⚠️ Potential issueQuote shell variables in conditional checks for robustness
In the label check step, the conditional
if [ $LABEL_NAMES != "" ];should quote the variable to prevent errors if$LABEL_NAMESis unset or contains spaces. Use[ -n "$LABEL_NAMES" ]for a safe, POSIX-compliant check.- if [ $LABEL_NAMES != "" ]; then + if [ -n "$LABEL_NAMES" ]; thenThis prevents unbound variable errors and improves script reliability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/cicd.yml(5 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/cicd.yml
173-173: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
181-181: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build, Test and Push
🔇 Additional comments (3)
.github/workflows/cicd.yml (3)
245-275: Excellent: Load balancer update script is robust and follows best practicesThe load balancer update step now includes proper error handling, atomic file operations, and re-imports the modified config. Temporary files are managed and the logic is clear. This addresses all previously raised concerns.
109-140: Conditional Docker push logic is correct and efficientThe conditional execution of GCP authentication, Artifact Registry login, and Docker image pushes ensures that these steps only run for relevant pull request actions. This optimizes workflow runs and avoids unnecessary operations.
149-243: Ephemeral staging deployment logic is well-structuredThe new
deploy-prjob is well-organized, with clear separation of label checking, repo checkouts, Terraform setup, GCP authentication, and deployment steps. The use of environment variables and conditional execution is appropriate.🧰 Tools
🪛 actionlint (1.7.4)
173-173: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
181-181: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
rmgpinto
force-pushed
the
ephemeral-staging-envs
branch
from
bb6739c to
fdb9276
Compare
rmgpinto
force-pushed
the
ephemeral-staging-envs
branch
from
fdb9276 to
ff014ca
Compare
rmgpinto
force-pushed
the
ephemeral-staging-envs
branch
from
ff014ca to
400c875
Compare
rmgpinto
force-pushed
the
ephemeral-staging-envs
branch
from
400c875 to
9c04fb5
Compare
rmgpinto
force-pushed
the
ephemeral-staging-envs
branch
from
9c04fb5 to
02d8c6b
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds ephemeral staging environments by updating the CICD workflow triggers, modifying job environments, and introducing a new deploy-pr job to conditionally deploy based on pull request labels.
- Updated pull request event types and conditions in the workflow.
- Changed the build-test-push job’s environment from "staging" to "build".
- Added a deploy-pr job with label checking and load balancer updates for ephemeral staging.
.github/workflows/cicd.yml
Outdated
| run: | | ||
| export LABEL_NAMES=$(echo "$LABELS" | jq -r '[.[] | select(.name | test("\\.ghost\\.is$")) | .name] | join(",")') | ||
| echo "Label names: $LABEL_NAMES" | ||
| if [ $LABEL_NAMES != "" ]; then |
There was a problem hiding this comment.
Consider quoting $LABEL_NAMES (e.g. if [ "$LABEL_NAMES" != "" ]; then) to prevent word splitting or potential errors when the variable is empty.
| if [ $LABEL_NAMES != "" ]; then | |
| if [ "$LABEL_NAMES" != "" ]; then |
.github/workflows/cicd.yml
Outdated
| export PR_SERVICE="https://www.googleapis.com/compute/v1/projects/ghost-activitypub/global/backendServices/stg-pr-${{ github.event.pull_request.number }}-api" | ||
| # Add host rules and path matchers if they don't exist | ||
| yq '.hostRules = (.hostRules // [{"hosts": ["activitypub.infra.ghost.is"], "pathMatcher": "staging-environments"}])' config.yml > config.yml.tmp | ||
| yq '.hostRules = ([{"hosts": ["activitypub.infra.ghost.is"], "pathMatcher": "staging-environments"}])' config.yml > config.yml.tmp |
There was a problem hiding this comment.
The .hostRules field is being set consecutively (lines 258 and 259); if the duplication is not intentional, please remove the redundant command to avoid unexpected overwrites.
| yq '.hostRules = ([{"hosts": ["activitypub.infra.ghost.is"], "pathMatcher": "staging-environments"}])' config.yml > config.yml.tmp |
| build-test-push: | ||
| name: Build, Test and Push | ||
| environment: staging | ||
| environment: build |
There was a problem hiding this comment.
[nitpick] The build-test-push job’s environment was changed from 'staging' to 'build'. Please confirm that this alteration is intentional and does not inadvertently affect deployment behavior.
| environment: build | |
| environment: staging |
closes https://linear.app/ghost/issue/AP-976 - Create ephemeral staging environment on PR labels *.ghost.is
rmgpinto
force-pushed
the
ephemeral-staging-envs
branch
from
02d8c6b to
e33a7ae
Compare
closes https://linear.app/ghost/issue/AP-976 - Create ephemeral staging environment on PR labels *.ghost.is
closes https://linear.app/ghost/issue/AP-976