aws-cloudfront icon indicating copy to clipboard operation
aws-cloudfront copied to clipboard

Improved handling of origins and lambda triggers

Open gehan opened this issue 5 years ago • 10 comments

Related to https://github.com/serverless-components/aws-cloudfront/issues/18

  • 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

gehan avatar May 01 '20 10:05 gehan

+1

janoist1 avatar May 01 '20 11:05 janoist1

+1

evilrob666 avatar May 01 '20 11:05 evilrob666

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

gehan avatar May 14 '20 11:05 gehan

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 🚀

krish-dev avatar Jun 08 '20 07:06 krish-dev

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!

gehan avatar Jun 08 '20 08:06 gehan

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>"
      }
    }
  ]
}

krish-dev avatar Jun 08 '20 08:06 krish-dev

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

gehan avatar Jun 08 '20 09:06 gehan

@gehan @danielcondemarin any update on the same.

krish-dev avatar Jun 12 '20 06:06 krish-dev

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

bboure avatar Aug 11 '20 15:08 bboure

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

gehan avatar Aug 19 '20 16:08 gehan