Changed cicd to deploy the external service#747
Conversation
ref https://linear.app/ghost/issue/PROD-1908 - Changed CI/CD to deploy the external CloudRun service.
WalkthroughThis change updates the GitHub Actions CI/CD workflow by altering the GCP Load Balancer URL map configuration for ephemeral staging deployments, specifically renaming host rule and path matcher references from "staging-environments" to "all-paths." The workflow also introduces new deployment steps for an "ActivityPub External" service in both staging and production jobs, deploying this service to Cloud Run in the same regions as existing ActivityPub services and using the same Docker image version. Service naming conventions are updated to reflect the environment and region. No changes are made to authentication, environment variables, or control flow. Possibly related PRs
Suggested labels
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![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 (3)
.github/workflows/cicd.yml (3)
285-286: Validate priority fallback logic for new route rules.
The use ofmax // 0ensuresMAX_PRIORITYis0if no existingrouteRulesare found, yieldingNEXT_PRIORITY=1. Confirm this aligns with your intended priority scheme. Adding a brief inline comment explaining the fallback could improve future maintainability.
371-379: Review new staging deploy step for ActivityPub External service.
The addedDeploy ActivityPub External to Cloud Runstep correctly follows the pattern of the existing API deployment, using${{ needs.build-test-push.outputs.activitypub_docker_version }}and theserviceparameter.
- Verify that
activitypub_docker_versionis the correct image tag for this external service.- Ensure the service name
stg-${{ matrix.region_name }}-activitypub-externalmatches your Terraform or infra naming conventions.- Optional: extract common parameters into a reusable step or composite action to reduce duplication across services.
435-443: Review new production deploy step for ActivityPub External service.
Similarly, theDeploy ActivityPub External to Cloud Runstep in production matches the staging pattern.
- Confirm you have the appropriate IAM permissions and service account configuration (
prd-activitypub-cicd@ghost-activitypub.iam.gserviceaccount.com) to deploy this additional service.- Validate the service name
prd-${{ matrix.region_name }}-activitypub-externalis declared in your infra code (Terraform/GCP config).
| yq -i '.hostRules = (.hostRules // [{"hosts": ["activitypub.ghostinfra.net"], "pathMatcher": "all-paths"}])' config.yml | ||
| yq -i '.pathMatchers = (.pathMatchers // [{"name": "all-paths", "defaultService": "'"$DEFAULT_SERVICE"'", "routeRules": []}])' config.yml | ||
| # Remove existing route rules for the PR service |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential logic issue merging URL map defaults with // operator.
The expressions use (.hostRules // [...]) and (.pathMatchers // [...]), which only set defaults when these fields are missing or null, leaving existing arrays untouched instead of appending or updating entries. This may prevent the all-paths matcher from being applied for ephemeral staging if the fields already exist.
Consider using appends (+=) or transforming existing entries (e.g., map) to update or add the new matcher.
🤖 Prompt for AI Agents
In .github/workflows/cicd.yml around lines 280 to 282, the current use of the
`//` operator only sets default values if `.hostRules` or `.pathMatchers` are
missing or null, but does not append or update existing arrays. To fix this,
modify the yq commands to append the new host rule and path matcher to the
existing arrays using the `+=` operator or use a map transformation to update or
add the `all-paths` matcher, ensuring the ephemeral staging configuration is
correctly applied even when these fields already exist.
1 participant
ref https://linear.app/ghost/issue/PROD-1908