-
Notifications
You must be signed in to change notification settings - Fork 170
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
[THREESCALE-2755] Return "auth failed" instead of "limits exceeded" for disabled metrics #1066
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor nitpick :-)
gateway/src/apicast/proxy.lua
Outdated
-- This header is set only when enabled via the '3scale-options' header of the | ||
-- request. | ||
local function limit_max_value(response_headers) | ||
return response_headers and response_headers['3scale-limit-max-value'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would change here to return tonumber(response_headers['3scale-limit-max-value'])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would still need to be tonumber(response_headers and response_headers['3scale-limit-max-value'])
.
gateway/src/apicast/proxy.lua
Outdated
-- APIcast, we want to distinguish between limits exceeded vs disabled | ||
-- metric. That's why we reset the reason. The generic auth fail (403) will | ||
-- be returned. | ||
if tonumber(limit_max) == 0 and unauthorized_reason == 'limits_exceeded' then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we return the number in the limit_max_value, here tonumber is not needed at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed 👍
54993ba
to
5931571
Compare
Fixes THREESCALE-2755
See the JIRA epic for a more detailed description: THREESCALE-2752
This uses the new
3scale-limit-max-value
header returned by apisonator: 3scale/apisonator#103