ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add environment prefix to consul keys. 

Since some environments share a consul datacenter we need to be able to
differentiate configurations.

Fixes: CNVS-34341

Test Plan:
- Nothing uses this yet but we need to make sure we haven't broken JWT
  secrets, the RCE, and Address Book.

Change-Id: I496a8f7d2cafd02c3177a28b348679e552965c0d
Reviewed-on: https://gerrit.instructure.com/99650
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Tyler Pickett <tpickett@instructure.com>
This commit is contained in:
Tyler Pickett 2017-01-16 09:27:22 -07:00
parent 7038ae2a5e
commit 7b45e0fd24
13 changed files with 194 additions and 103 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
config/consul.yml.example
View File

@ -1,13 +1,15 @@
production:
environment: "production"
host: 10.11.12.13
port: 8500
ssl: true
acl_token: "xxxxxxxx-yyyy-zzzz-1111-222222222222"
test:
environment: "test"
host: test.domain
port: 8500
ssl: false
init_values:
init_values_without_env:
canvas:
signing-secret: astringthatisactually32byteslong
encryption-secret: astringthatisactually32byteslong
@ -21,10 +23,11 @@ test:
app-host: http://les.docker
sad-panda: null
development:
environment: "dev"
host: dev.domain
port: 8500
ssl: false
init_values:
init_values_without_env:
canvas:
signing-secret: astringthatisactually32byteslong
encryption-secret: astringthatisactually32byteslong

43
lib/canvas/dynamic_settings.rb
View File

@ -27,7 +27,7 @@ module Canvas
KV_NAMESPACE = "config/canvas".freeze
class << self
attr_accessor :config, :cache, :fallback_data
attr_accessor :config, :cache, :environment, :fallback_data
def config=(conf_hash)
@config = conf_hash
@ -43,17 +43,19 @@ module Canvas
config.receive_timeout = conf_hash['receive_timeout'] if conf_hash['receive_timeout']
end
init_data = conf_hash.fetch("init_values", {})
init_values(init_data)
@environment = conf_hash['environment']
init_values(conf_hash.fetch("init_values", {}))
init_values(conf_hash.fetch("init_values_without_env", {}), use_env: false)
end
end
def find(key)
def find(key, use_env: true)
if config.nil?
return fallback_data.fetch(key) if fallback_data.present?
raise(ConsulError, "Unable to contact consul without config")
else
store_get(key)
store_get(key, use_env: use_env)
end
end
@ -61,12 +63,12 @@ module Canvas
# the first time they're asked for, and then can only be cleared with a SIGHUP
# or restart of the process. Make sure that's the behavior you want before
# you use this method, or specify a timeout
def from_cache(key, expires_in: nil)
def from_cache(key, expires_in: nil, use_env: true)
reset_cache! if cache.nil?
cached_value = get_from_cache(key, expires_in)
return cached_value if cached_value.present?
# cache miss or timeout
value = self.find(key)
value = self.find(key, use_env: use_env)
set_in_cache(key, value)
value
end
@ -94,25 +96,27 @@ module Canvas
cache[key] = {value: value, timestamp: Time.zone.now.to_i}
end
def init_values(hash)
def init_values(hash, use_env: true)
hash.each do |parent_key, settings|
settings.each do |child_key, value|
store_put("#{parent_key}/#{child_key}", value)
store_put("#{parent_key}/#{child_key}", value, use_env: use_env)
end
end
rescue Imperium::TimeoutError
return false
end
def store_get(key)
def store_get(key, use_env: true)
# store all values that we get here to
# kind-of recover in case of big failure
@strategic_reserve ||= {}
consul_value = consul_get(key)
parent_key = add_prefix_to(key, use_env)
consul_response = kv_client.get(parent_key, *CONSUL_READ_OPTIONS)
consul_value = consul_response.values
@strategic_reserve[key] = consul_value
consul_value
rescue Imperium::TimeoutError
rescue Imperium::TimeoutError => exception
if @strategic_reserve.key?(key)
# we have an old value for this key, log the error but recover
Canvas::Errors.capture_exception(:consul, exception)
@ -123,14 +127,17 @@ module Canvas
end
end
def store_put(key, value)
kv_client.put("#{KV_NAMESPACE}/#{key}", value)
def store_put(key, value, use_env: true)
full_key = add_prefix_to(key, use_env)
kv_client.put(full_key, value)
end
def consul_get(key)
parent_key = "#{KV_NAMESPACE}/#{key}"
consul_response = kv_client.get(parent_key, *CONSUL_READ_OPTIONS)
consul_response.values
def add_prefix_to(key, use_env)
if use_env && environment
"#{KV_NAMESPACE}/#{environment}/#{key}"
else
"#{KV_NAMESPACE}/#{key}"
end
end
end
end

