getlift / lift

Expanding Serverless Framework beyond functions using the AWS CDK
MIT License
920 stars 112 forks source link

Lambda is updated before assets are uploaded #398

Open jaulz opened 4 months ago

jaulz commented 4 months ago

Description

Currently, assets are uploaded after Lambda is updated in Server Side Website mode. This means that there is a short period of time where Lambda references to paths that do not exist yet and result in 404s. Is this a conscious design decision?

How to Reproduce

service: my-app
provider:
    name: aws

functions:
    home:
        handler: home.handler
        events:
            -   httpApi: 'GET /'

constructs:
    website:
        type: server-side-website
        assets:
            '/css/*': public/css
            '/js/*': public/js

plugins:
    - serverless-lift

Additional Information

No response

mnapoli commented 4 months ago

Yes, this was an accepted compromise from the start. Building a zero-downtime deployment would be much more complex unfortunately.

jaulz commented 4 months ago

Oh, too bad. Do you have any ideas how it could be implemented in user land instead?

jaulz commented 4 months ago

@mnapoli I wonder if we could make the path that is about to be invalidated configurable: https://github.com/getlift/lift/blob/0d56286371e98fa921a88635a0b98cd3404eb488/src/classes/aws.ts#L45

In my case it's actually just the manifest.json which needs to be invalidated and the rest can live as long as they are cached.

mnapoli commented 4 months ago

Ah, we're trying as hard as possible to avoid adding new options. I'm not sure if there are good alternatives for this problem 🤔

jaulz commented 4 months ago

I'm running a server side website (Laravel) with built assets (Vite). Since assets will be uploaded afterwards, the new app version must

  1. use/refer to old assets as long as the new ones are not available
  2. and old assets of course must also be still available as long as the app refers to these.

By tweaking the Vite asset resolver in Laravel and using Redis I am at least able to still refer to old assets. However, CloudFormation currently invalidates all files and hence old files are immediately not available anymore.

I don't think that it will be possible to find a generic solution for the server side website case because the asset resolver of the app is key (which can obviously not be changed by this library). However, I think by configuring the paths that are about to be invalidated we could at least have a solution that not only works for this scenario but also for other scenarios where manifest files are used (SPA).

mnapoli commented 4 months ago

Now that I think about it, i don't think the invalidation is the problem: there is no guarantee that older files are in cache (everywhere on the planet).

To solve this 100%, the files must exist in S3 while they are still used.

To be clear what I'm saying is that I don't think changing the CloudFront invalidation is the right solution, does that make sense?

jaulz commented 4 months ago

Yep, that totally makes sense 😊 I just thought that this would be a compromise to keep the current S3 sync behaviour. In that case we simply need to be able to define which folders should be synced (i.e. old files should be deleted) and which folder shouldn't?

mnapoli commented 4 months ago

How about https://github.com/getlift/lift/issues/369#issuecomment-1781322473? That would avoid any new option.