Skip to content

fix(redis scaler): use literal command names in Lua script for Alibaba Cloud compatibility#7759

Merged
wozniakjan merged 6 commits into
kedacore:mainfrom
cxhello:fix/redis-lua-literal-command
May 25, 2026
Merged

fix(redis scaler): use literal command names in Lua script for Alibaba Cloud compatibility#7759
wozniakjan merged 6 commits into
kedacore:mainfrom
cxhello:fix/redis-lua-literal-command

Conversation

@cxhello
Copy link
Copy Markdown
Contributor

@cxhello cxhello commented May 22, 2026

The Redis scaler's Lua script uses a table lookup variable as the first argument to redis.call(), which fails on Alibaba Cloud Redis Cluster instances that enforce strict Lua script validation requiring literal strings.

Before (fails on Alibaba Cloud Redis Cluster):

local cmd = {
    zset = 'zcard',
    set = 'scard',
    list = 'llen',
    hash = 'hlen',
    none = 'llen'
}
return redis.call(cmd[listType], listName)

After (compatible with all Redis deployments):

if listType == 'zset' then
    return redis.call('zcard', listName)
elseif listType == 'set' then
    return redis.call('scard', listName)
elseif listType == 'hash' then
    return redis.call('hlen', listName)
else
    return redis.call('llen', listName)
end

Checklist

Fixes #7758

…a Cloud compatibility

The Redis scaler's Lua script uses a table lookup variable as the first
argument to redis.call(), which fails on Alibaba Cloud Redis Cluster
instances that enforce strict Lua script validation.

Replace the table lookup pattern with if/elseif branches so every
redis.call() has a literal string as the first argument, compatible
with both standard Redis and Alibaba Cloud Redis Cluster.

Fixes kedacore#7758

Signed-off-by: cxhello <caixiaohuichn@gmail.com>
@cxhello cxhello requested a review from a team as a code owner May 22, 2026 05:18
@github-actions
Copy link
Copy Markdown

Thank you for your contribution! 🙏

Please understand that we will do our best to review your PR and give you feedback as soon as possible, but please bear with us if it takes a little longer as expected.

While you are waiting, make sure to:

  • Add an entry in our changelog in alphabetical order and link related issue
  • Update the documentation, if needed
  • Add unit & e2e tests for your changes
  • GitHub checks are passing
  • Is the DCO check failing? Here is how you can fix DCO issues

Once the initial tests are successful, a KEDA member will ensure that the e2e tests are run. Once the e2e tests have been successfully completed, the PR may be merged at a later date. Please be patient.

Learn more about our contribution guide.

@keda-automation keda-automation requested a review from a team May 22, 2026 05:18
@snyk-io
Copy link
Copy Markdown

snyk-io Bot commented May 22, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>
@wozniakjan
Copy link
Copy Markdown
Member

wozniakjan commented May 25, 2026

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

passed tests: 0
failed tests: 12
Execution of tests/scalers/redis/redis_sentinel_streams_lag/redis_sentinel_streams_lag_test.go, has failed after "three" attempts
Execution of tests/scalers/redis/redis_sentinel_streams_length/redis_sentinel_streams_length_test.go, has failed after "three" attempts
Execution of tests/scalers/redis/redis_cluster_streams_pending_entries/redis_cluster_streams_pending_entries_test.go, has failed after "three" attempts
Execution of tests/scalers/redis/redis_cluster_streams_lag/redis_cluster_streams_lag_test.go, has failed after "three" attempts
Execution of tests/scalers/redis/redis_standalone_lists/redis_standalone_lists_test.go, has failed after "three" attempts
Execution of tests/scalers/redis/redis_cluster_streams_length/redis_cluster_streams_length_test.go, has failed after "three" attempts
Execution of tests/scalers/redis/redis_cluster_lists/redis_cluster_lists_test.go, has failed after "three" attempts
Execution of tests/scalers/redis/redis_standalone_streams_lag/redis_standalone_streams_lag_test.go, has failed after "three" attempts
Execution of tests/scalers/redis/redis_sentinel_streams_pending_entries/redis_sentinel_streams_pending_entries_test.go, has failed after "three" attempts
Execution of tests/scalers/redis/redis_sentinel_lists/redis_sentinel_lists_test.go, has failed after "three" attempts
Execution of tests/scalers/redis/redis_standalone_streams_length/redis_standalone_streams_length_test.go, has failed after "three" attempts
Execution of tests/scalers/redis/redis_standalone_streams_pending_entries/redis_standalone_streams_test_pending_entries_test.go, has failed after "three" attempts

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Redis scaler’s Lua script to avoid passing a variable/table lookup as the first argument to redis.call, improving compatibility with Alibaba Cloud Redis Cluster’s strict Lua script validation.

Changes:

  • Replace Lua table-based command selection (cmd[listType]) with if/elseif branches that call Redis commands via literal strings.
  • Add an Unreleased changelog entry documenting the Redis scaler compatibility fix.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
