Merge pull request #155 from da-ar/transport_to_device

(PDK-1271) Allow a transport to be wrapped and used like a device

lib/puppet/resource_api/transport.rb

@@ -31,10 +31,22 @@ def connect(name, connection_info)
     validate(name, connection_info)
     require "puppet/transport/#{name}"
     class_name = name.split('_').map { |e| e.capitalize }.join
+    # passing the copy as it may have been stripped on invalid key/values by validate
     Puppet::Transport.const_get(class_name).new(get_context(name), connection_info)
   end
   module_function :connect # rubocop:disable Style/AccessModifierDeclarations
 
+  def inject_device(name, transport)
+    transport_wrapper = Puppet::ResourceApi::Transport::Wrapper.new(name, transport)
+
+    if Puppet::Util::NetworkDevice.respond_to?(:set_device)
+      Puppet::Util::NetworkDevice.set_device(name, transport_wrapper)
+    else
+      Puppet::Util::NetworkDevice.instance_variable_set(:@current, transport_wrapper)
+    end
+  end
+  module_function :inject_device # rubocop:disable Style/AccessModifierDeclarations
+
   def self.validate(name, connection_info)
     init_transports
     require "puppet/transport/schema/#{name}" unless @transports[@environment].key? name

lib/puppet/resource_api/transport/wrapper.rb

@@ -6,20 +6,28 @@
 class Puppet::ResourceApi::Transport::Wrapper
   attr_reader :transport, :schema
 
-  def initialize(name, url_or_config)
-    if url_or_config.is_a? String
-      url = URI.parse(url_or_config)
-      raise "Unexpected url '#{url_or_config}' found. Only file:/// URLs for configuration supported at the moment." unless url.scheme == 'file'
+  def initialize(name, url_or_config_or_transport)
+    if url_or_config_or_transport.is_a? String
+      url = URI.parse(url_or_config_or_transport)
+      raise "Unexpected url '#{url_or_config_or_transport}' found. Only file:/// URLs for configuration supported at the moment." unless url.scheme == 'file'
       raise "Trying to load config from '#{url.path}, but file does not exist." if url && !File.exist?(url.path)
       config = self.class.deep_symbolize(Hocon.load(url.path, syntax: Hocon::ConfigSyntax::HOCON) || {})
-    else
-      config = url_or_config
+    elsif url_or_config_or_transport.is_a? Hash
+      config = url_or_config_or_transport
+    elsif transport_class?(name, url_or_config_or_transport)
+      @transport = url_or_config_or_transport
     end
 
-    @transport = Puppet::ResourceApi::Transport.connect(name, config)
+    @transport ||= Puppet::ResourceApi::Transport.connect(name, config)
     @schema = Puppet::ResourceApi::Transport.list[name]
   end
 
+  def transport_class?(name, transport)
+    class_name = name.split('_').map { |e| e.capitalize }.join
+    expected = Puppet::Transport.const_get(class_name).to_s
+    expected == transport.class.to_s
+  end
+
   def facts
     context = Puppet::ResourceApi::PuppetContext.new(@schema)
     # @transport.facts + custom_facts  # look into custom facts work by TP

lib/puppet/resource_api/type_definition.rb

@@ -154,10 +154,10 @@ def check_schema(resource)
     end
 
     # Returns an array of keys that where not found in the type schema
-    # Modifies the resource passed in, leaving only valid attributes
+    # No longer modifies the resource passed in
     def check_schema_keys(resource)
       rejected = []
-      resource.reject! { |key| rejected << key if key != :title && attributes.key?(key) == false }
+      resource.reject { |key| rejected << key if key != :title && attributes.key?(key) == false }
       rejected
     end
 

spec/puppet/resource_api/base_type_definition_spec.rb

@@ -38,10 +38,16 @@
 
     context 'when resource contains invalid keys' do
       let(:resource) { { name: 'test_string', wibble: '1', foo: '2' } }
+      let(:resource_copy) { { name: 'test_string', wibble: '1', foo: '2' } }
 
       it 'returns an array containing the bad keys' do
         expect(type.check_schema_keys(resource)).to eq([:wibble, :foo])
       end
+
+      it 'does not modify the resource passed in' do
+        type.check_schema_keys(resource)
+        expect(resource).to eq(resource_copy)
+      end
     end
   end
 
@@ -84,7 +90,7 @@
         it 'displays up to 100 warnings' do
           expect(Puppet).to receive(:warning).with(message).exactly(100).times
           110.times do
-            type.check_schema(resource.dup)
+            type.check_schema(resource)
           end
         end
       end
@@ -121,7 +127,7 @@
         it 'displays up to 100 warnings' do
           expect(Puppet).to receive(:warning).with(message).exactly(100).times
           110.times do
-            type.check_schema(resource.dup)
+            type.check_schema(resource)
           end
         end
       end

spec/puppet/resource_api/transport/wrapper_spec.rb

@@ -32,6 +32,21 @@
         described_class.new('wibble', config)
       end
     end
+
+    before(:each) do
+      module Puppet::Transport
+        class SomethingSomethingDarkside; end
+      end
+    end
+
+    context 'when called with a transport class' do
+      let(:transport) { Puppet::Transport::SomethingSomethingDarkside.new }
+      let(:instance) { described_class.new('something_something_darkside', transport) }
+
+      it 'will set the @transport class variable' do
+        expect(instance.instance_variable_get(:@transport)).to eq(transport)
+      end
+    end
   end
 
   describe '#facts' do

spec/puppet/resource_api/transport_spec.rb

@@ -247,6 +247,39 @@ def change_environment(name = nil)
     end
   end
 
+  describe '#inject_device(name, transport)' do
+    let(:device_name) { 'wibble' }
+    let(:transport) { instance_double(Puppet::Transport::Wibble, 'transport') }
+    let(:wrapper) { instance_double(Puppet::ResourceApi::Transport::Wrapper, 'wrapper') }
+
+    before(:each) do
+      module Puppet::Transport
+        class Wibble; end
+      end
+    end
+
+    context 'when puppet has set_device' do
+      it 'wraps the transport and calls set_device within NetworkDevice' do
+        expect(Puppet::ResourceApi::Transport::Wrapper).to receive(:new).with(device_name, transport).and_return(wrapper)
+        allow(Puppet::Util::NetworkDevice).to receive(:respond_to?).with(:set_device).and_return(true)
+        expect(Puppet::Util::NetworkDevice).to receive(:set_device).with(device_name, wrapper)
+
+        described_class.inject_device(device_name, transport)
+      end
+    end
+
+    context 'when puppet does not have set_device' do
+      it 'wraps the transport and sets it as current in NetworkDevice' do
+        expect(Puppet::ResourceApi::Transport::Wrapper).to receive(:new).with(device_name, transport).and_return(wrapper)
+        expect(Puppet::Util::NetworkDevice).to receive(:respond_to?).with(:set_device).and_return(false)
+
+        described_class.inject_device(device_name, transport)
+
+        expect(Puppet::Util::NetworkDevice.current).to eq(wrapper)
+      end
+    end
+  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} }