4
lib/canvas/security.rb
View File

@ -342,11 +342,11 @@ module Canvas::Security
end
def services_encryption_secret
Canvas::DynamicSettings.from_cache("canvas")["encryption-secret"]
Canvas::DynamicSettings.from_cache("canvas", use_env: false)["encryption-secret"]
end
def services_signing_secret
Canvas::DynamicSettings.from_cache("canvas")["signing-secret"]
Canvas::DynamicSettings.from_cache("canvas", use_env: false)["signing-secret"]
end
end
end

19
lib/canvas/security/services_jwt.rb
View File

@ -120,7 +120,6 @@ class Canvas::Security::ServicesJwt
context: context)
end
private
def self.create_payload(payload_data)
if payload_data[:sub].nil?
@ -137,6 +136,16 @@ class Canvas::Security::ServicesJwt
})
end
def self.encryption_secret
Canvas::DynamicSettings.from_cache("canvas", use_env: false)["encryption-secret"]
end
def self.signing_secret
Canvas::DynamicSettings.from_cache("canvas", use_env: false)["signing-secret"]
end
private
def encryption_secret
self.class.encryption_secret
end
@ -145,14 +154,6 @@ class Canvas::Security::ServicesJwt
self.class.signing_secret
end
def self.encryption_secret
Canvas::DynamicSettings.from_cache("canvas")["encryption-secret"]
end
def self.signing_secret
Canvas::DynamicSettings.from_cache("canvas")["signing-secret"]
end
class << self
private

2
lib/services/address_book.rb
View File

@ -110,7 +110,7 @@ module Services
class << self
private
def setting(key)
settings = Canvas::DynamicSettings.from_cache("address-book", expires_in: 5.minutes)
settings = Canvas::DynamicSettings.from_cache("address-book", expires_in: 5.minutes, use_env: false)
settings[key]
rescue Imperium::TimeoutError => e
Canvas::Errors.capture_exception(:address_book, e)

2
lib/services/live_events_subscription_service.rb
View File

@ -72,7 +72,7 @@ module Services
end
def settings
Canvas::DynamicSettings.from_cache("live-events-subscription-service", expires_in: 5.minutes)
Canvas::DynamicSettings.from_cache("live-events-subscription-service", expires_in: 5.minutes, use_env: false)
rescue Imperium::TimeoutError => e
Canvas::Errors.capture_exception(:live_events_subscription, e)
nil

2
lib/services/rich_content.rb
View File

