[php] Add symfony framework#7193
Conversation
|
|
|
🎉 All green!🧪 All tests passed 🔄 Datadog auto-retried 1 job - 1 passed on retry 🔗 Commit SHA: 27cdcbf | Docs | Datadog PR Page | Give us feedback! |
estringana
force-pushed
the
estringana/symfony
branch
2 times, most recently
from
421f8d9 to
acc6539
Compare
estringana
force-pushed
the
estringana/add-laravel-weblog
branch
2 times, most recently
from
3c04eef to
8eb08bc
Compare
estringana
force-pushed
the
estringana/symfony
branch
2 times, most recently
from
ac7cac8 to
4e373c0
Compare
estringana
force-pushed
the
estringana/symfony
branch
from
faf23d9 to
27cdcbf
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27cdcbf696
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| tests/appsec/test_automated_payment_events.py: v1.17.0-dev | ||
| tests/appsec/test_automated_payment_events.py: | ||
| - weblog_declaration: | ||
| "*": missing_feature (Stripe endpoints not implemented) |
There was a problem hiding this comment.
For non-symfony7x PHP weblogs this wildcard now marks the entire automated payment suite as missing_feature, replacing the previous file-level v1.17.0-dev activation. The existing PHP apps already expose the Stripe endpoints (utils/build/docker/php/common/rewrite-rules.conf maps them for plain, and utils/build/docker/php/weblogs/laravel11x/routes/web.php defines them for Laravel), so this silently stops exercising payment-event coverage for all existing PHP variants when their tracer version is new enough. Keep the default enabled and override only weblogs that truly lack the endpoints.
Useful? React with 👍 / 👎.
| return new Response('Invalid status', 400, ['Content-Type' => 'text/plain']); | ||
| } | ||
|
|
||
| #[Route('/otel_drop_in_baggage_api_otel', name: 'otel_drop_in_baggage_api_otel', methods: ['GET'])] |
There was a problem hiding this comment.
When WEBLOG_VARIANT=symfony7x runs the integrations scenario, manifests/php.yml leaves tests/integrations/test_otel_drop_in.py::Test_Otel_Drop_In enabled for PHP 8.2-style weblogs, and that test calls /otel_drop_in to instantiate CachedInstrumentation and emit otel.* telemetry. This controller only adds the baggage-specific OTel routes, so /otel_drop_in returns Symfony's 404 and never emits the telemetry the test waits for; add the route or mark symfony7x missing_feature for that test.
Useful? React with 👍 / 👎.
| # Pre-create .env with APP_SECRET and APP_ENV | ||
| RUN mkdir -p /var/www/html && \ | ||
| echo "APP_ENV=prod" > /var/www/html/.env && \ | ||
| php -r "echo 'APP_SECRET=' . bin2hex(random_bytes(16)) . PHP_EOL;" >> /var/www/html/.env && \ |
There was a problem hiding this comment.
This only writes APP_SECRET into .env, but neither bin/console nor public/index.php loads Dotenv, and symfony/dotenv is not required by the new app. Because config/packages/framework.yaml reads %env(APP_SECRET)%, the Apache runtime environment will not contain the secret that Symfony's security/session services need, so login/session-backed requests can fail even though the file exists. Export the value into the container environment or explicitly load Dotenv before booting the kernel.
Useful? React with 👍 / 👎.
| assert self.config_state_2.state == rc.ApplyState.ACKNOWLEDGED | ||
|
|
||
| if context.library not in libs_without_user_id: | ||
| if login_success_includes_usr_id_meta(): |
There was a problem hiding this comment.
For WEBLOG_VARIANT=symfony7x, the new helper returns false, so test_login_event_blocking_auto_id only checks the initial login and remote-config ACKs and never validates that self.r_login_blocked produced the block-user-id attack or a 403. That turns an enabled test into a no-op for this weblog; mark the Symfony case in the manifest or split the expectation instead of gating the core assertions at runtime.
Useful? React with 👍 / 👎.
| return new Response('OK', 200, ['Content-Type' => 'text/plain']); | ||
| } | ||
|
|
||
| #[Route('/endpoint_fallback.php', name: 'endpoint_fallback', methods: ['GET'])] |
There was a problem hiding this comment.
The PHP manifest enables tests/test_resource_renaming.py for PHP weblogs, and those tests call /resource_renaming/... expecting an HTTP endpoint tag. I don't see any matching Symfony route in this new controller, so symfony7x will return 404 for those requests and the endpoint-tag assertions will fail; add the catch-all resource-renaming route or mark this weblog as unsupported for that test.
Useful? React with 👍 / 👎.
1 participant
Motivation
Adds a new symfony7x PHP weblog built with Symfony's idiomatic patterns (security component, form-based login, Doctrine ORM). The app implements the standard system-tests endpoint surface including AppSec, login events, automated payment events, and user/session
tracking.
Updates manifests/php.yml with per-test declarations for symfony7x, marking unsupported scenarios as missing_feature where the weblog or tracer version doesn't yet cover them.
Changes
Workflow
🚀 Once your PR is reviewed and the CI green, you can merge it!
🛟 #apm-shared-testing 🛟
Reviewer checklist
tests/ormanifests/is modified ? I have the approval from R&P teambuild-XXX-imagelabel is present