feat: add nsq scaler by Ulminator · Pull Request #6230 · kedacore/keda

Ulminator · 2024-10-11T00:06:26Z

This change adds a new scaler for NSQ, enabling auto-scaling based on topic/channel depth. The main components of NSQ that the scaler interacts with are nsqlookupd and nsqd. First the scaler makes /lookup requests to the nsqlookupd instances in the cluster to determine which nsqd instances have messages for the given topic. Then for each nsqd instance the scaler makes /stats requests to and aggregates the results to determine a final depth value.

NSQ consumers create the channel they are reading if they do not exist. As such, if the channel does not exist on an nsqd host, the topic depth is used. The consumer pods that KEDA would be responsible in spawning would then create the channel.

Checklist

When introducing a new scaler, I agree with the scaling governance policy
I have verified that my change is according to the deprecations & breaking changes policy
Tests have been added
Changelog has been updated and is aligned with our changelog requirements
A PR is opened to update the documentation on (repo) (if applicable)
Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #3281

Relates to:

pkg/scalers/nsq_scaler_test.go

semgrep-app · 2024-10-11T00:07:55Z

Semgrep found 7 no-direct-write-to-responsewriter findings:

pkg/scalers/nsq_scaler_test.go
- L160 - Triage
- L169 - Triage
- L315 - Triage
- L394 - Triage
- L463 - Triage
- L575 - Triage
- L636 - Triage

Detected directly writing or similar in 'http.ResponseWriter.write()'. This bypasses HTML escaping that prevents cross-site scripting vulnerabilities. Instead, use the 'html/template' package and render data using 'template.Execute()'.

_{Ignore this finding from no-direct-write-to-responsewriter.}

Semgrep found 20 sprintf-host-port findings:

pkg/scalers/nsq_scaler_test.go
- L176 - Triage
- L176 - Triage
- L176 - Triage
- L322 - Triage
- L322 - Triage
- L402 - Triage
- L402 - Triage
- L402 - Triage
- L470 - Triage
- L470 - Triage
- 7 more - Triage
pkg/scalers/nsq_scaler.go
- L188 - Triage
- L188 - Triage
- L188 - Triage

use net.JoinHostPort instead of fmt.Sprintf(parsedURL.Port(), parsedURL.Hostname())

_{Ignore this finding from sprintf-host-port.}

semgrep-app · 2024-10-11T01:15:12Z

Semgrep found 7 no-fprintf-to-responsewriter findings:

pkg/scalers/nsq_scaler_test.go
- L160 - Triage
- L169 - Triage
- L315 - Triage
- L394 - Triage
- L463 - Triage
- L576 - Triage
- L637 - Triage

Detected 'Fprintf' or similar writing to 'http.ResponseWriter'. This bypasses HTML escaping that prevents cross-site scripting vulnerabilities. Instead, use the 'html/template' package to render data to users.

_{Ignore this finding from no-fprintf-to-responsewriter.}

Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com>

Ulminator · 2024-10-14T21:34:28Z

@JorTurFer thanks for merging in kedacore/test-tools#175! I updated the code here to utilize the image that can be created from that PR.

I also wanted to let you know that I have created the corresponding docs PR as well here: kedacore/keda-docs#1485

The last remaining TODO's seem to just be getting these CI checks to pass. Is it safe to ignore the remaining semgrep checks since they just relate to tests? I initially got flagged for no-direct-write-to-responsewriter and now I'm getting no-fprintf-to-responsewriter, yet I do see both of these patterns being used in other test files.

CHANGELOG.md

zroubalik · 2024-10-15T11:27:38Z

/run-e2e nsq
Update: You can check the progress here

Ulminator · 2024-10-15T17:07:23Z

I see the e2e tests have failed, but I'm wondering if the e2e test image (ghcr.io/kedacore/tests-nsq:latest) was created? 🤔 I don't see it here. I'm thinking the issue was an image pull error due to it not existing yet.

# Local run w output snippet from describing a consumer pod
  Warning  Failed     9s    kubelet            Failed to pull image "ghcr.io/kedacore/tests-nsq:latest": failed to pull and unpack image "ghcr.io/kedacore/tests-nsq:latest": failed to resolve reference "ghcr.io/kedacore/tests-nsq:latest": failed to authorize: failed to fetch anonymous token: unexpected status from GET request to https://ghcr.io/token?scope=repository%3Akedacore%2Ftests-nsq%3Apull&service=ghcr.io: 401 Unauthorized
  Warning  Failed     9s    kubelet            Error: ErrImagePull
  Normal   BackOff    8s    kubelet            Back-off pulling image "ghcr.io/kedacore/tests-nsq:latest"
  Warning  Failed     8s    kubelet            Error: ImagePullBackOff

