From f052a621185a7af4a8ab7ea52b78ef55e2ecab0b Mon Sep 17 00:00:00 2001 From: Dave Armstrong Date: Thu, 31 Jan 2019 09:47:37 +0000 Subject: [PATCH 1/4] Fix missing environment check in validate --- lib/puppet/resource_api/transport.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/puppet/resource_api/transport.rb b/lib/puppet/resource_api/transport.rb index eb0d7efc..9ac38e44 100644 --- a/lib/puppet/resource_api/transport.rb +++ b/lib/puppet/resource_api/transport.rb @@ -37,7 +37,7 @@ def connect(name, connection_info) def self.validate(name, connection_info) init_transports - require "puppet/transport/schema/#{name}" unless @transports.key? name + require "puppet/transport/schema/#{name}" unless @transports[@environment].key? name transport_schema = @transports[@environment][name] if transport_schema.nil? raise Puppet::DevError, 'Transport for `%{target}` not registered with `%{environment}`' % { From f3b32bcb1e1e16cb853de86fc057fe7a65aa8e03 Mon Sep 17 00:00:00 2001 From: Dave Armstrong Date: Thu, 31 Jan 2019 09:49:31 +0000 Subject: [PATCH 2/4] (FM-7701) Support device providers when using Transport Wrapper --- lib/puppet/resource_api.rb | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/puppet/resource_api.rb b/lib/puppet/resource_api.rb index 870eb952..1c426f26 100644 --- a/lib/puppet/resource_api.rb +++ b/lib/puppet/resource_api.rb @@ -40,7 +40,12 @@ def register_type(definition) # Keeps a copy of the provider around. Weird naming to avoid clashes with puppet's own `provider` member define_singleton_method(:my_provider) do @my_provider ||= Hash.new { |hash, key| hash[key] = Puppet::ResourceApi.load_provider(definition[:name]).new } - @my_provider[Puppet::Util::NetworkDevice.current.class] + + if Puppet::Util::NetworkDevice.current.is_a? Puppet::ResourceApi::Transport::Wrapper + @my_provider[Puppet::Util::NetworkDevice.current.transport.class] + else + @my_provider[Puppet::Util::NetworkDevice.current.class] + end end # make the provider available in the instance's namespace @@ -414,7 +419,11 @@ def load_provider(type_name) nil else # extract the device type from the currently loaded device's class - Puppet::Util::NetworkDevice.current.class.name.split('::')[-2].downcase + if Puppet::Util::NetworkDevice.current.is_a? Puppet::ResourceApi::Transport::Wrapper + Puppet::Util::NetworkDevice.current.schema.name + else + Puppet::Util::NetworkDevice.current.class.name.split('::')[-2].downcase + end end device_class_name = class_name_from_type_name(device_name) From 968a148e5c3f0e379aa2a4f42b4ac71a72ebbfb8 Mon Sep 17 00:00:00 2001 From: Dave Armstrong Date: Tue, 5 Feb 2019 11:27:23 +0000 Subject: [PATCH 3/4] (maint) Remove message due to breakages in existing modules --- lib/puppet/resource_api/transport/wrapper.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/puppet/resource_api/transport/wrapper.rb b/lib/puppet/resource_api/transport/wrapper.rb index c1ac50a0..9934ce07 100644 --- a/lib/puppet/resource_api/transport/wrapper.rb +++ b/lib/puppet/resource_api/transport/wrapper.rb @@ -32,7 +32,6 @@ def respond_to_missing?(name, _include_private) def method_missing(method_name, *args, &block) if @transport.respond_to? method_name - puts "Delegating #{method_name}" @transport.send(method_name, *args, &block) else super From 203f6336ea151bbfa8e84bc1722c4e1bc45ca47d Mon Sep 17 00:00:00 2001 From: Dave Armstrong Date: Tue, 5 Feb 2019 18:46:35 +0000 Subject: [PATCH 4/4] (maint) Update unit tests for maximum coverage --- lib/puppet/resource_api.rb | 11 ++-- spec/puppet/resource_api/transport_spec.rb | 2 +- spec/puppet/resource_api_spec.rb | 59 ++++++++++++++++++++++ 3 files changed, 65 insertions(+), 7 deletions(-) diff --git a/lib/puppet/resource_api.rb b/lib/puppet/resource_api.rb index 1c426f26..5e84b628 100644 --- a/lib/puppet/resource_api.rb +++ b/lib/puppet/resource_api.rb @@ -6,6 +6,7 @@ require 'puppet/resource_api/puppet_context' unless RUBY_PLATFORM == 'java' require 'puppet/resource_api/read_only_parameter' require 'puppet/resource_api/transport' +require 'puppet/resource_api/transport/wrapper' require 'puppet/resource_api/type_definition' require 'puppet/resource_api/value_creator' require 'puppet/resource_api/version' @@ -417,13 +418,11 @@ def load_provider(type_name) type_name_sym = type_name.to_sym device_name = if Puppet::Util::NetworkDevice.current.nil? nil - else + elsif Puppet::Util::NetworkDevice.current.is_a? Puppet::ResourceApi::Transport::Wrapper # extract the device type from the currently loaded device's class - if Puppet::Util::NetworkDevice.current.is_a? Puppet::ResourceApi::Transport::Wrapper - Puppet::Util::NetworkDevice.current.schema.name - else - Puppet::Util::NetworkDevice.current.class.name.split('::')[-2].downcase - end + Puppet::Util::NetworkDevice.current.schema.name + else + Puppet::Util::NetworkDevice.current.class.name.split('::')[-2].downcase end device_class_name = class_name_from_type_name(device_name) diff --git a/spec/puppet/resource_api/transport_spec.rb b/spec/puppet/resource_api/transport_spec.rb index be019e97..0e6d8aca 100644 --- a/spec/puppet/resource_api/transport_spec.rb +++ b/spec/puppet/resource_api/transport_spec.rb @@ -268,7 +268,7 @@ def change_environment(name = nil) described_class.register(schema) - expect(described_class).to receive(:require).with('puppet/transport/schema/validate') + expect(described_class).not_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) diff --git a/spec/puppet/resource_api_spec.rb b/spec/puppet/resource_api_spec.rb index 7daa44e6..d8db9afc 100644 --- a/spec/puppet/resource_api_spec.rb +++ b/spec/puppet/resource_api_spec.rb @@ -1216,6 +1216,37 @@ class OtherDevice; end it('loads the device provider') { expect(described_class.load_provider('multi_provider').name).to eq 'Puppet::Provider::MultiProvider::SomeDevice' } end end + + context 'with a transport configured' do + let(:definition) { { name: 'multi_provider', attributes: {} } } + let(:transport) { instance_double('Puppet::ResourceApi::Transport::Wrapper', 'transport') } + let(:schema_def) { instance_double('Puppet::ResourceApi::TransportSchemaDef', 'schema_def') } + + before(:each) do + allow(Puppet::Util::NetworkDevice).to receive(:current).with(no_args).and_return(transport) + allow(transport).to receive(:is_a?).with(Puppet::ResourceApi::Transport::Wrapper).and_return(true) + allow(transport).to receive(:schema).and_return(schema_def) + allow(schema_def).to receive(:name).and_return(schema_name) + + module ::Puppet::Provider::MultiProvider + class MultiProvider; end + class SomeDevice; end + class OtherDevice; end + end + end + + context 'with no device-specific provider' do + let(:schema_name) { 'multi_provider' } + + it('loads the default provider') { expect(described_class.load_provider('multi_provider').name).to eq 'Puppet::Provider::MultiProvider::MultiProvider' } + end + + context 'with a device-specific provider' do + let(:schema_name) { 'some_device' } + + it('loads the device provider') { expect(described_class.load_provider('multi_provider').name).to eq 'Puppet::Provider::MultiProvider::SomeDevice' } + end + end end context 'with a provider that does canonicalization', agent_test: true do @@ -1601,6 +1632,10 @@ def set(_context, changes) stub_const('Puppet::Provider::Remoter::Remoter', provider_class) allow(provider_class).to receive(:new).and_return(provider) Puppet.settings[:strict] = :warning + + module ::Puppet::Transport + class Wibble; end + end end it 'is seen as a supported feature' do @@ -1618,6 +1653,30 @@ def set(_context, changes) expect(type.context.type).to be_feature('remote_resource') end end + + describe '#self.my_provider' do + subject(:type) { Puppet::Type.type(:remoter) } + + let(:instance) { type.new(name: 'remote_thing', test_string: 'wibble') } + let(:wrapper) { instance_double('Puppet::ResourceApi::Transport::Wrapper', 'wrapper') } + let(:transport) { instance_double('Puppet::Transport::Wibble', 'transport') } + + before(:each) do + allow(described_class).to receive(:load_provider).and_return(provider) + allow(provider).to receive(:new).and_return(provider) + end + + context 'when a transport is returned by NetworkDevice.current' do + it 'stores the provider with the the name of the transport' do + allow(Puppet::Util::NetworkDevice).to receive(:current).and_return(wrapper) + allow(wrapper).to receive(:is_a?).with(Puppet::ResourceApi::Transport::Wrapper).and_return(true) + allow(wrapper).to receive(:transport).and_return(transport) + allow(transport).to receive(:class).and_return(Puppet::Transport::Wibble) + + expect(instance.my_provider).to eq provider + end + end + end end context 'with a `supports_noop` provider', agent_test: true do