-
Notifications
You must be signed in to change notification settings - Fork 0
1 debian packaging for easy deploy and update #2
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.
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
base: main
Are you sure you want to change the base?
Conversation
Incorporates additional Debian-specific directories and files into the .gitignore to prevent unnecessary tracking of build artifacts. This change helps maintain a cleaner repository by excluding files generated during the packaging process, thus improving overall project management.
Introduces a cache file for PHP CS Fixer to maintain code style consistency across the project. Enhances the formatting in various PHP files, including updating comments for clarity, standardizing whitespace, and ensuring strict type declarations. These changes aim to improve code readability and maintainability, aligning with best practices for PHP development.
WalkthroughThis PR introduces comprehensive Debian packaging infrastructure with lifecycle scripts, development tooling (Makefile, PHP-CS-Fixer, Composer), multi-stage CI/CD pipelines for parallel Debian/Ubuntu builds, and enhancements to PHP libraries with retry mechanisms, data formatting, and validation utilities. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
composer.json (1)
12-14: Critical: Duplicate require-dev key in JSON.The
require-devkey appears twice (lines 12-14 and lines 22-28), which creates invalid JSON. The first occurrence should be removed, keeping only the expanded version at lines 22-28.🔎 Proposed fix
"require": { "php": ">=7.4", "ext-json": "*", "ext-soap": "*", "ext-openssl": "*" }, - "require-dev": { - "phpunit/phpunit": "^9.5" - }, "autoload": { "psr-4": { "ISPConfigMonitoring\\": "src/lib/" } }, "minimum-stability": "stable", "prefer-stable": true, "require-dev": { "phpunit/phpunit": "*", "phpstan/phpstan": "*", "friendsofphp/php-cs-fixer": "^3.85", "ergebnis/composer-normalize": "^2.47", "ergebnis/php-cs-fixer-config": "^6.51" },Also applies to: 22-28
🟠 Major comments (11)
composer.json-24-24 (1)
24-24: Use specific version constraints for PHPStan.Similar to PHPUnit, using
*forphpstan/phpstanis overly permissive. Consider using a specific version constraint like^1.0or^2.0.src/lib/ISPConfigClient.php-244-255 (1)
244-255: SSL verification bypass is a security risk.Allowing SSL verification to be disabled (
verify_ssl: false) can expose the application to man-in-the-middle attacks. While this may be necessary for development or testing environments, ensure that:
- This option is clearly documented as a security risk
- Production configurations never use this option
- Consider adding a warning when SSL verification is disabled
🔎 Proposed enhancement to add warning
// SSL verification settings $verifySSL = $this->config['verify_ssl'] ?? true; if (!$verifySSL) { + error_log('WARNING: SSL certificate verification is disabled. This is insecure and should only be used in development environments.'); $soapOptions['stream_context'] = stream_context_create([ 'ssl' => [ 'verify_peer' => false, 'verify_peer_name' => false, 'allow_self_signed' => true, ], ]); }.build/php-cs-fixer/.php-cs-fixer.cache-1-1 (1)
1-1: Add.build/directory and.php-cs-fixer.cacheto.gitignoreand remove from version control.The
.php-cs-fixer.cachefile is a generated artifact containing environment-specific data (PHP version8.4.15, file hashes, absolute paths, configuration). This file is currently tracked in git and will cause merge conflicts and repository bloat across different development environments.Add to
.gitignore:.build/ .php-cs-fixer.cacheThen remove the file from git:
git rm --cached .build/php-cs-fixer/.php-cs-fixer.cachecomposer.json-23-23 (1)
23-23: Use specific version constraints for PHPUnit.The wildcard version constraint (
*) forphpunit/phpunitallows any version, which could lead to compatibility issues or unexpected behavior. Consider using a specific version range like^9.5or^10.0based on your PHP version requirements.🔎 Proposed fix
"require-dev": { - "phpunit/phpunit": "*", + "phpunit/phpunit": "^9.5", "phpstan/phpstan": "*", "friendsofphp/php-cs-fixer": "^3.85", "ergebnis/composer-normalize": "^2.47", "ergebnis/php-cs-fixer-config": "^6.51" },debian/Jenkinsfile.release-7-8 (1)
7-8: Image prefix appears to be for a different project.The
imagePrefix = 'multiflexi-'and the downstream job referenceMultiFlexi-publishsuggest this Jenkinsfile was copied from the MultiFlexi project. Forispconfig-zabbix-monitoring, the image prefix and job references should be updated accordingly.debian/Jenkinsfile.release-150-161 (1)
150-161: Hardcoded publish destination appears to be for MultiFlexi project.The downstream job
MultiFlexi-publishand the SSH target[email protected]seem to be from a different project. Update these to match the correct publish destination forispconfig-zabbix-monitoring.debian/templates-16-20 (1)
16-20: Security concern: Default password in debconf template.Having a default password value (even for a demo server) in the package template is a security risk. Users may inadvertently deploy with these defaults. Consider removing the default or setting it to an empty string, forcing users to explicitly provide credentials.
🔎 Suggested fix
Template: abraflexi-zabbix/abraflexi_password Type: password -Default: winstrom +Default: Description: AbraFlexi API password Enter the password for the AbraFlexi API user.debian/postinst-44-48 (1)
44-48: Validate zabbix group exists before attempting chown.Lines 46 and 60 use
|| trueto silently ignorechownfailures. If thezabbixgroup doesn't exist during installation, files will have incorrect ownership (likelyroot:root), preventing the Zabbix agent from reading configuration or executing scripts. This violates the fail-fast principle ofset -e.🔎 Proposed fix
Add group validation before the ownership operations:
+ # Ensure zabbix group exists + if ! getent group zabbix > /dev/null 2>&1; then + echo "Error: zabbix group does not exist. Please install zabbix-agent or zabbix-agent2 first." >&2 + exit 1 + fi + # Set proper ownership and permissions for .env file if it exists if [ -f /etc/abraflexi-zabbix/.env ]; then - chown root:zabbix /etc/abraflexi-zabbix/.env 2>/dev/null || true + chown root:zabbix /etc/abraflexi-zabbix/.env chmod 640 /etc/abraflexi-zabbix/.env fiAnd for line 60:
# Set proper ownership - chown -R zabbix:zabbix /usr/lib/abraflexi-zabbix/ 2>/dev/null || true + chown -R zabbix:zabbix /usr/lib/abraflexi-zabbix/Also applies to: 60-60
Makefile-42-43 (1)
42-43: Declarecompanylldas.PHONY.The
companylldtarget is missing the.PHONYdeclaration. Without it, Make will treat it as a file target and skip execution if a file namedcompanylldexists in the directory.🔎 Proposed fix
+.PHONY: companylld companylld: cd src; php company_lld.phpMakefile-14-16 (1)
14-16: Verify Symfony CLI availability or use standard PHP.Line 16 invokes
symfony phpwithout checking if the Symfony CLI is installed. This will fail with "command not found" if the CLI is unavailable. Either add a dependency on acheck-symfonytarget (see previous comment) or use the standardvendor/bin/phpunitinvocation.🔎 Proposed fix using standard PHP
.PHONY: tests tests: vendor - symfony php vendor/bin/phpunit tests + vendor/bin/phpunit testsdebian/postinst-25-41 (1)
25-41: Quote variables in .env file generation to prevent injection.The variables
$ABRAFLEXI_URL,$ABRAFLEXI_LOGIN, and$ABRAFLEXI_PASSWORDare expanded without quotes in the heredoc. If debconf values contain spaces, special characters ($,`,\), or newlines, the resulting .env file will be malformed or vulnerable to shell expansion issues.🔎 Proposed fix
if [ ! -f /etc/abraflexi-zabbix/.env ] || [ "${DEBCONF_RECONFIGURE:-}" = "1" ]; then cat > /etc/abraflexi-zabbix/.env << EOF # AbraFlexi Zabbix Monitoring Configuration # Generated during package installation or reconfiguration # AbraFlexi server URL (required) -ABRAFLEXI_URL=$ABRAFLEXI_URL +ABRAFLEXI_URL="$ABRAFLEXI_URL" # AbraFlexi API credentials (required) -ABRAFLEXI_LOGIN=$ABRAFLEXI_LOGIN -ABRAFLEXI_PASSWORD=$ABRAFLEXI_PASSWORD +ABRAFLEXI_LOGIN="$ABRAFLEXI_LOGIN" +ABRAFLEXI_PASSWORD="$ABRAFLEXI_PASSWORD" # Optional: Request timeout in seconds (default: 30) # ABRAFLEXI_TIMEOUT=30 # Optional: Debug mode (uncomment for verbose logging) # DEBUG=1 EOF fi
🟡 Minor comments (8)
debian/source/format-1-1 (1)
1-1: Change source format to3.0 (quilt)for upstream project.The non-native Debian package in the Debian source format "3.0 (quilt)" is the most normal Debian source package format. Native packages are normally used for software that has no independent existence outside of Debian. Since this project is hosted on GitHub as an upstream repository, it should use
3.0 (quilt)to properly separate upstream code from Debian-specific changes. This allows clearer tracking of upstream updates and is the standard practice for packaging upstream projects in Debian.debian/compat-1-1 (1)
1-1: Use modern debhelper-compat declaration instead of the deprecated compat file.The separate
debian/compatfile is no longer the recommended approach. The debhelper-compat build-dependency is recommended whenever possible.Replace this file by:
- Removing
debian/compat- Adding
debhelper-compat (= 13)to theBuild-Dependsfield indebian/controlThis approach aligns with current Debian packaging standards and simplifies build dependencies in debian/control.
src/lib/ISPConfigClient.php-286-289 (1)
286-289: Session error detection is fragile.The session reset logic relies on checking if the error message contains the substring 'session', which is case-sensitive and may not catch all session-related errors. Consider:
- Making the check case-insensitive
- Checking for specific error codes if the SOAP API provides them
- Looking for additional session-related keywords
🔎 Proposed fix for case-insensitive check
if ($attempt < $this->retryAttempts) { error_log("Attempt {$attempt} failed, retrying in {$this->retryDelay} seconds..."); sleep($this->retryDelay); // Reset session ID on authentication errors - if (str_contains($e->getMessage(), 'session')) { + if (stripos($e->getMessage(), 'session') !== false || stripos($e->getMessage(), 'auth') !== false) { $this->sessionId = null; } }debian/templates-1-20 (1)
1-20: Missing trailing newline and potential package name mismatch.Two minor issues:
- The file is missing a trailing newline, which is required per Debian packaging standards.
- The template namespace
abraflexi-zabbixdoesn't match the package nameispconfig-zabbix-monitoringseen indebian/install. Verify this is intentional (e.g., if this is for a related but different package)..php-cs-fixer.dist.php-5-12 (1)
5-12: Header references wrong package name.The header comment mentions "EaseCore package" but this is the
ispconfig-zabbix-monitoringproject. Update the header to reflect the correct package name.🔎 Suggested fix
/** - * This file is part of the EaseCore package. + * This file is part of the ispconfig-zabbix-monitoring package. * * (c) Vítězslav Dvořák <[email protected]> * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. */And similarly update the
$headervariable:$header = <<<'HEADER' -This file is part of the EaseCore package. +This file is part of the ispconfig-zabbix-monitoring package. (c) Vítězslav Dvořák <[email protected]>Also applies to: 18-25
debian/Jenkinsfile-117-117 (1)
117-117: Typo: "INSTALATION" should be "INSTALLATION".🔎 Suggested fix
- sh 'echo "INSTALATION"' + sh 'echo "INSTALLATION"'debian/Jenkinsfile.release-110-110 (1)
110-110: Typo: "INSTALATION" should be "INSTALLATION".🔎 Suggested fix
- sh 'echo "INSTALATION"' + sh 'echo "INSTALLATION"'.php-cs-fixer.dist.php-96-99 (1)
96-99:rector.phpdoes not exist in the repository root.The configuration in
.php-cs-fixer.dist.php(lines 96-99) appends a reference torector.php, but this file is missing from the repository. Either create the file or remove the reference if it's no longer needed.
🧹 Nitpick comments (10)
debian/watch (1)
1-2: Watch file is incomplete or unnecessary.The watch file only declares
version=3without any upstream URL pattern. For a native Debian package (format3.0 (native)per debian/source/format), a watch file is typically not needed since there's no upstream to track. Consider either:
- Removing this file entirely if this is a native package
- Adding a proper upstream URL pattern if you plan to track upstream releases
- Updating to
version=4(current standard) if keeping the filedebian/clean (1)
1-1: Remove empty placeholder file.The debian/clean file contains only a comment with no actual file patterns to clean. This file should either list specific files to remove during the build clean phase (e.g., generated files, build artifacts) or be removed entirely if not needed.
debian/links (1)
1-2: Remove empty placeholder file.The debian/links file is used to define symlinks that should be created during package installation, but it currently contains only a blank line and a placeholder comment.
This file should either:
- Define actual symlinks (format:
source_path destination_path)- Be removed entirely if no symlinks are needed
debian/config (1)
17-17: Consider using high priority for password prompt.The password prompt uses "medium" priority, which might be skipped in non-interactive installations. Security-sensitive credentials should typically use "high" priority to ensure they're always requested.
🔎 Suggested change
-db_input medium ispconfig-zabbix-monitoring/ispconfig_password || true +db_input high ispconfig-zabbix-monitoring/ispconfig_password || truesrc/lib/ISPConfigClient.php (2)
31-32: Consider making retry configuration customizable.The retry parameters are hardcoded. Consider accepting these as optional constructor parameters to allow users to configure retry behavior based on their environment.
🔎 Proposed enhancement
- private int $retryAttempts = 3; - private int $retryDelay = 2; // seconds + private int $retryAttempts; + private int $retryDelay;And update the constructor:
/** * Initialize ISPConfig client. * * @param array $config Configuration array with keys: * - soap_uri: ISPConfig SOAP endpoint URL * - soap_location: SOAP location URL * - username: ISPConfig username * - password: ISPConfig password * - verify_ssl: Whether to verify SSL certificates (default: true) + * - retry_attempts: Number of retry attempts (default: 3) + * - retry_delay: Delay between retries in seconds (default: 2) * * @throws ISPConfigException */ public function __construct(array $config) { self::validateConfig($config); $this->config = $config; $this->username = $config['username']; $this->password = $config['password']; + $this->retryAttempts = $config['retry_attempts'] ?? 3; + $this->retryDelay = $config['retry_delay'] ?? 2; $this->initializeSoapClient(); }
213-226: Consider validating soap_location URL as well.The validation checks
soap_uribut notsoap_location. Since both are URLs, consider applying the same validation tosoap_locationfor consistency.🔎 Proposed enhancement
if (!filter_var($config['soap_uri'], \FILTER_VALIDATE_URL)) { throw new ISPConfigException('Invalid SOAP URI'); } + + if (!filter_var($config['soap_location'], \FILTER_VALIDATE_URL)) { + throw new ISPConfigException('Invalid SOAP location'); + } }debian/Jenkinsfile.release (1)
1-161: Consider extracting shared logic withdebian/Jenkinsfile.There is significant code duplication between
JenkinsfileandJenkinsfile.release. The main differences are the image prefix and the final publish step. Consider extracting the common stages into a shared library or parameterized function to reduce maintenance burden.debian/postinst (1)
50-54: Clarify why line 54 is optional while lines 51-53 are required.Lines 51-53 set executable permissions on scripts without error handling, implying they must exist. Line 54 uses
|| trueto ignore failures, suggesting/usr/lib/zabbix/externalscripts/abraflexi-update-check.shis optional. Consider adding a comment explaining why this script is optional while the others are mandatory, or verify that all required files are properly declared indebian/install.Makefile (2)
34-36: Verify coverage output path is correct.Line 36 outputs coverage to
tests/coverage, but the standard convention is justcoverageat the project root (as mentioned in the AI summary). Verify this path is correct, or update to match the conventional location.🔎 Proposed fix if using conventional path
.PHONY: test-coverage test-coverage: vendor ## Run PHPUnit tests with coverage - vendor/bin/phpunit --coverage-html tests/coverage + vendor/bin/phpunit --coverage-html coverage
1-43: Consider adding conventionalallandcleantargets.The Makefile lacks the conventional
allandcleanphony targets. While not strictly required, these are widely expected in Make-based projects:
all: Default target to build/prepare everythingclean: Remove generated artifacts (vendor/, coverage/, cache files)Based on learnings, static analysis tools flagged this.
🔎 Proposed additions
Add these targets at the beginning of the Makefile:
# vim: set tabstop=8 softtabstop=8 noexpandtab: +.PHONY: all +all: vendor ## Default target - install dependencies + +.PHONY: clean +clean: ## Remove generated files and dependencies + rm -rf vendor/ coverage/ .phpunit.result.cache + .PHONY: help help: ## Displays this list of targets with descriptions @grep -E '^[a-zA-Z0-9_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[32m%-30s\033[0m %s\n", $$1, $$2}'
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
.build/php-cs-fixer/.php-cs-fixer.cache(1 hunks).gitignore(1 hunks).php-cs-fixer.dist.php(1 hunks)Makefile(1 hunks)composer.json(1 hunks)debian/Jenkinsfile(1 hunks)debian/Jenkinsfile.release(1 hunks)debian/README.Debian(1 hunks)debian/changelog(1 hunks)debian/clean(1 hunks)debian/compat(1 hunks)debian/conf/composer.json(1 hunks)debian/config(1 hunks)debian/control(1 hunks)debian/copyright(1 hunks)debian/files(1 hunks)debian/install(1 hunks)debian/links(1 hunks)debian/postinst(1 hunks)debian/postrm(1 hunks)debian/rules(1 hunks)debian/salsa-ci.yml(1 hunks)debian/source/format(1 hunks)debian/templates(1 hunks)debian/tests/control(1 hunks)debian/tests/package-install(1 hunks)debian/watch(1 hunks)src/autodiscovery/websites.php(1 hunks)src/keys/websites.php(3 hunks)src/lib/ISPConfigClient.php(8 hunks)src/lib/ZabbixHelper.php(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/keys/websites.php (2)
src/lib/ISPConfigClient.php (3)
ISPConfigClient(24-296)ISPConfigException(301-303)getWebsite(142-159)src/lib/ZabbixHelper.php (2)
ZabbixHelper(22-320)formatItemValue(83-102)
src/autodiscovery/websites.php (2)
src/lib/ISPConfigClient.php (3)
ISPConfigClient(24-296)ISPConfigException(301-303)getWebsites(120-133)src/lib/ZabbixHelper.php (4)
ZabbixHelper(22-320)outputJSON(132-135)formatWebsitesDiscovery(60-73)validateLLDData(154-174)
🪛 checkmake (0.2.2)
Makefile
[warning] 38-38: Missing required phony target "all"
(minphony)
[warning] 38-38: Missing required phony target "clean"
(minphony)
🔇 Additional comments (21)
debian/rules (1)
1-3: LGTM!This is the standard minimal
debian/rulesformat using debhelper, which follows current Debian packaging best practices. The file correctly delegates all build targets todhfor automatic handling.debian/changelog (1)
1-5: LGTM!The changelog entry follows proper Debian changelog format (RFC 5322 style) with:
- Correct package name, version, distribution, and urgency
- Proper change description referencing issue closure
- Valid maintainer line with timestamp
The format is compliant with Debian packaging standards.
debian/config (1)
20-20: Verify error suppression is intentional.The
|| truesuffix ondb_gosuppresses errors. While this is sometimes necessary for non-interactive installations, it could mask configuration failures. Please confirm this is intentional and consider whether error handling should be more explicit.src/autodiscovery/websites.php (1)
1-88: LGTM! Formatting improvements.The changes are purely cosmetic (whitespace, string formatting, concatenation style) with no functional impact. These align with the PHP-CS-Fixer configuration introduced in this PR.
src/keys/websites.php (1)
1-144: LGTM! Formatting improvements.The changes are purely cosmetic (whitespace, header updates, string formatting) with no functional impact. Consistent with the code formatting standards introduced in this PR.
debian/conf/composer.json (1)
11-13: Verify vendor directory permissions and integration.The vendor directory is set to
/var/lib/composer/ispconfig-zabbix-monitoring, which is unusual. Please ensure:
- The Debian packaging scripts create this directory with appropriate permissions
- The application can access this location at runtime
- The PSR-4 autoload path
src/lib/resolves correctly from this vendor locationsrc/lib/ZabbixHelper.php (2)
75-125: Well-designed API additions.The new public methods (
formatItemValue,createItemKey) provide clean, type-safe interfaces for Zabbix data formatting and key generation. The refactoring of helpers to private static methods improves code organization and clarity.
127-174: Good validation and output utilities.The new
validateLLDData,outputJSON, andoutputJSONPrettymethods add robust validation and consistent output formatting. The macro validation regex pattern correctly enforces Zabbix's{#MACRO_NAME}format.src/lib/ISPConfigClient.php (9)
46-54: LGTM!The constructor properly validates configuration, stores credentials, and initializes the SOAP client. The static validation method call is appropriate.
59-62: Good practice to ensure cleanup.The destructor properly ensures logout is called when the object is destroyed. The logout method already handles exceptions internally by logging errors rather than throwing, which is appropriate for destructor usage.
71-90: LGTM!The login method properly implements session reuse, retry logic, and validation. The empty session ID check prevents silent failures, and error handling is robust.
95-111: LGTM!The logout method appropriately handles errors by logging rather than throwing exceptions, which is suitable for cleanup operations. Session state is properly managed.
120-133: LGTM!The method properly ensures authentication, uses retry logic, and defensively handles non-array responses. Error handling is consistent with the class pattern.
142-159: LGTM!The method appropriately handles nullable returns, validates response structure, and extracts the first result. Defensive programming with array checks is well-implemented.
168-181: LGTM!Consistent implementation with good error context and defensive array handling.
193-206: LGTM!The generic call method provides flexibility for arbitrary API operations while maintaining consistent session management and error handling.
298-303: LGTM!Simple, clean custom exception class for domain-specific error handling.
debian/tests/control (1)
1-3: LGTM!Standard Debian autopkgtest control file with appropriate configuration. The
allow-stderrrestriction is reasonable for tests that may produce diagnostic output.debian/control (1)
11-11: JSON and OpenSSL extensions are provided by the base php package.The JSON extension is a core PHP extension always enabled, and OpenSSL is similarly builtin. Since the package requires PHP >=7.4, both extensions are automatically available through the base
phpdependency already listed indebian/control. No additional packages need to be added.debian/install (1)
1-7: LGTM!The install file correctly maps source directories to FHS-compliant installation paths. Documentation is properly placed under
/usr/share/doc/.debian/Jenkinsfile (1)
159-162: VerifypublishDebToAptly()is available from a shared library.The function
publishDebToAptly()is called but not defined in this file. Ensure the Jenkins shared library providing this function is properly configured in the Jenkins instance.
| "require": { | ||
| "php": ">=7.4", | ||
| "ext-curl": "*", | ||
| "ext-yaml": "*" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for PHP 8.0+ functions that are incompatible with PHP 7.4
echo "=== Searching for str_contains usage (PHP 8.0+) ==="
rg -n 'str_contains\(' --type php
echo -e "\n=== Searching for str_starts_with usage (PHP 8.0+) ==="
rg -n 'str_starts_with\(' --type php
echo -e "\n=== Searching for str_ends_with usage (PHP 8.0+) ==="
rg -n 'str_ends_with\(' --type php
echo -e "\n=== Searching for match expression usage (PHP 8.0+) ==="
rg -n 'match\s*\(' --type phpRepository: Spoje-NET/ispconfig-zabbix-monitoring
Length of output: 742
Update PHP requirement to 8.0 or replace PHP 8.0+ functions.
The configuration requires PHP >=7.4, but the codebase uses str_contains() at lines 191 and 314 in src/lib/ZabbixHelper.php and line 287 in src/lib/ISPConfigClient.php. The str_contains() function was introduced in PHP 8.0, causing runtime errors on PHP 7.4 systems. Either update the PHP requirement to >=8.0 in composer.json or replace str_contains() with strpos() for backward compatibility.
🤖 Prompt for AI Agents
debian/conf/composer.json lines 14-17: the composer PHP requirement is set to
">=7.4" but the code uses PHP 8.0-only function str_contains() (used in
src/lib/ZabbixHelper.php lines ~191 and ~314 and src/lib/ISPConfigClient.php
line ~287), causing runtime errors on 7.4; fix by either updating composer.json
to require "php": ">=8.0" (preferred) or replace each str_contains($haystack,
$needle) call with a PHP 7.4-compatible check using strpos($haystack, $needle)
!== false to preserve behavior, and run tests to ensure no regressions.
|
|
||
| Package: ispconfig-zabbix-monitoring | ||
| Architecture: all | ||
| Depends: ${misc:Depends}, php, php-curl, php-yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing php-soap dependency.
The source code requires the soap PHP extension (as seen in composer.json line 9: "ext-soap": "*"), but php-soap is not listed in the package dependencies. This will cause runtime errors if the SOAP extension is not installed.
🔎 Proposed fix
-Depends: ${misc:Depends}, php, php-curl, php-yaml
+Depends: ${misc:Depends}, php, php-curl, php-yaml, php-soap📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Depends: ${misc:Depends}, php, php-curl, php-yaml | |
| Depends: ${misc:Depends}, php, php-curl, php-yaml, php-soap |
🤖 Prompt for AI Agents
In debian/control around line 11, the package dependencies list omits the
php-soap package while the project requires the PHP SOAP extension
(composer.json references "ext-soap"). Update the Depends line to include
php-soap so the runtime SOAP extension is installed by the package manager;
ensure the new dependency is added alongside the existing php, php-curl, and
php-yaml entries and follow Debian dependency formatting conventions.
| ispconfig-zabbix-monitoring_0.1_all.deb admin optional | ||
| ispconfig-zabbix-monitoring_0.1_amd64.buildinfo admin optional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not commit debian/files to version control.
The debian/files file is auto-generated by dpkg-genchanges during the package build process and should never be committed to version control. This file will be regenerated on every build and committing it can cause merge conflicts and packaging issues.
Required actions:
- Remove
debian/filesfrom the repository:git rm debian/files - Add
debian/filesto.gitignoreto prevent future accidental commits - Also ensure other generated files are ignored (e.g.,
debian/*.debhelper,debian/*.substvars,debian/tmp/,debian/.debhelper/)
🔎 Script to verify other potentially committed build artifacts
#!/bin/bash
# Description: Check for other commonly generated Debian packaging files that shouldn't be committed
echo "Checking for generated Debian packaging files in git..."
echo ""
# Check for common generated files
for pattern in "debian/files" "debian/*.debhelper*" "debian/*.substvars" "debian/tmp" "debian/.debhelper" "debian/*-stamp"; do
if git ls-files "$pattern" 2>/dev/null | grep -q .; then
echo "⚠️ Found committed generated files matching: $pattern"
git ls-files "$pattern"
fi
done
echo ""
echo "Checking .gitignore for Debian build artifacts..."
if [ -f .gitignore ]; then
if grep -q "debian/files" .gitignore; then
echo "✓ debian/files is in .gitignore"
else
echo "✗ debian/files is NOT in .gitignore"
fi
else
echo "✗ No .gitignore file found"
fi🤖 Prompt for AI Agents
In debian/files (lines 1-2) the file is an auto-generated dpkg-genchanges
artifact and must not be committed; remove it from the repository (use git to
remove the tracked file), add debian/files to .gitignore and add common
generated Debian packaging patterns to .gitignore (e.g. debian/*.debhelper,
debian/*.substvars, debian/tmp/, debian/.debhelper/, debian/*-stamp) so they are
not accidentally committed, then commit the updated .gitignore and the removal;
optionally run a quick scan (git ls-files) to ensure no other generated Debian
packaging files remain tracked.
| sh 'cd $WORKSPACE/dist/debian/ ; dpkg-scanpackages . /dev/null > Packages; gzip -9c Packages > Packages.gz; cd $WORKSPACE' | ||
| sh 'echo "deb [trusted=yes] file://///$WORKSPACE/dist/debian/ ./" | sudo tee /etc/apt/sources.list.d/local.list' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File URI has too many slashes - will break APT.
The local APT source uses file://///$WORKSPACE which has 5 slashes. The correct format is file://$WORKSPACE (for absolute paths) or file:///$WORKSPACE. This will cause APT to fail to find the local repository.
🔎 Suggested fix
- sh 'echo "deb [trusted=yes] file://///$WORKSPACE/dist/debian/ ./" | sudo tee /etc/apt/sources.list.d/local.list'
+ sh 'echo "deb [trusted=yes] file://$WORKSPACE/dist/debian/ ./" | sudo tee /etc/apt/sources.list.d/local.list'🤖 Prompt for AI Agents
In debian/Jenkinsfile around lines 71 to 72, the APT source URI mistakenly uses
five slashes (file://///$WORKSPACE) which will break apt; replace it with a
correct file URI (e.g., file:///$WORKSPACE for an absolute path) in the echo/tee
command so the sources.list entry reads the proper file:// URI pointing to
$WORKSPACE/dist/debian/.
| sh 'cd $WORKSPACE/dist/debian/ ; dpkg-scanpackages . /dev/null > Packages; gzip -9c Packages > Packages.gz; cd $WORKSPACE' | ||
| sh 'echo "deb [trusted=yes] file://///$WORKSPACE/dist/debian/ ./" | sudo tee /etc/apt/sources.list.d/local.list' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File URI has too many slashes - will break APT.
Same issue as in debian/Jenkinsfile: the local APT source uses file://///$WORKSPACE which has 5 slashes. This will cause APT to fail.
🔎 Suggested fix
- sh 'echo "deb [trusted=yes] file://///$WORKSPACE/dist/debian/ ./" | sudo tee /etc/apt/sources.list.d/local.list'
+ sh 'echo "deb [trusted=yes] file://$WORKSPACE/dist/debian/ ./" | sudo tee /etc/apt/sources.list.d/local.list'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sh 'cd $WORKSPACE/dist/debian/ ; dpkg-scanpackages . /dev/null > Packages; gzip -9c Packages > Packages.gz; cd $WORKSPACE' | |
| sh 'echo "deb [trusted=yes] file://///$WORKSPACE/dist/debian/ ./" | sudo tee /etc/apt/sources.list.d/local.list' | |
| sh 'cd $WORKSPACE/dist/debian/ ; dpkg-scanpackages . /dev/null > Packages; gzip -9c Packages > Packages.gz; cd $WORKSPACE' | |
| sh 'echo "deb [trusted=yes] file://$WORKSPACE/dist/debian/ ./" | sudo tee /etc/apt/sources.list.d/local.list' |
🤖 Prompt for AI Agents
In debian/Jenkinsfile.release around lines 64-65, the APT file URI uses too many
slashes ("file://///$WORKSPACE") which will break APT; replace it with the
correct construction "file://$WORKSPACE" (so the expanded value becomes
file:///absolute/path) to remove the extra slashes and update the sh command
that writes /etc/apt/sources.list.d/local.list accordingly.
| abraflexi-zabbix for Debian | ||
| ============================ | ||
|
|
||
| AbraFlexi Zabbix Monitoring Package | ||
|
|
||
| This package provides comprehensive monitoring for AbraFlexi accounting server | ||
| using Zabbix with Low Level Discovery, system metrics, and granular alerting. | ||
|
|
||
| INSTALLATION | ||
| ============ | ||
|
|
||
| 1. Install the package (done automatically if you're reading this) | ||
|
|
||
| 2. Configure AbraFlexi connection: | ||
|
|
||
| Create /etc/abraflexi-zabbix/.env with your AbraFlexi server details: | ||
|
|
||
| ABRAFLEXI_URL=https://your-abraflexi-server:5434 | ||
| ABRAFLEXI_LOGIN=your-username | ||
| ABRAFLEXI_PASSWORD=your-password | ||
|
|
||
| 3. Import Zabbix template: | ||
|
|
||
| - Open Zabbix web interface | ||
| - Go to Configuration → Templates | ||
| - Click Import | ||
| - Upload /usr/share/doc/abraflexi-zabbix/abraflexi-template.xml | ||
| - Apply template to your AbraFlexi server host | ||
|
|
||
| 4. Test the configuration: | ||
|
|
||
| sudo -u zabbix zabbix_agent2 -t abraflexi.company.lld | ||
| sudo -u zabbix zabbix_agent2 -t abraflexi.system.version | ||
| sudo -u zabbix zabbix_agent2 -t abraflexi.network.connectivity | ||
|
|
||
| FEATURES | ||
| ======== | ||
|
|
||
| - Low Level Discovery of AbraFlexi companies/databases | ||
| - System metrics from /status.json (memory, CPU, users, sessions) | ||
| - Network connectivity monitoring (TCP, auth, service health) | ||
| - Granular error detection with specific exit codes | ||
| - Immediate alerting for critical issues | ||
| - Performance monitoring and thresholds | ||
| - Company availability tracking | ||
|
|
||
| FILES | ||
| ===== | ||
|
|
||
| Configuration: | ||
| - /etc/abraflexi-zabbix/.env - Connection settings (create this file) | ||
| - /etc/zabbix/zabbix_agent2.d/abraflexi.conf - Zabbix user parameters | ||
|
|
||
| Scripts: | ||
| - /usr/lib/abraflexi-zabbix/company_lld.php - Company discovery | ||
| - /usr/lib/abraflexi-zabbix/system_status.php - System metrics | ||
| - /usr/lib/abraflexi-zabbix/network_check.php - Network diagnostics | ||
| - /usr/bin/abraflexi-zabbix-lld-company - Legacy wrapper | ||
|
|
||
| Documentation: | ||
| - /usr/share/doc/abraflexi-zabbix/abraflexi-template.xml - Zabbix template | ||
|
|
||
| TROUBLESHOOTING | ||
| =============== | ||
|
|
||
| 1. Check script execution: | ||
| php /usr/lib/abraflexi-zabbix/company_lld.php | ||
|
|
||
| 2. Test Zabbix agent items: | ||
| sudo -u zabbix zabbix_agent2 -t abraflexi.network.overall | ||
|
|
||
| 3. Check logs: | ||
| tail -f /var/log/zabbix/zabbix_agent2.log | ||
| tail -f /var/log/syslog | grep abraflexi | ||
|
|
||
| 4. Verify permissions: | ||
| ls -la /usr/lib/abraflexi-zabbix/ | ||
|
|
||
| SUPPORT | ||
| ======= | ||
|
|
||
| GitHub: https://github.com/VitexSoftware/abraflexi-zabbix | ||
| Documentation: See README.md in the source repository | ||
|
|
||
|
|
||
| Zabbix version support | ||
| ---------------------- | ||
| This package supports only Zabbix Agent 2 (zabbix-agent2, Zabbix 7.x and newer). | ||
| Legacy zabbix-agent (zabbix_agentd) is not supported and no configuration is | ||
| installed into /etc/zabbix/zabbix_agentd.d/. | ||
|
|
||
|
|
||
| Update check items | ||
| ================== | ||
|
|
||
| The package provides external scripts and agent2 items to monitor latest available | ||
| AbraFlexi versions from Vitex testing and official production repositories: | ||
|
|
||
| - /usr/lib/zabbix/externalscripts/abraflexi-update-testing.sh | ||
| - /usr/lib/zabbix/externalscripts/abraflexi-update-production.sh | ||
|
|
||
| These are exposed via agent2 user parameters in /etc/zabbix/zabbix_agent2.d/abraflexi.conf: | ||
|
|
||
| - abraflexi.update.latestTesting | ||
| - abraflexi.update.latestProduction | ||
|
|
||
| The Zabbix template also contains dependent numeric items and triggers that compare | ||
| current AbraFlexi version with these latest available versions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify package name and check for mismatched references
echo "=== Package name from debian/control ==="
head -5 debian/control
echo ""
echo "=== All files in debian directory ==="
ls -la debian/
echo ""
echo "=== References to 'abraflexi' in debian/ directory ==="
rg -i "abraflexi" debian/ -C 1 || echo "No matches found"
echo ""
echo "=== References to 'ispconfig' in debian/ directory ==="
rg -i "ispconfig" debian/ -C 1 || echo "No matches found"Repository: Spoje-NET/ispconfig-zabbix-monitoring
Length of output: 15844
README.Debian describes AbraFlexi monitoring, not ISPConfig.
The entire README.Debian file (lines 1-108) contains content for "abraflexi-zabbix" monitoring package, but this repository is "ispconfig-zabbix-monitoring". The package configuration, installation instructions, and all references must be updated for ISPConfig:
- Line 1: Title should reference ISPConfig, not AbraFlexi
- Lines 4-7: Description should cover ISPConfig web hosting control panel, not AbraFlexi accounting server
- Line 16: Configuration path should be
/etc/ispconfig-zabbix-monitoring/not/etc/abraflexi-zabbix/ - Lines 18-20: Environment variables should match ISPConfig (ispconfig_url, ispconfig_login, ispconfig_password) not AbraFlexi
- All file paths, scripts, and examples reference abraflexi instead of ispconfig
- SUPPORT section references wrong GitHub repository (VitexSoftware/abraflexi-zabbix)
Additionally, debian/tests/package-install contains the same mismatched AbraFlexi references throughout the test script.
The file must be completely rewritten to document ISPConfig monitoring instead of AbraFlexi.
🤖 Prompt for AI Agents
debian/README.Debian lines 1-108: the README currently documents the
abraflexi-zabbix package and contains AbraFlexi-specific titles, descriptions,
configuration paths, env vars, file lists, scripts, troubleshooting steps and
support links; replace all AbraFlexi references with ISPConfig equivalents.
Update the title to reference ISPConfig, rewrite the short description to
describe ISPConfig web hosting control panel monitoring, change configuration
directory to /etc/ispconfig-zabbix-monitoring/, rename environment variables to
ISPConfig names (e.g. ISPCONFIG_URL, ISPCONFIG_LOGIN, ISPCONFIG_PASSWORD),
update all file paths and script names from abraflexi-* to ispconfig-*
(including templates, scripts, and agent config), correct examples and test
commands to use ispconfig-specific items, and change the SUPPORT
GitHub/documentation links to the ispconfig-zabbix-monitoring repository; also
mirror these replacements in debian/tests/package-install so the tests reference
ISPConfig instead of AbraFlexi.
| #!/bin/bash | ||
| set -e | ||
|
|
||
| # Test basic package installation and file presence | ||
| test -f /usr/lib/abraflexi-zabbix/company_lld.php | ||
| test -f /usr/lib/abraflexi-zabbix/system_status.php | ||
| test -f /usr/lib/abraflexi-zabbix/cached_system_status.php | ||
| test -f /usr/lib/abraflexi-zabbix/network_check.php | ||
| test -f /usr/lib/abraflexi-zabbix/AbraFlexi/Zabbix/Feeder.php | ||
|
|
||
| test -f /usr/bin/abraflexi-zabbix-lld-company | ||
| test -f /usr/bin/abraflexi-zabbix-status | ||
| test -f /usr/bin/abraflexi-zabbix-network | ||
|
|
||
| # Test configuration file | ||
| test -f /etc/abraflexi-zabbix/.env | ||
|
|
||
| # Test commands with demo configuration | ||
| export ABRAFLEXI_URL=https://demo.flexibee.eu:5434 | ||
| export ABRAFLEXI_LOGIN=winstrom | ||
| export ABRAFLEXI_PASSWORD=winstrom | ||
|
|
||
| echo "Testing command execution..." | ||
|
|
||
| # Test system status command | ||
| echo "Testing abraflexi-zabbix-status..." | ||
| /usr/bin/abraflexi-zabbix-status --debug || echo "Status command test completed with exit code $?" | ||
|
|
||
| # Test network check command | ||
| echo "Testing abraflexi-zabbix-network..." | ||
| /usr/bin/abraflexi-zabbix-network --debug || echo "Network command test completed with exit code $?" | ||
|
|
||
| # Test company LLD command | ||
| echo "Testing abraflexi-zabbix-lld-company..." | ||
| /usr/bin/abraflexi-zabbix-lld-company --debug || echo "Company LLD command test completed with exit code $?" | ||
|
|
||
| # Test color output | ||
| echo "Testing color output..." | ||
| /usr/bin/abraflexi-zabbix-status --debug --color || echo "Color output test completed with exit code $?" | ||
|
|
||
| echo "Package installation and command tests completed" No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Test script is for the wrong project.
This test script checks for abraflexi-zabbix files and commands, but the package being tested is ispconfig-zabbix-monitoring. All references need to be corrected:
- Lines 5-9:
/usr/lib/abraflexi-zabbix/→/usr/lib/ispconfig-zabbix/ - Lines 11-13:
/usr/bin/abraflexi-zabbix-*→/usr/bin/ispconfig-zabbix-* - Line 16:
/etc/abraflexi-zabbix/→/etc/ispconfig-zabbix/ - Lines 19-21:
ABRAFLEXI_*environment variables →ISPCONFIG_* - Lines 26-39: Command paths and names need to be updated
The demo configuration (lines 19-21) also points to AbraFlexi demo server (demo.flexibee.eu), which is incorrect for testing an ISPConfig monitoring package.
Note: This test script needs to be completely rewritten for ISPConfig. You'll need:
- Correct file paths for ISPConfig monitoring scripts
- Correct binary names
- ISPConfig-specific test configuration (not AbraFlexi)
- Appropriate test commands for ISPConfig monitoring
🤖 Prompt for AI Agents
In debian/tests/package-install around lines 1 to 41, the script tests the wrong
project (abraflexi-zabbix) instead of ispconfig-zabbix-monitoring; update all
file paths, binary names, and env vars to ISPConfig equivalents (change
/usr/lib/abraflexi-zabbix/ → /usr/lib/ispconfig-zabbix/,
/usr/bin/abraflexi-zabbix-* → /usr/bin/ispconfig-zabbix-*,
/etc/abraflexi-zabbix/ → /etc/ispconfig-zabbix/, and ABRAFLEXI_* → ISPCONFIG_*),
replace the AbraFlexi demo URL and credentials with appropriate ISPConfig test
configuration or safe placeholders, and rewrite the command tests (lines ~26–39)
to call the correct ispconfig-zabbix binaries and verify their expected
outputs/exit codes; ensure the script asserts presence of ISPConfig-specific
files, uses correct env var names, and contains only ISPConfig-relevant test
commands.
| .PHONY: static-code-analysis-baseline | ||
| static-code-analysis-baseline: check-symfony vendor ## Generates a baseline for static code analysis with phpstan/phpstan | ||
| symfony php vendor/bin/phpstan analyze --configuration=phpstan-default.neon.dist --generate-baseline=phpstan-default-baseline.neon --memory-limit=-1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Define missing check-symfony target or remove dependency.
Line 11 declares a dependency on check-symfony, but this target is not defined in the Makefile. Running make static-code-analysis-baseline will fail with "No rule to make target 'check-symfony'".
🔎 Proposed fix
Either remove the undefined dependency if Symfony CLI is not required:
.PHONY: static-code-analysis-baseline
-static-code-analysis-baseline: check-symfony vendor ## Generates a baseline for static code analysis with phpstan/phpstan
- symfony php vendor/bin/phpstan analyze --configuration=phpstan-default.neon.dist --generate-baseline=phpstan-default-baseline.neon --memory-limit=-1
+static-code-analysis-baseline: vendor ## Generates a baseline for static code analysis with phpstan/phpstan
+ vendor/bin/phpstan analyze --configuration=phpstan-default.neon.dist --generate-baseline=phpstan-default-baseline.neon --memory-limit=-1Or define the check-symfony target:
+.PHONY: check-symfony
+check-symfony: ## Checks that Symfony CLI is available
+ @which symfony > /dev/null || (echo "Symfony CLI not found. Install from https://symfony.com/download" && exit 1)
+
.PHONY: static-code-analysis-baseline
static-code-analysis-baseline: check-symfony vendor ## Generates a baseline for static code analysis with phpstan/phpstan
symfony php vendor/bin/phpstan analyze --configuration=phpstan-default.neon.dist --generate-baseline=phpstan-default-baseline.neon --memory-limit=-1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .PHONY: static-code-analysis-baseline | |
| static-code-analysis-baseline: check-symfony vendor ## Generates a baseline for static code analysis with phpstan/phpstan | |
| symfony php vendor/bin/phpstan analyze --configuration=phpstan-default.neon.dist --generate-baseline=phpstan-default-baseline.neon --memory-limit=-1 | |
| .PHONY: static-code-analysis-baseline | |
| static-code-analysis-baseline: vendor ## Generates a baseline for static code analysis with phpstan/phpstan | |
| vendor/bin/phpstan analyze --configuration=phpstan-default.neon.dist --generate-baseline=phpstan-default-baseline.neon --memory-limit=-1 |
🤖 Prompt for AI Agents
In Makefile around lines 10 to 12, the target static-code-analysis-baseline
declares a dependency on check-symfony which is not defined; either remove
check-symfony from the dependency list if the Symfony CLI check is not required,
or add a new check-symfony phony target that verifies the Symfony CLI is
available (e.g., a small recipe that checks symfony --version or exits with a
clear error) and mark it .PHONY so make succeeds; pick one approach and update
the Makefile accordingly.
| if (!str_contains($key, '.')) { | ||
| return $default; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Replace PHP 8.0+ function with PHP 7.4 compatible alternative.
The str_contains() function requires PHP 8.0+, but debian/conf/composer.json specifies PHP >=7.4. This will cause a fatal error on PHP 7.4 systems.
🔎 Proposed fix for PHP 7.4 compatibility
- if (!str_contains($key, '.')) {
+ if (strpos($key, '.') === false) {
return $default;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!str_contains($key, '.')) { | |
| return $default; | |
| } | |
| if (strpos($key, '.') === false) { | |
| return $default; | |
| } |
🤖 Prompt for AI Agents
In src/lib/ZabbixHelper.php around lines 191 to 193, the code uses PHP 8's
str_contains($key, '.') which breaks compatibility with PHP >=7.4; replace this
with a PHP 7.4-safe check using strpos and strict comparison (e.g., if
strpos($key, '.') === false) so the function returns $default when the dot is
not present.
| if (str_contains($param, ' ')) { | ||
| $param = "\"{$param}\""; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Replace PHP 8.0+ function with PHP 7.4 compatible alternative.
Another instance of str_contains() that is incompatible with PHP 7.4.
🔎 Proposed fix for PHP 7.4 compatibility
- if (str_contains($param, ' ')) {
+ if (strpos($param, ' ') !== false) {
$param = "\"{$param}\"";
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (str_contains($param, ' ')) { | |
| $param = "\"{$param}\""; | |
| } | |
| if (strpos($param, ' ') !== false) { | |
| $param = "\"{$param}\""; | |
| } |
🤖 Prompt for AI Agents
In src/lib/ZabbixHelper.php around lines 314-316, replace the PHP 8+ call to
str_contains($param, ' ') with a PHP 7.4-compatible check: use strpos($param, '
') !== false to detect spaces; keep the same behavior of quoting the parameter
when a space is found (i.e., if (strpos($param, ' ') !== false) { $param =
"\"{$param}\""; }). Ensure you don't change surrounding logic or variable types.
Summary by CodeRabbit
Release Notes
New Features
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.