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 api_gateway_authorizer caching ttl in case explicitly set to 0 #8267

Closed
wants to merge 1 commit into from
Closed

Fix api_gateway_authorizer caching ttl in case explicitly set to 0 #8267

wants to merge 1 commit into from

Conversation

f3lang
Copy link
Contributor

@f3lang f3lang commented Apr 10, 2019

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Fixes #705

Changes proposed in this pull request:

The GetOk will compare the value to the default zero value of a type. When the ttl is set to 0, the value matches the default zero value of the type Integer and will be ignored for the api request.

Normally not a problem, but in this case, this results in an authorizer with a cache ttl of null, which defaults to 300s on AWS side. This is also not visible properly in the AWS console, since the value is displayed as "undefined" and the checkbox for caching is not activated. So it looks like the caching is disabled, but the default caching ttl of 300s kicks in and produces unreliable results in the API Gateway behaviour.

So this change uses the GetOkExists function for the parameter validation which will not compare
the value to the type zero value and therefore the resource will now accept an explicitly value of 0 for
the parameter authorizer_result_ttl_in_seconds.

The GetOk will compare the value to the default zero value of a
type. When the ttl is set to 0, the value matches the default zero
value of the type Integer and will be ignored for the api request.
Normally not a problem, but in this case, this results in an authorizer
with a cache ttl of null, which defaults to 300s on AWS side.
fixes #705
@ghost ghost added size/XS Managed by automation to categorize the size of a PR. service/apigateway Issues and PRs that pertain to the apigateway service. labels Apr 10, 2019
@f3lang
Copy link
Contributor Author

f3lang commented Apr 16, 2019

Hi, is there anything missing, which i should add to this pr?

@f3lang
Copy link
Contributor Author

f3lang commented May 5, 2019

I'll just try @bflad
I hope you are the right guy for this pr :) Can you have a look at it?

@aeschright aeschright requested a review from a team June 26, 2019 16:44
@f3lang
Copy link
Contributor Author

f3lang commented Jul 22, 2019

Hi, any updates on this?

@bflad bflad added the bug Addresses a defect in current functionality. label Aug 7, 2019
@bflad
Copy link
Contributor

bflad commented Aug 7, 2019

Hi @f3lang 👋 Thank you for submitting this and sorry for the delays in getting it reviewed. In practice we have found that d.GetOkExists() only works in certain scenarios, so unfortunately it is not a preferable solution we would be looking for here.

Since the API denotes a default of 300, but omits it from the API response if its unset, a solution like the one recently submitted in #9605 may cover the problem better here, but it needs to be verified, especially with existing configurations.

Would you be able to take a look at #9605 and see if it seems like a reasonable fix? Thanks.

@aeschright aeschright added the waiting-response Maintainers are waiting on response from community or contributor. label Sep 12, 2019
@bflad
Copy link
Contributor

bflad commented Nov 1, 2019

Closing due to lack of response -- please see #8267 (comment) for additional information and further tracking of fixing this issue.

@bflad bflad closed this Nov 1, 2019
@ghost
Copy link

ghost commented Mar 29, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 29, 2020
@breathingdust breathingdust removed the waiting-response Maintainers are waiting on response from community or contributor. label Sep 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/apigateway Issues and PRs that pertain to the apigateway service. size/XS Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom Authorizer for API Gateway does not insert '0' when specified in script
4 participants