Skip to content

Fix imagine hostnames 2 #10937

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

Jump to bottom
Merged
loks0n merged 1 commit into from
Dec 11, 2025
Merged

Fix imagine hostnames 2 #10937

loks0n merged 1 commit into from
Dec 11, 2025

Conversation

@loks0n
Copy link
Member

@loks0n loks0n commented Dec 11, 2025

No description provided.

@loks0n loks0n changed the base branch from main to 1.8.x December 11, 2025 18:36
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 11, 2025 •
edited
Loading

📝 Walkthrough

Walkthrough

This pull request renames platform config keys and shifts domain handling to a unified hostnames list. domain → apiHostname and consoleDomain → consoleHostname. A new hostnames public key is added as a deduplicated, filtered list of API and console hostnames. The domains public resource is removed and replaced by platform-provided hostnames; many constructors, injected dependencies, route/init closures, controllers, templates, views, and platform modules now accept or derive platform (and platform['hostnames']) instead of a separate domains array. The router() signature no longer takes $domains.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • app/controllers/general.php — router() signature and many route/init closures updated; verify call sites and in_array checks against platform['hostnames'].
  • app/init/resources.php — removal of domains resource and allowedHostnames resource signature change; ensure endpoint assembly and allowed host augmentation use platform['hostnames'] correctly.
  • Multiple platform modules & proxy rules (src/.../Create.php, Get.php, etc.) — constructor injections switched from domains to platform; confirm internal extraction $domains = $platform['hostnames'] ?? [] preserves previous behavior.
  • OAuth/VCS/account controllers and templates/views — confirm references changed from consoleDomain to consoleHostname and URL construction still respects protocol/port logic.

Possibly related PRs

  • feat: multiple app domains #10911 — touches platform/domain plumbing and multi-domain/console hostname propagation; closely related to renaming and platform hostnames changes.

Suggested reviewers

  • TorstenDittmann

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 2 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Fix imagine hostnames 2' is vague and does not clearly convey the actual changes made, which involve renaming platform configuration keys and refactoring hostname-related logic throughout the codebase. Use a more descriptive title that reflects the main changes, such as 'Refactor domain configuration to use hostnames' or 'Update platform configuration key names from domain to hostname'.
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess relevance to the changeset. Add a detailed description explaining the purpose of the changes, the motivation for renaming configuration keys, and how the new hostnames structure improves the codebase.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-imagine-hostnames-2
📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7efe840 and c6ffa18.

