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

Any way we can configure the connection through an async function? #3340

Closed
Ali-Dalal opened this issue Jul 9, 2019 · 11 comments
Closed

Any way we can configure the connection through an async function? #3340

Ali-Dalal opened this issue Jul 9, 2019 · 11 comments

Comments

@Ali-Dalal
Copy link

So, we have the db config stored in AWS secret manager. and we use knex with serverless labmda.

so the question is is it possible to load the config as an async function which calls the secret manager rather than setting a simple config object?

    production: {
        client: config.db.production.client,
        connection: async function () {
            // load secrets from secret manager as await function
           // then feed the required values to the return statement.
            return {
                host: secret.db.production.host,
                database:
                secret.db.production.name,
                user:
                secret.db.production.user,
                password:
                secret.db.production.password
            };
        },
        pool: {
            min: Number(config.db.production.min_pool) || 0,
            max: Number(config.db.staging.max_pool) || 1
        },
        migrations: {
            tableName: 'knex_migrations'
        }
    }
@kibertoad
Copy link
Collaborator

This is not a currently supported feature, similar (but somewhat different) feature was requested here: #2732

API proposal and (after proposal is approved) PR for this would be welcome.

@elhigu
Copy link
Member

elhigu commented Jul 9, 2019

I think this API is good (function can be also non async) and it could be specified in a way that as long as it returns the same configuration object === knex will not try to reinitialize anything and if configuration object is different, then knex internally tries to create new connections to new destination.. (at this phase if configuration object changes we may just throw an error).

Later on if it is needed we could also allow fetching the whole knex configuration object in async manner.

@Ali-Dalal
Copy link
Author

@elhigu Thanks for the explanation.

I read some of the comments you and other people mention and suggest.

for example, to load the config to a file via prescript and then load that file into knex.

The problem with that approach is security wise. the entire idea of loading the config via AWS secret manager is to keep the variables in memory. so no direct access via code files.

I tried to do async module export and load but the code gets really ugly.

what cause the issue to be really painful is I use bookshelf in the project and if there is a relation between two tables and we need to reflect the relation via bookshelf models then it can't be done via async require.

I will try to dig in knex code base and I will be really happy If I can implement that feature.

I already stuck into this problem since 4 days :(

@elhigu
Copy link
Member

elhigu commented Jul 9, 2019

Lets leave this opened until someone takes this over :) (I already closed those older issues about this same feature request).

@elhigu elhigu reopened this Jul 9, 2019
@Ali-Dalal
Copy link
Author

Hi @elhigu ,
hope you are doing well.

I try to modify some part of the code and I come up with a solution.

The solution allows the connection to be entered as async function

config: {
connection: async function(){/*...*/}
/*rest of config*/
}

The files I modifed are:

  1. client.js
  2. dialects/postgres/index.js

what I did was to check if the type of config is a function (in client.js) then don't do anything, just pass the config to dialects.

in dialects, I am checking if the type of config is function, then execute the function (with await and async to the top function) and load the config from the result.

//client.js
if(typeof config.connection == 'function')
        this.connectionSettings = config.connection
//postgres/index.js
acquireRawConnection() {
    const client = this;
    return new Bluebird(async function(resolver, rejecter) {
      if (typeof client.connectionSettings === 'function') {
                client.connectionSettings = await client.connectionSettings();
            }
      const connection = new client.driver.Client(client.connectionSettings);

If you agree of the way to implement this, then I can configure the rest of dialects and submit PR

@kibertoad
Copy link
Collaborator

Could you create WIP PR with changes so far? Would be easier to discuss.

@Ali-Dalal
Copy link
Author

@kibertoad I will try this weekend to write a POC code to support only one adapter (postgres).
If looks good then I can extend to support all the adapters

@mbelles
Copy link

mbelles commented Jul 16, 2019

I am looking for a solution to this as well. Cannot +1 this idea enough. Thanks!

@Ali-Dalal
Copy link
Author

@mbelles I will try to write a PR this weekend

@Ali-Dalal
Copy link
Author

Ali-Dalal commented Jul 20, 2019

@elhigu @kibertoad Can you please give a resolution on what I've done?

Also, I am a bit lost where to write the tests for the new connection config. (guide me please)

if you are ok with the PG client, then I will write the code to support other clients

example:

//...settings
connection: async function () {
                let secret = await functionToGetDBConfigFromAnyWhere(); //maybe SSM
                let connection = JSON.parse(secret);
                return {
                    host: connection.host,
                    user: connection.username,
                    password: connection.password,
                    database: connection.db_name
                };
            }
//...rest of the settings

@kibertoad
Copy link
Collaborator

Released in 0.20.1

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

4 participants