Update Symfony documentation #923

ker0x · 2021-05-04T09:46:16Z

Due to recent changes in the Symfony Bridge package and my personal experience using Bref with Symfony, I have updated the documentation accordingly.

All feedbacks and improvments are welcome 😃!

When this PR will be reviewed and approved, a new released of the the symfony-bridge should be made because currently the bridge don't require bref to be install.

t-richard

@ker0x thank you very much for this.

TBH, I was planning to do it this week 😄

I think the bridge is now stable enough to make this move and encourage new comers to use it

@mnapoli it would be nice to tag a new version of the bridge to get the latest fixes from brefphp/symfony-bridge#37, brefphp/symfony-bridge#39 and brefphp/symfony-bridge#41 without having to require dev-master 🙂

Here is a first round of review !

docs/frameworks/symfony.md

t-richard

Here are some other small things I noticed 🙂

docs/frameworks/symfony.md

…the recipe, fix wording

t-richard

Just a minor comment but all seem good to me ✔️

Thanks @ker0x

docs/frameworks/symfony.md

Co-authored-by: Thibault RICHARD <[email protected]>

t-richard · 2021-06-01T17:05:40Z

@mnapoli @deleugpn is there something preventing the merge of this PR ? Except a new release in bref/symfony-bridge ?

Let me know if I can be of any help in moving this forward (cc @ker0x)

deleugpn · 2021-06-02T09:33:04Z

My knowledge in Symfony is completely null and since this PR is a documentation-only, I can't be much of help. It would be nice if more Symfony users could weight in on these changes.

docs/frameworks/symfony.md

t-richard · 2021-06-02T09:54:46Z

@deleugpn I understand that you are not knowledgable with Symfony.

I'll try to ask on Slack if Symfony users could check this PR and review/approve it.

Nyholm

Thank you.

I've added some thoughts. Feel free to skip, ignore or add to a follow-up PR.

Nyholm · 2021-06-15T06:09:50Z

docs/frameworks/symfony.md

-    # trust "X-Forwarded-*" headers coming from API Gateway
-    trusted_headers: ['x-forwarded-for', 'x-forwarded-proto', 'x-forwarded-port']
+  # trust the remote address because API Gateway has no fixed IP or CIDR range that we can target
+  trusted_proxies: '127.0.0.1,REMOTE_ADDR'


Isn't $_SERVER['REMOTE_ADDR'] always 127.0.0.1 on Lambda?

I think it is https://github.com/brefphp/bref/blob/master/src/Event/Http/FpmHandler.php#L184

So we could change it to only keep the IP

Suggested change

trusted_proxies: '127.0.0.1,REMOTE_ADDR'

trusted_proxies: '127.0.0.1'

Nyholm · 2021-06-15T06:11:59Z

docs/frameworks/symfony.md

-to whitelist [every CloudFront IP](https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/LocationsOfEdgeServers.html)
+**When using CloudFront** on top of API Gateway, you will not be able to retrieve the client IP address, and you will instead get one of Cloudfront's IP when
+calling `Request::getClientIp()`. If you really need this, you will need to
+whitelist [every CloudFront IP](https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/LocationsOfEdgeServers.html)


I know you did not change this. But this is not really true.

You can just update your index.php file with:

// Get user IP: if (isset($_SERVER['LAMBDA_REQUEST_CONTEXT'])) { $context = json_decode($_SERVER['LAMBDA_REQUEST_CONTEXT'], true); $_SERVER['HTTP_X_FORWARDED_FOR'] = $context['identity']['sourceIp'] ?? ''; }

There was some discussions about this on Slack and it seems to work fine but we weren't able to say if this is working in all scenarios. And I wasn't able to find AWS docs that states clearly what they put in it.

If you are sure about that, I'm a 💯 to document it 👍

Nyholm · 2021-06-15T06:14:38Z

docs/frameworks/symfony.md


 ```bash
 php bin/console assets:install --env prod
 # if using Webpack Encore, additionally run
 yarn encore production
-aws s3 sync public/ s3://<bucket-name>/ --delete --exclude index.php
+aws s3 sync public/ s3://<bucket-name>/ \
+  --delete \


If you delete stuff, it would mean that there is some downtime for your application. (ie no assets)

True, I think the solution is to sync twice. First without --delete and then with the --delete option.

Or do you have another solution ?

I'll just leave the old files :)

Nyholm · 2021-06-15T06:14:48Z

docs/frameworks/symfony.md

+  --delete \
+  --exclude index.php \
+  --exclude public/build/manifest.json \
+  --exclude public/build/entrypoint.json


Also, one should configure cace control (But only if one use unique filenames, ie hashes)

--cache-control max-age=31449600,immutable

File hasing is enabled by default with Webpack Encore. But I'm not sure it's true for third party bundles providing assets (like EasyAdmin or Sonata for example) moved by bin/console assets:install in bundles.

Nyholm · 2021-06-15T06:15:13Z

docs/frameworks/symfony.md


 ```yaml
