Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat:support redis username #76

Merged
merged 11 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 22 additions & 7 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,34 @@ jobs:
- name: Checkout
uses: actions/checkout@v4

- name: Start containers
run: docker-compose -f "docker-compose.yml" up -d --build

- name: Install node
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}

- name: install redis-cli
run: sudo apt-get install redis-tools

- name: Install dependencies
run: npm install

- name: Run tests
run: npm run test
- name: Setup Standalone Tests
run: make test-standalone-setup

- name: Run Standalone tests
run: make test-standalone

- name: Teardown Standalone Tests
run: make test-standalone-teardown

- name: Setup Clustered Tests
run: make test-cluster-setup

- name: Check Redis Cluster
run: timeout 60 bash <<< "until redis-cli -c -p 16371 cluster info | grep 'cluster_state:ok'; do sleep 1; done"

- name: Run Clustered tests
run: make test-cluster

- name: Stop containers
run: docker-compose -f "docker-compose.yml" down
- name: Teardown Clustered Tests
run: make test-cluster-teardown
22 changes: 22 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
export CLUSTER_NO_TLS_VALIDATION=true

test-standalone-setup:
docker-compose up -d

test-standalone-teardown:
docker-compose down

test-cluster-setup:
docker-compose -f docker-compose-cluster.yml up -d

test-cluster-teardown:
docker-compose -f docker-compose-cluster.yml down

test-cluster:
npm run test-cluster
test-standalone:
npm run test-standalone

