Follow-up from "redis: Fix password auth with UNIX domain sockets"
The following discussion from !7573 (merged) should be addressed:
-
@brodock started a discussion: (+1 comment) should we perhaps extract the logic to a class that extends URI, like the
URI::Redis
one?this makes it much cleaner to test the URI without the chef part.
-
@brodock started a discussion: (+1 comment) if we refactor this the rest of the code can also be simplified and becomes easier to understand.
Some ideas:
extract into two methods, one for sentinel the other for regular.
instead of returning an array that we don't know what each value means, unless we read the code again, go for a hash or a struct,
something like:
{host: host, port: port, password: password}
or even better (so we can typecheck later on):
RedisCredential = Struct.new(:host, :port, :password, keyword_init: true) RedisClusterCredential = Struct.new(:name, :port, :password, keyword_init: true) #... RedisCredential.new(host: host, port: port, password: password) RedisClusterCredential.new(...)
Perhaps something like this:
RedisCredential = Struct.new(:host, :port, :password) RedisSentinelCredential = Struct.new(:name, :port, :password) def build_redis_credentials(support_sentinel_groupname: true) if RedisHelper::Checks.has_sentinels? && support_sentinel_groupname redis_sentinel_credentials else redis_credentials end end def check_redis_settings! raise 'Redis announce_ip and announce_ip_from_hostname are mutually exclusive, please unset one of them' if redis['announce_ip'] && redis['announce_ip_from_hostname'] end def redis_credentials(service_config: @node['gitlab']['gitlab_rails']) host = service_config['redis_host'] || Gitlab['redis']['master_ip'] port = service_config['redis_port'] || Gitlab['redis']['master_port'] password = service_config['redis_password'] || Gitlab['redis']['master_password'] RedisCredential.new(host: host, port: port, password: password) end def redis_sentinel_credentials name = redis['master_name'] port = redis['master_port'] password = redis['master_password'] RedisSentinelCredential.new(name: name, port: port, password: password) end end
we still need to call
check_redis_settings!
somewhere, if we want similar behavior put that inside the build method