Skip to content

Commit

Permalink
Streamline configuration of account/admin hosts
Browse files Browse the repository at this point in the history
There is one reasonable default for the tenant domain template, if only
the admin_host is configured, namely `"%{tenant}.#{admin_host}"`.
Adding one piece of logic in code means we no longer have to specify a
base in `settings.yml` only to override it.  This flexibility will be
useful when we try to standardize development with multitenancy enabled.

This is backwards compatible and all previous configs will still work.
  • Loading branch information
atz committed Apr 25, 2017
1 parent 60fd770 commit 1dc9298
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 19 deletions.
21 changes: 10 additions & 11 deletions app/models/account.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,19 @@ class Account < ActiveRecord::Base
# @see Settings.multitenancy.default_host
def self.default_cname(piece)
return unless piece
default_host = Settings.multitenancy.default_host
default_host = Settings.multitenancy.default_host || "%{tenant}.#{admin_host}"
format(default_host, tenant: piece.parameterize)
end

# Canonicalize the account cname or request host for comparison
# @param [String] cname distinct part of host name
# @return [String] canonicalized host name
def self.canonical_cname(cname)
# DNS host names are case-insensitive. Convert complete domain names to relative names.
cname &&= cname.downcase.sub(/\.\Z/, '')
cname
end

def self.admin_host
host = Settings.multitenancy.admin_host
host ||= ENV['HOST']
Expand Down Expand Up @@ -49,16 +58,6 @@ def self.single_tenant_default
end
end

# Canonicalize the account cname or request host for comparison
#
# @param [String] cname distinct part of host name
# @return [String] canonicalized host name
def self.canonical_cname(cname)
# DNS host names are case-insensitive. Convert complete domain names to relative names.
cname &&= cname.downcase.sub(/\.\Z/, '')
cname
end

# Make all the account specific connections active
def switch!
solr_endpoint.switch! if solr_endpoint
Expand Down
2 changes: 1 addition & 1 deletion config/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#
multitenancy:
enabled: false
default_host: "%{tenant}.dev"
default_host: # "%{tenant}.dev"
admin_host:

ssl_configured: false
Expand Down
2 changes: 2 additions & 0 deletions config/settings/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@ zookeeper:

multitenancy:
enabled: false
# admin_host: 'localhost'
# default_host: "%{tenant}.localhost"
2 changes: 1 addition & 1 deletion spec/controllers/account_sign_up_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
expect do
post :create, params: { account: valid_attributes }
end.to change(Account, :count).by(1)
expect(assigns(:account).cname).to eq('x.dev')
expect(assigns(:account).cname).to eq('x.localhost')
expect(assigns(:account).errors).to be_empty

expect do # now repeat the same action
Expand Down
42 changes: 36 additions & 6 deletions spec/models/account_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,19 +119,49 @@
expect(account1.errors).to be_empty
end

it 'builds default cname from name' do
account1 = described_class.create(tenant: 'example', name: 'example')
expect(account1.errors).to be_empty
expect(account1.cname).to eq('example.dev')
context 'default_host' do
let(:account1) { described_class.create(tenant: 'example', name: 'example') }

context 'is set' do
it 'builds default cname from name and default_host' do
allow(Settings.multitenancy).to receive(:default_host).and_return "%{tenant}.dev"
expect(account1.errors).to be_empty
expect(account1.cname).to eq('example.dev')
end
end

context 'is unset' do
it 'builds default cname from name and admin_host' do
allow(Settings.multitenancy).to receive(:admin_host).and_return('admin-host')
expect(account1.errors).to be_empty
expect(account1.cname).to eq('example.admin-host')
end
end
end

it 'prevents duplicate cname and tenant values' do
account1 = described_class.create(name: 'example', tenant: 'example', cname: 'example.dev')
account2 = described_class.create(name: 'example', tenant: 'example', cname: 'example.dev')
account1 = described_class.create(name: 'example', tenant: 'example_tenant', cname: 'example.dev')
account2 = described_class.create(name: 'example', tenant: 'example_tenant', cname: 'example.dev')
expect(account1.errors).to be_empty
expect(account2.errors).not_to be_empty
expect(account2.errors.messages).to match a_hash_including(:tenant, :cname)
expect(account2.errors.messages).not_to include(:name)
end

it 'prevents duplicate cname from only name' do
account1 = described_class.create(name: 'example')
account2 = described_class.create(name: 'example')
expect(account1.errors).to be_empty
expect(account2.errors).not_to be_empty
expect(account2.errors.messages).to match a_hash_including(:cname)
expect(account2.errors.messages).not_to include(:name)
end

it 'prevents conflicting new object saves' do
described_class.create(name: 'example', title: 'Original Example')
account2 = described_class.new(name: 'example', title: 'Fancy Example')
expect(account2.save).to be false
expect(account2.errors).to match a_hash_including(:cname)
end
end
end

0 comments on commit 1dc9298

Please sign in to comment.