I also noticed I had only updated one of the refs of my custom image to ghcr.io/kedacore/tests-nsq:latest so I went ahead and made that fix.

Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com>

JorTurFer · 2024-10-16T18:23:34Z

The image was created but GH creates them as private by default, now it's public

JorTurFer · 2024-10-16T18:23:38Z

/run-e2e nsq
Update: You can check the progress here

JorTurFer · 2024-10-16T18:31:43Z

Do we need to update the policy to include any extra permission? https://github.com/kedacore/testing-infrastructure/blob/main/terraform/modules/aws/iam/main.tf#L88-L96

If yes, could you open a PR there?

Ulminator · 2024-10-17T17:59:39Z

Do we need to update the policy to include any extra permission? https://github.com/kedacore/testing-infrastructure/blob/main/terraform/modules/aws/iam/main.tf#L88-L96

If yes, could you open a PR there?

No additional permissions are required 👍

Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com>

Ulminator · 2024-10-23T22:34:59Z

@JorTurFer just wanted to check in and see if you need anything else before merging this in?

Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com>

Ulminator · 2024-11-05T05:27:33Z

@JorTurFer thanks for taking a look! I believe I addressed all of your comments, but let me know if anything else needs to change of course.

I'm worried about the lifecycle of the project, I closed the issue with the feature request a year and a half ago because there wasn't activity, and it looked as no longer maintained.

I'm an engineer at Bitly, which is where NSQ was initially developed. We use it extensively throughout our stack, and while updates to the project are not as frequent as they were in the past, it is stable. There are plans for updates in the near future and it is under active development.

tests/scalers/nsq/nsq_test.go

Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com>

JorTurFer · 2024-11-05T21:08:53Z

/run-e2e nsq
Update: You can check the progress here

JorTurFer

LGTM! Awesome job! 🙇

tests/scalers/nsq/nsq_test.go

JorTurFer · 2024-11-05T21:47:29Z

@zroubalik @wozniakjan PTAL

Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es> Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com>

Ulminator · 2024-11-05T21:51:28Z

@JorTurFer thanks for approving the PR and for your prompt feedback!

JorTurFer · 2024-11-05T22:20:35Z

/run-e2e nsq
Update: You can check the progress here

wozniakjan

lgtm, thank you!

not sure what is wrong with Semgrep / Analyze Semgrep action, it doesn't allow me to see anything in there

Ulminator · 2024-11-15T00:37:12Z

Thanks for the review @wozniakjan !

@JorTurFer, I have updated this PR along with kedacore/docs#1485 to account for the 2.16 release that happened last week, PTAL!

Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com>

JorTurFer · 2024-11-23T20:05:52Z

/run-e2e nsq
Update: You can check the progress here

JorTurFer · 2024-11-23T20:47:41Z

Thanks for your contribution!

* feat: add nsq scaler Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * fix changelog and image ref Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * fix changelog formatting Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * fix: address comments Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * feat: add useHttps/unsafeSsl to nsqScaler Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * fix: make nsq e2e tests more reliable Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * Update tests/scalers/nsq/nsq_test.go Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es> Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * fix: update changelog Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> --------- Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es> Signed-off-by: michael pechner <mike.pechner@akasa.com>

* feat: add nsq scaler Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * fix changelog and image ref Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * fix changelog formatting Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * fix: address comments Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * feat: add useHttps/unsafeSsl to nsqScaler Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * fix: make nsq e2e tests more reliable Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * Update tests/scalers/nsq/nsq_test.go Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es> Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * fix: update changelog Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> --------- Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>

* feat: add nsq scaler Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * fix changelog and image ref Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * fix changelog formatting Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * fix: address comments Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * feat: add useHttps/unsafeSsl to nsqScaler Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * fix: make nsq e2e tests more reliable Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * Update tests/scalers/nsq/nsq_test.go Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es> Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> * fix: update changelog Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> --------- Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com> Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es> Signed-off-by: Chirag Bhatia <chiragbhatia8@gmail.com>

Ulminator requested a review from a team as a code owner October 11, 2024 00:06

semgrep-app bot reviewed Oct 11, 2024

View reviewed changes

pkg/scalers/nsq_scaler_test.go Outdated Show resolved Hide resolved

semgrep-app bot reviewed Oct 11, 2024

View reviewed changes

pkg/scalers/nsq_scaler_test.go Outdated Show resolved Hide resolved

semgrep-app bot reviewed Oct 11, 2024

View reviewed changes

pkg/scalers/nsq_scaler_test.go Outdated Show resolved Hide resolved

semgrep-app bot reviewed Oct 11, 2024

View reviewed changes

pkg/scalers/nsq_scaler_test.go Outdated Show resolved Hide resolved

