Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 7 additions & 7 deletions .github/workflows/deploy-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ jobs:
- name: Destroy Tests databases
if: ${{ steps.check-labels.outputs.is_ephemeral_staging == 'true' }}
env:
GCP_PROJECT: ghost-activitypub
GCP_PROJECT: ghost-activitypub-stg
run: |
TEST_DATABASES=$(gcloud sql databases list --instance=stg-netherlands-activitypub --filter="name~pr_${{ github.event.pull_request.number }}_test*" --format="value(name)" --project ${GCP_PROJECT})
for TEST_DATABASE in ${TEST_DATABASES}; do
Expand All @@ -114,28 +114,28 @@ jobs:
if: ${{ steps.check-labels.outputs.is_ephemeral_staging == 'true' }}
env:
LABELS: ${{ toJson(github.event.pull_request.labels) }}
GCP_PROJECT: ghost-activitypub
GCP_PROJECT: ghost-activitypub-stg
run: |
set -euo pipefail
# Get current config
gcloud compute url-maps export stg-activitypub --global --project ${GCP_PROJECT} > config.yml
# Delete unnecessary fields
yq -i 'del(.fingerprint)' config.yml
yq -i 'del(.creationTimestamp)' config.yml
export DEFAULT_SERVICE="https://www.googleapis.com/compute/v1/projects/ghost-activitypub/global/backendServices/stg-netherlands-activitypub-api"
export PR_SERVICE="https://www.googleapis.com/compute/v1/projects/ghost-activitypub/global/backendServices/stg-pr-${{ github.event.pull_request.number }}-api"
export DEFAULT_SERVICE="https://www.googleapis.com/compute/v1/projects/${GCP_PROJECT}/global/backendServices/stg-netherlands-activitypub-api"
export PR_SERVICE="https://www.googleapis.com/compute/v1/projects/${GCP_PROJECT}/global/backendServices/stg-pr-${{ github.event.pull_request.number }}-api"
# Add host rules and path matchers if they don't exist
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
yq -i '.pathMatchers[] |= (.routeRules |= map(select((.routeAction.weightedBackendServices // []) | length == 0 or .routeAction.weightedBackendServices[0].backendService != env(PR_SERVICE))))' config.yml
# Add new route rules for the PR service
export MAX_PRIORITY=$(yq '[.pathMatchers[] | select(.name == "all-paths") | .routeRules[]?.priority] | max // 0' config.yml)
export NEXT_PRIORITY=$((MAX_PRIORITY + 1))
export MIN_PRIORITY=$(yq '[.pathMatchers[] | select(.name == "all-paths") | .routeRules[]?.priority] | min // 0' config.yml)
export NEXT_PRIORITY=$((MIN_PRIORITY - 1))
Comment on lines +133 to +134
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Guard against negative route priorities

MIN_PRIORITY falls back to 0, so the first time we run without any existing rules we compute NEXT_PRIORITY=-1. Cloud Load Balancer rejects negative priorities (valid range is 0..2147483647), so the import will fail and the workflow breaks. Please clamp the fallback and ensure we never subtract below zero.

-          export MIN_PRIORITY=$(yq '[.pathMatchers[] | select(.name == "all-paths") | .routeRules[]?.priority] | min // 0' config.yml)
-          export NEXT_PRIORITY=$((MIN_PRIORITY - 1))
+          export MIN_PRIORITY=$(yq '[.pathMatchers[] | select(.name == "all-paths") | .routeRules[]?.priority] | min // 1' config.yml)
+          if [ "$MIN_PRIORITY" -le 0 ]; then
+            export NEXT_PRIORITY=0
+          else
+            export NEXT_PRIORITY=$((MIN_PRIORITY - 1))
+          fi
📝 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
export MIN_PRIORITY=$(yq '[.pathMatchers[] | select(.name == "all-paths") | .routeRules[]?.priority] | min // 0' config.yml)
export NEXT_PRIORITY=$((MIN_PRIORITY - 1))
export MIN_PRIORITY=$(yq '[.pathMatchers[] | select(.name == "all-paths") | .routeRules[]?.priority] | min // 1' config.yml)
if [ "$MIN_PRIORITY" -le 0 ]; then
export NEXT_PRIORITY=0
else
export NEXT_PRIORITY=$((MIN_PRIORITY - 1))
fi
🤖 Prompt for AI Agents
In .github/workflows/deploy-pr.yml around lines 133-134, the script computes
NEXT_PRIORITY as MIN_PRIORITY - 1 which can produce -1 when MIN_PRIORITY falls
back to 0; Cloud Load Balancer rejects negative priorities. Change the logic so
MIN_PRIORITY is treated as unsigned and clamp NEXT_PRIORITY to never go below 0
(e.g. if MIN_PRIORITY is 0 or missing, set NEXT_PRIORITY to 0; otherwise set
NEXT_PRIORITY to MIN_PRIORITY - 1).

LABELS_JSON=$(echo "$LABELS" | jq -c '[.[] | select(.name | test("\\.ghost\\.is$")) | .name]')
for LABEL in $(echo "$LABELS_JSON" | jq -r '.[]'); do
echo "Adding route for label: $LABEL"
yq -i '.pathMatchers[0].routeRules += [{"priority": '$NEXT_PRIORITY', "matchRules": [{"prefixMatch": "/", "headerMatches": [{ "headerName": "X-Forwarded-Host", "exactMatch": "'$LABEL'" }]}], "routeAction": {"weightedBackendServices": [ { "backendService": "'$PR_SERVICE'", "weight": 100 } ] }, "headerAction": { "requestHeadersToAdd": [ { "headerName": "X-ActivityPub-PR", "headerValue": "${{ github.event.pull_request.number }}" }, { "headerName": "X-ActivityPub-Commit", "headerValue": "${{ github.event.pull_request.head.sha }}" } ] } }]' config.yml
yq -i '.pathMatchers[0].routeRules = [{"priority": '${NEXT_PRIORITY}', "matchRules": [{"prefixMatch": "/", "headerMatches": [{"headerName": "X-Forwarded-Host", "exactMatch": "'"${LABEL}"'"}]}], "routeAction": {"weightedBackendServices": [{"backendService": "'"${PR_SERVICE}"'", "weight": 100}]}, "headerAction": {"requestHeadersToAdd": [{"headerName": "X-ActivityPub-PR", "headerValue": "${{ github.event.pull_request.number }}"}, {"headerName": "X-ActivityPub-Commit", "headerValue": "${{ github.event.pull_request.head.sha }}"}]}}] + .pathMatchers[0].routeRules' config.yml
export NEXT_PRIORITY=$((NEXT_PRIORITY + 1))
done
echo "Updating url map with:"
Expand Down
4 changes: 2 additions & 2 deletions adr/0002-frontend-backend-versioning.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ Implement a backend-driven frontend configuration endpoint that tells the JS bun
```json
{
"client": {
"name": "admin-x-activitypub",
"name": "activitypub",
"version": "^1.0.0",
"cdnUrl": "https://cdn.jsdelivr.net/ghost/admin-x-activitypub@1/dist/admin-x-activitypub.js"
"cdnUrl": "https://cdn.jsdelivr.net/ghost/activitypub@1/dist/activitypub.js"
}
}
```
Expand Down
2 changes: 1 addition & 1 deletion src/http/api/client-config.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export class ClientConfigController {
@APIRoute('GET', 'client-config', 'stable')
async handleGetClientConfig(_ctx: AppContext) {
const major = 1;
const name = 'admin-x-activitypub';
const name = 'activitypub';
const client = {
name,
version: `^${major}.0.0`,
Expand Down
Loading