add config options for redis password (#1545)

* add config options for redis password

* redis password test and failure test

* changed redis name and made a minimal redis.conf

add drone volume

volume fix

remove volumes from drone.

repoint redis path

Co-authored-by: Aaron Schlesinger <70865+arschles@users.noreply.github.com>
Co-authored-by: Marwan Sulaiman <marwan.sameer@gmail.com>

.drone.yml

@@ -46,6 +46,8 @@ steps:
     ATHENS_MONGO_STORAGE_URL: mongodb://mongo:27017
     ATHENS_MINIO_ENDPOINT: minio:9000
     REDIS_TEST_ENDPOINT: redis:6379
+    PROTECTED_REDIS_TEST_ENDPOINT: protectedredis:6380
+    ATHENS_PROTECTED_REDIS_PASSWORD: AthensPass1
     GCS_SERVICE_ACCOUNT:
       from_secret: GCS_SERVICE_ACCOUNT
     GCS_PROJECT_ID: 
@@ -154,6 +156,13 @@ services:
   image: redis
   ports:
     - 6379
+- name: protectedredis
+  image: redis
+  ports:
+    - 6380
+  commands:
+    - "redis-server ./test/redis.conf"
+
 - name: athens-proxy
   image: gomods/athens:canary
   pull: always

cmd/proxy/actions/app_proxy.go

@@ -132,7 +132,7 @@ func getSingleFlight(c *config.Config, checker storage.Checker) (stash.Wrapper,
 		if c.SingleFlight == nil || c.SingleFlight.Redis == nil {
 			return nil, fmt.Errorf("Redis config must be present")
 		}
-		return stash.WithRedisLock(c.SingleFlight.Redis.Endpoint, checker)
+		return stash.WithRedisLock(c.SingleFlight.Redis.Endpoint, c.SingleFlight.Redis.Password, checker)
 	case "gcp":
 		if c.StorageType != "gcp" {
 			return nil, fmt.Errorf("gcp SingleFlight only works with a gcp storage type and not: %v", c.StorageType)

config.dev.toml

@@ -291,6 +291,10 @@ SingleFlightType = "memory"
         # Env override: ATHENS_REDIS_ENDPOINT
         Endpoint = "127.0.0.1:6379"
 
+        # Password is the password for a redis SingleFlight lock.
+        # Env override: ATHENS_REDIS_PASSWORD
+        Password = ""
+
 [Storage]
     # Only storage backends that are specified in Proxy.StorageType are required here
     [Storage.CDN]

docker-compose.yml

@@ -81,6 +81,13 @@ services:
     image: redis
     ports:
       - 6379:6379
+  protectedredis:
+    image: redis
+    ports:
+      - "6380:6380"
+    volumes:
+      - "./test/redis.conf:/usr/local/etc/redis/redis.conf"
+    entrypoint: ["redis-server", "/usr/local/etc/redis/redis.conf"]
   etcd0:
     image: quay.io/coreos/etcd
     ports:

pkg/config/config.go

@@ -163,7 +163,7 @@ func defaultConfig() *Config {
 		RobotsFile:       "robots.txt",
 		SingleFlight: &SingleFlight{
 			Etcd:  &Etcd{"localhost:2379,localhost:22379,localhost:32379"},
-			Redis: &Redis{"127.0.0.1:6379"},
+			Redis: &Redis{"127.0.0.1:6379", ""},
 		},
 	}
 }

pkg/config/singleflight.go

@@ -19,4 +19,5 @@ type Etcd struct {
 // to connect to redis as a SingleFlight implementation.
 type Redis struct {
 	Endpoint string `envconfig:"ATHENS_REDIS_ENDPOINT"`
+	Password string `envconfig:"ATHENS_REDIS_PASSWORD"`
 }

pkg/stash/with_redis.go

@@ -14,11 +14,12 @@ import (
 
 // WithRedisLock returns a distributed singleflight
 // using an redis cluster. If it cannot connect, it will return an error.
-func WithRedisLock(endpoint string, checker storage.Checker) (Wrapper, error) {
+func WithRedisLock(endpoint string, password string, checker storage.Checker) (Wrapper, error) {
 	const op errors.Op = "stash.WithRedisLock"
 	client := redis.NewClient(&redis.Options{
 		Network:  "tcp",
 		Addr:     endpoint,
+		Password: password,
 	})
 	_, err := client.Ping().Result()
 	if err != nil {

pkg/stash/with_redis_test.go

@@ -19,6 +19,7 @@ import (
 // and therefore all 5 responses should have no error.
 func TestWithRedisLock(t *testing.T) {
 	endpoint := os.Getenv("REDIS_TEST_ENDPOINT")
+	password := os.Getenv("ATHENS_REDIS_PASSWORD")
 	if len(endpoint) == 0 {
 		t.SkipNow()
 	}
@@ -27,7 +28,7 @@ func TestWithRedisLock(t *testing.T) {
 		t.Fatal(err)
 	}
 	ms := &mockRedisStasher{strg: strg}
-	wrapper, err := WithRedisLock(endpoint, strg)
+	wrapper, err := WithRedisLock(endpoint, password, strg)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -49,6 +50,63 @@ func TestWithRedisLock(t *testing.T) {
 	}
 }
 
+// Verify with WithRedisLock working with password protected redis
+// Same logic as the TestWithRedisLock test.
+func TestWithRedisLockWithPassword(t *testing.T) {
+	endpoint := os.Getenv("PROTECTED_REDIS_TEST_ENDPOINT")
+	password := os.Getenv("ATHENS_PROTECTED_REDIS_PASSWORD")
+	if len(endpoint) == 0 {
+		t.SkipNow()
+	}
+	strg, err := mem.NewStorage()
+	if err != nil {
+		t.Fatal(err)
+	}
+	ms := &mockRedisStasher{strg: strg}
+	wrapper, err := WithRedisLock(endpoint, password, strg)
+	if err != nil {
+		t.Fatal(err)
+	}
+	s := wrapper(ms)
+
+	var eg errgroup.Group
+	for i := 0; i < 5; i++ {
+		eg.Go(func() error {
+			ctx, cancel := context.WithTimeout(context.Background(), time.Second*10)
+			defer cancel()
+			_, err := s.Stash(ctx, "mod", "ver")
+			return err
+		})
+	}
+
+	err = eg.Wait()
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
+// Verify the WithRedisLock fails with the correct error when trying
+// to connect with the wrong password.
+func TestWithRedisLockWithWrongPassword(t *testing.T) {
+	endpoint := os.Getenv("PROTECTED_REDIS_TEST_ENDPOINT")
+	password := ""
+	if len(endpoint) == 0 {
+		t.SkipNow()
+	}
+	strg, err := mem.NewStorage()
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = WithRedisLock(endpoint, password, strg)
+	if err == nil {
+		t.Fatal("Expected Connection Error")
+	}
+
+	if err.Error() != "NOAUTH Authentication required." {
+		t.Fatalf("Wrong error was thrown %s\n", err.Error())
+	}
+}
+
 // mockRedisStasher is like mockStasher
 // but leverages in memory storage
 // so that redis can determine

test/redis.conf

@@ -0,0 +1,3 @@
+# protectedredis testing configuration file.
+port 6380
+requirepass AthensPass1