Conversation
issues-auto-labeler
bot
added
activitypub/mastodon
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
💤 Files with no reviewable changes (5)
📝 WalkthroughWalkthroughConsolidates strict HTTPS/TLS smoke-test behavior into the primary Mastodon smoke-test setup: deletes the separate strict workflow and compose file, adds TLS-terminating Caddy proxies, renames backend services, updates envs and provisioning to use HTTPS and WebFinger, and centralizes compose invocations in the workflow. Changes
Sequence DiagramsequenceDiagram
participant Client as External Client
participant CaddyH as Caddy Harness<br/>(TLS Proxy)
participant HarnessB as Fedify Harness<br/>(Backend)
participant CaddyM as Caddy Mastodon<br/>(TLS Proxy)
participant MastodonB as Mastodon<br/>(Backend)
participant DB as PostgreSQL
rect rgba(120, 180, 240, 0.5)
Note over Client,CaddyH: HTTPS health check via Caddy
end
Client->>CaddyH: HTTPS GET /_test/health (fedify-harness:443)
CaddyH->>HarnessB: HTTP GET /_test/health (backend)
HarnessB-->>CaddyH: 200 OK
CaddyH-->>Client: 200 OK
rect rgba(200, 140, 220, 0.5)
Note over Client,CaddyM: HTTPS Mastodon API flow
end
Client->>CaddyM: HTTPS GET /api/v1/... (mastodon:443)
CaddyM->>MastodonB: HTTP GET /api/v1/... (backend port 3000)
MastodonB->>DB: Query
DB-->>MastodonB: Results
MastodonB-->>CaddyM: 200 + Payload
CaddyM-->>Client: 200 + Payload (TLS)
rect rgba(160, 200, 120, 0.5)
Note over HarnessB,MastodonB: Local CA verification
end
HarnessB->>HarnessB: Verify with DENO_CERT=/certs/ca.crt
MastodonB->>MastodonB: Use SSL_CERT_FILE=/tmp/ca-bundle.crt
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/smoke/mastodon/docker-compose.yml`:
- Around line 90-93: Replace the simple process check in the Caddy healthcheck
(the healthcheck block containing test: ["CMD", "caddy", "version"]) with an
HTTP-based check that verifies TLS and proxying, e.g., change the test to run a
curl against the proxied endpoint (use ["CMD-SHELL", "curl --fail -sS --cacert
/path/to/ca.pem https://your-proxied-host/ || exit 1"] or, if using self-signed
certs in tests, use --insecure) so the container confirms Caddy is serving TLS
correctly; apply the same replacement to the other identical healthcheck block
(the second healthcheck instance present in the file).
- Line 89: Update the inline mapping brace spacing for the depends_on
conditions: replace occurrences of "{ condition: service_healthy }" with
"{condition: service_healthy}" (remove the spaces immediately inside the braces)
for the fedify-harness-backend depends_on entry and the other similar
occurrences (the same pattern on the other two depends_on lines).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7b70d0be-8e9c-429c-b272-27e1b53f6c73
📒 Files selected for processing (9)
.github/workflows/smoke-mastodon-strict.yml.github/workflows/smoke-mastodon.yml.gitignoretest/smoke/mastodon/docker-compose.strict.ymltest/smoke/mastodon/docker-compose.ymltest/smoke/mastodon/mastodon-strict.envtest/smoke/mastodon/mastodon.envtest/smoke/mastodon/provision-strict.shtest/smoke/mastodon/provision.sh
💤 Files with no reviewable changes (5)
- .gitignore
- test/smoke/mastodon/mastodon-strict.env
- .github/workflows/smoke-mastodon-strict.yml
- test/smoke/mastodon/docker-compose.strict.yml
- test/smoke/mastodon/provision-strict.sh
| smoke: | ||
| aliases: [fedify-harness] | ||
| depends_on: | ||
| fedify-harness-backend: { condition: service_healthy } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Optional: Address YAMLlint brace spacing warnings.
YAMLlint reports "too many spaces inside braces" for the depends_on conditions. This is a cosmetic issue that doesn't affect functionality.
♻️ Optional fix to satisfy YAMLlint
depends_on:
- fedify-harness-backend: { condition: service_healthy }
+ fedify-harness-backend: {condition: service_healthy}Apply similar changes to lines 136, 158, and 159.
Also applies to: 136-136, 158-159
🧰 Tools
🪛 YAMLlint (1.38.0)
[error] 89-89: too many spaces inside braces
(braces)
[error] 89-89: too many spaces inside braces
(braces)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/smoke/mastodon/docker-compose.yml` at line 89, Update the inline mapping
brace spacing for the depends_on conditions: replace occurrences of "{
condition: service_healthy }" with "{condition: service_healthy}" (remove the
spaces immediately inside the braces) for the fedify-harness-backend depends_on
entry and the other similar occurrences (the same pattern on the other two
depends_on lines).
| healthcheck: | ||
| test: ["CMD", "caddy", "version"] | ||
| interval: 5s | ||
| retries: 5 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider a more robust healthcheck for Caddy proxies.
The current healthcheck (caddy version) only confirms Caddy is running, not that it's serving TLS correctly. A curl against the proxied endpoint would catch configuration issues.
♻️ Optional: Use HTTP-based healthcheck
healthcheck:
- test: ["CMD", "caddy", "version"]
+ test: ["CMD-SHELL", "wget -q --spider --no-check-certificate https://localhost:443/ || exit 1"]
interval: 5s
retries: 5Note: This would require the backend to be healthy first, which is already enforced by depends_on. Alternatively, keep the simple check if startup ordering is sufficient.
Also applies to: 137-140
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/smoke/mastodon/docker-compose.yml` around lines 90 - 93, Replace the
simple process check in the Caddy healthcheck (the healthcheck block containing
test: ["CMD", "caddy", "version"]) with an HTTP-based check that verifies TLS
and proxying, e.g., change the test to run a curl against the proxied endpoint
(use ["CMD-SHELL", "curl --fail -sS --cacert /path/to/ca.pem
https://your-proxied-host/ || exit 1"] or, if using self-signed certs in tests,
use --insecure) so the container confirms Caddy is serving TLS correctly; apply
the same replacement to the other identical healthcheck block (the second
healthcheck instance present in the file).
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request consolidates the Mastodon smoke tests by unifying the standard and strict configurations into a single setup that supports HTTPS and signature verification. It introduces Caddy as a TLS proxy for both the Fedify harness and Mastodon services, enabling more realistic testing of federation protocols. Key changes include renaming backend services to avoid DNS collisions with proxies, configuring CA certificate bundles within containers for internal trust, and updating the provisioning script to utilize Mastodon's internal ResolveAccountService for WebFinger discovery instead of manual database manipulation. I have no feedback to provide.
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
The non-strict variant only exercised plain HTTP without signature verification, which no longer reflects how Fedify is used against real fediverse servers. The strict variant (HTTPS + HTTP signature verification via Caddy TLS proxies) is now the sole Mastodon smoke test. Renamed the strict files to canonical names so the layout matches the other service smoke tests: - docker-compose.strict.yml → docker-compose.yml - provision-strict.sh → provision.sh - mastodon-strict.env → mastodon.env - smoke-mastodon-strict.yml → smoke-mastodon.yml Assisted-by: Claude Code:claude-opus-4-6
sij411
force-pushed
the
fix/remove-non-strict-smoke-test
branch
from
f3f76f4 to
25912ce
Compare
2 participants
Summary
docker-compose.strict.yml→docker-compose.ymlprovision-strict.sh→provision.shmastodon-strict.env→mastodon.envsmoke-mastodon-strict.yml→smoke-mastodon.ymlTest plan
smoke-mastodonGitHub Actions workflow passes on the renamed files.🤖 Generated with Claude Code