feat: limit-count plugin with redis cluster support tls/ssl #8558
Conversation
t/plugin/limit-count-redis-cluster.t
Outdated
| } | ||
| --- error_code: 400 | ||
| --- error_log | ||
| Expected comma or object end but found T_STRING |
There was a problem hiding this comment.
This error message seems from a JSON parsing error, is it expected?
There was a problem hiding this comment.
Yes, i send a error type(string) cause check schema err.
There was a problem hiding this comment.
The check schema err isn't like this. Please check if the provided data is valid JSON.
t/plugin/limit-count-redis-cluster.t
Outdated
|
|
||
|
|
||
|
|
||
| === TEST 14: enable degradation switch for TEST 5 |
t/plugin/limit-count-redis-cluster.t
Outdated
|
|
||
|
|
||
|
|
||
| === TEST 17: up the limit |
There was a problem hiding this comment.
Why do we need TEST 16 as we have TEST 17?
There was a problem hiding this comment.
had changed it.
ci/pod/docker-compose.common.yml
Outdated
| @@ -102,3 +102,103 @@ services: | |||
| VAULT_DEV_ROOT_TOKEN_ID: root | |||
| VAULT_DEV_LISTEN_ADDRESS: 0.0.0.0:8200 | |||
| command: [ "vault", "server", "-dev" ] | |||
|
|
|||
|
|
|||
| ## RedisCluster Enable TLS | |||
There was a problem hiding this comment.
Let's move the redis cluster to https://github.com/apache/apisix/blob/master/ci/pod/docker-compose.plugin.yml, as this dependency is only used in the plugin test.
ci/pod/docker-compose.common.yml
Outdated
| - ./t/certs:/certs | ||
| environment: | ||
| - 'ALLOW_EMPTY_PASSWORD=yes' | ||
| - 'REDIS_NODES=redis-node-0 redis-node-1 redis-node-2 redis-node-3 redis-node-4 redis-node-5' |
There was a problem hiding this comment.
Could we reduce the number of redis nodes used in the CI?
There was a problem hiding this comment.
done. for a redis cluster, it may need at least 3 master nodes, so i reduce to 3 nodes.
ci/pod/docker-compose.common.yml
Outdated
| - 'ALLOW_EMPTY_PASSWORD=yes' | ||
| - 'REDIS_NODES=redis-node-0 redis-node-1 redis-node-2 redis-node-3 redis-node-4 redis-node-5' | ||
| - 'REDIS_TLS_ENABLED=yes' | ||
| - 'REDIS_TLS_CERT_FILE=/certs/redis.crt' |
There was a problem hiding this comment.
Maybe we can use the existing certificate under ./t/certs so there is no need to manage new certs
There was a problem hiding this comment.
done. i use t/certs/mtls_ca.crt instead.
ci/pod/docker-compose.plugin.yml
Outdated
| - '7000:6379' | ||
|
|
||
| ## RedisCluster Enable TLS | ||
| redis-node-0: |
There was a problem hiding this comment.
There are two redis-node-0?
There was a problem hiding this comment.
sorry, i will remove it.
t/plugin/limit-count-redis-cluster.t
Outdated
| } | ||
| --- error_code: 400 | ||
| --- error_log | ||
| Expected comma or object end but found T_STRING |
There was a problem hiding this comment.
The check schema err isn't like this. Please check if the provided data is valid JSON.
|
Will this be backported to 2.X? |
|
@tao12345666333 |
Description
Fixes #8413
i deploy a new redis-cluster enable tls with docker-compose, and add connect_opts to redis-cluster config.
Checklist