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

Add support for environment variables #39

Open
Bramzor opened this issue Oct 4, 2017 · 25 comments
Open

Add support for environment variables #39

Bramzor opened this issue Oct 4, 2017 · 25 comments

Comments

@Bramzor
Copy link

Bramzor commented Oct 4, 2017

As best practices today is to use environment variables as config parameters, it would make sense to use it for environment based variables.
For example in my use case, I want to have rcpt_to.in_host_list plugin to check for environment variable to be set if there is no file. So kind of a fall back method.

@smfreegard
Copy link

That doesn't make sense - how can you possible replace the host_list file (which contains a list of domains) with an environment variable?

@Bramzor
Copy link
Author

Bramzor commented Oct 4, 2017

Easy... by setting HARAKA_HOST_LIST = "domain1.com domain2.com domain3.com"

@KingNoosh
Copy link
Member

Linked to haraka/Haraka#2098

@ghost
Copy link

ghost commented Oct 4, 2017

Good point @Bramzor you can have even an array environments in BASH nowadays.

@KingNoosh
Copy link
Member

I think what would be easier is to allow environment variables to be used within the config files.

Less complicated to get working and easier to use.

@Bramzor
Copy link
Author

Bramzor commented Oct 4, 2017

+1 for environment variables inside config files

@ghost
Copy link

ghost commented Oct 4, 2017

I was thinking the ability to override variables for example from smtp.ini.
Example (notice the __ separator)

HARAKA__SMTP_INI__LISTEN="[::0]:2525"

@KingNoosh
Copy link
Member

https://gist.github.com/KingNoosh/4461e2cacff887d55604d149e06745f0

Is what we have atm, I can easily take this since I already have this.

@Bramzor
Copy link
Author

Bramzor commented Oct 4, 2017

Advantage of using environment variables inside config files is that you still keep one place where this is defined instead of having multiple places which might easily be overlooked.

@msimerson
Copy link
Member

by setting HARAKA_HOST_LIST = "domain1.com domain2.com domain3.com"

another way is using URIs:

HARAKA_HOST_LIST="redis://redis.example.com:6179/0/host_list" or similar.
HARAKA_HOST_LIST="mysql://mysql.example.com/haraka/host_list" or similar.

@ghost
Copy link

ghost commented Oct 5, 2017

@msimerson love the idea of fetching stuff directly from DB! How about passwords? inline, config or another env?

@smfreegard
Copy link

@msimerson love the idea of fetching stuff directly from DB!

Unfortunately being as config is a synchronous API, getting config from a database like this is impossible unless it's only ever read at start-up.

@ghost
Copy link

ghost commented Oct 5, 2017

@smfreegard maybe cron (timer or something) like wrapper to refetch specific configs in master?

@smfreegard
Copy link

@smfreegard maybe cron like wrapper to refetch specific configs in master?

Yeah - that could work as I do something similar here - config gets read from the database in init_master and init_child and then an interval is set to re-read the database for only changes and deletions (I have a deleted flag and last_update timestamp in the database table that is set by triggers to do this).

However - you either need to use something that supports database triggers or pub/sub for it to work without requiring a complete re-read of the whole table on each interval, so PostgreSQL (like in my case) or Redis. I don't think MySQL would work as IIRC it doesn't have triggers like PostgreSQL.

It would need a fair amount of thought put into it for this to work generically for everyone that uses Haraka and not something that just works for a specific case.

Redis pub/sub seems like a sensible idea being as we already support Redis in one way or another. Redis keys would work fine for simple values, Hashes for INI/JSON type files (via dot notation) and Lists for List/Data types. However for list types, I don't think you can subscribe to see individual elements (but I might be wrong), so you might have to re-read the whole list - which wouldn't be ideal.

It's do-able, but I guess it would require a way to add plugins to config.

@ghost
Copy link

ghost commented Oct 5, 2017

@smfreegard there actually are some triggers in MySQL:

CREATE     
[DEFINER = { user | CURRENT_USER }]     
TRIGGER trigger_name     
trigger_time trigger_event     
ON tbl_name FOR EACH ROW     
trigger_body
trigger_time: { BEFORE | AFTER } 
trigger_event: { INSERT | UPDATE | DELETE }

Maybe introduce some new event to trigger the update into the API? So plugins could also listen for it.
Redis would be neat because it is pretty fast from my experience.

@smfreegard
Copy link

@smfreegard there actually are some triggers in MySQL:

They're a bit too basic. e.g. you need to be able to intercept a DELETE and either UPDATE the deleted flag or allow the deletion to complete if it's already set.

Maybe introduce some new event to trigger the update into the API? So plugins could also listen for it.
Redis would be neat because it is pretty fast from my experience.

Start a page on the Wiki or something to brainstorm any ideas or start a new Issue etc.

@ghost
Copy link

ghost commented Oct 5, 2017

@smfreegard

They're a bit too basic. e.g. you need to be able to intercept a DELETE and either UPDATE the deleted flag or allow the deletion to complete if it's already set.

Oh ok, I have only used some SET procedures on INSERT and UPDATE so far. So I have not faced this advanced issue yet.

Start a page on the Wiki or something to brainstorm any ideas or start a new Issue etc.

Yeah, I will probably leave that on someone more experienced. I was really throwing in my two cents here. 😄

@Bramzor
Copy link
Author

Bramzor commented Oct 5, 2017

We'll keep it simple for now and use the environments variables inside configs. Anosh already has this figured out anyway 👍

@jkroepke
Copy link

This patch wont work anymore

@celesteking
Copy link

Be extremely careful with these kind of changes. Using ENV vars is a security disaster.
Bringing expansion in will multiply the issue. (Unless there's someone very smart implementing those.)

@jkroepke
Copy link

jkroepke commented May 7, 2020

Using ENV vars is a security disaster.

Explain why

@jchook
Copy link

jchook commented Jun 30, 2020

Why not allow JavaScript configuration files, which would have access to process.env etc?

Edit: Looks like haraka-config does have support for JS.

@celesteking
Copy link

Using ENV vars is a security disaster.

Explain why

https://httpoxy.org/

I stay corrected, Using ENV vars *improperly* is a security disaster. And I'd like to repeat myself: Unless there's someone very smart implementing those..

Smart people are welcome with patches.

@jkroepke
Copy link

jkroepke commented Mar 2, 2021

I got a full understanding of https://httpoxy.org/ (I know it before).

But this issue describe a problem in cgi applications, not a problem with environment variables itself.

Please get in detail what is your understanding of a very smart implementation.

@taobojlen
Copy link

I'm using .js config files to read configuration via environment variables, and it works pretty well.

The downside is that many/most of Haraka's configuration values have hard-coded filenames, e.g. config.get('host_list', 'list'). In order to use environment variables, you have to fork the plugins and change this to config.get('host_list.js', 'js').

This is fine for plugins, but it becomes more of a challenge when you want to use a .js file to configure e.g. the hostname that's usually stored in the me file. You can't configure this via environment variables without forking Haraka.

A potential solution: in the config get function, always check if the given file exists with a .js extension and use that if so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants