haraka / haraka-plugin-redis

redis support for Haraka plugins
https://www.npmjs.com/package/haraka-plugin-redis
MIT License
3 stars 12 forks source link

Haraka main redis database instance is hardcoded to database 0 #18

Closed chlarsen closed 5 years ago

chlarsen commented 5 years ago

system info

Haraka: 2.8.24 Node: 11.13.0 OS: FreeBSD mail 12.0-RELEASE-p3 FreeBSD 12.0-RELEASE-p3 GENERIC amd64 OpenSSL: OpenSSL 1.1.1a-freebsd 20 Nov 2018

Expected behavior

The main Redis database, as set in redis.ini (default stanza: "db=0") should accept any value between 0 and 16.

Observed behavior

It seems that the main Redis database, as set in redis.ini (stanza: "db=0") cannot be changed to anything else.

Steps to reproduce

Trying to set a different database number seems to be ignored.

Am I missing something? Thank you

Chris

ad-m commented 5 years ago

What do you mena by redis.ini? There is a few seperate Redis configuration: https://github.com/haraka/Haraka/search?q=redis+path%3Aconfig%2F&unscoped_q=redis+path%3Aconfig%2F&type=Code

chlarsen commented 5 years ago

In haraka-plugin-redis/README.md, oit says:

The `redis.ini` file has the following sections (defaults shown):

### [server]

    ; host=127.0.0.1
    ; port=6379
    ; db=0

### [pubsub]

    ; host=127.0.0.1
    ; port=6379

Publish & Subscribe are DB agnostic and thus have no db setting. If host and port and not defined, they default to the same as [server] settings.

This is what I am referring to. My redis.ini looks like this:

[server]
;host=127.0.0.1
host=broker.jail.vlan
;port=6379
; presently, the haraka redis db is hardcoded to 0
;db=0

See my comment above :-). I do realise that the other plugins use other Redis databases, and they can be configured, indeed. Seems I am misunderstanding the README.md, but how to read it then? Thank you! Chris

msimerson commented 5 years ago

With this config, you still haven't specified a db. If you remove the comment character ; from in front of the db line (as shown below), it might work more like you expect.

[server]
host=broker.jail.vlan
db=5
chlarsen commented 5 years ago

Isn't the rule of configuration that defaults are taken from the, well, default config, and user *.ini files are only used to OVERRIDE defaults? When I state a different database, nothing happens; db 0 is still used. This is the issue here, at least from my end.

msimerson commented 5 years ago

Isn't the rule of configuration that defaults are taken from the, well, default config,

Yes. In this case, there isn't a default redis.ini file. The defaults are set inside the plugin.

used to OVERRIDE defaults

Yes.

When I state a different database, nothing happens;

Well then, presuming your have your db line NOT commented out (contrary to what you showed above), then that's a bug. Your settings are supposed to be honored.

chlarsen commented 5 years ago

Yep, this is what I have been trying to point out :-). I set, for instance: db=10 An get: [INFO] [-] [redis] connected to redis://broker.jail.vlan:6379 No db is stated, and Redis tells me that db 0 is used.

msimerson commented 5 years ago

I think I see the problem. Try setting db=10 in the [opts] section.

msimerson commented 5 years ago

PS: there's no hard coding anywhere. It's just that your config setting wasn't getting passed so you got the redis default db.

chlarsen commented 5 years ago

I tried it, but may not have set the [opts] section properly. Would you mind spinning an example with the right sections for redis.ini? Thank you!

msimerson commented 5 years ago

There's one included in PR #20.

here

chlarsen commented 5 years ago

Thank you! This worked for the redis.ini config file, both for the localised db number, as well as the addition of a password. However, I seem to be out of luck, when I want to add the same password value to the redis stanzas for limits, karma, etc. [opts] does not do the trick, keeping in mind that localised database numbers DO work for those sub-config files.

chlarsen commented 5 years ago

Allow me to add that dnsbl.ini does not seem to have an configuration settings for redis except host and port; [opts] does not seem to work there?

msimerson commented 5 years ago

In plugins, put all the options in the [redis] block. Example:

karma.ini

; other stuff
[redis]
host=redis.example.com
db=6
password=SeriouslyYou'reUsingThis?

That should work.

chlarsen commented 5 years ago

I finally had time to test this. I think the stumbling block is the dnsbl module with history enabled. The Redis server config syntax there is a bit different from the rest of the Redis-enabled modules, and I do not seem to get the Redis password across, neither using [redis] nor [opts]. Thank you!

msimerson commented 5 years ago

The DNSBL module(s) predate the redis plugin by quite a long time and it hasn't been updated. It likely just needs to be retrofitted with the shared calls the other "redis using" plugins load.

chlarsen commented 5 years ago

Yes, this was my feeling, too. Can we leave it on our (sic!) ToDo list, so it won't be forgotten?

msimerson commented 5 years ago

That's why it's a wiki, anyone can add to it. Better yet, a PR is welcome.

chlarsen commented 5 years ago

Exactly :-D. Thanks a lot and enjoy your weekend.