feat(site): add site:https command for enabling HTTPS - #97
Conversation
- Add SiteHttpsCommand class with domain selection and playbook execution - Add site-https.sh playbook that configures Caddy for automatic HTTPS - Support different WWW redirect modes (redirect-to-root, redirect-to-www) - Display HTTPS-enabled URL after successful configuration
WalkthroughAdds configurable WWW redirect handling across site provisioning and HTTPS enablement, a new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant CLI_Add as SiteAddCommand
participant CLI_HTTPS as SiteHttpsCommand
participant Traits
participant Playbook_Add as playbooks/site-add.sh
participant Playbook_HTTPS as playbooks/site-https.sh
participant Caddy
User->>CLI_Add: run site:add (optionally --www-mode)
CLI_Add->>Traits: gatherSiteInfo (returns wwwMode)
CLI_Add->>Playbook_Add: execute with DEPLOYER_WWW_MODE
Playbook_Add->>Caddy: write/update vhost file (mode-specific)
Playbook_Add-->>CLI_Add: exit status & output
CLI_Add->>User: show site URL and DNS guidance (based on wwwMode)
User->>CLI_HTTPS: run site:https --domain example.com
CLI_HTTPS->>Traits: select site, load server/site config (wwwMode)
CLI_HTTPS->>Playbook_HTTPS: execute with DEPLOYER_WWW_MODE
Playbook_HTTPS->>Caddy: update vhost file and reload services
Playbook_HTTPS-->>CLI_HTTPS: success output (https_enabled:true)
CLI_HTTPS->>User: display HTTPS URL (considering wwwMode)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Traits/SitesTrait.php (1)
137-158: Normalize domain consistently when resolving sites (selectSite vs validation)You now normalize domains in
validateSiteDomain()/gatherSiteInfo()vianormalizeDomain(), butselectSite()still passes the raw--domainoption straight into$this->sites->findByDomain($domain). That meanssite:https --domain=www.Example.comcan fail to match an existingexample.comentry that was normalized at creation time.Consider normalizing the domain in
selectSite()before lookup, e.g.:$domain = $this->normalizeDomain($domain); $site = $this->sites->findByDomain($domain);to keep domain handling consistent across creation, validation, and selection.
Also applies to: 160-172
🧹 Nitpick comments (3)
app/Traits/ServersTrait.php (1)
73-102: Consider reusinggetSiteConfig()indisplayServerInfo()
getSiteConfig()encapsulates the parsing/normalization forphp_version,www_mode, andhttps_enabledfromsites_config, butdisplayServerInfo()re-implements similar extraction logic when building the “Sites Config” lines.Not a correctness issue, but you could simplify and centralize behavior by using
getSiteConfig()inside the loop, e.g.:foreach ($info['sites_config'] as $domain => $_) { $config = $this->getSiteConfig($info, (string) $domain); if ($config === null) { continue; } $sitesItems[] = sprintf( '%s: PHP %s, %s, %s', $domain, $config['php_version'], $config['www_mode'], $config['https_enabled'] ? '<fg=green>HTTPS</>' : '<fg=yellow>HTTP</>' ); }This keeps the parsing rules in one place and reduces duplication.
Also applies to: 425-449
playbooks/server-info.sh (1)
379-432: Sites configuration detection is coherent; consider updating header docsThe new
get_sites_config()implementation and thesites_configYAML emission align with howsite-addandsite-httpswrite Caddy configs (comments andhttp://vshttps://usage), so HTTPS status and WWW mode should be detected reliably.You might also:
- Update the header “Returns YAML with:” section to mention the new
sites_configblock for completeness.- (Optional) quote
php_version/www_mode/https_enabledin the YAML to avoid them being parsed as non-strings in some YAML consumers, though your PHP side already normalizes these safely.Also applies to: 595-621
app/Console/Site/SiteAddCommand.php (1)
41-43: WWW mode plumbing from CLI to playbooks is correct; consider validating CLI valuesThe new
--www-modeoption is correctly:
- Captured in
gatherSiteInfo()(with a sensible interactive default and description).- Returned as
wwwModeand wired into:
DEPLOYER_WWW_MODEfor thesite-addplaybook.- The initial HTTP URL shown in “Next steps”.
- The replay payload for
site:add.This end-to-end wiring looks solid.
You might optionally add validation for CLI-provided
--www-mode(similar to how you validatephp-version) so that typos are caught before hitting the remote playbook, rather than only via the “Invalid WWW mode” error from Bash.Also applies to: 105-111, 130-141, 168-181, 187-195, 304-407
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/Console/Site/SiteAddCommand.php(8 hunks)app/Console/Site/SiteHttpsCommand.php(1 hunks)app/SymfonyApp.php(2 hunks)app/Traits/ServersTrait.php(2 hunks)app/Traits/SitesTrait.php(2 hunks)playbooks/server-info.sh(2 hunks)playbooks/site-add.sh(5 hunks)playbooks/site-https.sh(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/00-main.mdc)
**/*.php: Eliminate single-use methods: inline if a method is called only once
Cache computed values: initialize expensive calculations in the constructor
Avoid method call overhead: prefer direct property access when appropriate
Files:
app/Traits/SitesTrait.phpapp/Traits/ServersTrait.phpapp/Console/Site/SiteHttpsCommand.phpapp/SymfonyApp.phpapp/Console/Site/SiteAddCommand.php
🧠 Learnings (8)
📚 Learning: 2025-09-22T11:10:21.459Z
Learnt from: CR
Repo: deployer-plus/deployer-php PR: 0
File: .cursor/rules/01-architecture.mdc:0-0
Timestamp: 2025-09-22T11:10:21.459Z
Learning: Applies to app/**/Command/**/*.php : Only Commands perform console input/output operations
Applied to files:
app/SymfonyApp.php
📚 Learning: 2025-09-22T11:10:21.459Z
Learnt from: CR
Repo: deployer-plus/deployer-php PR: 0
File: .cursor/rules/01-architecture.mdc:0-0
Timestamp: 2025-09-22T11:10:21.459Z
Learning: Applies to app/**/Command/**/*.php : Commands must not duplicate orchestration logic—extract to shared Services
Applied to files:
app/SymfonyApp.php
📚 Learning: 2025-09-22T11:10:21.459Z
Learnt from: CR
Repo: deployer-plus/deployer-php PR: 0
File: .cursor/rules/01-architecture.mdc:0-0
Timestamp: 2025-09-22T11:10:21.459Z
Learning: Applies to app/**/Command/**/*.php : Commands may only depend on Services (not other Commands)
Applied to files:
app/SymfonyApp.php
📚 Learning: 2025-09-22T11:10:21.459Z
Learnt from: CR
Repo: deployer-plus/deployer-php PR: 0
File: .cursor/rules/01-architecture.mdc:0-0
Timestamp: 2025-09-22T11:10:21.459Z
Learning: Applies to app/**/Command/**/*.php : Commands receive Services via constructor injection
Applied to files:
app/SymfonyApp.php
📚 Learning: 2025-09-22T11:10:21.459Z
Learnt from: CR
Repo: deployer-plus/deployer-php PR: 0
File: .cursor/rules/01-architecture.mdc:0-0
Timestamp: 2025-09-22T11:10:21.459Z
Learning: Applies to app/**/Command/**/*.php : Commands must not contain business logic—delegate to Services
Applied to files:
app/SymfonyApp.php
📚 Learning: 2025-09-22T11:10:21.459Z
Learnt from: CR
Repo: deployer-plus/deployer-php PR: 0
File: .cursor/rules/01-architecture.mdc:0-0
Timestamp: 2025-09-22T11:10:21.459Z
Learning: Applies to app/**/@(Service|Services)/**/*.php : Extract complex orchestration shared by multiple Commands into dedicated Services
Applied to files:
app/SymfonyApp.php
📚 Learning: 2025-09-22T11:10:21.459Z
Learnt from: CR
Repo: deployer-plus/deployer-php PR: 0
File: .cursor/rules/01-architecture.mdc:0-0
Timestamp: 2025-09-22T11:10:21.459Z
Learning: Applies to app/**/Command/**/*.php : Commands handle user interaction (input/output) and orchestrate Services
Applied to files:
app/SymfonyApp.php
📚 Learning: 2025-09-22T11:10:21.459Z
Learnt from: CR
Repo: deployer-plus/deployer-php PR: 0
File: .cursor/rules/01-architecture.mdc:0-0
Timestamp: 2025-09-22T11:10:21.459Z
Learning: Applies to app/**/Command/**/*.php : Commands should not invoke other commands (no proxy commands)
Applied to files:
app/SymfonyApp.php
🧬 Code graph analysis (3)
app/Traits/SitesTrait.php (2)
app/Repositories/SiteRepository.php (1)
delete(133-147)app/Console/Server/ServerDeleteCommand.php (1)
site(93-93)
app/Traits/ServersTrait.php (2)
app/Services/IOService.php (2)
displayDeets(543-566)writeln(463-469)app/Console/Server/ServerListCommand.php (1)
AsCommand(15-62)
playbooks/site-https.sh (2)
playbooks/helpers.sh (1)
run_cmd(18-24)playbooks/site-delete.sh (1)
reload_caddy(59-67)
🔇 Additional comments (3)
playbooks/site-add.sh (1)
34-35: WWW mode wiring and Caddy HTTP config look consistent with HTTPS flowThe new
DEPLOYER_WWW_MODErequirement and the dynamic Caddy config generation forredirect-to-root/redirect-to-wwware consistent with thesite-httpsplaybook andserver-info.sh’ssites_configdetection (comments andhttp://patterns line up). I don’t see functional issues here; behavior should be predictable from initial HTTP provisioning through later HTTPS enablement.Also applies to: 141-231
app/SymfonyApp.php (1)
22-23: Command registration forsite:httpsis correctImporting
SiteHttpsCommandand adding it to the$commandsarray cleanly wires the new HTTPS flow into the app and is consistent with existing command registration.Also applies to: 165-171
playbooks/site-https.sh (1)
25-36: HTTPS Caddy update logic is coherent with provisioning and detectionThe new
site-httpsplaybook correctly:
- Validates all required
DEPLOYER_*env vars (includingDEPLOYER_WWW_MODE).- Rewrites the vhost to HTTPS-friendly Caddy config using the same
site_block_configshape assite-add, but with Auto HTTPS listeners and HTTPS redirects that line up withserver-info.sh’shttps_enabledheuristics.- Reloads Caddy and surfaces failures clearly, then emits a simple YAML result.
This should behave idempotently and integrate cleanly with the existing provisioning flow.
Also applies to: 47-124, 130-155
Prevent the site-https command from proceeding with playbook execution when PHP version cannot be detected from server configuration, avoiding confusing failures during playbook execution with invalid phpunknown-fpm.sock paths.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/Console/Site/SiteHttpsCommand.php (1)
116-126: Optional: make site config access more defensive
$config['php_version']and$config['www_mode']are used assuming both keys are always present. You already guard the'unknown'PHP version case, but if the structure of$configever changes (e.g., missingwww_mode), this would raise notices and break the command.You could optionally harden this by providing defaults or validating keys up front, e.g. defaulting
www_modeto'redirect-to-root'if missing, and failing fast ifphp_versionis absent entirely.Also applies to: 140-142
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/Console/Site/SiteHttpsCommand.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/00-main.mdc)
**/*.php: Eliminate single-use methods: inline if a method is called only once
Cache computed values: initialize expensive calculations in the constructor
Avoid method call overhead: prefer direct property access when appropriate
Files:
app/Console/Site/SiteHttpsCommand.php
🔇 Additional comments (2)
app/Console/Site/SiteHttpsCommand.php (2)
42-151: HTTPS execution flow looks solidThe end-to-end flow in
execute()(site/server selection, server info, site config lookup,php_version === 'unknown'guard, playbook execution, and HTTPS URL display honoringwww_mode) is coherent and consistent. No blocking issues from my side here.
35-36: Confirm--domainoption is actually used for site selectionThe command declares a required
domainoption, butexecute()never reads it directly. This is fine ifselectSite()(or shared command infrastructure) internally consults thedomainoption; otherwise the option would be effectively unused.Please double‑check that
selectSite()honors thedomainoption for filtering/preselection; if not, consider wiring it through explicitly.
Use getSiteConfig() method instead of duplicating extraction logic when building Sites Config display lines. This reduces code duplication and ensures consistent parsing behavior.
- Extract WWW mode options into reusable array - Switch to getValidatedOptionOrPrompt for proper validation - Add validation logic to ensure only valid WWW modes are accepted
- Add quotes around php_version, www_mode, and https_enabled values - Add documentation comment for sites_config field in output
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/Traits/ServersTrait.php (2)
73-102: Robust site config extraction; consider intent around single-use helper and HTTPS unknownsThe helper cleanly normalizes per-site config (
php_version,www_mode,https_enabled) and guards against malformedsites_configentries, which makes downstream rendering code much safer.Two small points to consider:
- Given the current codebase,
getSiteConfig()is only called fromdisplayServerInfo(). Per the PHP guidelines about avoiding single‑use helpers/method‑call overhead, you could inline this logic into the Sites Config section to reduce indirection. That said, if you expect other commands/traits to consume site config soon (e.g.,site:https), keeping this helper is reasonable for reuse and consistency.https_enabledfalls back tofalsewhen missing, which renders asHTTP. If the playbook might ever omit this key for “unknown” rather than “explicitly not HTTPS”, you may want a tri‑state (true/false/unknown) or a dedicated sentinel so the UI can distinguish “definitely HTTP” from “no data yet”.
425-446: Sites Config display is clear; consider domain normalization for consistencyThe new “Sites Config” section is straightforward and user‑friendly:
- Skips invalid entries via
getSiteConfig(), so it won’t explode on bad data.- Sensible fallbacks (
?for unknown PHP / WWW mode, color‑coded HTTPS vs HTTP) make the output readable.Two minor refinements to consider:
- If site domains are stored in a normalized form elsewhere (e.g., via
normalizeDomain()inSitesTrait), you might want to normalize$domainbefore display so this view always matches how domains are shown in other commands.- If you anticipate a lot of sites per server, you might later want to sort
array_keys($info['sites_config'])to produce a stable, alphabetical listing.app/Console/Site/SiteAddCommand.php (1)
308-308: Domain normalization and WWW mode selection are solid; consider centralizing allowed modesThe updated
gatherSiteInfo()flow looks good:
- Domain is validated first and then normalized via
normalizeDomain(), which prevents issues likewww.www.example.comwhen building display URLs or Caddy config.www-modeis obtained viagetValidatedOptionOrPrompt, with a clear prompt, helpful labels (Redirect www to non-www/Redirect non-www to www), and strict validation against the allowed keys. Invalid CLI values cleanly cause the method to returnnull, which the caller already treats as a failure.- The return type and payload now correctly include
wwwMode, keeping the structure explicit.One suggestion:
- The
$wwwModesarray (and the corresponding set of valid keys) will almost certainly be needed inSiteHttpsCommandand possibly in other flows. You might consider centralizing these allowed modes (e.g., a small enum, constant, or a helper inSitesTrait) so you don’t have to keep multiple definitions in sync across commands.Also applies to: 328-357, 410-416
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/Console/Site/SiteAddCommand.php(8 hunks)app/Traits/ServersTrait.php(2 hunks)playbooks/server-info.sh(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- playbooks/server-info.sh
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/00-main.mdc)
**/*.php: Eliminate single-use methods: inline if a method is called only once
Cache computed values: initialize expensive calculations in the constructor
Avoid method call overhead: prefer direct property access when appropriate
Files:
app/Console/Site/SiteAddCommand.phpapp/Traits/ServersTrait.php
🧬 Code graph analysis (2)
app/Console/Site/SiteAddCommand.php (2)
app/Services/IOService.php (3)
writeln(463-469)getValidatedOptionOrPrompt(164-187)promptSelect(301-317)app/Traits/SitesTrait.php (1)
normalizeDomain(163-172)
app/Traits/ServersTrait.php (1)
app/Services/IOService.php (3)
info(474-477)displayDeets(543-566)writeln(463-469)
🔇 Additional comments (2)
app/Console/Site/SiteAddCommand.php (2)
41-42: CLI options for PHP version and WWW mode are consistent and well-scopedDefining
--php-versionand--www-modehere aligns the CLI surface with howselectPhpVersion()and the new wwwMode handling work later in the command. Names match the internal option keys used in prompts and replay, so this will behave predictably for both interactive and scripted usage.
105-111: End‑to‑end wwwMode propagation and UX look coherentThe new
wwwModewiring is cohesive:
- It’s captured from
gatherSiteInfo()and destructured alongside other site info.- It’s passed to the playbook as
DEPLOYER_WWW_MODE, so provisioning can generate the correct Caddy config.- The “Next steps” URL reflects the chosen mode (
http://www.{domain}vshttp://{domain}), and the DNS guidance now clearly calls out both root andwwwrecords.- Command replay includes
'www-mode' => $wwwMode, which keeps automation reproducible.Behaviorally this matches the intent of “redirect-to-root” vs “redirect-to-www” without surprising double‑
wwwcases (thanks to earlier domain normalization).Also applies to: 139-140, 168-181, 193-194
Summary by CodeRabbit
New Features
--www-modeoption when adding sites and a new site:https command to enable HTTPS and show the accessible HTTPS URL.Refactor
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.