getlift / lift

Expanding Serverless Framework beyond functions using the AWS CDK
MIT License
917 stars 113 forks source link

Server-side website: ability to define static HTML pages #94

Open piolet opened 3 years ago

piolet commented 3 years ago

In Server-side-website, we can configure the error page with :

constructs:
    website:
        # ...
        errorPage: error500.html

But we can't set the CDN's default page, like this :

constructs:
    website:
        # ...
        defaultPage: index.html

In my case, for example, index.html is added in my assets (it's a fully static page), but if i go to the page https://mydomain.com i redirect to the lambda mangement. If i go directly to https://mydomain.com/index.html, it's ok, but not really top.

What do you think ?

t-richard commented 3 years ago

The server-side website is targeted at frameworks like Ruby on Rails, Symfony/Laravel, Django, Express, etc where the HTML rendering would be done server-side (with erb, twig/blade, DTL/Jinja, ejs/nunjucks, etc).

What you are willing to do looks like the static website construct, to run a static website generated by Jekyll or Hugo for example. Or Single page apps built with Vue.js or React.

Can you tell us a bit more about the use case here ?

piolet commented 3 years ago

In my case, some page are static (full html) and some other are dynamic (php rendering). All is ok, except for home page, my index.html. It's not a SPA.

t-richard commented 3 years ago

This is somewhat out of scope of this construct (at least for now).

Maybe you could trick the construct by doing something like

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

or

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

This is from the top of my head, not sure it works but might be worth a try.

piolet commented 3 years ago

with the first, I arrive on the list of the bucket :

<ListBucketResult xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
<Name>websiteassets2a73bb69-mhl8dw28rjm3</Name>
<Prefix/>
<Marker/>
<MaxKeys>1000</MaxKeys>
<IsTruncated>false</IsTruncated>
<Contents>
<Key>assets/css/styles.css</Key>
...

for the second, it's an error : An error occurred: websiteCDN060D946D - Resource handler returned message: "Invalid request provided: The parameter globPattern is too small or empty.

"flûte"

mnapoli commented 3 years ago

👍 this indeed isn't outside of the scope the construct in its current state. (because it's a static page, not a server-rendered page)

Too bad the workarounds mentioned by @t-richard do not work :/ (that would have been an easy way to work around the limitation)

I'm curious to understand this use case a bit more: why can't you return/render the page (or event serve the HTML file) from Lambda?

piolet commented 3 years ago

@mnapoli for me, maybe I'm wrong, but Lambda = Dynamic (need to compute things, to retrieve information from database, etc.). And anything static goes into an S3 Bucket.

But in my case, my index.html page is completely static. No dynamic information, only links to other pages of the site which can be dynamic or static (depending on the page).

I thought that this constructor was completely appropriate and could handle such a structure.

Especially since it manages it well at 99%, because if my index.html was dynamic (and thus managed by lambda) everything would work perfectly ;)

And to answer the question completely, my index.html is static to try to get better performances on the page loading time (and thus to have a gain in SEO level, the devil is in the details 😈 )

mnapoli commented 3 years ago

maybe I'm wrong, but Lambda = Dynamic (need to compute things, to retrieve information from database, etc.). And anything static goes into an S3 Bucket.

That's definitely correct, but we're talking about the "Server-side website". It's made for websites that are served server-side (Lambda), nothing static, which is not 100% the case here, and that's why it's not working well.

I would say: let's keep this issue open and gather feedback. Maybe your use case is more common than we thought. If that's the case, then we could expand the scope of the server-side-website construct.

piolet commented 3 years ago

okay, i hope ;)

I hope I won't have to create a dynamic index.html just to "hack" this.

Because then, it's just in the CDN configuration that it happens.

If we define a parameter in the constructor (defaultStaticPage for example), we could fill in the default page of the CDN (the way we fill in the error page).

Well, I say this as if I knew all the subtleties of this constructor ;) (but it is not the case ;) )

t-richard commented 3 years ago

First let me clear things up : I'm not trying to derail, what you're trying to accomplish seems like a thoughtful way to do (on several aspects like performance, costs, caching, etc). I also understand that making this page dynamic just to please the tool is frustrating.