📒 Files selected for processing (13)
  • app/config/platform.php (1 hunks)
  • app/config/templates/site.php (1 hunks)
  • app/controllers/api/account.php (3 hunks)
  • app/controllers/api/vcs.php (3 hunks)
  • app/controllers/general.php (12 hunks)
  • app/init/resources.php (2 hunks)
  • app/views/general/error.phtml (1 hunks)
  • src/Appwrite/Platform/Modules/Console/Http/Resources/Get.php (2 hunks)
  • src/Appwrite/Platform/Modules/Proxy/Http/Rules/API/Create.php (1 hunks)
  • src/Appwrite/Platform/Modules/Proxy/Http/Rules/Function/Create.php (1 hunks)
  • src/Appwrite/Platform/Modules/Proxy/Http/Rules/Redirect/Create.php (1 hunks)
  • src/Appwrite/Platform/Modules/Proxy/Http/Rules/Site/Create.php (1 hunks)
  • src/Appwrite/Vcs/Comment.php (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • app/config/templates/site.php
  • app/controllers/api/vcs.php
  • app/views/general/error.phtml
  • app/controllers/api/account.php
  • src/Appwrite/Vcs/Comment.php
🧰 Additional context used
🧬 Code graph analysis (3)
src/Appwrite/Platform/Modules/Proxy/Http/Rules/API/Create.php (3)
src/Appwrite/Platform/Modules/Proxy/Http/Rules/Redirect/Create.php (1)
  • action (82-219)
src/Appwrite/Platform/Modules/Proxy/Http/Rules/Site/Create.php (1)
  • action (79-215)
src/Appwrite/Platform/Modules/Functions/Http/Functions/Create.php (1)
  • action (121-422)
src/Appwrite/Platform/Modules/Proxy/Http/Rules/Redirect/Create.php (3)
src/Appwrite/Platform/Modules/Proxy/Http/Rules/API/Create.php (1)
  • action (74-197)
src/Appwrite/Platform/Modules/Proxy/Http/Rules/Site/Create.php (1)
  • action (79-215)
src/Appwrite/Platform/Modules/Sites/Http/Deployments/Create.php (1)
  • action (93-378)
app/controllers/general.php (3)
src/Appwrite/Platform/Modules/Proxy/Http/Rules/API/Create.php (1)
  • action (74-197)
src/Appwrite/Platform/Modules/Sites/Http/Deployments/Create.php (1)
  • action (93-378)
src/Appwrite/Platform/Modules/Sites/Http/Deployments/Template/Create.php (1)
  • action (85-225)
🪛 PHPMD (2.15.0)
app/controllers/general.php

62-62: Avoid unused parameters such as '$utopia'. (undefined)

(UnusedFormalParameter)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: E2E Service Test (VCS)
  • GitHub Check: E2E Service Test (Storage)
  • GitHub Check: E2E Service Test (Webhooks)
  • GitHub Check: E2E Service Test (Realtime)
  • GitHub Check: E2E Service Test (Functions)
  • GitHub Check: E2E Service Test (FunctionsSchedule)
  • GitHub Check: E2E Service Test (Proxy)
  • GitHub Check: E2E Service Test (GraphQL)
  • GitHub Check: E2E Service Test (Projects)
  • GitHub Check: E2E Service Test (Databases/Legacy)
  • GitHub Check: E2E Service Test (Locale)
  • GitHub Check: E2E Service Test (Avatars)
  • GitHub Check: E2E Service Test (Account)
  • GitHub Check: E2E Service Test (Databases/TablesDB)
  • GitHub Check: E2E Service Test (Console)
  • GitHub Check: E2E Service Test (Site Screenshots)
  • GitHub Check: E2E Service Test (Dev Keys)
  • GitHub Check: Unit Test
  • GitHub Check: E2E General Test
  • GitHub Check: Benchmark
🔇 Additional comments (10)
src/Appwrite/Platform/Modules/Proxy/Http/Rules/Redirect/Create.php (1)

78-84: LGTM! Consistent refactoring to platform-based hostnames.

The changes correctly shift from injecting domains to injecting platform, updating the action signature, and deriving $domains from platform['hostnames']. This aligns with the same pattern used in API/Create.php, Site/Create.php, and Function/Create.php.

app/init/resources.php (2)

178-178: LGTM! Correct usage of apiHostname for endpoint construction.

The platform endpoint now correctly uses $platform['apiHostname'] instead of the previous domain key, consistent with the renamed configuration keys in app/config/platform.php.

186-208: LGTM! Consistent refactoring of allowedHostnames resource.

The signature change from array $domains to array $platform and the derivation of hostnames from $platform['hostnames'] aligns with the broader PR refactoring. The logic for augmenting with project hostnames, dev keys, and origin-derived hostnames is preserved.

src/Appwrite/Platform/Modules/Proxy/Http/Rules/API/Create.php (1)

70-76: LGTM! Consistent with platform-based hostname refactoring.

The changes follow the established pattern: inject platform, update the action signature to accept array $platform, and derive $domains from platform['hostnames']. This matches the implementation in other rule creation classes.

src/Appwrite/Platform/Modules/Proxy/Http/Rules/Site/Create.php (1)

75-81: LGTM! Follows the platform-based hostname pattern.

The refactoring is consistent with API/Create.php, Function/Create.php, and Redirect/Create.php. The domain validation and deny-list logic remains intact, now deriving domains from the platform context.

src/Appwrite/Platform/Modules/Console/Http/Resources/Get.php (1)

62-73: LGTM! Consistent platform-based domain validation.

The refactoring aligns with the proxy rule creation classes, using platform injection and deriving domains from platform['hostnames'] for resource availability checks.

src/Appwrite/Platform/Modules/Proxy/Http/Rules/Function/Create.php (1)

75-81: LGTM! Consistent with the platform-based refactoring.

The changes follow the established pattern across all rule creation classes, ensuring uniform hostname derivation from the platform context.

app/controllers/general.php (3)

62-84: LGTM! Router signature correctly updated for platform-based hostnames.

The router function signature has been updated to accept array $platform instead of array $domains, and the implementation correctly derives hostnames from $platform['hostnames'] and uses $platform['consoleHostname'] for URL construction. This is a significant but necessary change to centralize hostname handling.

862-873: LGTM! Consistent hostname validation in router invocation.

The router invocation correctly passes the $platform parameter and uses $platformHostnames for hostname validation checks. The condition !in_array($hostname, $platformHostnames) ensures the router only processes external domains.

1033-1121: LGTM! Automatic certificate generation updated for platform hostnames.

The certificate generation logic correctly:

  • Derives $platformHostnames from platform['hostnames']
  • Validates against platform hostnames before creating rules
  • Caches hostnames using Config::setParam('hostnames', ...)

The refactoring maintains the original certificate generation flow while using platform-based hostname sources.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@loks0n loks0n force-pushed the fix-imagine-hostnames-2 branch from e9e788d to 6c6c2a8 Compare December 11, 2025 18:38
@github-actions
Copy link

github-actions bot commented Dec 11, 2025 •
edited
Loading

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
libpng 1.6.51-r0 CVE-2025-66293 HIGH
libpng-dev 1.6.51-r0 CVE-2025-66293 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

@loks0n loks0n force-pushed the fix-imagine-hostnames-2 branch from 6c6c2a8 to 7efe840 Compare December 11, 2025 18:42
@loks0n loks0n force-pushed the fix-imagine-hostnames-2 branch from 7efe840 to c6ffa18 Compare December 11, 2025 18:54
@github-actions
Copy link

github-actions bot commented Dec 11, 2025 •
edited
Loading

✨ Benchmark results

  • Requests per second: 1,185
  • Requests with 200 status code: 213,309
  • P99 latency: 0.166226835

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,185 1,227
200 213,309 220,870
P99 0.166226835 0.16815785

coderabbitai[bot]
coderabbitai bot reviewed Dec 11, 2025
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cd8e93d and 7efe840.

📒 Files selected for processing (8)
  • app/config/platform.php (1 hunks)
  • app/config/templates/site.php (1 hunks)
  • app/controllers/api/account.php (3 hunks)
  • app/controllers/api/vcs.php (3 hunks)
  • app/controllers/general.php (12 hunks)
  • app/init/resources.php (2 hunks)
  • app/views/general/error.phtml (1 hunks)
  • src/Appwrite/Vcs/Comment.php (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/init/resources.php (1)
src/Appwrite/Event/Build.php (1)
  • setResource (43-48)
🪛 PHPMD (2.15.0)
app/controllers/general.php

62-62: Avoid unused parameters such as '$utopia'. (undefined)

(UnusedFormalParameter)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Benchmark
  • GitHub Check: E2E Service Test (Databases/Legacy)
  • GitHub Check: E2E Service Test (Databases/TablesDB)
🔇 Additional comments (15)
app/config/templates/site.php (1)

10-15: Use consoleHostname for templates base URL

Switching $hostname to $platform['consoleHostname'] aligns this file with the new platform config key and keeps URL construction behavior intact (protocol + console host). No further issues here.

app/views/general/error.phtml (1)

18-26: Console error view now keyed off consoleHostname

Using $platform['consoleHostname'] for $hostname is consistent with the config rename and with other files in this PR; the dev localhost override remains unchanged. Looks good.

src/Appwrite/Vcs/Comment.php (1)

119-123: VCS comment URLs now correctly use consoleHostname

Both the PR comment body (generateComment) and image helper (generatImage) now derive the base host from $this->platform['consoleHostname'], matching the new platform contract without changing URL shapes or paths. As long as consoleHostname is configured, this remains behaviorally equivalent to the previous consoleDomain usage.

Also applies to: 233-239

app/controllers/api/account.php (3)

1328-1342: OAuth2 session redirects now based on consoleHostname

Using $platform['consoleHostname'] for $redirectBase keeps default success/failure URLs pointing at the console while aligning with the new config naming. Callback handling still uses the API host, so the flow remains consistent.

1981-1997: OAuth2 token flow redirect base updated to consoleHostname

The token-oriented OAuth2 endpoint now also builds its default success/failure redirects from $platform['consoleHostname'], mirroring the session endpoint. This keeps console redirects consistent across both flows.

2154-2165: Magic URL default redirect now uses consoleHostname

When url is omitted, the magic URL token flow now composes /console/auth/magic-url using $platform['consoleHostname'] plus protocol/port. This is a direct key rename from consoleDomain and should behave identically assuming consoleHostname is set in the platform config.

app/controllers/api/vcs.php (1)

135-135: LGTM! Consistent key rename from consoleDomain to consoleHostname.

All three occurrences correctly use the new consoleHostname key with appropriate fallback handling via ?? ''.

Also applies to: 558-558, 617-617

app/init/resources.php (2)

178-178: LGTM! Endpoint construction updated to use apiHostname.

The endpoint URL is correctly built using the renamed apiHostname key.

186-208: LGTM! Resource refactoring from domains to platform['hostnames'].

The allowedHostnames resource correctly:

  • Updated signature to accept $platform instead of $domains
  • Uses $platform['hostnames'] ?? [] with appropriate fallback
  • Updated dependency array to reference 'platform' instead of 'domains'
app/controllers/general.php (6)

62-84: LGTM! Router function updated to use platform-derived hostnames.

The router() function signature correctly:

  • Removed the $domains parameter
  • Now receives the full $platform array
  • Derives consoleHostname and platformHostnames from the platform configuration

1033-1058: LGTM! Certificate generation uses platform hostnames consistently.

The SSL certificate generation logic correctly:

  • Injects platform instead of domains
  • Uses $platform['hostnames'] for hostname validation
  • Updated the cache parameter key from 'domains' to 'hostnames'

862-873: LGTM! App::init action updated for platform-based hostname checks.

The routing decision correctly uses $platform['hostnames'] to determine whether to invoke the router.

1144-1154: LGTM! Options handler updated for platform-based hostname checks.

Consistent with other handlers in using $platform['hostnames'] for routing decisions.

1453-1462: LGTM! robots.txt handler updated for platform-based hostname checks.

The logic correctly checks against $platform['hostnames'] to determine whether to serve the default robots.txt or delegate to the router.

1485-1495: LGTM! humans.txt handler updated for platform-based hostname checks.

Consistent with the robots.txt handler pattern.

@loks0n loks0n merged commit e4a7b1b into 1.8.x Dec 11, 2025
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@TorstenDittmann TorstenDittmann TorstenDittmann approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@loks0n @TorstenDittmann