From d6a631929245ba03cb1fe253f007ddfb014e524e Mon Sep 17 00:00:00 2001 From: ramonskie Date: Mon, 9 Feb 2026 12:42:27 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20feat:=20add=20PHP=20version=20place?= =?UTF-8?q?holder=20resolution=20and=20#{VAR}=20syntax=20support?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add {PHP_XX_LATEST} placeholder resolution in options.json - Resolves {PHP_83_LATEST}, {PHP_82_LATEST}, {PHP_81_LATEST} to actual versions - Provides clear error messages for invalid placeholders - Add #{VAR} placeholder syntax for backward compatibility with v4.x - Support both @{VAR} and #{VAR} syntax in all config files - Restore compatibility for users migrating from v4.x buildpack - Add comprehensive integration tests - 10 test cases covering placeholder resolution - Test fixtures for all scenarios - Unit tests for both features Fixes #1208 --- .../.bp-config/options.json | 4 + .../.bp-config/php/php.ini.d/custom.ini | 2 + fixtures/custom_libdir_hash/index.php | 2 + fixtures/custom_libdir_hash/phpinfo.php | 2 + .../.bp-config/httpd/httpd.conf | 29 ++ .../.bp-config/options.json | 4 + fixtures/custom_webdir_config/index.php | 2 + .../.bp-config/options.json | 4 + fixtures/custom_webdir_value/public/index.php | 3 + .../.bp-config/httpd/httpd.conf | 29 ++ .../.bp-config/options.json | 5 + fixtures/httpd_hash_placeholder/index.php | 2 + .../.bp-config/httpd/httpd.conf | 29 ++ .../.bp-config/options.json | 4 + fixtures/mixed_placeholder_syntax/index.php | 2 + .../.bp-config/options.json | 4 + fixtures/php_82_version_placeholder/index.php | 2 + fixtures/php_extension_dir_test/index.php | 2 + fixtures/php_extension_dir_test/phpinfo.php | 2 + fixtures/php_extensions_test/extensions.php | 22 ++ fixtures/php_extensions_test/index.php | 2 + .../.bp-config/options.json | 4 + fixtures/php_invalid_placeholder/index.php | 2 + .../.bp-config/options.json | 4 + fixtures/php_version_placeholder/index.php | 2 + src/php/finalize/finalize.go | 15 + src/php/finalize/finalize_test.go | 5 +- src/php/integration/init_test.go | 1 + src/php/integration/placeholder_test.go | 261 ++++++++++++++++++ src/php/options/options.go | 14 + src/php/options/options_test.go | 81 ++++++ 31 files changed, 543 insertions(+), 3 deletions(-) create mode 100644 fixtures/custom_libdir_hash/.bp-config/options.json create mode 100644 fixtures/custom_libdir_hash/.bp-config/php/php.ini.d/custom.ini create mode 100644 fixtures/custom_libdir_hash/index.php create mode 100644 fixtures/custom_libdir_hash/phpinfo.php create mode 100644 fixtures/custom_webdir_config/.bp-config/httpd/httpd.conf create mode 100644 fixtures/custom_webdir_config/.bp-config/options.json create mode 100644 fixtures/custom_webdir_config/index.php create mode 100644 fixtures/custom_webdir_value/.bp-config/options.json create mode 100644 fixtures/custom_webdir_value/public/index.php create mode 100644 fixtures/httpd_hash_placeholder/.bp-config/httpd/httpd.conf create mode 100644 fixtures/httpd_hash_placeholder/.bp-config/options.json create mode 100644 fixtures/httpd_hash_placeholder/index.php create mode 100644 fixtures/mixed_placeholder_syntax/.bp-config/httpd/httpd.conf create mode 100644 fixtures/mixed_placeholder_syntax/.bp-config/options.json create mode 100644 fixtures/mixed_placeholder_syntax/index.php create mode 100644 fixtures/php_82_version_placeholder/.bp-config/options.json create mode 100644 fixtures/php_82_version_placeholder/index.php create mode 100644 fixtures/php_extension_dir_test/index.php create mode 100644 fixtures/php_extension_dir_test/phpinfo.php create mode 100644 fixtures/php_extensions_test/extensions.php create mode 100644 fixtures/php_extensions_test/index.php create mode 100644 fixtures/php_invalid_placeholder/.bp-config/options.json create mode 100644 fixtures/php_invalid_placeholder/index.php create mode 100644 fixtures/php_version_placeholder/.bp-config/options.json create mode 100644 fixtures/php_version_placeholder/index.php create mode 100644 src/php/integration/placeholder_test.go diff --git a/fixtures/custom_libdir_hash/.bp-config/options.json b/fixtures/custom_libdir_hash/.bp-config/options.json new file mode 100644 index 000000000..dfa573b3b --- /dev/null +++ b/fixtures/custom_libdir_hash/.bp-config/options.json @@ -0,0 +1,4 @@ +{ + "WEB_SERVER": "httpd", + "LIBDIR": "lib" +} diff --git a/fixtures/custom_libdir_hash/.bp-config/php/php.ini.d/custom.ini b/fixtures/custom_libdir_hash/.bp-config/php/php.ini.d/custom.ini new file mode 100644 index 000000000..ddbc9721b --- /dev/null +++ b/fixtures/custom_libdir_hash/.bp-config/php/php.ini.d/custom.ini @@ -0,0 +1,2 @@ +; Custom PHP configuration using #{LIBDIR} placeholder +include_path = ".:/usr/share/php:#{HOME}/#{LIBDIR}" diff --git a/fixtures/custom_libdir_hash/index.php b/fixtures/custom_libdir_hash/index.php new file mode 100644 index 000000000..61ace196d --- /dev/null +++ b/fixtures/custom_libdir_hash/index.php @@ -0,0 +1,2 @@ + + Options Indexes FollowSymLinks + AllowOverride All + Require all granted + + +DirectoryIndex index.php index.html +TypesConfig conf/mime.types + + + SetHandler "proxy:fcgi://@{PHP_FPM_LISTEN}" + + +ErrorLog logs/error.log +LogLevel warn diff --git a/fixtures/custom_webdir_config/.bp-config/options.json b/fixtures/custom_webdir_config/.bp-config/options.json new file mode 100644 index 000000000..c81b4a18d --- /dev/null +++ b/fixtures/custom_webdir_config/.bp-config/options.json @@ -0,0 +1,4 @@ +{ + "WEB_SERVER": "httpd", + "WEBDIR": "htdocs" +} diff --git a/fixtures/custom_webdir_config/index.php b/fixtures/custom_webdir_config/index.php new file mode 100644 index 000000000..61ace196d --- /dev/null +++ b/fixtures/custom_webdir_config/index.php @@ -0,0 +1,2 @@ + + Options Indexes FollowSymLinks + AllowOverride All + Require all granted + + +DirectoryIndex index.php index.html +TypesConfig conf/mime.types + + + SetHandler "proxy:fcgi://#{PHP_FPM_LISTEN}" + + +ErrorLog logs/error.log +LogLevel warn diff --git a/fixtures/httpd_hash_placeholder/.bp-config/options.json b/fixtures/httpd_hash_placeholder/.bp-config/options.json new file mode 100644 index 000000000..9209b39ff --- /dev/null +++ b/fixtures/httpd_hash_placeholder/.bp-config/options.json @@ -0,0 +1,5 @@ +{ + "WEB_SERVER": "httpd", + "WEBDIR": "htdocs", + "ADMIN_EMAIL": "test@example.com" +} diff --git a/fixtures/httpd_hash_placeholder/index.php b/fixtures/httpd_hash_placeholder/index.php new file mode 100644 index 000000000..61ace196d --- /dev/null +++ b/fixtures/httpd_hash_placeholder/index.php @@ -0,0 +1,2 @@ + + Options Indexes FollowSymLinks + AllowOverride All + Require all granted + + +DirectoryIndex index.php index.html +TypesConfig conf/mime.types + + + SetHandler "proxy:fcgi://#{PHP_FPM_LISTEN}" + + +ErrorLog logs/error.log +LogLevel warn diff --git a/fixtures/mixed_placeholder_syntax/.bp-config/options.json b/fixtures/mixed_placeholder_syntax/.bp-config/options.json new file mode 100644 index 000000000..e16ca91b4 --- /dev/null +++ b/fixtures/mixed_placeholder_syntax/.bp-config/options.json @@ -0,0 +1,4 @@ +{ + "WEB_SERVER": "httpd", + "ADMIN_EMAIL": "test@example.com" +} diff --git a/fixtures/mixed_placeholder_syntax/index.php b/fixtures/mixed_placeholder_syntax/index.php new file mode 100644 index 000000000..61ace196d --- /dev/null +++ b/fixtures/mixed_placeholder_syntax/index.php @@ -0,0 +1,2 @@ +/tmp/app/php")) + }) + }) + + context("WEBDIR placeholder in user configs", func() { + it("resolves WEBDIR in custom httpd.conf", func() { + deployment, logs, err := platform.Deploy. + WithEnv(map[string]string{ + "BP_DEBUG": "1", + }). + Execute(name, filepath.Join(fixtures, "custom_webdir_config")) + Expect(err).NotTo(HaveOccurred(), logs.String) + + Expect(logs).NotTo(ContainSubstring("DocumentRoot '/home/vcap/app/#{WEBDIR}' is not a directory")) + Expect(logs).NotTo(ContainSubstring("DocumentRoot '/home/vcap/app/@{WEBDIR}' is not a directory")) + + Eventually(deployment).Should(Serve( + ContainSubstring("PHP Version"), + )) + }) + + it("resolves custom WEBDIR value from options.json", func() { + deployment, logs, err := platform.Deploy. + WithEnv(map[string]string{ + "BP_DEBUG": "1", + }). + Execute(name, filepath.Join(fixtures, "custom_webdir_value")) + Expect(err).NotTo(HaveOccurred(), logs.String) + + // App has WEBDIR set to "public" in options.json + Eventually(deployment).Should(Serve( + ContainSubstring("Custom WEBDIR: public"), + )) + }) + }) + } +} + +// Helper function to read response body +func ReadAll(r io.Reader) []byte { + data, _ := io.ReadAll(r) + return data +} diff --git a/src/php/options/options.go b/src/php/options/options.go index 49200c185..93339ffee 100644 --- a/src/php/options/options.go +++ b/src/php/options/options.go @@ -211,8 +211,22 @@ func (o *Options) validate() error { } // GetPHPVersion returns the PHP version to use, either from user config or default +// Resolves placeholders like {PHP_83_LATEST} to actual versions func (o *Options) GetPHPVersion() string { if o.PHPVersion != "" { + // Check if it's a placeholder like {PHP_83_LATEST} + if strings.HasPrefix(o.PHPVersion, "{") && strings.HasSuffix(o.PHPVersion, "}") { + // Extract the placeholder name (remove { and }) + placeholderName := strings.TrimPrefix(strings.TrimSuffix(o.PHPVersion, "}"), "{") + + // Look up the actual version from PHPVersions map + if actualVersion, exists := o.PHPVersions[placeholderName]; exists { + return actualVersion + } + + // If placeholder not found, return as-is (will fail with clear error message) + // This allows the buildpack to show which placeholder was invalid + } return o.PHPVersion } return o.PHPDefault diff --git a/src/php/options/options_test.go b/src/php/options/options_test.go index 76238c7ef..e696793f2 100644 --- a/src/php/options/options_test.go +++ b/src/php/options/options_test.go @@ -259,3 +259,84 @@ func TestGetPHPVersion(t *testing.T) { t.Errorf("Expected 8.2.15, got %s", opts.GetPHPVersion()) } } + +func TestGetPHPVersion_PlaceholderResolution(t *testing.T) { + tests := []struct { + name string + phpVersion string + phpVersions map[string]string + expected string + }{ + { + name: "Resolves {PHP_83_LATEST} placeholder", + phpVersion: "{PHP_83_LATEST}", + phpVersions: map[string]string{ + "PHP_81_LATEST": "8.1.34", + "PHP_82_LATEST": "8.2.29", + "PHP_83_LATEST": "8.3.30", + }, + expected: "8.3.30", + }, + { + name: "Resolves {PHP_82_LATEST} placeholder", + phpVersion: "{PHP_82_LATEST}", + phpVersions: map[string]string{ + "PHP_81_LATEST": "8.1.34", + "PHP_82_LATEST": "8.2.29", + "PHP_83_LATEST": "8.3.30", + }, + expected: "8.2.29", + }, + { + name: "Resolves {PHP_81_LATEST} placeholder", + phpVersion: "{PHP_81_LATEST}", + phpVersions: map[string]string{ + "PHP_81_LATEST": "8.1.34", + "PHP_82_LATEST": "8.2.29", + "PHP_83_LATEST": "8.3.30", + }, + expected: "8.1.34", + }, + { + name: "Returns invalid placeholder as-is (for clear error message)", + phpVersion: "{PHP_99_LATEST}", + phpVersions: map[string]string{ + "PHP_81_LATEST": "8.1.34", + "PHP_82_LATEST": "8.2.29", + "PHP_83_LATEST": "8.3.30", + }, + expected: "{PHP_99_LATEST}", + }, + { + name: "Returns exact version without placeholder syntax", + phpVersion: "8.3.21", + phpVersions: map[string]string{ + "PHP_83_LATEST": "8.3.30", + }, + expected: "8.3.21", + }, + { + name: "Returns version with partial placeholder syntax (not a placeholder)", + phpVersion: "PHP_83_LATEST", + phpVersions: map[string]string{ + "PHP_83_LATEST": "8.3.30", + }, + expected: "PHP_83_LATEST", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + opts := &options.Options{ + PHPDefault: "8.1.32", + PHPVersion: tt.phpVersion, + PHPVersions: tt.phpVersions, + } + + result := opts.GetPHPVersion() + if result != tt.expected { + t.Errorf("Expected %s, got %s", tt.expected, result) + } + }) + } +}