Skip to content
This repository was archived by the owner on Jan 28, 2025. It is now read-only.

Add support for custom headers in custom origins#1448

Merged
dphang merged 7 commits into
serverless-nextjs:masterfrom
Xaroth:origin-with-custom-headers
Jul 27, 2021
Merged

Add support for custom headers in custom origins#1448
dphang merged 7 commits into
serverless-nextjs:masterfrom
Xaroth:origin-with-custom-headers

Conversation

@Xaroth

@Xaroth Xaroth commented Jul 26, 2021

Copy link
Copy Markdown
Contributor

This mostly mimics serverless-components/aws-cloudfront#20 , but then converted to ts.

This adds the 'headers' option for origins, which allows one to set custom headers to be set with these origin requests.

@dphang

dphang commented Jul 27, 2021

Copy link
Copy Markdown
Collaborator

Thanks, can you just make sure you are formatting the file according to prettier/eslint rules, so the codebase is consistent in style? We have git hooks via husky for that. Please ensure you are running them. Looks like the codacy analysis failed because of those?

EDIT: also it looks like the tests are failing as well. Please fix and I will approve and merge.

@Xaroth

Xaroth commented Jul 27, 2021

Copy link
Copy Markdown
Contributor Author

Will work on this when I get into the office. I'm not very familiar with the test suite being used (I normally don't develop much js/ts), so this might take me a bit 🙂

@Xaroth

Xaroth commented Jul 27, 2021

Copy link
Copy Markdown
Contributor Author

@dphang Both points should be fixed. Ironically, the linting issues I already had staged, just not committed, which was a bad on my part. The tests failing was a failure at my end when converting it to ts.

@Xaroth

Xaroth commented Jul 27, 2021

Copy link
Copy Markdown
Contributor Author

Hm, the test I see failing seems to be related to the fix you just merged in master, so is not related to this patch.

@dphang

dphang commented Jul 27, 2021

Copy link
Copy Markdown
Collaborator

Hm, the test I see failing seems to be related to the fix you just merged in master, so is not related to this patch.

Yup, I was trying to upgrade a dependency but it failed the build, will revert the change.

Ran e2e tests: https://github.com/serverless-nextjs/serverless-next.js/actions/runs/1070362351

@slsnextbot

slsnextbot commented Jul 27, 2021

Copy link
Copy Markdown
Collaborator

Handler Size Report

No changes to handler sizes.

Base Handler Sizes (kB) (commit bcc9f73)

{
    "Lambda@Edge": {
        "Default Lambda": {
            "Standard": 1226,
            "Minified": 432
        },
        "API Lambda": {
            "Standard": 86,
            "Minified": 34
        },
        "Image Lambda": {
            "Standard": 894,
            "Minified": 354
        },
        "Regeneration Lambda": {
            "Standard": 620,
            "Minified": 220
        }
    }
}

New Handler Sizes (kB) (commit b3c656c)

{
    "Lambda@Edge": {
        "Default Lambda": {
            "Standard": 1226,
            "Minified": 432
        },
        "API Lambda": {
            "Standard": 86,
            "Minified": 34
        },
        "Image Lambda": {
            "Standard": 894,
            "Minified": 354
        },
        "Regeneration Lambda": {
            "Standard": 620,
            "Minified": 220
        }
    }
}

@codecov

codecov Bot commented Jul 27, 2021

Copy link
Copy Markdown

Codecov Report

Merging #1448 (09065d3) into master (bcc9f73) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 09065d3 differs from pull request most recent head b3c656c. Consider uploading reports for the commit b3c656c to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1448      +/-   ##
==========================================
+ Coverage   84.14%   84.15%   +0.01%     
==========================================
  Files          96       96              
  Lines        3367     3371       +4     
  Branches      995      996       +1     
==========================================
+ Hits         2833     2837       +4     
  Misses        474      474              
  Partials       60       60              
Impacted Files Coverage Δ
...s-components/aws-cloudfront/src/getOriginConfig.ts 95.83% <100.00%> (+0.83%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bcc9f73...b3c656c. Read the comment docs.

@Xaroth

Xaroth commented Jul 27, 2021

Copy link
Copy Markdown
Contributor Author

I see the e2e runs generated some errors, but I don't think those errors are related to the code, more to the service being used for that test.

@dphang

dphang commented Jul 27, 2021

Copy link
Copy Markdown
Collaborator

I see the e2e runs generated some errors, but I don't think those errors are related to the code, more to the service being used for that test.

Yea, it's because it was hitting the homepage of https://jsonplaceholder.typicode.com/ for some tests and doing some comparison for rewrites, but because it's HTML and they recently added Cloudflare browser insights, there is some UUID generated that differs on each page load. Fixed it in another PR.

@Xaroth

Xaroth commented Jul 27, 2021

Copy link
Copy Markdown
Contributor Author

Ah, that does explain a lot then, yeah 🙂

@dphang dphang merged commit dd2fa6e into serverless-nextjs:master Jul 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants