From 61be8e7b1b2e405dd74a952cc0557791c27cb3e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Andr=C3=A9=20dos=20Santos=20Lopes?= Date: Thu, 10 Sep 2026 17:48:12 +0100 Subject: [PATCH] fix appsec int tests fail on frankenphp --- appsec/CMakeLists.txt | 5 +- appsec/cmake/ddtrace.cmake | 2 - appsec/cmake/extension.cmake | 2 - appsec/cmake/patchelf.cmake | 22 ----- appsec/cmake/run_tests.cmake | 15 +++- appsec/cmake/strip_libc.sh | 17 ---- appsec/tests/integration/build.gradle | 88 +++++++++++++------ .../appsec/php/docker/AppSecContainer.groovy | 9 +- 8 files changed, 84 insertions(+), 76 deletions(-) delete mode 100644 appsec/cmake/patchelf.cmake delete mode 100755 appsec/cmake/strip_libc.sh diff --git a/appsec/CMakeLists.txt b/appsec/CMakeLists.txt index 125af0074fa..04cd617e028 100644 --- a/appsec/CMakeLists.txt +++ b/appsec/CMakeLists.txt @@ -35,13 +35,16 @@ endif() option(DD_APPSEC_BUILD_EXTENSION "Whether to builder the extension" ON) option(DD_APPSEC_ENABLE_COVERAGE "Whether to enable coverage calculation" OFF) option(DD_APPSEC_TESTING "Whether to enable testing" ON) +set(DD_APPSEC_TEST_EXTENSION "" CACHE FILEPATH + "Use an existing AppSec extension when running extension tests") +set(DD_APPSEC_TEST_TRACER "" CACHE FILEPATH + "Use an existing tracer extension when running extension tests") option(DD_APPSEC_DDTRACE_ALT "Whether to build appsec with cmake" OFF) option(DD_APPSEC_SSI "Whether to build ddtrace in split configuration (slim ddtrace.so + libdatadog_php.so)" OFF) option(DD_APPSEC_EXTENSION_STATIC_LIBSTDCXX "Whether to link the extension with -static-libstdc++ (not available on macOS)" OFF) add_subdirectory(third_party EXCLUDE_FROM_ALL) -include("cmake/patchelf.cmake") include("cmake/coverage.cmake") include("cmake/boost.cmake") diff --git a/appsec/cmake/ddtrace.cmake b/appsec/cmake/ddtrace.cmake index 541aa2dc8a4..8fbc01f2ca4 100644 --- a/appsec/cmake/ddtrace.cmake +++ b/appsec/cmake/ddtrace.cmake @@ -288,5 +288,3 @@ if(DD_APPSEC_PHP_SIDECAR_MOCKGEN) add_dependencies(ddtrace ddtrace_weaken_php_symbols) endif() add_dependencies(ddtrace ddtrace_exports) - -patch_away_libc(ddtrace) diff --git a/appsec/cmake/extension.cmake b/appsec/cmake/extension.cmake index a3bbdf4fb97..719c176b4ad 100644 --- a/appsec/cmake/extension.cmake +++ b/appsec/cmake/extension.cmake @@ -111,8 +111,6 @@ if(DD_APPSEC_EXTENSION_STATIC_LIBSTDCXX AND NOT APPLE) target_link_options(extension PRIVATE -static-libstdc++) endif() -patch_away_libc(extension) - if(DD_APPSEC_TESTING) maybe_enable_coverage(extension) diff --git a/appsec/cmake/patchelf.cmake b/appsec/cmake/patchelf.cmake deleted file mode 100644 index 4ae68738281..00000000000 --- a/appsec/cmake/patchelf.cmake +++ /dev/null @@ -1,22 +0,0 @@ -function(patch_away_libc target) - if(NOT ${DD_APPSEC_ENABLE_PATCHELF_LIBC}) - return() - endif() - - if (CMAKE_SYSTEM_NAME STREQUAL Darwin) - return() - endif() - - find_program(PATCHELF patchelf) - find_program(READELF readelf) - if(PATCHELF STREQUAL "PATCHELF-NOTFOUND") - message(WARNING "Patchelf not found. Can't build glibc + musl binaries") - else() - if(READELF STREQUAL "READELF-NOTFOUND") - message(WARNING "readelf not found. Can't build glibc + musl binaries") - else() - add_custom_command(TARGET ${target} POST_BUILD - COMMAND ${CMAKE_SOURCE_DIR}/cmake/strip_libc.sh "${PATCHELF}" "${READELF}" $) - endif() - endif() -endfunction() diff --git a/appsec/cmake/run_tests.cmake b/appsec/cmake/run_tests.cmake index 53bd00e4b5b..d75507a1d94 100644 --- a/appsec/cmake/run_tests.cmake +++ b/appsec/cmake/run_tests.cmake @@ -1,5 +1,7 @@ if(DD_APPSEC_DDTRACE_ALT) set(DD_APPSEC_TRACER_EXT_FILE $) +elseif(DD_APPSEC_TEST_TRACER) + set(DD_APPSEC_TRACER_EXT_FILE "${DD_APPSEC_TEST_TRACER}") else() get_filename_component(DD_APPSEC_TRACER_EXT_FILE "${CMAKE_SOURCE_DIR}/../tmp/build_extension/modules/ddtrace.so" REALPATH) get_target_property(_DD_APPSEC_PCRE2_INCLUDE_DIRS @@ -18,6 +20,12 @@ else() WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/../) endif() +if(DD_APPSEC_TEST_EXTENSION) + set(DD_APPSEC_TEST_EXTENSION_FILE "${DD_APPSEC_TEST_EXTENSION}") +else() + set(DD_APPSEC_TEST_EXTENSION_FILE "$") +endif() + add_custom_target(xtest-prepare COMMAND mkdir -p /tmp/appsec-ext-test) @@ -29,7 +37,7 @@ add_custom_target(xtest run-tests-internal.php -n -c ${CMAKE_SOURCE_DIR}/tests/extension/test-php.ini -d "extension_dir=${CMAKE_BINARY_DIR}/extensions" - -d "extension=$" + -d "extension=${DD_APPSEC_TEST_EXTENSION_FILE}" --show-diff ${CMAKE_SOURCE_DIR}/tests/extension/ WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}) @@ -41,6 +49,9 @@ if(DD_APPSEC_ENABLE_COVERAGE) "gcovr -r ${CMAKE_SOURCE_DIR} --html --html-details -s -d -o coverage.html") endif() -add_dependencies(xtest xtest-prepare ddtrace) +add_dependencies(xtest xtest-prepare) +if(TARGET ddtrace) + add_dependencies(xtest ddtrace) +endif() add_subdirectory(tests/mock_helper EXCLUDE_FROM_ALL) diff --git a/appsec/cmake/strip_libc.sh b/appsec/cmake/strip_libc.sh deleted file mode 100755 index 128f566b284..00000000000 --- a/appsec/cmake/strip_libc.sh +++ /dev/null @@ -1,17 +0,0 @@ -#!/bin/sh - -set -e - -main() { - local patchelf=$1 - local readelf=$2 - local target=$3 - - "$patchelf" $( - "$readelf" -d "$target" 2>/dev/null | grep libc\\. | grep NEEDED | \ - awk -F'[][]' '{print "--remove-needed " $2;}' | xargs - ) \ - "$target" -} - -main "$@" diff --git a/appsec/tests/integration/build.gradle b/appsec/tests/integration/build.gradle index 7f28b86d94b..50e8ad3dfe7 100644 --- a/appsec/tests/integration/build.gradle +++ b/appsec/tests/integration/build.gradle @@ -345,6 +345,10 @@ def phpSdkVersion = { String version, String variant -> variant in ['release', 'release-musl'] ? version : "$version-$variant" } +def portableBuildVariant = { String variant -> + variant == 'release-musl' ? 'release' : variant +} + def buildTracerTask = { String version, String variant -> buildRunInDockerTask( baseName: 'buildTracer', @@ -355,7 +359,7 @@ def buildTracerTask = { String version, String variant -> needsAppsec: false, needsBoostCache: false, needsCargoCache: false, - description: 'Build tracer for PHP', + description: 'Build portable tracer extension for PHP', inputs: [ dirs: [ '../../../ext', @@ -798,47 +802,64 @@ def buildLoaderTask = { String version, String variant -> ) } -def buildAppSecTask = { String version, String variant, altBaseTag = null -> +def buildAppSecTask = { String version, String variant -> def buildType = variant.contains('debug') ? 'Debug' : 'RelWithDebInfo' buildRunInDockerTask( baseName: 'buildAppsec', - baseTag: altBaseTag ?: 'php', + imageTag: 'php-buildonly-rust', version: version, variant: variant, needsTracer: false, needsCargoCache: false, - description: 'Build appsec for PHP', + description: 'Build portable appsec extension for PHP', inputs: [ dirs: [ '../../../cmake', + '../../../components-rs', '../../../zend_abstract_interface', '../../cmake', '../../third_party', '../../src'], - files: ['../../CMakeLists.txt'], + files: [ + '../../../VERSION', + '../../CMakeLists.txt'], ], outputs: [ volume: 'php-appsec', files: ['ddappsec.so'], ], + volumes: [ + ("php-appsec-buildonly-${version}-${variant}"): [ + mountPoint: '/project/tmp', + ], + ], command: [ '-e', '-c', """ - cd /appsec - test -f CMakeCache.txt || \\ - cmake -DCMAKE_BUILD_TYPE=$buildType \\ - -DCMAKE_INSTALL_PREFIX=/appsec \\ - -DDD_APPSEC_ENABLE_PATCHELF_LIBC=ON \\ - -DDD_APPSEC_TESTING=ON \\ - -DBOOST_CACHE_PREFIX=/var/boost-cache /project/appsec - make -j extension && \\ - touch ddappsec.so + cd /project/tmp + + export PHP_SDK_VERSION=${phpSdkVersion(version, variant)} + cmake -DCMAKE_BUILD_TYPE=$buildType \\ + -DCMAKE_C_FLAGS=-Wno-static-in-inline \\ + -DCMAKE_INSTALL_PREFIX=/appsec \\ + -DDD_APPSEC_TESTING=OFF \\ + -DBOOST_CACHE_PREFIX=/var/boost-cache \\ + /project/appsec + make -j extension + versionInfo=\$(readelf --version-info ddappsec.so) + if grep -q 'GLIBC_' <<< \"\$versionInfo\"; then + echo 'ddappsec.so has versioned glibc symbols' >&2 + exit 1 + fi + cp ddappsec.so /appsec/ddappsec.so + touch /appsec/ddappsec.so """ ] ) } def runUnitTestsTask = { String phpVersion, String variant -> + def buildType = variant.contains('debug') ? 'Debug' : 'RelWithDebInfo' def env = '' if (project.hasProperty('tests')) { env = "TESTS='${project.getProperty('tests')}' " @@ -848,21 +869,37 @@ def runUnitTestsTask = { String phpVersion, String variant -> baseTag: 'php', version: phpVersion, variant: variant, - needsBoostCache: false, + needsAppsec: false, needsCargoCache: false, - description: 'Build appsec for PHP', + description: 'Run appsec extension tests for PHP', + volumes: [ + ("php-appsec-${phpVersion}-${variant}"): [ + mountPoint: '/appsec-artifact', + readonly: true, + ], + ("php-appsec-unit-${phpVersion}-${variant}"): [ + mountPoint: '/appsec', + ], + ], command: [ '-e', '-c', """ cd /appsec + cmake -DCMAKE_BUILD_TYPE=$buildType \\ + -DCMAKE_INSTALL_PREFIX=/appsec \\ + -DDD_APPSEC_TEST_EXTENSION=/appsec-artifact/ddappsec.so \\ + -DDD_APPSEC_TEST_TRACER=/project/tmp/build_extension/modules/ddtrace.so \\ + -DDD_APPSEC_TESTING=ON \\ + -DBOOST_CACHE_PREFIX=/var/boost-cache \\ + /project/appsec ${env}make -j xtest """ ] ) task.configure { - dependsOn "buildTracer-$phpVersion-$variant" - dependsOn "buildAppsec-$phpVersion-$variant" + dependsOn "buildTracer-$phpVersion-${portableBuildVariant(variant)}" + dependsOn "buildAppsec-$phpVersion-${portableBuildVariant(variant)}" } } @@ -881,8 +918,8 @@ def runMainTask = { String phpVersion, String variant -> systemProperty 'PHP_VERSION', phpVersion systemProperty 'VARIANT', variant - dependsOn "buildTracer-$phpVersion-$variant" - dependsOn "buildAppsec-$phpVersion-$variant" + dependsOn "buildTracer-$phpVersion-${portableBuildVariant(variant)}" + dependsOn "buildAppsec-$phpVersion-${portableBuildVariant(variant)}" } } @@ -892,12 +929,10 @@ def runMainTask = { String phpVersion, String variant -> def isMusl = variant =~ /\bmusl\b/ - buildTracerTask(phpVersion, variant) if (!isMusl) { + buildTracerTask(phpVersion, variant) buildTracerCmakeTask(phpVersion, variant) - } - buildAppSecTask(phpVersion, variant, isMusl ? 'nginx-fpm-php' : null) - if (!isMusl) { + buildAppSecTask(phpVersion, variant) runUnitTestsTask(phpVersion, variant) } if (project.hasProperty('testClass')) { @@ -942,8 +977,9 @@ def runMainTask = { String phpVersion, String variant -> it.systemProperty 'USE_CMAKE', 'true' } - dependsOn project.hasProperty('useCmake') ? "buildTracerCmake-${phpVersion}-${variant}" : "buildTracer-${phpVersion}-${variant}" - dependsOn "buildAppsec-${phpVersion}-${variant}" + String artifactVariant = portableBuildVariant(variant) + dependsOn project.hasProperty('useCmake') ? "buildTracerCmake-${phpVersion}-${artifactVariant}" : "buildTracer-${phpVersion}-${artifactVariant}" + dependsOn "buildAppsec-${phpVersion}-${artifactVariant}" if (phpVersion in ['7.0', '7.1']) { dependsOn downloadComposerOld diff --git a/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/docker/AppSecContainer.groovy b/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/docker/AppSecContainer.groovy index 007a61095d5..2f21e25f16a 100644 --- a/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/docker/AppSecContainer.groovy +++ b/appsec/tests/integration/src/main/groovy/com/datadog/appsec/php/docker/AppSecContainer.groovy @@ -544,8 +544,9 @@ class AppSecContainer> extends GenericContain withFileSystemBind('src/test/resources/gdbinit', '/root/.gdbinit', BindMode.READ_ONLY) withFileSystemBind('src/test/bin/enable_extensions.sh', '/usr/local/bin/enable_extensions.sh', BindMode.READ_ONLY) + String artifactVariant = phpVariant == 'release-musl' ? 'release' : phpVariant if (System.getProperty('SSI')) { - addVolumeMount("php-appsec-$phpVersion-$phpVariant", '/appsec') + addVolumeMount("php-appsec-$phpVersion-$artifactVariant", '/appsec') def ssiTracerVol = System.getProperty('USE_CMAKE') ? "php-tracer-ssi-cmake-$phpVersion-$phpVariant" : "php-tracer-ssi-$phpVersion-$phpVariant" @@ -580,10 +581,10 @@ class AppSecContainer> extends GenericContain cmd.hostConfig.withCapAdd(com.github.dockerjava.api.model.Capability.SYS_PTRACE) } } else { - addVolumeMount("php-appsec-$phpVersion-$phpVariant", '/appsec') + addVolumeMount("php-appsec-$phpVersion-$artifactVariant", '/appsec') def tracerVol = System.getProperty('USE_CMAKE') - ? "php-tracer-cmake-$phpVersion-$phpVariant" - : "php-tracer-$phpVersion-$phpVariant" + ? "php-tracer-cmake-$phpVersion-$artifactVariant" + : "php-tracer-$phpVersion-$artifactVariant" addVolumeMount(tracerVol, '/project/tmp') } withEnv 'RUST_BACKTRACE', '1'