-# config/packages/prod/framework.yaml
+# config/packages/prod/assets.yaml
+
 framework:
  assets:
    base_urls: 'https://<bucket-name>.s3.amazonaws.com'


Should we really care about S3? Why not do cloudfront directly?

This is simpler to setup for beginners and if you put Cloudfront in front of your app, then this is not needed I think.

t-richard · 2021-06-15T07:04:07Z

@Nyholm thank you, this is very much appreciated.

I think we can address the trusted proxies IP here, and I'll open a follow-up PR for assets and the source IP thing because they could need more work to get them right.

This would allow us to move this PR forward and merge it.

mnapoli · 2021-06-15T13:19:41Z

Thank you all!

t-richard · 2021-06-15T14:28:16Z

Thank you @mnapoli and @Nyholm for your time on that matter.

And thank you @ker0x for this PR, the contributions on the recipe and the bridge ❤️

We'll be able to build upon that. I'll hopefully come up with some new PRs soon.

Update Symfony documentation

1e22ead

ker0x force-pushed the symfony-doc branch from b100cad to 1e22ead Compare May 4, 2021 09:46

t-richard requested changes May 4, 2021

View reviewed changes

t-richard requested changes May 9, 2021

View reviewed changes

docs/frameworks/symfony.md Outdated Show resolved Hide resolved

docs/frameworks/symfony.md Show resolved Hide resolved

docs/frameworks/symfony.md Show resolved Hide resolved

docs/frameworks/symfony.md Show resolved Hide resolved

docs/frameworks/symfony.md Outdated Show resolved Hide resolved

mnapoli added the documentation label May 11, 2021

ker0x force-pushed the symfony-doc branch 3 times, most recently from 9b62e9a to b4c960d Compare May 14, 2021 10:16

Replace default serverless configuration example with direct link to …

d94225b

…the recipe, fix wording

ker0x force-pushed the symfony-doc branch from b4c960d to d94225b Compare May 14, 2021 10:23

t-richard approved these changes May 15, 2021

View reviewed changes

docs/frameworks/symfony.md Outdated Show resolved Hide resolved

Fix missing space

9763bc0

Co-authored-by: Thibault RICHARD <[email protected]>

t-richard reviewed Jun 2, 2021

View reviewed changes

docs/frameworks/symfony.md Outdated Show resolved Hide resolved

Fix patterns typo

913ed9d

deleugpn added the help wanted label Jun 4, 2021

Nyholm reviewed Jun 15, 2021

View reviewed changes

mnapoli merged commit 913ed9d into brefphp:master Jun 15, 2021

deleugpn removed the help wanted label Aug 6, 2021

t-richard mentioned this pull request Sep 8, 2021

[Docs] Document server-side construct in Symfony docs #1014

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Symfony documentation #923

Update Symfony documentation #923

ker0x commented May 4, 2021

t-richard left a comment

t-richard left a comment

t-richard left a comment

t-richard commented Jun 1, 2021

deleugpn commented Jun 2, 2021

t-richard commented Jun 2, 2021

Nyholm left a comment

Nyholm Jun 15, 2021

t-richard Jun 15, 2021

Nyholm Jun 15, 2021

Nyholm Jun 15, 2021

t-richard Jun 15, 2021

Nyholm Jun 15, 2021

t-richard Jun 15, 2021

Nyholm Jun 15, 2021

Nyholm Jun 15, 2021

t-richard Jun 15, 2021

Nyholm Jun 15, 2021

t-richard Jun 15, 2021

t-richard commented Jun 15, 2021 •

edited

Loading

mnapoli commented Jun 15, 2021

t-richard commented Jun 15, 2021

	trusted_proxies: '127.0.0.1,REMOTE_ADDR'
	trusted_proxies: '127.0.0.1'

Update Symfony documentation #923

Update Symfony documentation #923

Conversation

ker0x commented May 4, 2021

t-richard left a comment

Choose a reason for hiding this comment

t-richard left a comment

Choose a reason for hiding this comment

t-richard left a comment

Choose a reason for hiding this comment

t-richard commented Jun 1, 2021

deleugpn commented Jun 2, 2021

t-richard commented Jun 2, 2021

Nyholm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

t-richard commented Jun 15, 2021 • edited Loading

mnapoli commented Jun 15, 2021

t-richard commented Jun 15, 2021

t-richard commented Jun 15, 2021 •

edited

Loading