diff --git a/config.dev.toml b/config.dev.toml index 976af56d..5049a89f 100755 --- a/config.dev.toml +++ b/config.dev.toml @@ -334,7 +334,9 @@ ShutdownTimeout = 60 # Env override: ATHENS_ETCD_ENDPOINTS Endpoints = "localhost:2379,localhost:22379,localhost:32379" [SingleFlight.Redis] - # Endpoint is the redis endpoint for a SingleFlight lock. + # Endpoint is the redis endpoint for a SingleFlight lock. Should be either a host:port + # pair or redis url such as: + # redis[s]://user:password@127.0.0.1:6379/0?protocol=3 # TODO(marwan): enable multiple endpoints for redis clusters. # Env override: ATHENS_REDIS_ENDPOINT Endpoint = "127.0.0.1:6379" diff --git a/docs/content/configuration/storage.md b/docs/content/configuration/storage.md index 48c5d080..b98e58dc 100644 --- a/docs/content/configuration/storage.md +++ b/docs/content/configuration/storage.md @@ -445,6 +445,22 @@ You can also optionally specify a password to connect to the redis server with # Env override: ATHENS_REDIS_PASSWORD Password = "" +Connecting to Redis via a [redis url](https://github.com/redis/redis-specifications/blob/master/uri/redis.txt) is also +supported: + + SingleFlightType = "redis" + + [SingleFlight] + [SingleFlight.Redis] + # Endpoint is the redis endpoint for the single flight mechanism + # Env override: ATHENS_REDIS_ENDPOINT + # Note, if TLS is required use rediss:// instead. + Endpoint = "redis://user:password@127.0.0.1:6379:6379/0?protocol=3" + +If the redis url is invalid or cannot be parsed, Athens will fall back to treating `Endpoint` as if it were +a normal `host:port` pair. If a password is supplied in the redis url, in addition to being provided in the `Password` +configuration option, the two values must match otherwise Athens will fail to start. + ##### Customizing lock configurations: If you would like to customize the distributed lock options then you can optionally override the default lock config to better suit your use-case: diff --git a/pkg/stash/with_redis.go b/pkg/stash/with_redis.go index ff32fb89..4d743dbd 100644 --- a/pkg/stash/with_redis.go +++ b/pkg/stash/with_redis.go @@ -18,22 +18,56 @@ type RedisLogger interface { Printf(ctx context.Context, format string, v ...any) } +var errPasswordsDoNotMatch = goerrors.New("a redis url was parsed that contained a password but the configuration also defined a specific redis password, please ensure these values match or use only one of them") + +// getRedisClientOptions takes an endpoint and password and returns *redis.Options to use +// with the redis client. endpoint may be a redis url or host:port combination. If a redis +// url is used and a password is also used this function checks to make sure the parsed redis +// url has produced the same password. Preferably, one should use EITHER a redis url or a host:port +// combination w/password but not both. More information on the redis url structure can be found +// here: https://github.com/redis/redis-specifications/blob/master/uri/redis.txt +func getRedisClientOptions(endpoint, password string) (*redis.Options, error) { + // Try parsing the endpoint as a redis url first. The redis library does not define + // a specific error when parsing the url so we fall back on the old config here + // which passed in arguments. + options, err := redis.ParseURL(endpoint) + if err != nil { + return &redis.Options{ //nolint:nilerr // We are specifically falling back here and ignoring the error on purpose. + Network: "tcp", + Addr: endpoint, + Password: password, + }, nil + } + + // Ensure the password is either empty or that it matches the password + // parsed from the url into redis.Options. This ensures that if the + // config supplies the password but a redis url doesn't the behavior + // is clear vs. failing later on at the time of the first connection + // with an 'invalid password' like error. + if password != "" && options.Password != password { + return nil, errPasswordsDoNotMatch + } + + return options, nil +} + // WithRedisLock returns a distributed singleflight // using a redis cluster. If it cannot connect, it will return an error. func WithRedisLock(l RedisLogger, endpoint, password string, checker storage.Checker, lockConfig *config.RedisLockConfig) (Wrapper, error) { redis.SetLogger(l) const op errors.Op = "stash.WithRedisLock" - client := redis.NewClient(&redis.Options{ - Network: "tcp", - Addr: endpoint, - Password: password, - }) - _, err := client.Ping(context.Background()).Result() + + options, err := getRedisClientOptions(endpoint, password) if err != nil { return nil, errors.E(op, err) } + client := redis.NewClient(options) + if _, err := client.Ping(context.Background()).Result(); err != nil { + return nil, errors.E(op, err) + } + lockOptions, err := lockOptionsFromConfig(lockConfig) if err != nil { return nil, errors.E(op, err) diff --git a/pkg/stash/with_redis_test.go b/pkg/stash/with_redis_test.go index ba49dba2..862ca012 100644 --- a/pkg/stash/with_redis_test.go +++ b/pkg/stash/with_redis_test.go @@ -9,7 +9,9 @@ import ( "testing" "time" + "github.com/go-redis/redis/v8" "github.com/gomods/athens/pkg/config" + "github.com/gomods/athens/pkg/errors" "github.com/gomods/athens/pkg/storage" "github.com/gomods/athens/pkg/storage/mem" "golang.org/x/sync/errgroup" @@ -120,6 +122,73 @@ func TestWithRedisLockWithWrongPassword(t *testing.T) { } } +type getRedisClientOptionsFacet struct { + endpoint string + password string + options *redis.Options + err error +} + +func Test_getRedisClientOptions(t *testing.T) { + facets := []*getRedisClientOptionsFacet{ + { + endpoint: "127.0.0.1:6379", + options: &redis.Options{ + Addr: "127.0.0.1:6379", + }, + }, + { + endpoint: "127.0.0.1:6379", + password: "1234", + options: &redis.Options{ + Addr: "127.0.0.1:6379", + Password: "1234", + }, + }, + { + endpoint: "rediss://username:password@127.0.0.1:6379", + password: "1234", // Ignored because password was parsed + err: errors.E("stash.WithRedisLock", errPasswordsDoNotMatch), + }, + { + endpoint: "rediss://username:password@127.0.0.1:6379", + password: "1234", // Ignored because password was parsed + err: errors.E("stash.WithRedisLock", errPasswordsDoNotMatch), + }, + } + + for i, facet := range facets { + options, err := getRedisClientOptions(facet.endpoint, facet.password) + if err != nil && facet.err == nil { + t.Errorf("Facet %d: no error produced", i) + continue + } + if facet.err != nil { + if err == nil { + t.Errorf("Facet %d: no error produced", i) + } else { + if err.Error() != facet.err.Error() { + t.Errorf("Facet %d: expected %q, got %q", i, facet.err, err) + } + } + } + + if err != nil { + continue + } + if facet.options.Addr != options.Addr { + t.Errorf("Facet %d: Expected Addr %q, got %q", i, facet.options.Addr, options.Addr) + } + if facet.options.Username != options.Username { + t.Errorf("Facet %d: Expected Username %q, got %q", i, facet.options.Username, options.Username) + } + if facet.options.Password != options.Password { + t.Errorf("Facet %d: Expected Password %q, got %q", i, facet.options.Password, options.Password) + } + + } +} + // mockRedisStasher is like mockStasher // but leverages in memory storage // so that redis can determine diff --git a/scripts/build-image/Dockerfile b/scripts/build-image/Dockerfile index 1c501e4e..86df5295 100644 --- a/scripts/build-image/Dockerfile +++ b/scripts/build-image/Dockerfile @@ -4,12 +4,12 @@ WORKDIR /tmp # Install Helm ENV HELM_VERSION=2.13.0 -RUN curl -sLO https://kubernetes-helm.storage.googleapis.com/helm-v${HELM_VERSION}-linux-amd64.tar.gz && \ +RUN curl -sLO https://get.helm.sh/helm-v${HELM_VERSION}-linux-amd64.tar.gz && \ tar -zxvf helm-v${HELM_VERSION}-linux-amd64.tar.gz && \ mv linux-amd64/helm /usr/local/bin/ # Install a tiny azure client -ENV AZCLI_VERSION=v0.3.1 +ENV AZCLI_VERSION=v0.3.2 RUN curl -sLo /usr/local/bin/az https://github.com/carolynvs/az-cli/releases/download/$AZCLI_VERSION/az-linux-amd64 && \ chmod +x /usr/local/bin/az