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

(FM-7690) Fix transports cache to be environment aware #151

Merged
merged 1 commit into from
Jan 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 34 additions & 16 deletions lib/puppet/resource_api/transport.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
# rubocop:disable Style/Documentation
module Puppet; end
module Puppet::ResourceApi; end
# rubocop:enable Style/Documentation
module Puppet::ResourceApi; end # rubocop:disable Style/Documentation

# Remote target transport API
module Puppet::ResourceApi::Transport
Expand All @@ -12,19 +9,21 @@ def register(schema)
raise Puppet::DevError, 'requires `:connection_info`' unless schema.key? :connection_info
raise Puppet::DevError, '`:connection_info` must be a hash, not `%{other_type}`' % { other_type: schema[:connection_info].class } unless schema[:connection_info].is_a?(Hash)

@transports ||= {}
raise Puppet::DevError, 'Transport `%{name}` is already registered.' % { name: schema[:name] } unless @transports[schema[:name]].nil?
@transports[schema[:name]] = Puppet::ResourceApi::TransportSchemaDef.new(schema)
init_transports
unless @transports[@environment][schema[:name]].nil?
raise Puppet::DevError, 'Transport `%{name}` is already registered for `%{environment}`' % {
name: schema[:name],
environment: @environment,
}
end
@transports[@environment][schema[:name]] = Puppet::ResourceApi::TransportSchemaDef.new(schema)
end
module_function :register # rubocop:disable Style/AccessModifierDeclarations

# retrieve a Hash of transport schemas, keyed by their name.
def list
if @transports
Marshal.load(Marshal.dump(@transports))
else
{}
end
init_transports
Marshal.load(Marshal.dump(@transports[@environment]))
end
module_function :list # rubocop:disable Style/AccessModifierDeclarations

Expand All @@ -37,17 +36,36 @@ def connect(name, connection_info)
module_function :connect # rubocop:disable Style/AccessModifierDeclarations

def self.validate(name, connection_info)
@transports ||= {}
init_transports
require "puppet/transport/schema/#{name}" unless @transports.key? name
transport_schema = @transports[name]
raise Puppet::DevError, 'Transport for `%{target}` not registered' % { target: name } if transport_schema.nil?
transport_schema = @transports[@environment][name]
if transport_schema.nil?
raise Puppet::DevError, 'Transport for `%{target}` not registered with `%{environment}`' % {
target: name,
environment: @environment,
}
end

transport_schema.check_schema(connection_info)
transport_schema.validate(connection_info)
end
private_class_method :validate

def self.get_context(name)
require 'puppet/resource_api/puppet_context'
Puppet::ResourceApi::PuppetContext.new(@transports[name])
Puppet::ResourceApi::PuppetContext.new(@transports[@environment][name])
end
private_class_method :get_context

def self.init_transports
lookup = Puppet.lookup(:current_environment) if Puppet.respond_to? :lookup
@environment = if lookup.nil?
:transports_default
else
lookup.name
end
@transports ||= {}
@transports[@environment] ||= {}
end
private_class_method :init_transports
end
106 changes: 104 additions & 2 deletions spec/puppet/resource_api/transport_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,24 @@
require 'spec_helper'

RSpec.describe Puppet::ResourceApi::Transport do
def change_environment(name = nil)
environment = class_double(Puppet::Node::Environment)

if name.nil?
allow(Puppet).to receive(:respond_to?).and_return(false)
else
allow(Puppet).to receive(:respond_to?).and_return(true)
end

allow(Puppet).to receive(:lookup).with(:current_environment).and_return(environment)

# allow clean up scripts to run unhindered
allow(Puppet).to receive(:lookup).with(:root_environment).and_call_original
allow(Puppet).to receive(:lookup).with(:environments).and_call_original

allow(environment).to receive(:name).and_return(name)
end

let(:strict_level) { :error }