But at the same time, it seems to me like an edge case. I don't have absolute knowledge but I've never seen nor needed this kind of things on my projects.

The closest thing that comes to mind is Netlify with Netlify functions and redirect rules but this is way more complex to achieve than what we are doing here and would probably involve compute at the edge to do so. Maybe you could do something with Cloudfront functions here :thinking:

I'm also not sure that the fallback page would solve the issue here. Because the default page would be served if nothing else matched, but because the API Gateway is the default behaviour of the Cloudfront distribution. This means the default index.html would never be used.

If you manage to do what you are trying to do by updating the distribution in the AWS console or using Cloudfront functions, I'd be happy to discuss it again and see how this could fit in the construct :slightly_smiling_face:

Please keep us updated if you have some ideas :pray:

piolet commented 3 years ago

no worries I fully understand that you can accede to all requests and treat all particular cases. You are already doing a monstrous job to simplify our task.

I just tested a change in the AWS console that works. I simply went to the CloudFront configuration and set the root object to: index.html, plain and simple. And it does the job, at least, it does what I want ;)

After that, I don't know if on your side, it's something that can be passed as a constructor parameter to adjust CDN behavior

t-richard commented 3 years ago

But that will only work for the homepage /, not other static routes right ? Or are other static routes fine with what we have now ?

piolet commented 3 years ago

it works for everything

piolet commented 3 years ago

I created a PR #95 that goes in this direction, hoping that it makes sense for you

piolet commented 3 years ago

hey hey, everything is not so "ok" ;)

by setting my root object, it's ok for the main index.html, i.e. for https://my-domain.com it's well redirected to https://my-domain.com/index.html

but for a subfolder, it's not good: https://my-domain.com/subfolder/ does not redirect to https://my-domain.com/subfolder/index.html

a priori, I have to use a Lambda@edge (like in this example : https://aws.amazon.com/fr/blogs/compute/implementing-default-directory-indexes-in-amazon-s3-backed-amazon-cloudfront-origins-using-lambdaedge/), but it doesn't work yet on my side

piolet commented 3 years ago

an other way, more simple, don't use directly the s3 for origin in cloud front, but his endpoint (for this, the S3 must be configured as static web host).

to be discuss

yhortuk commented 2 years ago

Are there any plans to add support for the option to deploy CloudFront on top of the s3 website, for nested folder support? I'm just wondering if it makes sense to send a pull request for this.

My use case is to deploy a jigsaw static website with a pretty URL enabled. It is using the folders to make pretty URLs. With current static website deployment subfolders are not working, since it is not supported by CloudFront. But is working fine with the s3 static website being served behind the CloudFront distribution.

I'm referring to the solution mentioned here "Using a website endpoint as the origin, with access restricted by a Referer header" https://aws.amazon.com/premiumsupport/knowledge-center/cloudfront-serve-static-website/.

Probably we can add a new config option to the current config to expose s3 as static website limited to some secret referer generated during deployment.

piolet commented 2 years ago

This my solution to avoid this limitation, for the moment. A patch with node module "patch-package" :

diff --git a/node_modules/serverless-lift/dist/src/constructs/aws/ServerSideWebsite.js b/node_modules/serverless-lift/dist/src/constructs/aws/ServerSideWebsite.js
index d9aedaf..fef49f2 100644
--- a/node_modules/serverless-lift/dist/src/constructs/aws/ServerSideWebsite.js
+++ b/node_modules/serverless-lift/dist/src/constructs/aws/ServerSideWebsite.js
@@ -65,7 +65,8 @@ const SCHEMA = {
     },
     redirectToMainDomain: { type: "boolean" },
     certificate: { type: "string" },
-    forwardedHeaders: { type: "array", items: { type: "string" } }
+    forwardedHeaders: { type: "array", items: { type: "string" } },
+    defaultRootObject: { type: "string" }
   },
   additionalProperties: false
 };
