Improved handling of origins and lambda triggers

gehan commented 4 years ago

Handles s3 website urls differently - S3 buckets used to serve websites should use CustomOrigin as per https://docs.aws.amazon.com/cloudfront/latest/APIReference/API_S3OriginConfig.html
Allow multiple origins from same s3 bucket - Sets origin to ID to include path
Allow setting of origin protocol policy option
Handles IncludeBody for lambda triggers correctly - ie allows option and requires set to false for response triggers

janoist1 commented 4 years ago

+1

evilrob666 commented 4 years ago

+1

gehan commented 4 years ago

Will update. Changes should not break anything but will confirm.

krish-dev commented 4 years ago

Wonderful PR, I also encountered with exactly same issue like

InvalidArgument: The parameter origin ID must be unique.

@gehan Can you please 🙏 confirm that changes should not break anything so that @danielcondemarin can take this further 🚀

gehan commented 4 years ago

Wonderful PR, I also encountered with exactly same issue like
InvalidArgument: The parameter origin ID must be unique.
@gehan Can you please 🙏 confirm that changes should not break anything so that @danielcondemarin can take this further 🚀

What config are you using to get this error? I'll update accordingly!

krish-dev commented 4 years ago

What config are you using to get this error? I'll update accordingly!

Hi, @gehan I think you already fix the issue in this PR.

Actually, I have 2 origins with the same domain but with different paths like this configuration.

{
    "defaults": {...},
    "origins": [
        {
            "url": "http://xxx-pwa-prod.s3.amazonaws.com",
            "private": true,
            "pathPatterns": {
                "_next/*": {
                    "ttl": 86400
                },
                "static/*": {
                    "ttl": 86400
                }
            }
        },
        {
            "url": "http://xxx-pwa-prod.s3.amazonaws.com/_next/static",
            "private": true,
            "pathPatterns": {
                "service-worker.js": {
                    "ttl": 0
                }
            }
        }
    ]
}

and its end-up with same Id like I just added a console.log(JSON.stringify(Origins)); in this line to get this result.

{
  "Quantity": 2,
  "Items": [
    {
      "Id": "xxx-pwa-prod",
      "DomainName": "xxx-pwa-prod.s3.amazonaws.com",
      "CustomHeaders": {
        "Quantity": 0,
        "Items": []
      },
      "OriginPath": "",
      "S3OriginConfig": {
        "OriginAccessIdentity": "origin-access-identity/cloudfront/<identity>"
      }
    },
    {
      "Id": "xxx-pwa-prod",
      "DomainName": "xxx-pwa-prod.s3.amazonaws.com",
      "CustomHeaders": {
        "Quantity": 0,
        "Items": []
      },
      "OriginPath": "/_next/static",
      "S3OriginConfig": {
        "OriginAccessIdentity": "origin-access-identity/cloudfront/<identity>"
      }
    }
  ]
}

gehan commented 4 years ago

@krish-dev oh yes sorry I see! misunderstood. i'll check my PR for breaking changes 👍

krish-dev commented 4 years ago

@gehan @danielcondemarin any update on the same.

bboure commented 4 years ago

Any update on this? I am getting errors due to IncludeBody being true for on-supported events. This PR would fix the issue. Thanks

gehan commented 4 years ago

LGTM aside from just a few things as per my comments 🚀

It also doesn't seem like the change would be breaking, could you confirm?

Long delay here! Updated as per comments and is non-breaking

serverless-components / aws-cloudfront

Improved handling of origins and lambda triggers #19