Carmine v3.2+ conn-spec URIs with usernames presume Redis ACL (Redis 6+) support

[liamchzh]

Hello!

After upgrading to Carmine 3.2.0, we're encountering a "wrong number of arguments for 'auth' command" error when trying to connect to Redis. After looking into the changelog, we believe this is due to Carmine now supporting ACL starting from version 3.2.0, which caused a change in the way the Redis connection URI is being parsed. We're using Redis version 5, which doesn't accept the username as a parameter for the "AUTH" command.

Our connection URI pattern looks like this: redis://u:${REDIS_PWD}@${REDIS_HOST}:${REDIS_PORT}, in which u is just a placeholder for username. One of the solutions that we came up with is to remove the u but it seems the username is still being passed even though the username is not set - Carmine sets an empty username instead of ignoring it entirely. We're wondering if this behavior is intentional or a bug.

Let me know if you need more info from me. Thank you!

[ptaoussanis]

@liamchzh Hi Liam, thanks a lot for the report!

So what's happening here is the following:

With Carmine < v3.2: (parse-uri "redis://user:pwd@foo.bar.com:6379/1") => {:host "foo.bar.com", :port 6379, :db 1, :password "pwd"}
With Carmine   v3.2: (parse-uri "redis://user:pwd@foo.bar.com:6379/1") => {:host "foo.bar.com", :port 6379, :db 1, :password "pwd", :username "user"}

i.e. previous versions of Carmine basically ignored any username in the conn URI.
But from v3.2, Carmine assumes that any username in the conn URI is intended for inclusion in auth calls to Redis.

That assumption is what's tripping up in your case.

One easy solution (workaround?): instead of providing your conn-spec to Carmine as a URI, you can just provide the map directly (over which you have full control).

I.e. instead of (wcar {:spec "redis://user:pwd@foo.bar.com:6379"} ...), you could use
(wcar {:spec {:host "foo.bar.com", :port 6379, :password "pwd"}}) and effectively parse the URI yourself.

An open question still is: should the behaviour in 3.2 be changed, or some other option added? I'm a little unsure.

I could add support for something like {:spec {:uri "<...>" :uri-incl-username? false}} - but at that point I wonder if it doesn't instead make sense to just provide the spec directly since it's not much more work than a URI and offers full control.

The auto URI parsing was meant to be a small convenience, so I feel reluctant to start adding options to it and potentially making it clunky.

An alternative could be to make the parse-uri function public, and maybe offer some options there.

What do you think?

[liamchzh]

Thanks for the reply!

One easy solution (workaround?): instead of providing your conn-spec to Carmine as a URI, you can just provide the map directly (over which you have full control).

Your assumption is right. That's what trips up us. I didn't aware of the easy workaround. This sounds like a solution to us when we need to bump to >= 3.2 in the future.

What do you think?

I am not sure which alternatives would be more suitable. I also don't want the parse-uri become too complicated.
Can we document this change in the changelog so that someone in our situation can carefully review it before upgrading to 3.2?

[ptaoussanis]

Can we document this change in the changelog so that someone in our situation can carefully review it before upgrading to 3.2?

That's a good idea, I've made mention of the change and referenced back to this issue for more info: https://github.com/ptaoussanis/carmine/releases/tag/v3.2.0

Apologies that this change caught you by surprise, this change should have been highlighted from the beginning. Your kind of case hadn't occurred to me, so I didn't realise the change could negatively impact anyone.