-
Notifications
You must be signed in to change notification settings - Fork 2.7k
fix(redis): add default auth to redis clusters #37337
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
base: main
Are you sure you want to change the base?
fix(redis): add default auth to redis clusters #37337
Conversation
The createCluster call is only authenticating with the rootNode, connections to other nodes in the cluster require default username and password when creating the client
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need some time to check with some known users of cluster mode to confirm that none of them are using username/password. If they are, then we need to reconfirm the assumptions which lead to this PR
Hi there, Please don't merge from Thanks, The Renovate team |
|
||
describe('init', () => { | ||
beforeEach(() => { | ||
vi.clearAllMocks(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vi.clearAllMocks(); |
done globally
vi.mocked(createClient).mockReturnValue({ | ||
connect: vi.fn(), | ||
} as unknown as RedisClientType); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use our partial helper method
Lines 25 to 30 in 643d489
export function partial<T>(): T; | |
export function partial<T>(obj: Partial<T>): T; | |
export function partial<T>(obj: Partial<T>[]): T[]; | |
export function partial(obj: unknown = {}): unknown { | |
return obj; | |
} |
clusterConfig.defaults = { | ||
...clusterConfig.defaults, | ||
password: parsedUrl.password, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clusterConfig.defaults = { | |
...clusterConfig.defaults, | |
password: parsedUrl.password, | |
}; | |
clusterConfig.defaults ??= { } | |
clusterConfig.defaults.password = parsedUrl.password; |
Changes
Using redis in a cluster with authentication causes the following error:
This change adds the username and password from the redis URL as defaults for connections to non-root node servers as described in the node redis docs
Context
Logs and motivation in #37319
Documentation (please check one with an [x])
How I've tested my work (please select one)
I have verified these changes via: