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 reporting only 50% of calls to 3scale backend [THREESCALE-1080] #774

Merged
merged 1 commit into from
Jun 25, 2018

Conversation

mikz
Copy link
Contributor

@mikz mikz commented Jun 19, 2018

Due to a bug in credentials juggling APIcast would send raw JWT to 3scale backend instead of just app_id.
That token is of course not understood and the reporting call rejected.

The fix changes how credentials are moved around to reduce risk of future issues.

Fixes THREESCALE-1080

@3scale/qe you might want to incorporate some tests that test metrics reporting accuracy to catch this kind of issues.

@mikz mikz requested a review from a team as a code owner June 19, 2018 12:33
@mikz mikz changed the title [OIDC] Fix reporting only 50% of calls to 3scale backend [THREESCALE-1080] [WIP] Fix reporting only 50% of calls to 3scale backend [THREESCALE-1080] Jun 19, 2018
t/apicast-oidc.t Outdated



=== TEST 2: Report calls to 3scale backend
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this test is doing the same as the one above but making 2 requests instead of one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly. I wanted it to be super obvious. Not sure if setting repeat_each(2) would be obvious enough.

Copy link
Contributor

Choose a reason for hiding this comment

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

In that case I would add a comment. Figuring out why this is being tested twice can be difficult without having some context like the PR title.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point 👍

@mikz mikz force-pushed the oidc-reporting branch 4 times, most recently from 2031991 to 3392d3c Compare June 20, 2018 14:24
@mikz mikz changed the title [WIP] Fix reporting only 50% of calls to 3scale backend [THREESCALE-1080] Fix reporting only 50% of calls to 3scale backend [THREESCALE-1080] Jun 20, 2018
@mikz mikz requested a review from davidor June 20, 2018 15:40
@mikz mikz merged commit 1bff272 into master Jun 25, 2018
@mikz mikz deleted the oidc-reporting branch June 25, 2018 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants