Skip to content
This repository has been archived by the owner on Dec 10, 2022. It is now read-only.

auto-configuration detection by default #37

Open
kares opened this issue Apr 20, 2015 · 4 comments
Open

auto-configuration detection by default #37

kares opened this issue Apr 20, 2015 · 4 comments

Comments

@kares
Copy link
Contributor

kares commented Apr 20, 2015

looking at commits reminded me that I wanted the plugin's .yml to go through erb (just like in Rails)

@pierre added it already 336b0aa ... so I was thinking about getting it to the 'next' level - generating a default yaml file that auto-detects whether it is running inside KB :

currently on kares-performance branch I'm actually able to use something like :

:stripe:
  :api_secret_key: <%= ENV['API_SECRET_KEY'] %>
  :api_publishable_key: <%= ENV['API_PUBLISHABLE_KEY'] %>
  :test: true

<% if defined? JRUBY_VERSION &&
      (Java::JavaClass.for_name('org.killbill.billing.osgi.bundles.jruby.JRubyPlugin') rescue false) %>
:database:
# In Kill Bill
  :adapter: mysql
  :jndi: 'killbill/osgi/jdbc'
  :pool: false # false-pool (JNDI pool's max)
  # uncomment if pool does not support JDBC4 :
  #:connection_alive_sql: 'select 1'
  # MySQL adapter #configure_connection defaults :
  #    @@SESSION.sql_auto_is_null = 0,
  #    @@SESSION.wait_timeout = 2147483,
  #    @@SESSION.sql_mode = 'STRICT_ALL_TABLES'
  # ... can be disabled (on AR-JDBC 1.4) using :
  :configure_connection: false
<% else %>
:database:
# SQLite (development)
  :adapter: sqlite3
  :database: test.db
# For MySQL
  :adapter: mysql
  :username: root
  :database: 'killbill' # or set the URL :
  :driver: org.mariadb.jdbc.Driver # as in KB
  :pool: 30 # AR's default is max 5 connections
<% end %>

... what this lucks is a simple way of detecting KB - e.g. defined? KILLBILL_SERVER_VERSION or exporting the KillbillServerConfig in a global constant ~ defined? KILLBILL_CONFIG (such a constant could be set before plugins are started) from JRubyPlugin. or one could simply use ENV I'm really not sure about what the "officlal" (best-approach) for the next KB could be ...

p.s. just so I do not forget - tuning erb to work with <%- could be useful as well ...

@pierre
Copy link
Member

pierre commented Apr 20, 2015

Some background about configuration:

  • the yml file isn't required anymore when running in multi-tenant mode (default), as the configuration can be picked up from the database (see curl example in the Stripe README for instance)
  • sane defaults are now pre-configured when running inside Kill Bill for the jdbc connection
  • it is unfortunately still required for production deployments, regardless of tenancy
  • it is (of course) still required to run tests outside of Kill Bill, and I had allowed ERB for this usecase (we run integration tests on Travis now, passing gateway credentials as secure environment variables)

About this ticket:

  • 👍 for defining a global constant whether we are running inside Kill Bill (we have a similar use-case today around JBunder)
  • I also would love to streamline the database configuration for tests:
    • I haven't figured out a good way to switch between DB drivers for Travis for instance (and this is going to be important as we'll have support for PostgreSQL soon)
    • The overall configuration is a mess: the generator has to generate it today which seems unnecessary, and even worse it is not DRY

@kares
Copy link
Contributor Author

kares commented Apr 21, 2015

ah - cool, missed the "sane defaults" - before changing the ticket's concerns I'd like to understand some:

it is unfortunately still required for production deployments, regardless of tenancy

not sure what do you mean by "unfortunately" :) ... you need a place to configure GW credentials or ?
... I also wonder how it would look like - if it's possible to use different credentials per tenant.

for defining a global constant whether we are running inside Kill Bill

OK, thus this can probably go into a separate feature request. also I'm not sure what it should be if you'd like me to do it (seems that exporting the server configuration as KILLBILL_CONFIG might turn our useful for plugins but than it might be confusing if there's a plugin configuration as well - really not sure).


back to database configuration - related to tests.
... if the .yml is to stay what if we used it for test database configuration e.g.

:stripe:
  :api_secret_key: <%= ENV['API_SECRET_KEY'] %>
  :api_publishable_key: <%= ENV['API_PUBLISHABLE_KEY'] %>
  :test: true

<% if ENV['KILLBILL_ENV'] == 'test' %>
:database:
  :adapter: <%= ENV['AR_ADAPTER'] || 'mariadb' %>
  :username: <%= ENV['AR_USERNAME'] || 'killbill' %>
  :database: <%= ENV['AR_DATABASE'] || 'killbill' %>
  :pool: 30 # AR's default is max 5 connections
<% end %>
  • spec_helper.rb would default to ENV['KILLBILL_ENV'] ||= 'test'
  • it still needs to be generated (spec_helper.rb would than read the .yml and use it to connect)
  • driver switching should work - e.g. adapter: postresql will try to load 'jdbc/postres' (just need to make sure they're listed as development gem dependencies)

@pierre
Copy link
Member

pierre commented Apr 21, 2015

not sure what do you mean by "unfortunately" :) ... you need a place to configure GW credentials or ?
... I also wonder how it would look like - if it's possible to use different credentials per tenant.

In multi-tenant mode, the latest version of Kill Bill lets you store this YAML file in the database (see the curl example in the Stripe README for instance).

At runtime, we retrieve these per-tenant credentials from Kill Bill, using the tenant id.

OK, thus this can probably go into a separate feature request.

👍

also I'm not sure what it should be if you'd like me to do it (seems that exporting the server configuration as KILLBILL_CONFIG might turn our useful for plugins but than it might be confusing if there's a plugin configuration as well - really not sure).

The KillbillServerConfig won't be visible to the plugin (OSGI isolation), and I don't think it should be. Plugins are potentially running untrusted code and we don't want them to have access to the main database credentials for instance (there are actually ways to break this isolation today, but it's the direction we're headed).

I'm actually not sure either what the best way to auto-detect Kill Bill would be. Does JRuby know it is running in a ScriptingContainer? Otherwise detecting JRubyPlugin may be the way to go as you did above (I'm actually surprised it is visible from the plugin, I don't think it's exported explicitly).

  • spec_helper.rb would default to ENV['KILLBILL_ENV'] ||= 'test'

That sounds good (and the sane defaults would be picked up instead when run inside Kill Bill).

  • it still needs to be generated (spec_helper.rb would than read the .yml and use it to connect)

I've actually been learning from the refactoring you did in the specs about some of possibilities with RSpec (I'm still at RSpec 101 😄). Wouldn't it be possible to do something similar and have the plugin tests inherit from a shared spec_helper? We already share some test utilities today.

  • driver switching should work - e.g. adapter: postresql will try to load 'jdbc/postres' (just need to make sure they're listed as development gem dependencies)

Actually, that what my main roadblock when I had looked at it a while back. Are all the options passed to ActiveRecord::Base.establish_connection compatible cross drivers?

@kares
Copy link
Contributor Author

kares commented Apr 21, 2015

In multi-tenant mode, the latest version of Kill Bill lets you store this YAML file in the database (see the curl example in the Stripe README for instance).

najs!

The KillbillServerConfig won't be visible to the plugin (OSGI isolation), and I don't think it should be. Plugins are potentially running untrusted code and we don't want them to have access to the main database credentials for instance (there are actually ways to break this isolation today, but it's the direction we're headed).

right, makes sense - thanks for clarifying

I'm actually not sure either what the best way to auto-detect Kill Bill would be. Does JRuby know it is running in a ScriptingContainer? Otherwise detecting JRubyPlugin may be the way to go as you did above (I'm actually surprised it is visible from the plugin, I don't think it's exported explicitly).

knowing that we're on ScriptingContainer would be possible via "hacks", and I do not like the current JRubyPlugin way either as it's long and not supposed to work, something "simpler" would be great to have ... opened #39 for tracking the feature

That sounds good (and the sane defaults would be picked up instead when run inside Kill Bill).

yy, unless of course the user explicitly configured something for KB to use ... (e.g. wants to use pool: 20)

Actually, that what my main roadblock when I had looked at it a while back. Are all the options passed to ActiveRecord::Base.establish_connection compatible cross drivers?

yes, except when you use :properties or custom ones the common ones will work across adapters, custom ones e.g. encoding: xxx for MySQL simply gets ignored on SQLite3 ... for test I do not see a major issue here esp. since if there is one we can use ERB to tune the .yml configuration

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

No branches or pull requests

2 participants