Skip to content
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

Added information in regards to DFS and static files #150

Open
wants to merge 2 commits into
base: 1.0
Choose a base branch
from

Conversation

mateuszbieniek
Copy link
Contributor

@mateuszbieniek mateuszbieniek commented Jun 18, 2021

Question Answer
Type Improvement
Target version 1.0
BC breaks no
Doc needed yes

If the project is set to use DFS to serve static files, the Response's Vary header will always be set to Cookie, Authorization due to the missing X-User-Hash header inside Request coming from Varnish.

@mateuszbieniek mateuszbieniek added Improvement Doc needed The changes require some documentation labels Jun 18, 2021
@lserwatka
Copy link
Member

@ezsystems/documentation-team for review.

Copy link
Member

@vidarl vidarl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proposal to remove the given if will likely fix the issue yes ( not tested on my part though).
However, we go for this change, the fix needs to be added to Fastly's vcl too..

It must also be documented better ( in the docs for setting of dfs cluster )

Also, in regards to Fastly, we (or actually Platform.sh) don't want clients to run custom vcls, that means that we need to provide two vcls (for instance fastly/ez_main.vcl and fastly/ez_main_dfs.vcl)

However, it would also be possible to autodetect from the vcl if backend is runnning in dfs or not:

  • Create a controller that returns whatever the dfs handler is enabled or not
  • in vcl, call that controller in the same way already do with invalidate token and check with that response if we should do the return (hash) or not

As a last thought, would it be so bad to simply remove that return (hash) to everyone? yes, there will then always be a restart for fetching X-User-Context-Hash, also when fetching assets. But it would anyway be cached and overhead should be minimal.

@mateuszbieniek
Copy link
Contributor Author

Hi @vidarl - Please take a second look - I've probably updated the text when you were writing your review.
But I agree that this might not be the best solution - probably we should introduce a second file for the dfs setup and take advantage of the fact, that User context should be applied only for:

/var/([^/]+/)?storage/images(-versioned)?/

👍

@vidarl
Copy link
Member

vidarl commented Jun 18, 2021

I didn't see that last commit while doing the review, correct.
And you are right, it is quite different if a file is located in assets/ or var/
But I think I would rater say skip User Context for any file in assets/, and get User Context for anything else.

For instance, a .pdf file would never be located in (....)/storage/images(...), but we still would like to get User Context for it

@alongosz
Copy link
Member

@vidarl @mateuszbieniek this one looks like a "low hanging fruit" mergabe without QA as it applies to code comment only. What is the status here?

I see that this applies to eZ Platform 2.5, which is now past EOM, so it would require a rebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Doc needed The changes require some documentation Improvement
Development

Successfully merging this pull request may close these issues.

4 participants