Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add HTTPS support for Edge Functions in Netlify Dev #4567

Merged
merged 12 commits into from
May 3, 2022

Conversation

eduardoboucas
Copy link
Member

Summary

Fixes an issue where origin requests in Edge Functions will fail when running Netlify Dev with HTTPS enabled.

Builds on top of https://github.com/netlify/edge-functions-bootstrap/pull/30 and netlify/edge-bundler#7 (the tests will fail until both of these are merged and released).

Closes https://github.com/netlify/pillar-runtime/issues/323.

@eduardoboucas eduardoboucas added the type: bug code to address defects in shipped code label Apr 28, 2022
@eduardoboucas eduardoboucas changed the title feat: add https support for Edge Functions feat: add HTTPS support for Edge Functions in Netlify Dev Apr 28, 2022
@github-actions github-actions bot added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Apr 28, 2022
@github-actions
Copy link

github-actions bot commented Apr 28, 2022

📊 Benchmark results

Comparing with 6695113

Package size: 274 MB

⬆️ 0.17% increase vs. 6695113

^  380 MB  380 MB  380 MB  380 MB  380 MB  380 MB  380 MB  380 MB  380 MB  380 MB  380 MB  380 MB         
│   ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐          
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |          
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |          
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |          
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |          
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |   274 MB 
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    ┌──┐  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-12    T-11    T-10    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T    
Legend

minivan
minivan previously approved these changes May 2, 2022
Copy link
Contributor

@Skn0tt Skn0tt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT, this will require every developer cloning this repository to run npm run certs as part of setting up their development environment. Could we use something like https://www.npmjs.com/package/openssl-nodejs (or execa) to generate the certificates as part of the test run? Or is that too resource-intensive?

@eduardoboucas
Copy link
Member Author

AFAICT, this will require every developer cloning this repository to run npm run certs as part of setting up their development environment. Could we use something like https://www.npmjs.com/package/openssl-nodejs (or execa) to generate the certificates as part of the test run? Or is that too resource-intensive?

@Skn0tt I've added certs to the test:dev script in c43991f. I think that should cover it?

@Skn0tt
Copy link
Contributor

Skn0tt commented May 3, 2022

Yes, that's good 👍

@eduardoboucas eduardoboucas added automerge Add to Kodiak auto merge queue and removed type: bug code to address defects in shipped code labels May 3, 2022
@kodiakhq kodiakhq bot merged commit f51066e into main May 3, 2022
@kodiakhq kodiakhq bot deleted the feat/edge-functions-https branch May 3, 2022 13:29
This was referenced May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Add to Kodiak auto merge queue type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants