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

fix(AWS API Gateway): Ensure consistent default for cors conf #9909

Conversation

issea1015
Copy link
Contributor

Closes: #9858

@codecov
Copy link

codecov bot commented Sep 3, 2021

Codecov Report

Merging #9909 (109fc8f) into master (f096440) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9909      +/-   ##
==========================================
- Coverage   86.25%   86.18%   -0.07%     
==========================================
  Files         330      330              
  Lines       12879    13005     +126     
==========================================
+ Hits        11109    11209     +100     
- Misses       1770     1796      +26     
Impacted Files Coverage Δ
...ins/aws/package/compile/events/apiGateway/index.js 93.65% <ø> (+0.20%) ⬆️
.../package/compile/events/apiGateway/lib/validate.js 96.84% <100.00%> (-0.50%) ⬇️
lib/plugins/aws/info/display.js 77.14% <0.00%> (-12.02%) ⬇️
lib/plugins/aws/deploy/lib/extendedValidate.js 95.23% <0.00%> (-4.77%) ⬇️
lib/plugins/aws/deploy/index.js 96.36% <0.00%> (-3.64%) ⬇️
lib/plugins/aws/lib/monitorStack.js 94.56% <0.00%> (-2.88%) ⬇️
lib/classes/CLI.js 91.42% <0.00%> (-2.86%) ⬇️
lib/classes/PluginManager.js 94.65% <0.00%> (-0.51%) ⬇️
.../compile/events/apiGateway/lib/requestValidator.js 98.38% <0.00%> (-0.13%) ⬇️
... and 19 more

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 f096440...109fc8f. Read the comment docs.

@issea1015
Copy link
Contributor Author

Let it add a test to assert this

@issea1015 issea1015 changed the title fix(AWS API Gateway): Have consistent default for CORS configuration WIP: fix(AWS API Gateway): Have consistent default for CORS configuration Sep 3, 2021
@issea1015 issea1015 changed the title WIP: fix(AWS API Gateway): Have consistent default for CORS configuration fix(AWS API Gateway): Have consistent default for CORS configuration Sep 3, 2021
@issea1015
Copy link
Contributor Author

Ready to be reviewed

Copy link
Contributor

@pgrzesik pgrzesik left a comment

Choose a reason for hiding this comment

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

Thank you @issea1015 - looks good, I only have a few minor questions - please let me know what do you think 🙇

@issea1015
Copy link
Contributor Author

@pgrzesik Thanks for the feedback :) I will resolve them and let you know.

Copy link
Contributor

@pgrzesik pgrzesik left a comment

Choose a reason for hiding this comment

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

Thank you @issea1015 🙇 While the refactoring looks nice, I would rather avoid it, please see my comment about testing and let me know what do you think

lib/classes/PluginManager.js Outdated Show resolved Hide resolved
@issea1015
Copy link
Contributor Author

@pgrzesik Thanks for the guidance! Let me take a look and resolve them.

Copy link
Contributor

@pgrzesik pgrzesik left a comment

Choose a reason for hiding this comment

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

Thank you @issea1015 - it looks awesome, there's one last small thing related to test and we're good to go 👍

@issea1015
Copy link
Contributor Author

Thank you for all of these kind guidances 👍 👍 @pgrzesik

Copy link
Contributor

@pgrzesik pgrzesik left a comment

Choose a reason for hiding this comment

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

Great job @issea1015 👏

@pgrzesik pgrzesik changed the title fix(AWS API Gateway): Have consistent default for CORS configuration fix(AWS API Gateway): Ensure consistent default for cors conf Sep 13, 2021
@pgrzesik pgrzesik merged commit 7cd3966 into serverless:master Sep 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AWS API Gateway: Improve validation of CORS configuration
2 participants