@@ -82,7 +83,9 @@ const _ServerSideWebsite = class extends import_abstracts.AwsConstruct {
       throw new Error(`Invalid configuration in 'constructs.${id}.errorPage': the custom error page must be a static HTML file. '${configuration.errorPage}' does not end with '.html'.`);
     }
     const bucket = new import_aws_s3.Bucket(this, "Assets", {
-      removalPolicy: import_core.RemovalPolicy.DESTROY
+      publicReadAccess: true,
+      removalPolicy: import_core.RemovalPolicy.DESTROY,
+      websiteIndexDocument: "index.html"
     });
     const cloudFrontOAI = new import_aws_cloudfront.OriginAccessIdentity(this, "OriginAccessIdentity", {
       comment: `Identity that represents CloudFront for the ${id} website.`
@@ -101,13 +104,15 @@ const _ServerSideWebsite = class extends import_abstracts.AwsConstruct {
       defaultTtl: import_core.Duration.seconds(0),
       headerBehavior: import_aws_cloudfront.CacheHeaderBehavior.allowList("Authorization")
     });
-    const s3Origin = new import_aws_cloudfront_origins.S3Origin(bucket, {
-      originAccessIdentity: cloudFrontOAI
-    });
+    const bucketOrigin = new import_aws_cloudfront_origins.HttpOrigin(bucket.bucketWebsiteDomainName, {
+      protocolPolicy: import_aws_cloudfront.OriginProtocolPolicy.HTTP_ONLY
+    })
     const apiId = configuration.apiGateway === "rest" ? this.provider.naming.getRestApiLogicalId() : this.provider.naming.getHttpApiLogicalId();
     const apiGatewayDomain = import_core.Fn.join(".", [import_core.Fn.ref(apiId), `execute-api.${this.provider.region}.amazonaws.com`]);
     this.domains = configuration.domain !== void 0 ? (0, import_lodash.flatten)([configuration.domain]) : void 0;
     const certificate = configuration.certificate !== void 0 ? acm.Certificate.fromCertificateArn(this, "Certificate", configuration.certificate) : void 0;
+    this.bucketName = bucket.bucketName;
+    const functionAssociations = this.createRequestFunction();
     this.distribution = new import_aws_cloudfront.Distribution(this, "CDN", {
       comment: `${provider.stackName} ${id} website CDN`,
       defaultBehavior: {
@@ -120,16 +125,17 @@ const _ServerSideWebsite = class extends import_abstracts.AwsConstruct {
         originRequestPolicy: backendOriginPolicy,
         functionAssociations: [
           {
-            function: this.createRequestFunction(),
+            function: functionAssociations,
             eventType: import_aws_cloudfront.FunctionEventType.VIEWER_REQUEST
           }
         ]
       },
-      additionalBehaviors: this.createCacheBehaviors(s3Origin),
+      additionalBehaviors: this.createCacheBehaviors(bucketOrigin, functionAssociations),
       errorResponses: this.createErrorResponses(),
       httpVersion: import_aws_cloudfront.HttpVersion.HTTP2,
       certificate,
-      domainNames: this.domains
+      domainNames: this.domains,
+      defaultRootObject: configuration.defaultRootObject
     });
     this.bucketNameOutput = new import_core.CfnOutput(this, "AssetsBucketName", {
       description: "Name of the bucket that stores the website assets.",
@@ -155,6 +161,7 @@ const _ServerSideWebsite = class extends import_abstracts.AwsConstruct {
   outputs() {
     return {
       url: () => this.getUrl(),
+      bucketName: () => this.getBucketName(),
       cname: () => this.getCName()
     };
   }
@@ -163,7 +170,8 @@ const _ServerSideWebsite = class extends import_abstracts.AwsConstruct {
     const domain = (_a = this.getMainCustomDomain()) != null ? _a : this.distribution.distributionDomainName;
     return {
       url: import_core.Fn.join("", ["https://", domain]),
-      cname: this.distribution.distributionDomainName
+      cname: this.distribution.distributionDomainName,
+      bucketName: this.bucketName
     };
   }
   async postDeploy() {
@@ -263,14 +271,20 @@ const _ServerSideWebsite = class extends import_abstracts.AwsConstruct {
     }
     return import_aws_cloudfront.OriginRequestHeaderBehavior.allowList("Accept", "Accept-Language", "Content-Type", "Origin", "Referer", "User-Agent", "X-Requested-With", "X-Forwarded-Host");
   }
-  createCacheBehaviors(s3Origin) {
+  createCacheBehaviors(s3Origin, functionAssociations) {
     const behaviors = {};
     for (const pattern of Object.keys(this.getAssetPatterns())) {
       behaviors[pattern] = {
         origin: s3Origin,
         allowedMethods: import_aws_cloudfront.AllowedMethods.ALLOW_GET_HEAD_OPTIONS,
         cachePolicy: import_aws_cloudfront.CachePolicy.CACHING_OPTIMIZED,
-        viewerProtocolPolicy: import_aws_cloudfront.ViewerProtocolPolicy.REDIRECT_TO_HTTPS
+        viewerProtocolPolicy: import_aws_cloudfront.ViewerProtocolPolicy.REDIRECT_TO_HTTPS,
+        functionAssociations: [
+          {
+            function: functionAssociations,
+            eventType: import_aws_cloudfront.FunctionEventType.VIEWER_REQUEST
+          }
+        ]
       };
     }
     return behaviors;

With this kind of construct, this is ok. But an option in the construct would indeed be appreciated ;)

piolet commented 2 years ago

Oups, my patch failed with v.1.12 :(

jaulz commented 1 year ago

@piolet did you find a solution? This is exactly what I am stumbling about right now...

piolet commented 1 year ago

@piolet did you find a solution? This is exactly what I am stumbling about right now...

Yes, I stayed in an older version of Lift with the patch above unfortunately.

jaulz commented 1 year ago

Too bad. I am dealing with exactly the same scenario and think I will got for a simple redirect as a workaround.

jaulz commented 1 year ago

For those who are looking for a version where you can define the defaultRootObject:

diff --git a/dist/src/constructs/aws/ServerSideWebsite.js b/dist/src/constructs/aws/ServerSideWebsite.js
index 915725b5e8d8f111d439033dd4d010639034899d..4c4a30533d05c8314f50037138ac7f382d28ac4e 100644
--- a/dist/src/constructs/aws/ServerSideWebsite.js
+++ b/dist/src/constructs/aws/ServerSideWebsite.js
@@ -72,7 +72,8 @@ const SCHEMA = {
     },
     redirectToMainDomain: { type: "boolean" },
     certificate: { type: "string" },
-    forwardedHeaders: { type: "array", items: { type: "string" } }
+    forwardedHeaders: { type: "array", items: { type: "string" } },
+    defaultRootObject: { type: "string" }
   },
   additionalProperties: false
 };
@@ -136,7 +137,8 @@ const _ServerSideWebsite = class extends import_abstracts.AwsConstruct {
       // Enable http2 & http3 transfer for better performances
       httpVersion: import_aws_cloudfront.HttpVersion.HTTP2_AND_3,
       certificate,
-      domainNames: this.domains
+      domainNames: this.domains,
+      defaultRootObject: configuration.defaultRootObject
     });
     this.bucketNameOutput = new import_aws_cdk_lib.CfnOutput(this, "AssetsBucketName", {
       description: "Name of the bucket that stores the website assets.",

@mnapoli @t-richard would you consider merging this at all? My use case is that the main route is a SPA (which is built via Next) and I still have a couple of other dynamic routes. Thanks for your feedback ✌️

piolet commented 1 year ago

This solution is it ok for another subpage ? (not only main page) If you go on https://yourdomain.com/a-page/ directly with browser (not by action in your SPA) is it ok ?

jaulz commented 1 year ago

@piolet no, I don't need such complex behaviour. It's just the root object that matters for me.

jaulz commented 1 year ago

@piolet by the way, maybe this is interesting for you: https://stackoverflow.com/a/69157535

mnapoli commented 1 year ago

@jaulz it seems it would only cover your scenario, right? So while the diff is super simple, I doesn't seem to address most cases described here, correct?

jaulz commented 1 year ago

@mnapoli actually, I think my suggested change exactly covers the original request and the typical use case if you have a SPA that still has server-side rendered routes (e.g. OAuth callbacks). Only later @piolet mentioned that it would also be beneficial for sub folders but I think that's an even more specific request which requires a more complex solution and thus should be picked up separately?