pkg/scalers/redis_scaler.go Updates the Redis scaler Lua script to use literal command names in redis.call.
CHANGELOG.md Adds an Unreleased “Redis Scaler” fix entry (but also removes an unrelated AWS scalers entry).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/scalers/redis_scaler.go Outdated
Comment thread pkg/scalers/redis_scaler.go
Comment thread CHANGELOG.md
@rickbrouwer
Copy link
Copy Markdown
Member

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

@wozniakjan: I don't think this one will succeed; I believe we have some Redis E2E issues (given that all Redis tests fail in both main and E2E).

@wozniakjan
Copy link
Copy Markdown
Member

wozniakjan commented May 25, 2026

@wozniakjan: I don't think this one will succeed; I believe we have some Redis E2E issues (given that all Redis tests fail in both main and E2E).

I also suspected that, haven't investigated but thought that maybe randomly this PR fixes it :). If it fails here too then I will troubleshoot the redis e2e tests in main.

EDIT: #7768

@wozniakjan wozniakjan added the nice-to-have:keda-v2.20 Not strictly necessary, but nice if you can bring it along label May 25, 2026
cxhello added 2 commits May 25, 2026 17:43
… entry

- Add explicit handling for 'list' and 'none' key types in Lua script
- Return error_reply for unsupported key types (e.g. string, stream)
- Restore accidentally removed AWS Scalers changelog entry

Signed-off-by: cxhello <caixiaohuichn@gmail.com>
…order

Signed-off-by: cxhello <caixiaohuichn@gmail.com>
@cxhello
Copy link
Copy Markdown
Contributor Author

cxhello commented May 25, 2026

Thanks for the review! Addressed all feedback in the latest push:

  1. Restored the accidentally removed AWS Scalers changelog entry
  2. Added explicit handling for list/none key types and error_reply for unsupported types (e.g. string, stream)
  3. Deduplicated the Redis Scaler changelog entry and kept alphabetical order

@wozniakjan
Copy link
Copy Markdown
Member

wozniakjan commented May 25, 2026

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

@wozniakjan wozniakjan enabled auto-merge (squash) May 25, 2026 10:38
@wozniakjan wozniakjan added the Awaiting/2nd-approval This PR needs one more approval review label May 25, 2026
@rickbrouwer rickbrouwer added ok-to-merge This PR can be merged waiting-for-e2e and removed Awaiting/2nd-approval This PR needs one more approval review labels May 25, 2026
@JorTurFer
Copy link
Copy Markdown
Member

JorTurFer commented May 25, 2026

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

@keda-automation keda-automation requested a review from a team May 25, 2026 12:38
@JorTurFer
Copy link
Copy Markdown
Member

JorTurFer commented May 25, 2026

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

passed tests: 12
Execution of tests/scalers/redis/redis_standalone_streams_pending_entries/redis_standalone_streams_test_pending_entries_test.go, has passed after "one" attempts
Execution of tests/scalers/redis/redis_standalone_streams_length/redis_standalone_streams_length_test.go, has passed after "one" attempts
Execution of tests/scalers/redis/redis_standalone_streams_lag/redis_standalone_streams_lag_test.go, has passed after "one" attempts
Execution of tests/scalers/redis/redis_standalone_lists/redis_standalone_lists_test.go, has passed after "one" attempts
Execution of tests/scalers/redis/redis_sentinel_streams_length/redis_sentinel_streams_length_test.go, has passed after "one" attempts
Execution of tests/scalers/redis/redis_cluster_streams_length/redis_cluster_streams_length_test.go, has passed after "one" attempts
Execution of tests/scalers/redis/redis_cluster_streams_pending_entries/redis_cluster_streams_pending_entries_test.go, has passed after "one" attempts
Execution of tests/scalers/redis/redis_cluster_streams_lag/redis_cluster_streams_lag_test.go, has passed after "one" attempts
Execution of tests/scalers/redis/redis_sentinel_lists/redis_sentinel_lists_test.go, has passed after "one" attempts
Execution of tests/scalers/redis/redis_cluster_lists/redis_cluster_lists_test.go, has passed after "one" attempts
Execution of tests/scalers/redis/redis_sentinel_streams_pending_entries/redis_sentinel_streams_pending_entries_test.go, has passed after "one" attempts
Execution of tests/scalers/redis/redis_sentinel_streams_lag/redis_sentinel_streams_lag_test.go, has passed after "one" attempts
failed tests: 0

@wozniakjan wozniakjan merged commit 04f6e3c into kedacore:main May 25, 2026
23 of 24 checks passed
@rickbrouwer rickbrouwer removed ok-to-merge This PR can be merged waiting-for-e2e labels May 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

nice-to-have:keda-v2.20 Not strictly necessary, but nice if you can bring it along

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Redis scaler Lua script fails on Alibaba Cloud Redis Cluster due to variable command name

5 participants