Skip to content

fix(migration): keep a site whose Let's Encrypt certificate is pending on HTTP - #1955

Open
mrrobot47 wants to merge 2 commits into
EasyEngine:developfrom
mrrobot47:fix/migration-keep-pending-ssl-on-http
Open

mrrobot47 wants to merge 2 commits into
EasyEngine:developfrom
mrrobot47:fix/migration-keep-pending-ssl-on-http

Conversation

@mrrobot47

Copy link
Copy Markdown
Member

What

The image migration regenerates each affected site's docker-compose.yml in SiteContainers::generate_site_docker_compose_file() and decided HTTPS only from site_ssl. It now also keeps a site on plain HTTP while its Let's Encrypt certificate is pending.

Why

EasyEngine/site-command#521 adds a pending Let's Encrypt state: after a manual DNS-01 challenge (no Cloudflare API key) the site keeps site_ssl = le but has no certificate until ee site ssl-verify issues it, and the site is served over HTTP meanwhile. An image migration that touched such a site turned HTTPS back on, so nginx-proxy redirected it to HTTPS and served it with the closest matching certificate of another site (e.g. the parent domain's) until ssl-verify ran.

The check is site-command's own \EE\Site\Utils\is_le_ssl_pending(), the same one refresh and update --php use in #521. It is called only when it exists: with a site-command that doesn't have it (the current release), the migration behaves exactly as before.

Note: a site left pending by an older site-command (HTTPS still on in its compose file, WordPress URLs on https://) is switched to HTTP by the migration once #521 is in, the same as ee site refresh does with #521. Moving such sites' WordPress URLs along is a separate follow-up.

Depends on / pairs with

How tested

  • Unit tests (PHPUnit, PHP 7.4 and 8.5) for the HTTPS state written by generate_site_docker_compose_file(): no SSL, LE with and without certificate files (crt only, key only), self/custom/inherit, and the fallback without the helper. Run against site-command develop and Databases not dropping properly - weakly-referenced object no longer exists #521; with Databases not dropping properly - weakly-referenced object no longer exists #521 they fail on develop's code.
  • E2E on GitHub Actions (run; Pebble as the ACME server, PHP 7.4 and 8.5): a pending html site and a pending WordPress site (manual DNS-01, site-command#521 + site-type-wp#247) and an html site with an issued certificate (HTTP-01), a certificate for the parent domain present, then every site's nginx moved to an older tag and the real migration triggered:
    • this branch: both pending sites stay nohttps, answer 200 over HTTP and aren't served with the parent's certificate, WordPress home stays http://; the issued site keeps HTTPS (200, Pebble certificate, HTTP redirects to HTTPS); ee site ssl-verify afterwards turns HTTPS on for the pending site.
    • develop's SiteContainers.php with the same packages: the migration removes nohttps from both pending sites and HTTP redirects to HTTPS served with the parent domain's certificate.
    • this branch with site-command/site-type-wp develop: unchanged behaviour (pending sites keep HTTPS on, as before), no errors.

…g on HTTP

The image migration regenerated each site's docker-compose.yml with HTTPS on whenever site_ssl was set. A site waiting for `ee site ssl-verify` after a manual DNS-01 challenge has site_ssl=le but no certificate yet, so nginx-proxy then served it over HTTPS with the closest matching certificate of another site.

Use site-command's is_le_ssl_pending() when it exists, the same check refresh and `update --php` use. Older site-command versions don't have it and keep the previous behaviour.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused change preserves backward compatibility and has no identified blocking issues.

No open issues 🎉

What changed in this PR

Keeps sites with pending Let's Encrypt certificates on HTTP when image migrations regenerate their Docker Compose configuration.

Changes:

  • Uses the site-command pending-certificate helper to determine whether HTTPS should remain disabled.
  • Preserves existing behavior when the helper is unavailable or the site has no SSL configured.
File Description
php/​EE/​Migration/​SiteContainers.php Adds a backward-compatible HTTPS decision helper for site Compose regeneration.

Review effort: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants