-
Notifications
You must be signed in to change notification settings - Fork 35
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
feat: Add support for specifying if it should sign #62
Conversation
This adds a new parameter to specify if a closure should be signed or not.
Hi Olivier, I wanted to bring to your attention that the code in this package is very sensitive and while I understand your need for this feature, the fact that you are the only one who has requested it leads me to believe that we should not merge it. Is it possible to achieve your goal by archiving something like this, perhaps by overriding certain elements in the framework and extending the classes in this package? |
Hey @nunomaduro :) Yes, I can see some possible other solutions.
In regards to being the only one requesting this, I think that is true, however I am not the first one to experience it. After thinking about it my recommendations would be my first proposed solution here with a new class, as that seems like a very safe approach to this. |
Adds a new class for serializing closures without signing the closure.
I have created a secondary commit to showcase my other approach which in my opinion seems like a safe and backwards compatible way of solving this :) |
@nunomaduro please mark as ready for review when this is ready. |
Has this been forgotten? 👀 |
@olivernybroe This appears to be an unusual scenario, you can utilize the class you suggested on your end to resolve it. |
@nunomaduro I think it is quite common for people to wanna cache their routes while building the docker image, not after build 👀 But we will just enforce no route closures as that in theory should also fix this 👍 Don't wanna override |
This adds a new parameter to specify if a closure should be signed or not.
Why?
When caching routes in the Laravel/framework, they are currently being cached signed. This means if you cache your routes and then set/change your
APP_KEY
your route caches are invalid.This option allows the route caching logic to disable signing when caching the routes, which then means
APP_KEY
can be changed without the route caching being invalid.Usecase
When building a docker image it is currently impossible to cache your routes while building the image (if any routes are closures) as the
APP_KEY
should not be exposed to the build steps of the docker image.Caching the routes while building the image has a lot of advantages, as each instance does not have to cache it themselves. This can also prevent potential crashes of running out of storage when using k8s, as you cannot guarantee the amount of storage available outside of the image size unless you start mounting volumes etc.
A PR will be created to
laravel/framework
for the needed changes there to utilize this new option.laravel/framework#45527