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

[THREESCALE-10934] [3scale_batcher] Update regex to match key with special chars #1453

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

tkan145
Copy link
Contributor

@tkan145 tkan145 commented Apr 2, 2024

What

Fix https://issues.redhat.com/browse/THREESCALE-10934

Dev notes

In my opinion there are 2 ways to fix this:

  1. Replace the current regrex with the regrex from porta repo. The allowed key formats are describe in the linked JIRA. I don't like this approach because we always have to make sure that the APIcast regular expression matches the one from the porta. Also, the current code allows set keys of any format but restricting the format when get is also strange to me.
  2. Allow everything except spaces and let porta handle the validation during the authorization call. I think this is a better option but would be happy to discuss further.

Verification Steps

  • Configure a single Product A with user_key
aGVsbG93b3JsZAo=
  • Configure batcher policy with the following:
{ "name" : "apicast.policy.3scale_batcher", "configuration" : { "batch_report_seconds" : 1 } }
  • Start dev environment
make development
make dependencies
  • Run apicast locally
THREESCALE_DEPLOYMENT_ENV=staging APICAST_LOG_LEVEL=debug APICAST_WORKER=1 APICAST_CONFIGURATION_LOADER=lazy APICAST_CONFIGURATION_CACHE=0 THREESCALE_PORTAL_ENDPOINT=https://token@3scale-admin.example.com ./bin/apicast
  • Run query with the valid user_key
# capture apicast IP
APICAST_IP=$(docker inspect apicast_build_0-development-1 | yq e -P '.[0].NetworkSettings.Networks.apicast_build_0_default.IPAddress' -)

curl -v -k -H "Host: example.com:443" "http://${APICAST_IP}:8080/?user_key=aGVsbG93b3JsZAo="
  • Check that credentials not found error does not appear in the log. For example:
reports_batcher.lua:99: get_all(): failed to get report for key service_id:12,user_key:aGVsbG93b3JsZAo=,metric:Hits err: credentials not found, context: ngx.timer, client: 10.10.10.1, server: 0.0.0.0:8080 

@tkan145 tkan145 requested a review from a team as a code owner April 2, 2024 06:10
@eguzki eguzki changed the title [3scale_batcher] Update regrex to match key with special chars [THREESCALE-10934] [3scale_batcher] Update regrex to match key with special chars Apr 2, 2024
@eguzki eguzki changed the title [THREESCALE-10934] [3scale_batcher] Update regrex to match key with special chars [THREESCALE-10934] [3scale_batcher] Update regex to match key with special chars Apr 2, 2024
Copy link
Member

@eguzki eguzki left a comment

Choose a reason for hiding this comment

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

Verification steps working

@tkan145 tkan145 force-pushed the THREESCALE-10934-batcher-regrex branch from 9be2d37 to f3d8b72 Compare April 3, 2024 00:40
Currently, when key is base64 encoded or contains special chars, the
batcher policy will save it to the cache but won't be able to retrieve
it due to the regrex mismatch. This leads to two problems:
* Reports are not sent to the backend
* Report cache filling up over time.

This commit changes the regular expression key to accept any character
except spaces.
@tkan145 tkan145 force-pushed the THREESCALE-10934-batcher-regrex branch from f3d8b72 to 40e9bcb Compare April 3, 2024 04:10
@tkan145 tkan145 merged commit 849b5fb into 3scale:master Apr 4, 2024
12 checks passed
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