From e6bb9f1066d0464d81275ad37c8b7346e7c848db Mon Sep 17 00:00:00 2001 From: Oliver Palmer Date: Wed, 16 Oct 2024 17:09:00 -0400 Subject: [PATCH 1/4] Support using redis urls to construct the redis client Currently Athens only supports connecting to Redis using a hostname:port combination in addition to a password. While this works in most cases it also means that if you have other options you wish to supply Athens has to be updated to support them. As a basic example Redis clusters that require TLS currently are not supported by Athens but with this change you can simply supply a [redis url](https://github.com/redis/redis-specifications/blob/master/uri/redis.txt) to connect over TLS. It also makes it easy to override the password, set a username and more all from a single configuration option: `rediss://username:password@redis.example.com:6379/1?protocol=3` --- config.dev.toml | 4 +- docs/content/configuration/storage.md | 16 +++++++ pkg/stash/with_redis.go | 46 +++++++++++++++--- pkg/stash/with_redis_test.go | 69 +++++++++++++++++++++++++++ scripts/build-image/Dockerfile | 4 +- 5 files changed, 130 insertions(+), 9 deletions(-) diff --git a/config.dev.toml b/config.dev.toml index 976af56df..5049a89f7 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 48c5d080d..b98e58dc3 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 ff32fb89f..a7878ae00 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 string, 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{ + 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 ba49dba2c..862ca012b 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 1c501e4eb..86df5295a 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 From b0fd8436bc6e62521b05f2a0f03a6d3d421d733f Mon Sep 17 00:00:00 2001 From: Oliver Palmer Date: Wed, 16 Oct 2024 17:25:20 -0400 Subject: [PATCH 2/4] lint --- pkg/stash/with_redis.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/stash/with_redis.go b/pkg/stash/with_redis.go index a7878ae00..8fa34c4da 100644 --- a/pkg/stash/with_redis.go +++ b/pkg/stash/with_redis.go @@ -29,14 +29,14 @@ var errPasswordsDoNotMatch = goerrors.New("a redis url was parsed that contained func getRedisClientOptions(endpoint string, 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. + // which passed in arguments. options, err := redis.ParseURL(endpoint) if err != nil { return &redis.Options{ Network: "tcp", Addr: endpoint, Password: password, - }, nil + }, nil //nolint: nilerr } // Ensure the password is either empty or that it matches the password From 7d4bd05dcea3f8343adf4684bc369a93e32bfa68 Mon Sep 17 00:00:00 2001 From: Oliver Palmer Date: Wed, 16 Oct 2024 17:29:36 -0400 Subject: [PATCH 3/4] fix lint --- pkg/stash/with_redis.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/stash/with_redis.go b/pkg/stash/with_redis.go index 8fa34c4da..2bce68e18 100644 --- a/pkg/stash/with_redis.go +++ b/pkg/stash/with_redis.go @@ -32,11 +32,11 @@ func getRedisClientOptions(endpoint string, password string) (*redis.Options, er // which passed in arguments. options, err := redis.ParseURL(endpoint) if err != nil { - return &redis.Options{ + return &redis.Options{ //nolint:nilerr // We are specifically falling back here and ignoring the error on purpose. Network: "tcp", Addr: endpoint, Password: password, - }, nil //nolint: nilerr + }, nil } // Ensure the password is either empty or that it matches the password From 452d662143cc5825e138effc545cd733b6c4c2a8 Mon Sep 17 00:00:00 2001 From: Oliver Palmer Date: Wed, 16 Oct 2024 17:33:43 -0400 Subject: [PATCH 4/4] lint --- pkg/stash/with_redis.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/stash/with_redis.go b/pkg/stash/with_redis.go index 2bce68e18..4d743dbd6 100644 --- a/pkg/stash/with_redis.go +++ b/pkg/stash/with_redis.go @@ -26,7 +26,7 @@ var errPasswordsDoNotMatch = goerrors.New("a redis url was parsed that contained // 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 string, password string) (*redis.Options, error) { +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.