test: test-standalone test-cluster
test-setup: test-standalone-setup test-cluster-setup
test-teardown: test-standalone-teardown test-cluster-teardown
37 changes: 36 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,30 @@
`limitd-redis` is client for limits on top of `redis` using [Token Buckets](https://en.wikipedia.org/wiki/Token_bucket).
It's a fork from [LimitDB](https://github.com/limitd/limitdb).

## Table of Contents
pubalokta marked this conversation as resolved.
Show resolved Hide resolved

- [Installation](#installation)
- [Configure](#configure)
- [Options available](#options-available)
- [Buckets](#buckets)
- [Ping](#ping)
- [Overrides](#overrides)
- [ERL (Elevated Rate Limits)](#erl-elevated-rate-limits)
- [Prerequisites](#prerequisites)
- [Introduction](#introduction)
- [Configuration](#configuration)
- [ERL Quota](#erl-quota)
- [Use of Redis hash tags](#use-of-redis-hash-tags)
- [Breaking changes from `Limitdb`](#breaking-changes-from-limitdb)
- [TAKE](#take)
- [TAKEELEVATED](#takeelevated)
- [PUT](#put)
- [Overriding Configuration at Runtime](#overriding-configuration-at-runtime)
- [Overriding Configuration at Runtime with ERL](#overriding-configuration-at-runtime-with-erl)
- [Testing](#testing)
- [Author](#author)
- [License](#license)

## Installation

```
Expand Down Expand Up @@ -34,7 +58,9 @@ const limitd = new Limitd({
interval: 1000,
maxFailedAttempts: 5,
reconnectIfFailed: true
}
},
username: 'username',
password: 'password'
});
```

Expand All @@ -45,6 +71,8 @@ const limitd = new Limitd({
- `buckets` (object): Setup your bucket types.
- `prefix` (string): Prefix keys in Redis.
- `ping` (object): Configure ping to Redis DB.
- `username` (string): Redis username. Only to be used in Cluster mode. Needs Redis >= v6.
pubalokta marked this conversation as resolved.
Show resolved Hide resolved
- `password` (string): Redis password.

### Buckets:

Expand Down Expand Up @@ -345,6 +373,13 @@ const configOverride = {
}
```

## Testing

- Setup tests: `make test-setup`
- Run tests: `make test`
- Teardown tests: `make test-teardown`


## Author

[Auth0](auth0.com)
Expand Down
43 changes: 43 additions & 0 deletions docker-compose-cluster.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
services:
redis-1:
image: 'redis:6'
healthcheck:
interval: "1s"
test: [ "CMD", "redis-cli", "-p", "16371", "ping", "|", "grep", "PONG" ]
command: ["redis-server", "/etc/redis/redis.conf"]
volumes:
- ${PWD}/test-resources/redis-cluster/node-1/conf/redis.conf:/etc/redis/redis.conf
network_mode: host
redis-2:
image: 'redis:6'
healthcheck:
interval: "1s"
test: [ "CMD", "redis-cli", "-p", "16372", "ping", "|", "grep", "PONG" ]
command: [ "redis-server", "/etc/redis/redis.conf" ]
volumes:
- ${PWD}/test-resources/redis-cluster/node-2/conf/redis.conf:/etc/redis/redis.conf
network_mode: host
redis-3:
image: 'redis:6'
healthcheck:
interval: "1s"
test: [ "CMD", "redis-cli", "-p", "16373", "ping", "|", "grep", "PONG" ]
command: [ "redis-server", "/etc/redis/redis.conf" ]
volumes:
- ${PWD}/test-resources/redis-cluster/node-3/conf/redis.conf:/etc/redis/redis.conf
network_mode: host
redis-cluster-create:
image: 'redis:6'
command: '/usr/local/etc/redis/redis-cluster-create.sh'
depends_on:
redis-1:
condition: service_healthy
redis-2:
condition: service_healthy
redis-3:
condition: service_healthy
volumes:
- ${PWD}/test-resources/redis-cluster/redis-cluster-create.sh:/usr/local/etc/redis/redis-cluster-create.sh
network_mode: host
healthcheck:
test: ["CMD-SHELL", "redis-cli -p 16371 -c cluster info | grep cluster_state:ok"]
2 changes: 1 addition & 1 deletion lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class LimitdRedis extends EventEmitter {

this.db = new LimitDBRedis(_.pick(params, [
'uri', 'nodes', 'buckets', 'prefix', 'slotsRefreshTimeout', 'slotsRefreshInterval',
'password', 'tls', 'dnsLookup', 'globalTTL', 'cacheSize', 'ping']));
'username', 'password', 'tls', 'dnsLookup', 'globalTTL', 'cacheSize', 'ping']));

this.db.on('error', (err) => {
this.emit('error', err);
Expand Down
3 changes: 3 additions & 0 deletions lib/db.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ class LimitDBRedis extends EventEmitter {

this.redis = null;
if (config.nodes) {
if (config.username) {
clusterOptions.redisOptions.username = config.username;
}
this.redis = new Redis.Cluster(config.nodes, clusterOptions);
} else {
this.redis = new Redis(config.uri, redisOptions);
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
"url": "http://github.com/auth0/limitd-redis.git"
},
"scripts": {
"test": "trap 'docker-compose down --remove-orphans -v' EXIT; docker-compose up -d && NODE_ENV=test nyc mocha --exit"
"test-standalone": "CLUSTERED_ENV=false NODE_ENV=test nyc mocha --exit",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a huge fan of this way of running a subset of tests.

I think a better way would be to use mocha to filter for specific test file names. Mocha accepts a glob pattern for test files.

Not tested, but something like:

Suggested change
"test-standalone": "CLUSTERED_ENV=false NODE_ENV=test nyc mocha --exit",
"test-standalone": "NODE_ENV=test nyc mocha --exit '!**/*.clustermode.tests.js,**/*.standalone.tests.js'",

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Did it a bit different but same idea. Thanks for the suggestion

"test-cluster": "CLUSTERED_ENV=true NODE_ENV=test nyc mocha --exit"
},
"author": "Auth0",
"license": "MIT",
Expand Down
10 changes: 10 additions & 0 deletions test-resources/redis-cluster/node-1/conf/redis.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
port 16371
bind 0.0.0.0
cluster-enabled yes
cluster-config-file nodes.conf
cluster-node-timeout 5000
cluster-announce-ip 0.0.0.0
cluster-announce-port 16371
cluster-announce-bus-port 26371
appendonly yes
user testuser on >testpass ~* +@all
10 changes: 10 additions & 0 deletions test-resources/redis-cluster/node-2/conf/redis.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
port 16372
bind 0.0.0.0
cluster-enabled yes
cluster-config-file nodes.conf
cluster-node-timeout 5000
cluster-announce-ip 0.0.0.0
cluster-announce-port 16372
cluster-announce-bus-port 26372
appendonly yes
user testuser on >testpass ~* +@all
10 changes: 10 additions & 0 deletions test-resources/redis-cluster/node-3/conf/redis.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
port 16373
bind 0.0.0.0
cluster-enabled yes
cluster-config-file nodes.conf
cluster-node-timeout 5000
cluster-announce-ip 0.0.0.0
cluster-announce-port 16373
cluster-announce-bus-port 26373
appendonly yes
user testuser on >testpass ~* +@all
5 changes: 5 additions & 0 deletions test-resources/redis-cluster/redis-cluster-create.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
node_1_ip="0.0.0.0"
node_2_ip="0.0.0.0"
node_3_ip="0.0.0.0"

redis-cli --cluster create $node_1_ip:16371 $node_2_ip:16372 $node_3_ip:16373 --cluster-replicas 0 --cluster-yes
79 changes: 60 additions & 19 deletions test/client.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,31 @@ const _ = require('lodash');
const assert = require('chai').assert;
const LimitRedis = require('../lib/client');
const ValidationError = LimitRedis.ValidationError;
const clusterNodes = [{ host: '127.0.0.1', port: 16371 }, { host: '127.0.0.1', port: 16372 }, { host: '127.0.0.1', port: 16373 }];

describe('LimitdRedis', () => {
const standaloneClientFn = (params) => {
return new LimitRedis({ uri: 'localhost', buckets: {}, prefix: 'tests:', ..._.omit(params, ['nodes']) });
};
const clusteredClientFn = (params) => {
return new LimitRedis({ nodes: clusterNodes, buckets: {}, prefix: 'tests:', ..._.omit(params, ['uri']) });
};
let clientCreator = standaloneClientFn;
let clusteredEnv = false;

if (process.env.CLUSTERED_ENV && process.env.CLUSTERED_ENV === "true") {
pubalokta marked this conversation as resolved.
Show resolved Hide resolved
clusteredEnv = true;
clientCreator = clusteredClientFn;
}

describe("LimitdRedis", () => {
let client;
beforeEach((done) => {
client = new LimitRedis({ uri: 'localhost', buckets: {}, prefix: 'tests:' });
client = clientCreator();
client.on('error', done);
client.on('ready', done);
});

describe('#constructor', () => {
it('should call error if db fails', (done) => {
let called = false; // avoid uncaught
client = new LimitRedis({ uri: 'localhost:fail', buckets: {} });
client.on('error', () => {
if (!called) {
called = true;
return done();
}
});
});

it('should set up retry and circuitbreaker defaults', () => {
assert.equal(client.retryOpts.retries, 3);
assert.equal(client.retryOpts.minTimeout, 10);
Expand All @@ -37,14 +41,51 @@ describe('LimitdRedis', () => {
});

it('should accept circuitbreaker parameters', () => {
client = new LimitRedis({ uri: 'localhost', buckets: {}, circuitbreaker: { onTrip: () => {} } });
client = clientCreator({ circuitbreaker: { onTrip: () => {} } });
assert.ok(client.breakerOpts.onTrip);
});

it('should accept retry parameters', () => {
client = new LimitRedis({ uri: 'localhost', buckets: {}, retry: { retries: 5 } });
assert.equa;(client.retryOpts.retries, 5);
client = clientCreator({ retry: { retries: 5 } });
assert.equal(client.retryOpts.retries, 5);
});

if(!clusteredEnv) {
describe('when using the standalone #constructor', () => {
// in cluster mode, ioredis doesn't fail when given a bad node address, it keeps retrying
it('should call error if db fails', (done) => {
let called = false; // avoid uncaught
client = clientCreator({ uri: 'localhost:fail' });
client.on('error', () => {
if (!called) {
called = true;
return done();
}
});
});
});
} else {
describe('when using the clustered #constructor', () => {
it('should allow setting username and password', (done) => {
let client = clientCreator({ username: 'testuser', password: 'testpass' });
client.on('ready', () => {
client.db.redis.acl("WHOAMI", (err, res) => {
assert.equal(res, 'testuser');
done();
})
});
});
it('should use the default user if no one is provided', (done) => {
let client = clientCreator();
client.on('ready', () => {
client.db.redis.acl("WHOAMI", (err, res) => {
assert.equal(res, 'default');
done();
})
});
});
});
}
});

describe('#handler', () => {
Expand All @@ -69,7 +110,7 @@ describe('LimitdRedis', () => {
client.handler('take', 'test', 'test', done);
});
it('should not retry or circuitbreak on ValidationError', (done) => {
client = new LimitRedis({ uri: 'localhost', buckets: {}, circuitbreaker: { maxFailures: 3, onTrip: () => {} } });
client = clientCreator({ circuitbreaker: { maxFailures: 3, onTrip: () => {} } });
client.db.take = (params, cb) => {
return cb(new ValidationError('invalid config'));
};
Expand Down Expand Up @@ -109,7 +150,7 @@ describe('LimitdRedis', () => {
client.handler('take', 'test', 'test', done);
});
it('should circuitbreak', (done) => {
client = new LimitRedis({ uri: 'localhost', buckets: {}, circuitbreaker: { maxFailures: 3, onTrip: () => {} } });
client = clientCreator({ circuitbreaker: { maxFailures: 3, onTrip: () => {} } });
client.db.take = () => {};
client.handler('take', 'test', 'test', _.noop);
client.handler('take', 'test', 'test', _.noop);
Expand Down Expand Up @@ -205,4 +246,4 @@ describe('LimitdRedis', () => {
});
});
});
});
}, 'LimitdRedis');
Loading