Skip to content

Commit eb1b22f

Browse files
yCodeTechyCodeTech
authored
fix: replacing php port for isolation also replaced all non-php ports (#36)
A follow-up to PR #33 which introduced a bug that replaced **all** ports, not just the php-specific ones. So this meant that port 60, 80, etc. was all replaced which crashed nginx preventing it from obtaining and running the sites. --- This pull request introduces significant improvements to how Nginx site configurations handle PHP port overrides in Valet, along with a more robust upgrade process for deprecated Nginx directives. The changes ensure per-site PHP port flexibility, automate config upgrades, and enhance maintainability. **Nginx Site Configuration and PHP Port Handling:** * Updated the Nginx config stubs (`secure.valet.conf` and `unsecure.valet.conf`) to use a new variable `valet_site_php_port` instead of directly referencing `valet_php_port`, allowing for per-site PHP port overrides. * Refactored the `replacePhpVersionInSiteConf` method in `Site.php` to support replacing the PHP port with either a specific value or the global variable, and improved the removal of isolated PHP version comments. **Upgrade and Migration Automation:** * Added a new upgrade routine in `Upgrader.php` that scans all Nginx site configs and upgrades them to use the new per-site PHP port override format, ensuring this runs only once per install. * Introduced a mechanism to track which sites have had their Nginx configs upgraded during a run to avoid redundant operations. **Deprecated Directive Handling:** * Replaced the previous inline upgrade logic with a new process that detects deprecated Nginx directives (`http2` and `http2_push_preload`), warns the user, and upgrades affected site configs automatically. These changes collectively modernize Valet’s Nginx configuration management, making it more flexible and future-proof. ~ Summary generated by Copilot. For more detailed info, view the commit messages.
2 parents 56614cd + 24348c0 commit eb1b22f

4 files changed

Lines changed: 158 additions & 42 deletions

File tree

‎cli/Valet/Site.php‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -976,15 +976,26 @@ public function buildSecureNginxServer($url, $siteConf = null) {
976976
*
977977
* @param string $siteConf
978978
* @param string $phpPort
979+
* @param string|null $phpVersion
979980
*
980981
* @return array|string|null
981982
*/
982983
public function replacePhpVersionInSiteConf($siteConf, $phpPort, $phpVersion = null) {
983-
// Replace both the variable $valet_php_port and any existing specific port numbers
984-
$siteConf = preg_replace('/127\.0\.0\.1:(?:\$valet_php_port|\d+);/', "127.0.0.1:{$phpPort};", $siteConf);
985-
986984
// Remove `Valet isolated PHP version` line from config
987-
$siteConf = preg_replace('/# Valet isolated PHP version.*\n/', '', $siteConf);
985+
$siteConf = preg_replace('/^# Valet isolated PHP version.*\R/m', '', $siteConf);
986+
987+
// If a specific PHP version is provided, use that port value,
988+
// otherwise use the global variable.
989+
$phpPortValue = $phpVersion ? $phpPort : '$valet_php_port';
990+
991+
// Replace existing port number or global variable with the new port value
992+
// or global variable, depending on whether the site is isolated or not.
993+
$siteConf = preg_replace(
994+
'/(^\s*set \$valet_site_php_port\s+)[^;]+(\s*;)/m',
995+
"$1{$phpPortValue}$2",
996+
$siteConf,
997+
1
998+
);
988999

9891000
if ($phpVersion) {
9901001
$siteConf = '# Valet isolated PHP version : ' . $phpVersion . PHP_EOL . $siteConf;

‎cli/Valet/Upgrader.php‎

Lines changed: 137 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,13 @@ class Upgrader {
1818
*/
1919
protected $site;
2020

21+
/**
22+
* Sites that have had their Nginx configs upgraded during this run of Valet.
23+
*
24+
* @var array<string, bool>
25+
*/
26+
protected $upgradedNginxSites = [];
27+
2128
public function __construct(Filesystem $files, Configuration $config, Site $site) {
2229
$this->files = $files;
2330
$this->config = $config;
@@ -33,9 +40,9 @@ public function onEveryRun() {
3340
$this->prunePathsFromConfig();
3441
$this->pruneSymbolicLinks();
3542
$this->upgradeSymbolicLinks();
36-
$this->lintNginxConfigs();
37-
$this->upgradeNginxSiteConfigs();
43+
$this->upgradeDeprecatedNginxConfigDirectives();
3844
$this->fixOldSampleValetDriver();
45+
$this->upgradeNginxSitePhpPortOverrides();
3946
}
4047
}
4148

@@ -116,18 +123,74 @@ private function lintNginxConfigs($returnOutput = false) {
116123
/**
117124
* Upgrade Nginx site configurations.
118125
*
119-
* This method checks the Nginx configuration files for deprecated `http2` param
120-
* and `http2_push_preload` directive, then upgrades the site configurations accordingly.
126+
* To upgrade the Nginx config files for each site, this method will:
127+
* - Unisolate and reisolate isolated sites
128+
* - Delete and recreate proxy sites
129+
* - Unsecure and resecure secured sites
130+
*
131+
* @param string $site The site to upgrade.
132+
*/
133+
private function upgradeNginxSiteConfigs($site) {
134+
info("Upgrading Nginx config for site '{$site}'...");
135+
136+
$didUpgrade = false;
137+
138+
// If the site is isolated...
139+
if ($this->site->isIsolated($site)) {
140+
// Get the PHP version for the site.
141+
$phpVersion = $this->site->customPhpVersion($site);
142+
143+
// Unisolate the site and re-isolate it using the phpVersion to
144+
// upgrade the Nginx config file.
145+
$this->site->unisolate($site);
146+
$this->site->isolate($phpVersion, $site);
147+
$didUpgrade = true;
148+
}
149+
// If the site is a proxy...
150+
elseif ($this->site->isProxy($site)) {
151+
// Get the proxy host and whether the site is secured.
152+
$host = $this->site->getProxyHostForSite($site);
153+
$secured = $this->site->isSecured($site);
154+
155+
// Delete the proxy and re-create it using the host and
156+
// secured values to upgrade the Nginx config file.
157+
$this->site->proxyDelete($site);
158+
$this->site->proxyCreate($site, $host, $secured);
159+
$didUpgrade = true;
160+
}
161+
// If the site is secured...
162+
elseif ($this->site->isSecured($site)) {
163+
// Unsecure the site and re-secure it to upgrade
164+
// the Nginx config file.
165+
$this->site->unsecure($site);
166+
$this->site->secure($site);
167+
$didUpgrade = true;
168+
}
169+
170+
if ($didUpgrade) {
171+
$this->upgradedNginxSites[$site] = true;
172+
}
173+
}
174+
175+
/**
176+
* Upgrade deprecated Nginx configuration directives.
177+
*
178+
* This method checks the Nginx configuration files for deprecated `http2` param and `http2_push_preload` directive, then prompts the user to upgrade their Nginx site configurations accordingly.
121179
*/
122-
private function upgradeNginxSiteConfigs() {
180+
private function upgradeDeprecatedNginxConfigDirectives() {
123181
$output = $this->lintNginxConfigs(true);
124182

125183
// If output is not empty...
126184
if (!empty($output)) {
127-
$stringsToCheck = ['the "listen ... http2" directive is deprecated', '"http2_push_preload" directive is obsolete'];
185+
$stringsToCheck = [
186+
'the "listen ... http2" directive is deprecated',
187+
'"http2_push_preload" directive is obsolete'
188+
];
128189

129190
$outputArray = explode("\r\n", $output);
130191

192+
$sitesToUpgrade = [];
193+
131194
// For each line in the output...
132195
foreach ($outputArray as $line) {
133196
// If the line contains any of the strings in the array...
@@ -139,38 +202,23 @@ private function upgradeNginxSiteConfigs() {
139202
// Get the site name from file path in the matched string,
140203
// ie. gets the filename (site.conf) and removes the extension.
141204
$site = basename($matches[1], ".conf");
142-
143-
// If the site is isolated...
144-
if ($this->site->isIsolated($site)) {
145-
// Get the PHP version for the site.
146-
$phpVersion = $this->site->customPhpVersion($site);
147-
148-
// Unisolate the site and re-isolate it using the phpVersion to
149-
// upgrade the Nginx config file.
150-
$this->site->unisolate($site);
151-
$this->site->isolate($phpVersion, $site);
152-
}
153-
// If the site is a proxy...
154-
elseif ($this->site->isProxy($site)) {
155-
// Get the proxy host and whether the site is secured.
156-
$host = $this->site->getProxyHostForSite($site);
157-
$secured = $this->site->isSecured($site);
158-
159-
// Delete the proxy and re-create it using the host and
160-
// secured values to upgrade the Nginx config file.
161-
$this->site->proxyDelete($site);
162-
$this->site->proxyCreate($site, $host, $secured);
163-
}
164-
// If the site is secured...
165-
elseif ($this->site->isSecured($site)) {
166-
// Unsecure the site and re-secure it to upgrade
167-
// the Nginx config file.
168-
$this->site->unsecure($site);
169-
$this->site->secure($site);
170-
}
205+
// Add the site to the sitesToUpgrade array.
206+
// This ensures that if a site has multiple deprecated directives,
207+
// it will only be added to the array once.
208+
$sitesToUpgrade[$site] = true;
171209
}
172210
}
173211
}
212+
213+
// If there are any sites to upgrade...
214+
if (!empty($sitesToUpgrade)) {
215+
warning('Your Nginx configuration files contain some deprecated directives that need to be updated.');
216+
217+
// Upgrade the Nginx config files for each site that needs to be upgraded.
218+
foreach (array_keys($sitesToUpgrade) as $site) {
219+
$this->upgradeNginxSiteConfigs($site);
220+
}
221+
}
174222
}
175223
}
176224

@@ -202,4 +250,58 @@ public function fixOldSampleValetDriver(): void {
202250
}
203251
}
204252
}
253+
254+
/**
255+
* Upgrade all Nginx site configs to use per-site PHP port overrides.
256+
*/
257+
private function upgradeNginxSitePhpPortOverrides() {
258+
// If the PHP port definitions have already been upgraded, skip.
259+
if (!$this->shouldUpgradeNginxSitePhpPortOverrides()) {
260+
return;
261+
}
262+
263+
// If the Nginx config directory doesn't exist, skip and mark it as upgraded to prevent
264+
// this from running again.
265+
if (!$this->files->exists($this->site->nginxPath())) {
266+
$this->config->updateKey('php_port_overrides_upgraded', true);
267+
return;
268+
}
269+
270+
$upgraded = 0;
271+
$tld = $this->config->read()['tld'];
272+
273+
// Get all the Nginx config files for the sites.
274+
foreach ($this->files->scandir($this->site->nginxPath()) as $file) {
275+
// If the file doesn't end with ".{tld}.conf", skip it.
276+
if (!str_ends_with($file, ".{$tld}.conf")) {
277+
continue;
278+
}
279+
280+
// Remove the ".conf" extension from the filename to get the site name.
281+
$site = basename($file, '.conf');
282+
283+
// If the site has already been upgraded during this run, skip it.
284+
if (isset($this->upgradedNginxSites[$site])) {
285+
continue;
286+
}
287+
288+
$this->upgradeNginxSiteConfigs($site);
289+
$upgraded++;
290+
}
291+
292+
if ($upgraded > 0) {
293+
info("Upgraded {$upgraded} Nginx site config(s) to the new PHP port override format.");
294+
}
295+
296+
$this->config->updateKey('php_port_overrides_upgraded', true);
297+
}
298+
299+
/**
300+
* Determine whether Nginx site PHP port overrides should be upgraded.
301+
*
302+
* @return bool
303+
*/
304+
private function shouldUpgradeNginxSitePhpPortOverrides() {
305+
return !$this->config->get('php_port_overrides_upgraded', false);
306+
}
205307
}

‎cli/stubs/secure.valet.conf‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ server {
4949

5050
location ~ [^/]\.php(/|$) {
5151
fastcgi_split_path_info ^(.+\.php)(/.+)$;
52-
fastcgi_pass 127.0.0.1:$valet_php_port;
52+
set $valet_site_php_port $valet_php_port;
53+
fastcgi_pass 127.0.0.1:$valet_site_php_port;
5354
fastcgi_index "VALET_SERVER_PATH";
5455
include fastcgi_params;
5556
fastcgi_param SCRIPT_FILENAME "VALET_SERVER_PATH";
@@ -101,7 +102,8 @@ server {
101102

102103
location ~ [^/]\.php(/|$) {
103104
fastcgi_split_path_info ^(.+\.php)(/.+)$;
104-
fastcgi_pass 127.0.0.1:$valet_php_port;
105+
set $valet_site_php_port $valet_php_port;
106+
fastcgi_pass 127.0.0.1:$valet_site_php_port;
105107
fastcgi_index "VALET_SERVER_PATH";
106108
include fastcgi_params;
107109
fastcgi_param SCRIPT_FILENAME "VALET_SERVER_PATH";

‎cli/stubs/unsecure.valet.conf‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ server {
3939

4040
location ~ [^/]\.php(/|$) {
4141
fastcgi_split_path_info ^(.+\.php)(/.+)$;
42-
fastcgi_pass 127.0.0.1:$valet_php_port;
42+
set $valet_site_php_port $valet_php_port;
43+
fastcgi_pass 127.0.0.1:$valet_site_php_port;
4344
fastcgi_index "VALET_SERVER_PATH";
4445
include fastcgi_params;
4546
fastcgi_param SCRIPT_FILENAME "VALET_SERVER_PATH";

0 commit comments

Comments
 (0)