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

Escape queueName and vhostName in RabbitMQ Scaler before use them in query string (bug fix) #2055

Merged
merged 10 commits into from
Aug 26, 2021

Conversation

JorTurFer
Copy link
Member

@JorTurFer JorTurFer commented Aug 24, 2021

This PR escapes the values introduced by the user before add them into a query string. These values are the queueName (main goal) and vhostName. This second one is because the virtual host can contain also escapable values:
image

I update also the e2e test to ensure that it cover the regex use case

This PR is in WIP because I need to wait until this is merged kedacore/test-tools#17

Checklist

  • Commits are signed with Developer Certificate of Origin (DCO - learn more)
  • Tests have been added
  • A PR is opened to update our Helm chart (repo) (if applicable, ie. when deployment manifests are modified)
  • A PR is opened to update the documentation on (repo) (if applicable)
  • Changelog has been updated

Fixes #2054

Signed-off-by: jorturfer <jorge_turrado@hotmail.es>
…string

Signed-off-by: jorturfer <jorge_turrado@hotmail.es>
Signed-off-by: jorturfer <jorge_turrado@hotmail.es>
Signed-off-by: jorturfer <jorge_turrado@hotmail.es>
Signed-off-by: jorturfer <jorge_turrado@hotmail.es>
Copy link
Contributor

@coderanger coderanger left a comment

Choose a reason for hiding this comment

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

Code looks good, but I would like to move the image.

tests/scalers/rabbitmq-helpers.ts Outdated Show resolved Hide resolved
Signed-off-by: jorturfer <jorge_turrado@hotmail.es>
@JorTurFer JorTurFer marked this pull request as draft August 24, 2021 22:25
Signed-off-by: jorturfer <jorge_turrado@hotmail.es>
Signed-off-by: jorturfer <jorge_turrado@hotmail.es>
Signed-off-by: jorturfer <jorge_turrado@hotmail.es>
@JorTurFer JorTurFer marked this pull request as ready for review August 25, 2021 08:38
@JorTurFer
Copy link
Member Author

I have just updated the e2e images and moved from draft to open the PR :)

Signed-off-by: Jorge Turrado <jorge.turrado@docplanner.com>
@JorTurFer JorTurFer changed the title Escape queueName and vhostName in RabbitMQ Scaler before use them in query string Escape queueName and vhostName in RabbitMQ Scaler before use them in query string (bug fix) Aug 25, 2021
@ahmelsayed ahmelsayed merged commit 0e4a01f into kedacore:main Aug 26, 2021
@JorTurFer JorTurFer deleted the fix/rabbit_regex branch August 26, 2021 06:15
nilayasiktoprak pushed a commit to nilayasiktoprak/keda that referenced this pull request Oct 23, 2021
… in query string (bug fix) (kedacore#2055)

Signed-off-by: nilayasiktoprak <nilayasiktoprak@gmail.com>
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.

Bug getting queues with regex using RabbitMQ Scaler
4 participants