Ulminator mentioned this pull request Oct 11, 2024

feat: add nsq e2e image kedacore/test-tools#175

Merged

2 tasks

Ulminator force-pushed the nsq_autoscaler branch from bb102b3 to 7e342bd Compare October 11, 2024 01:13

Ulminator force-pushed the nsq_autoscaler branch 3 times, most recently from 7d7ee13 to 32b9535 Compare October 14, 2024 19:09

feat: add nsq scaler

063ca3d

Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com>

Ulminator force-pushed the nsq_autoscaler branch from 32b9535 to 063ca3d Compare October 14, 2024 19:47

Ulminator mentioned this pull request Oct 14, 2024

feat: nsq scaler docs kedacore/keda-docs#1485

Merged

1 task

zroubalik reviewed Oct 15, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

fix changelog and image ref

64f31ce

Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com>

Ulminator force-pushed the nsq_autoscaler branch from e43cf59 to 64f31ce Compare October 15, 2024 19:43

fix changelog formatting

87b8815

Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com>

Ulminator force-pushed the nsq_autoscaler branch from 58dcc69 to 87b8815 Compare October 17, 2024 18:16

Merge branch 'main' into nsq_autoscaler

5d32d68

JorTurFer mentioned this pull request Nov 3, 2024

Release: v2.16 #6260

Closed

28 tasks

Ulminator force-pushed the nsq_autoscaler branch from 1dbec8b to 54afb54 Compare November 5, 2024 01:26

feat: add useHttps/unsafeSsl to nsqScaler

56787db

Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com>

JorTurFer reviewed Nov 5, 2024

View reviewed changes

tests/scalers/nsq/nsq_test.go Outdated Show resolved Hide resolved

tests/scalers/nsq/nsq_test.go Outdated Show resolved Hide resolved

fix: make nsq e2e tests more reliable

5ebd84a

Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com>

Merge branch 'main' into nsq_autoscaler

2286b36

JorTurFer approved these changes Nov 5, 2024

View reviewed changes

tests/scalers/nsq/nsq_test.go Outdated Show resolved Hide resolved

Update tests/scalers/nsq/nsq_test.go

4a6a4a7

Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es> Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com>

wozniakjan approved these changes Nov 11, 2024

View reviewed changes

wozniakjan enabled auto-merge (squash) November 11, 2024 08:49

Merge branch 'main' into nsq_autoscaler

f6c2343

auto-merge was automatically disabled November 15, 2024 00:31
Head branch was pushed to by a user without write access

fix: update changelog

b0d2bb5

Signed-off-by: Matt Ulmer <25484774+Ulminator@users.noreply.github.com>

Ulminator force-pushed the nsq_autoscaler branch from 07f4d4a to b0d2bb5 Compare November 15, 2024 00:38

JorTurFer merged commit 92f3533 into kedacore:main Nov 23, 2024

Conversation

Ulminator commented Oct 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

semgrep-app bot commented Oct 11, 2024

Uh oh!

semgrep-app bot commented Oct 11, 2024

Uh oh!

Ulminator commented Oct 14, 2024

Uh oh!

Uh oh!

zroubalik commented Oct 15, 2024 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ulminator commented Oct 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JorTurFer commented Oct 16, 2024

Uh oh!

JorTurFer commented Oct 16, 2024 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JorTurFer commented Oct 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ulminator commented Oct 17, 2024

Uh oh!

Ulminator commented Oct 23, 2024

Uh oh!

Ulminator commented Nov 5, 2024

Uh oh!

Uh oh!

Uh oh!

JorTurFer commented Nov 5, 2024 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JorTurFer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JorTurFer commented Nov 5, 2024

Uh oh!

Ulminator commented Nov 5, 2024

Uh oh!

JorTurFer commented Nov 5, 2024 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wozniakjan left a comment

Choose a reason for hiding this comment

Uh oh!

Ulminator commented Nov 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JorTurFer commented Nov 23, 2024 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JorTurFer commented Nov 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Ulminator commented Oct 11, 2024 •

edited

Loading

zroubalik commented Oct 15, 2024 •

edited by github-actions bot

Loading

Ulminator commented Oct 15, 2024 •

edited

Loading

JorTurFer commented Oct 16, 2024 •

edited by github-actions bot

Loading

JorTurFer commented Oct 16, 2024 •

edited

Loading

JorTurFer commented Nov 5, 2024 •

edited by github-actions bot

Loading

JorTurFer commented Nov 5, 2024 •

edited by github-actions bot

Loading

Ulminator commented Nov 15, 2024 •

edited

Loading

JorTurFer commented Nov 23, 2024 •

edited by github-actions bot

Loading