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): Proper stage resolution for custom resource #9277

Merged
merged 1 commit into from Apr 9, 2021

Conversation

pgrzesik
Copy link
Contributor

@pgrzesik pgrzesik commented Apr 9, 2021

After recent changes the bug surfaced where the stage for custom resource needed for API Gateway logs did not take into account setting from provider.stage

Closes: #9240, #9268

@pgrzesik pgrzesik force-pushed the ensure-cw-custom-resource-properly-resolves-stage branch from 5e37760 to 072452f Compare April 9, 2021 08:26
@codecov
Copy link

codecov bot commented Apr 9, 2021

Codecov Report

Merging #9277 (a78005f) into master (dac06c8) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

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

@@            Coverage Diff             @@
##           master    #9277      +/-   ##
==========================================
- Coverage   86.95%   86.95%   -0.01%     
==========================================
  Files         315      315              
  Lines       11736    11735       -1     
==========================================
- Hits        10205    10204       -1     
  Misses       1531     1531              
Impacted Files Coverage Δ
lib/plugins/aws/customResources/index.js 98.57% <100.00%> (-0.03%) ⬇️

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 dac06c8...072452f. Read the comment docs.

@pgrzesik pgrzesik requested a review from medikoo April 9, 2021 08:33
Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

Looks great 👍

@pgrzesik pgrzesik merged commit 50e4425 into master Apr 9, 2021
@pgrzesik pgrzesik deleted the ensure-cw-custom-resource-properly-resolves-stage branch April 9, 2021 08:44
@chekalsky
Copy link

@pgrzesik @medikoo Hi! Does this PR breaks BC? One of my lambdas wasn't deploying with "Modifying service token is not allowed.." error and I suppose that problem is here (probably name change of a function). That alone doesn't really bother me as I can remove lambda and recreate it later, but this also somehow messed with CORS headers in either CloudFront responses, either in Api Gateway, and these headers aren't being send.

@pgrzesik
Copy link
Contributor Author

Hello @chekalsky - it should fix a previously missed bug - stage wasn't resolved correctly and was working "by chance" and it should not introduce any breaking changes but it's definitely possible. Could you provide a reproducible example of a bug you're experiencing so we could take a deeper look into it? Thanks in advance

@Frikitrok
Copy link

@pgrzesik hey, i am trying to upgrade my SLS infrastructure from 2.32.1 to latest 2.68.0 and can see just same error @chekalsky described. So i have different lambda handlers for cognitoUserPool and S3 events, all failing on "sls deploy" with error "An error occurred: CognitoCustomUserPool - Modifying service token is not allowed..". I would assume this is because SLS defining custom event resources in another way, respecting stage variable. Any suggestion how to migrate to latest version without downtime?

@pgrzesik
Copy link
Contributor Author

pgrzesik commented Dec 6, 2021

Hey @Frikitrok - sorry you've run into trouble. Did you add any other modifications to your config during the update or you just upgraded the Framework version and attempted a deployment?

@Frikitrok
Copy link

Frikitrok commented Dec 6, 2021

@pgrzesik If it helps, i've removed serverless-pseudo-parameters plugin because now it supported on SLS vanilla, and added new one serverless-prune-plugin.

@Frikitrok
Copy link

@pgrzesik ^

@pgrzesik
Copy link
Contributor Author

pgrzesik commented Dec 7, 2021

Are you able to share more about your configuration so I could try to reproduce it on my side?

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.

An error occurred: CustomApiGatewayAccountCloudWatchRole - Modifying service token is not allowed..
4 participants