before(:each) do
Expand Down Expand Up @@ -60,10 +78,64 @@
},
}
end
let(:schema2) do
{
name: 'schema2',
desc: 'basic transport',
connection_info: {
host: {
type: 'String',
desc: 'the host ip address or hostname',
},
},
}
end
let(:schema3) do
{
name: 'schema3',
desc: 'basic transport',
connection_info: {
user: {
type: 'String',
desc: 'the user to connect as',
},
password: {
type: 'String',
sensitive: true,
desc: 'the password to make the connection',
},
},
}
end

it 'adds to the transports register' do
expect { described_class.register(schema) }.not_to raise_error
end

context 'when a transports are added to multiple environments' do
it 'will record the schemas in the correct structure' do
change_environment(:wibble)
described_class.register(schema)
expect(described_class.instance_variable_get(:@transports)).to be_key(:wibble)
expect(described_class.instance_variable_get(:@transports)[:wibble][schema[:name]]).to be_a_kind_of(Puppet::ResourceApi::TransportSchemaDef)
expect(described_class.instance_variable_get(:@transports)[:wibble][schema[:name]].definition).to eq(schema)

change_environment(:foo)
described_class.register(schema)
described_class.register(schema2)
expect(described_class.instance_variable_get(:@transports)).to be_key(:foo)
expect(described_class.instance_variable_get(:@transports)[:foo][schema[:name]]).to be_a_kind_of(Puppet::ResourceApi::TransportSchemaDef)
expect(described_class.instance_variable_get(:@transports)[:foo][schema[:name]].definition).to eq(schema)
expect(described_class.instance_variable_get(:@transports)[:foo][schema2[:name]]).to be_a_kind_of(Puppet::ResourceApi::TransportSchemaDef)
expect(described_class.instance_variable_get(:@transports)[:foo][schema2[:name]].definition).to eq(schema2)

change_environment(:bar)
described_class.register(schema3)
expect(described_class.instance_variable_get(:@transports)).to be_key(:bar)
expect(described_class.instance_variable_get(:@transports)[:bar][schema3[:name]]).to be_a_kind_of(Puppet::ResourceApi::TransportSchemaDef)
expect(described_class.instance_variable_get(:@transports)[:bar][schema3[:name]].definition).to eq(schema3)
end
end
end

context 'when registering a transport with a bad type' do
Expand Down Expand Up @@ -176,8 +248,15 @@
end

describe '#validate(name, connection_info)', agent_test: true do
context 'when the transport does not exist' do
it { expect { described_class.send(:validate, 'wibble', {}) }.to raise_error LoadError, %r{(no such file to load|cannot load such file) -- puppet/transport/schema/wibble} }
end

context 'when the transport being validated has not be registered' do
it { expect { described_class.validate('wibble', {}) }.to raise_error LoadError, %r{(no such file to load|cannot load such file) -- puppet/transport/schema/wibble} }
it 'will throw an unregistered error message' do
expect(described_class).to receive(:require).with('puppet/transport/schema/wibble')
expect { described_class.send(:validate, 'wibble', {}) }.to raise_error Puppet::DevError, %r{ not registered with }
end
end

context 'when the transport being validated has been registered' do
Expand All @@ -189,10 +268,33 @@

described_class.register(schema)

expect(described_class).to receive(:require).with('puppet/transport/schema/validate')
expect(schema_def).to receive(:check_schema).with('connection_info').and_return(nil)
expect(schema_def).to receive(:validate).with('connection_info').and_return(nil)

described_class.validate('validate', 'connection_info')
described_class.send :validate, 'validate', 'connection_info'
end
end
end

describe '#init_transports' do
context 'when there is not a current_environment' do
it 'will return the default transport environment name' do
change_environment

described_class.send :init_transports

expect(described_class.instance_variable_get(:@environment)).to eq(:transports_default)
end
end

context 'when there is a current_environment' do
it 'will return the set environment name' do
change_environment(:something)

described_class.send :init_transports

expect(described_class.instance_variable_get(:@environment)).to eq(:something)
end
end
end
Expand Down