feat: allow to use environment variables for limit-count plugin settings#10607
Conversation
ikatlinsky
changed the title
|
@ikatlinsky thank you for your contribution 👍 |
| type = "boolean", default = false, | ||
| }, | ||
| }, | ||
| encrypt_fields = {"redis_username", "redis_password"}, |
There was a problem hiding this comment.
Does the redis_username need to be encrypted?
There was a problem hiding this comment.
actually, not really, will fix that
| "count": 2, | ||
| "time_window": 60, | ||
| "rejected_code": 503, | ||
| "key": "$ENV://LIMIT_COUNT_KEY" |
There was a problem hiding this comment.
same reasoning as above
juststillthinking
added
the
wait for update
| type = "boolean", default = false, | ||
| }, | ||
| }, | ||
| encrypt_fields = {"redis_password"}, |
There was a problem hiding this comment.
I think this is not correct, please refer to https://github.com/apache/apisix/blob/master/apisix/plugins/kafka-proxy.lua#L36, and add a corresponding test case
There was a problem hiding this comment.
your are correct, that i am doing 2 different things in a single PR, let me ever this change
|
@monkeyDluffy6017 looking at failing tests (https://github.com/apache/apisix/actions/runs/7124716224/job/19435705333?pr=10607) and it seems like tests for multiple plugins are failed, like request id, openwisk and etc. It that something that is expected? Not sure if my changes can break other plugins. Thanks |
|
@ikatlinsky It has nothing to do with you changes |
juststillthinking
added
approved
and removed
wait for update
juststillthinking
left a comment
juststillthinking
left a comment
There was a problem hiding this comment.
please resolve the conflicts
done |
apache#9247 apache#10353 apache#10607 apache#11095 --------- Signed-off-by: Nic <[email protected]> Co-authored-by: Nic <[email protected]>
5 participants
Description
As per #10597.
Fixes #10597
Checklist