@ -50,7 +50,7 @@ module Services
end
def service_settings
settings = Canvas::DynamicSettings.from_cache("rich-content-service", expires_in: 5.minutes)
settings = Canvas::DynamicSettings.from_cache("rich-content-service", expires_in: 5.minutes, use_env: false)
{
RICH_CONTENT_APP_HOST: settings["app-host"],
RICH_CONTENT_CDN_HOST: settings["cdn-host"]

4
spec/controllers/eportfolios_controller_spec.rb
View File

@ -62,7 +62,7 @@ describe EportfoliosController do
}
before do
Canvas::DynamicSettings.stubs(:find).with("canvas").returns(fake_secrets)
Canvas::DynamicSettings.stubs(:find).with("canvas", use_env: false).returns(fake_secrets)
end
after do
@ -78,7 +78,7 @@ describe EportfoliosController do
it "exposes the feature state for rich content service to js_env" do
@user.account.root_account.enable_feature!(:rich_content_service)
Canvas::DynamicSettings.stubs(:find).with("rich-content-service").returns({
Canvas::DynamicSettings.stubs(:find).with("rich-content-service", use_env: false).returns({
'app-host' => 'rce.docker',
'cdn-host' => 'rce.docker'
})

166
spec/lib/canvas/dynamic_settings_spec.rb
View File

@ -28,39 +28,40 @@ module Canvas
after do
begin
DynamicSettings.config = @cached_config
rescue Imperium::UnableToConnectError
rescue Imperium::UnableToConnectError, Imperium::TimeoutError
# don't fail the test if there is no consul running
end
Canvas::DynamicSettings.reset_cache!
Canvas::DynamicSettings.fallback_data = nil
end
let(:parent_key){ "rich-content-service" }
let(:parent_key){ 'rich-content-service' }
let(:imperium_read_options){ [:recurse, :stale] }
let(:kv_client) { DynamicSettings.kv_client }
let(:valid_config) do
{
'host' =>'consul',
'port' => 8500,
'ssl' => true,
'acl_token' => 'some-long-string',
'environment' => 'rspec',
}
end
describe ".config=" do
let(:valid_config) do
{
"host" =>"consul",
"port" => 8500,
"ssl" => true,
"acl_token" => "some-long-string"
}
end
it "configures imperium when config is set" do
DynamicSettings.kv_client.stubs(:put)
DynamicSettings.config = valid_config
expect(Imperium.configuration.url.to_s).to eq("https://consul:8500")
end
it "sends initial config data by de-nesting a hash into keys" do
it 'must send initial values w/o the environment to consul' do
client = DynamicSettings.kv_client
config = valid_config.merge({
'init_values' => {
"rich-content-service" => {
"app-host" => "rce.docker",
"cdn-host" => "rce.docker"
'init_values_without_env' => {
'rich-content-service' => {
'app-host' => 'rce.docker',
'cdn-host' => 'rce.docker'
}
}
})
@ -70,7 +71,6 @@ module Canvas
.and_return(true)
expect(kv_client).to receive(:put)
.with("config/canvas/rich-content-service/cdn-host", "rce.docker")
.and_return(true)
# This is super gross but the alternative is to use expect_any_instance
allow(Imperium::Client).to receive(:reset_default_clients).and_return(true)
@ -89,32 +89,108 @@ module Canvas
expect(client_config.send_timeout).to eq 2
expect(client_config.receive_timeout).to eq 3
end
it 'must send initial values with the environment to consul' do
client = DynamicSettings.kv_client
config = valid_config.merge({
'init_values' => {
'some-service' => {
'signing-key' => 'sekret'
}
}
})
expect(client).to receive(:put)
.with('config/canvas/rspec/some-service/signing-key', 'sekret')
# This is super gross but the alternative is to use expect_any_instance
allow(Imperium::Client).to receive(:reset_default_clients).and_return(true)
DynamicSettings.config = config
end
it 'must capture the environment name when supplied' do
DynamicSettings.config = valid_config.merge({
'environment' => 'foobar'
})
expect(DynamicSettings.environment).to eq 'foobar'
end
end
describe ".find" do
describe "with consul config" do
# we don't need to interact with a real consul for unit tests
before(:each) do
DynamicSettings.config = {} # just to be not nil
DynamicSettings.config = valid_config
DynamicSettings.fallback_data = nil
allow(kv_client).to receive(:get)
.with("config/canvas/#{parent_key}", *imperium_read_options)
.with("config/canvas/rspec/#{parent_key}", *imperium_read_options)
.with("config/canvas/rspec/#{parent_key}", *imperium_read_options)
.and_return(
Imperium::Testing.kv_get_response(
body: [
{ Key: "config/canvas/#{parent_key}/app-host", Value: "rce.insops.com"},
{ Key: "config/canvas/#{parent_key}/cdn-host", Value: "asdfasdf.cloudfront.com"}
{ Key: "config/canvas/rspec/#{parent_key}/app-host", Value: "rce.insops.net"},
{ Key: "config/canvas/rspec/#{parent_key}/cdn-host", Value: "asdfasdf.cloudfront.com"}
],
options: imperium_read_options,
prefix: "config/canvas/#{parent_key}"
prefix: "config/canvas/rspec/#{parent_key}"
)
)
allow(kv_client).to receive(:get)
.with("config/canvas/rspec/#{parent_key}/app-host", *imperium_read_options)
.and_return(
Imperium::Testing.kv_get_response(
body: [
{ Key: "config/canvas/rspec/#{parent_key}/app-host", Value: "rce.insops.net"},
],
options: imperium_read_options,
prefix: "config/canvas/rspec/#{parent_key}/app-host"
)
)
end
it 'must default to finding values using the included environment' do
value = DynamicSettings.find("#{parent_key}/app-host")
expect(value).to eq 'rce.insops.net'
end
it 'must fall back to not including the environment when the config is unset' do
DynamicSettings.environment = nil
expect(kv_client).to receive(:get)
.with('config/canvas/legacy-app', *imperium_read_options)
.and_return(
Imperium::Testing.kv_get_response(
body: [
{ Key: 'config/canvas/legacy-app', Value: 'value'},
],
options: imperium_read_options,
prefix: 'config/canvas/legacy-app'
)
)
value = DynamicSettings.find('legacy-app')
expect(value).to eq 'value'
end
it 'must allow finding values not including the configured environment' do
expect(kv_client).to receive(:get)
.with('config/canvas/legacy-app', *imperium_read_options)
.and_return(
Imperium::Testing.kv_get_response(
body: [
{ Key: 'config/canvas/legacy-app', Value: 'value'},
],
options: imperium_read_options,
prefix: 'config/canvas/legacy-app'
)
)
value = DynamicSettings.find('legacy-app', use_env: false)
expect(value).to eq 'value'
end
it "loads the children of a k/v node as a hash" do
rce_settings = DynamicSettings.find(parent_key)
expect(rce_settings).to eq({
"app-host" => "rce.insops.com",
"app-host" => "rce.insops.net",
"cdn-host" => "asdfasdf.cloudfront.com"
})
end
@ -124,12 +200,12 @@ module Canvas
DynamicSettings.find(parent_key)
# some values are now stored in case of connection failure
allow(kv_client).to receive(:get)
.with("config/canvas/#{parent_key}", imperium_read_options)
.with("config/canvas/rspec/#{parent_key}", *imperium_read_options)
.and_raise(Imperium::ConnectTimeout, "could not contact consul")
rce_settings = DynamicSettings.find(parent_key)
expect(rce_settings).to eq({
"app-host" => "rce.insops.com",
"app-host" => "rce.insops.net",
"cdn-host" => "asdfasdf.cloudfront.com"
})
end
@ -137,7 +213,7 @@ module Canvas
it "cant recover with no value cached for connection failure" do
DynamicSettings.reset_cache!(hard: true)
allow(kv_client).to receive(:get)
.with("config/canvas/#{parent_key}", *imperium_read_options)
.with("config/canvas/rspec/#{parent_key}", *imperium_read_options)
.and_raise(Imperium::ConnectTimeout)
expect{ DynamicSettings.find(parent_key) }.to(
@ -175,34 +251,34 @@ module Canvas
end
describe ".from_cache" do
before(:each){ DynamicSettings.config = {} } # just to be not nil
before(:each){ DynamicSettings.config = valid_config } # just to be not nil
after(:each){ DynamicSettings.reset_cache! }
def stub_consul_with(value)
allow(kv_client).to receive(:get)
.with("config/canvas/#{parent_key}", *imperium_read_options)
.with("config/canvas/rspec/#{parent_key}", *imperium_read_options)
.and_return(
Imperium::Testing.kv_get_response(
body: [
{ Key: "config/canvas/#{parent_key}/app-host", Value: value},
{ Key: "config/canvas/rspec/#{parent_key}/app-host", Value: value},
],
options: imperium_read_options,
prefix: "config/canvas/#{parent_key}"
prefix: "config/canvas/rspec/#{parent_key}"
)
)
end
it "only queries consul the first time" do
allow(kv_client).to receive(:get)
.with("config/canvas/#{parent_key}", *imperium_read_options)
.with("config/canvas/rspec/#{parent_key}", *imperium_read_options)
.once # and only once, going to hit it several times
.and_return(
Imperium::Testing.kv_get_response(
body: [
{ Key: "config/canvas/#{parent_key}/app-host", Value: 'rce.insops.net'},
{ Key: "config/canvas/rspec/#{parent_key}/app-host", Value: 'rce.insops.net'},
],
options: imperium_read_options,
prefix: "config/canvas/#{parent_key}"
prefix: "config/canvas/rspec/#{parent_key}"
)
)
5.times{ DynamicSettings.from_cache(parent_key) }
@ -211,16 +287,16 @@ module Canvas
end
it "definitely doesnt pickup new values once cached" do
stub_consul_with("rce.insops.com")
stub_consul_with("rce.insops.net")
value = DynamicSettings.from_cache(parent_key)
expect(value["app-host"]).to eq("rce.insops.com")
expect(value["app-host"]).to eq("rce.insops.net")
stub_consul_with("CHANGED VALUE")
value = DynamicSettings.from_cache(parent_key)
expect(value["app-host"]).to eq("rce.insops.com")
expect(value["app-host"]).to eq("rce.insops.net")
end
it "returns new values after a cache clear" do
stub_consul_with("rce.insops.com")
stub_consul_with("rce.insops.net")
DynamicSettings.from_cache(parent_key)
stub_consul_with("CHANGED VALUE")
DynamicSettings.reset_cache!
@ -229,16 +305,16 @@ module Canvas
end
it "caches values with timeouts" do
stub_consul_with("rce.insops.com")
stub_consul_with("rce.insops.net")
value = DynamicSettings.from_cache(parent_key, expires_in: 5.minutes)
expect(value["app-host"]).to eq("rce.insops.com")
expect(value["app-host"]).to eq("rce.insops.net")
stub_consul_with("CHANGED VALUE")
value = DynamicSettings.from_cache(parent_key, expires_in: 5.minutes)
expect(value["app-host"]).to eq("rce.insops.com")
expect(value["app-host"]).to eq("rce.insops.net")
end
it "loads new values when timeout is past" do
stub_consul_with("rce.insops.com")
stub_consul_with("rce.insops.net")
value = DynamicSettings.from_cache(parent_key, expires_in: 5.minutes)
Timecop.travel(Time.zone.now + 6.minutes) do
stub_consul_with("CHANGED VALUE")
@ -248,7 +324,7 @@ module Canvas
end
it "accepts a timeout on a previously inifinity key" do
stub_consul_with("rce.insops.com")
stub_consul_with("rce.insops.net")
value = DynamicSettings.from_cache(parent_key)
Timecop.travel(Time.zone.now + 11.minutes) do
stub_consul_with("CHANGED VALUE")
@ -261,7 +337,7 @@ module Canvas
let!(:now) { Time.zone.now }
before(:each) do
stub_consul_with("rce.insops.com")
stub_consul_with("rce.insops.net")
DynamicSettings.from_cache(parent_key) # prime cache
end
@ -278,14 +354,14 @@ module Canvas
with("config/canvas/#{parent_key}", imperium_read_options).
raises(Imperium::TimeoutError, "could not contact consul")
value = DynamicSettings.from_cache(parent_key, expires_in: 10.minutes)
expect(value["app-host"]).to eq("rce.insops.com")
expect(value["app-host"]).to eq("rce.insops.net")
end
it "returns old value during connection timeout" do
Imperium::KV.stubs(:get).
raises(Imperium::TimeoutError, "could not contact consul")
value = DynamicSettings.from_cache(parent_key, expires_in: 10.minutes)
expect(value["app-host"]).to eq("rce.insops.com")
expect(value["app-host"]).to eq("rce.insops.net")
end
end
end

2
spec/lib/services/address_book_spec.rb
View File

@ -24,7 +24,7 @@ module Services
@app_host = "address-book"
@secret = "opensesame"
allow(Canvas::DynamicSettings).to receive(:find).
with("address-book").
with("address-book", use_env: false).
and_return({ "app-host" => @app_host, "secret" => Canvas::Security.base64_encode(@secret) })
@sender = user_model
@course = course_model

27
spec/lib/services/live_events_subscription_service_spec.rb
View File

@ -24,11 +24,9 @@ module Services
context 'service unavailable' do
before do
Canvas::DynamicSettings.stubs(:find).with('live-events-subscription-service').returns(nil)
end
after do
Canvas::DynamicSettings.unstub(:find)
allow(Canvas::DynamicSettings).to receive(:find)
.with('live-events-subscription-service', {use_env: false})
.and_return(nil)
end
describe '.available?' do
@ -40,14 +38,17 @@ module Services
context 'service available' do
before do
Canvas::DynamicSettings.stubs(:find).with('live-events-subscription-service').returns({
"app-host" => "http://example.com",
"sad-panda" => nil
})
Canvas::DynamicSettings.stubs(:find).with('canvas').returns({
"signing-secret" => "astringthatisactually32byteslong",
"encryption-secret" => "astringthatisactually32byteslong"
})
allow(Canvas::DynamicSettings).to receive(:find)
.with('live-events-subscription-service', {use_env: false})
.and_return({
'app-host' => 'http://example.com',
})
allow(Canvas::DynamicSettings).to receive(:find)
.with('canvas', {use_env: false})
.and_return({
'signing-secret' => 'astringthatisactually32byteslong',
'encryption-secret' => 'astringthatisactually32byteslong'
})
end
after do

17
spec/lib/services/rich_content_spec.rb
View File

@ -21,10 +21,12 @@ require_dependency "services/rich_content"
module Services
describe RichContent do
before do
Canvas::DynamicSettings.stubs(:find).with("rich-content-service").returns({
"app-host" => "rce-app",
"cdn-host" => "rce-cdn"
})
allow(Canvas::DynamicSettings).to receive(:find)
.with('rich-content-service', use_env: false)
.and_return({
"app-host" => "rce-app",
"cdn-host" => "rce-cdn"
})
end
describe ".env_for" do
@ -42,8 +44,9 @@ module Services
end
it "populates hosts with an error signal when consul is down" do
Canvas::DynamicSettings.stubs(:find).with("rich-content-service").
raises(Imperium::UnableToConnectError, "can't talk to consul")
allow(Canvas::DynamicSettings).to receive(:find)
.with('rich-content-service', use_env: false)
.and_raise(Imperium::UnableToConnectError, "can't talk to consul")
root_account = stub("root_account", feature_enabled?: true)
env = described_class.env_for(root_account)
expect(env[:RICH_CONTENT_SERVICE_ENABLED]).to be_truthy
@ -52,7 +55,7 @@ module Services
end
it "logs errors for later consideration" do
Canvas::DynamicSettings.stubs(:find).with("rich-content-service").
Canvas::DynamicSettings.stubs(:find).with("rich-content-service", use_env: false).
raises(Canvas::DynamicSettings::ConsulError, "can't talk to consul")
root_account = stub("root_account", feature_enabled?: true)
Canvas::Errors.expects(:capture_exception).with do |type, e|

2
spec/support/jwt_env.rb
View File

@ -26,7 +26,7 @@ RSpec.shared_context "JWT setup" do
}
before do
Canvas::DynamicSettings.stubs(:find).with("canvas").returns(fake_secrets)
Canvas::DynamicSettings.stubs(:find).with("canvas", use_env: false).returns(fake_secrets)
end
after do