diff --git a/.rubocop.yml b/.rubocop.yml index 6d831a74..5457a9ea 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,6 +1,5 @@ --- require: rubocop-rspec -inherit_from: .rubocop_todo.yml AllCops: TargetRubyVersion: '2.1' Include: @@ -157,4 +156,4 @@ Naming/UncommunicativeMethodParamName: # This cop breaks syntax highlighting in VSCode Layout/ClosingHeredocIndentation: - Enabled: false \ No newline at end of file + Enabled: false diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml deleted file mode 100644 index 63295575..00000000 --- a/.rubocop_todo.yml +++ /dev/null @@ -1,17 +0,0 @@ -# This configuration was generated by -# `rubocop --auto-gen-config` -# on 2017-09-05 11:31:11 +0100 using RuboCop version 0.49.1. -# The point is for the user to remove these configuration records -# one by one as the offenses are removed from the code base. -# Note that changes in the inspected code, or installation of new -# versions of RuboCop, may require this file to be generated again. - -# Offense count: 6 -Style/Documentation: - Exclude: - - 'spec/**/*' - - 'test/**/*' - - 'lib/puppet/resource_api.rb' - - 'lib/puppet/resource_api/base_context.rb' - - 'lib/puppet/resource_api/io_context.rb' - - 'lib/puppet/resource_api/puppet_context.rb' diff --git a/.travis.yml b/.travis.yml index b2619621..978c4106 100644 --- a/.travis.yml +++ b/.travis.yml @@ -65,10 +65,5 @@ matrix: - rvm: 2.5.1 env: PUPPET_GEM_VERSION='https://github.com/puppetlabs/puppet.git#6.0.x' notifications: - hipchat: - rooms: - secure: 10a49kkZcghKHNnef8x7eBG+KjScL3i1VpygFg6DPAOK2YNbEoyEx1Kv9KLC7GSRYov/SQZOsZrvHZtDhEtFSKhhiAjOwxl1jV1t6aAMGMnN1IoZBOvdAJKrZsm54/bBeYp+je2wqnnoFNtLVFSoOX0LkFaDEWT+zGZ5xKJIH25GpeQEZf1eDxs/d8YX/m+RwbGXHVA//hOpvZo0ntvznh2EbW5OPODKSeUXbWZ+W4ndODTsKWFc/WLMSSgFDzW/Y2/9V40D4IC8lvSx6eKFryMfAQy6pO/d1aTB468awzyVcdYAMMCOITm7hlKGRKxNgq6NkOsXs5KLg6ifpn+a/Rhapbz6Qxbpjjho/7Wxngl4B3T+i35ap/mFrS/fOfKCq3gEQlYn29its9bEFArNGbr+/sXKABb+sRpgW4RTPWYDHJyHJendbevd5tZ+fd0JUBOi0Cb4PcXxQxM8IQrbuu2zso0K5MV05kL0S1DE/VsuUrPaK0RsF+b1+i6NfvtN8kgbYs1eiVku+guIG2ec3xIefQ1hsEOFPFNqSDfHp7nANnRVIbBCt8qw8DhmNEczsfN5Dp21euJUsO9qpau++NzD3jRhkE5Zki5cwsakU7hIQzw82BIb0eSQJCHygieExeEboWRqtDgy/IKIWPgIvEuU68ppR2bl2reKCHLCnWc= - template: - - '%{repository}#%{build_number} (%{branch} - %{commit} : %{author}): %{message} - (Details | PR/Diff)' - format: html + slack: + secure: aPXZYNow8LsmmlS8PQU3FjL0bc7FqUUA95d++wZfIu7YAjGboIUiekxYouQ0XnY+Aig8InvbTOIgBHgGNheyr/YFbFS90/jtulbF8oW7BitW+imgjeAHDCwlQZTCc4FFYde/2pI7QTT8H5NpLR9mKxlTU77Sqr8gFAIybuPdHcKMYQZdEZS07ma2pUp7+GyKS6PDQpzW2+mDCz/wfi3/JdsUvc0mclCZ8vxySc66j5P1E6nFDMzuakBOjwJHpgeDpreapbmSUQLAX0a3ZsFP+N+SNduLotlV2BWnJK2gcO6rGFP4Fz1D0bGXuBnYYdIiB+9OgI3wtXg9y1SifNHUG3IrOBAA8CGNyrebTGKtH0TS2O+HZLbaNX2g6udD5e3156vys9wScmJuQ/rSkVtQfXf1qUm5eijvlXI+DIbssbZHqm6QQGyM4p3NoULmNmF1C85bQoZ4GF7b1P/8mstsVE/HUfnzRPNbwD0r6j1aE/ck3PKMi7ZAhIi0Ja9RnAgP3wi0t62uERYcJGGYEycWohMWnrf2w6GFwGeuoiwAkASdHOLX0/AOMPc4mBOjlc621o8uYMrrZqfF5CrOAvJ151USSsWn2AhXaibIvnHo6X91paNvvNpU/GYu3CUAl6q8OhYovvjtRVPVnhs2DrpgoRB+6NWHnzjRG/wr6Z9U+vA= diff --git a/lib/puppet/resource_api.rb b/lib/puppet/resource_api.rb index 1d69ac0a..870eb952 100644 --- a/lib/puppet/resource_api.rb +++ b/lib/puppet/resource_api.rb @@ -12,6 +12,7 @@ require 'puppet/type' require 'puppet/util/network_device' +# This module contains the main API to register and access types, providers and transports. module Puppet::ResourceApi @warning_count = 0 @@ -353,7 +354,7 @@ def cache_current_state(resource_hash) end define_singleton_method(:context) do - @context ||= PuppetContext.new(definition) + @context ||= PuppetContext.new(TypeDefinition.new(definition)) end def context diff --git a/lib/puppet/resource_api/base_context.rb b/lib/puppet/resource_api/base_context.rb index bf877680..910a7d81 100644 --- a/lib/puppet/resource_api/base_context.rb +++ b/lib/puppet/resource_api/base_context.rb @@ -1,14 +1,24 @@ require 'puppet/resource_api/type_definition' +# rubocop:disable Style/Documentation module Puppet; end module Puppet::ResourceApi; end +# rubocop:enable Style/Documentation + +# This class provides access to all common external dependencies of providers and transports. +# The runtime environment will inject an appropriate implementation. class Puppet::ResourceApi::BaseContext attr_reader :type def initialize(definition) - raise ArgumentError, 'BaseContext requires definition to be a Hash' unless definition.is_a?(Hash) - @typename = definition[:name] - @type = Puppet::ResourceApi::TypeDefinition.new(definition) + if definition.is_a?(Hash) + # this is only for backwards compatibility + @type = Puppet::ResourceApi::TypeDefinition.new(definition) + elsif definition.is_a? Puppet::ResourceApi::BaseTypeDefinition + @type = definition + else + raise ArgumentError, 'BaseContext requires definition to be a child of Puppet::ResourceApi::BaseTypeDefinition, not <%{actual_type}>' % { actual_type: definition.class } + end end def device @@ -27,7 +37,7 @@ def feature_support?(feature) [:debug, :info, :notice, :warning, :err].each do |level| define_method(level) do |*args| if args.length == 1 - message = "#{@context || @typename}: #{args.last}" + message = "#{@context || @type.name}: #{args.last}" elsif args.length == 2 resources = format_titles(args.first) message = "#{resources}: #{args.last}" @@ -137,9 +147,9 @@ def send_log(_level, _message) def format_titles(titles) if titles.length.zero? && !titles.is_a?(String) - @typename + @type.name else - "#{@typename}[#{[titles].flatten.compact.join(', ')}]" + "#{@type.name}[#{[titles].flatten.compact.join(', ')}]" end end diff --git a/lib/puppet/resource_api/io_context.rb b/lib/puppet/resource_api/io_context.rb index 3cfb4afb..d31143c4 100644 --- a/lib/puppet/resource_api/io_context.rb +++ b/lib/puppet/resource_api/io_context.rb @@ -1,5 +1,7 @@ require 'puppet/resource_api/base_context' +# Implement Resource API Conext to log through an IO object, defaulting to `$stderr`. +# There is no access to a device here. class Puppet::ResourceApi::IOContext < Puppet::ResourceApi::BaseContext def initialize(definition, target = $stderr) super(definition) diff --git a/lib/puppet/resource_api/puppet_context.rb b/lib/puppet/resource_api/puppet_context.rb index 770c7692..6dd23a51 100644 --- a/lib/puppet/resource_api/puppet_context.rb +++ b/lib/puppet/resource_api/puppet_context.rb @@ -1,6 +1,8 @@ require 'puppet/resource_api/base_context' require 'puppet/util/logging' +# Implement Resource API Context to log through Puppet facilities +# and access/expose the puppet process' current device class Puppet::ResourceApi::PuppetContext < Puppet::ResourceApi::BaseContext def device # TODO: evaluate facter_url setting for loading config if there is no `current` NetworkDevice diff --git a/lib/puppet/resource_api/transport.rb b/lib/puppet/resource_api/transport.rb index 5858cd91..107d002a 100644 --- a/lib/puppet/resource_api/transport.rb +++ b/lib/puppet/resource_api/transport.rb @@ -1,7 +1,6 @@ # rubocop:disable Style/Documentation module Puppet; end module Puppet::ResourceApi; end -module Puppet::ResourceApi::Transport; end # rubocop:enable Style/Documentation # Remote target transport API @@ -19,24 +18,36 @@ def register(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 + end + module_function :list # rubocop:disable Style/AccessModifierDeclarations + def connect(name, connection_info) validate(name, connection_info) - begin - require "puppet/transport/#{name}" - class_name = name.split('_').map { |e| e.capitalize }.join - Puppet::Transport.const_get(class_name).new(connection_info) - rescue LoadError, NameError => detail - raise Puppet::DevError, 'Cannot load transport for `%{target}`: %{detail}' % { target: name, detail: detail } - end + require "puppet/transport/#{name}" + class_name = name.split('_').map { |e| e.capitalize }.join + Puppet::Transport.const_get(class_name).new(get_context(name), connection_info) end module_function :connect # rubocop:disable Style/AccessModifierDeclarations def self.validate(name, connection_info) @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.check_schema(connection_info) transport_schema.validate(connection_info) end + + def self.get_context(name) + require 'puppet/resource_api/puppet_context' + Puppet::ResourceApi::PuppetContext.new(@transports[name]) + end end diff --git a/lib/puppet/resource_api/transport/wrapper.rb b/lib/puppet/resource_api/transport/wrapper.rb index 97fe4fe0..81febbcb 100644 --- a/lib/puppet/resource_api/transport/wrapper.rb +++ b/lib/puppet/resource_api/transport/wrapper.rb @@ -8,24 +8,22 @@ class Puppet::ResourceApi::Transport::Wrapper 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' + 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' + raise "Trying to load config from '#{url.path}, but file does not exist." if url && !File.exist?(url.path) + config = (Hocon.load(url.path, syntax: Hocon::ConfigSyntax::HOCON) || {}).map { |k, v| [k.to_sym, v] }.to_h else - @config = url_or_config + config = url_or_config end @transport = Puppet::ResourceApi::Transport.connect(name, config) - end - - def config - raise "Trying to load config from '#{@url.path}, but file does not exist." if @url && !File.exist?(@url.path) - # symbolize top-level keys to match up with expected provider/resource data conventions - @config ||= (Hocon.load(@url.path, syntax: Hocon::ConfigSyntax::HOCON) || {}).map { |k, v| [k.to_sym, v] }.to_h + @schema = Puppet::ResourceApi::Transport.list[name] end def facts + context = Puppet::ResourceApi::PuppetContext.new(@schema) # @transport.facts + custom_facts # look into custom facts work by TP - @transport.facts + @transport.facts(context) end def respond_to_missing?(name, _include_private) diff --git a/spec/fixtures/test_module/lib/puppet/transport/test_device.rb b/spec/fixtures/test_module/lib/puppet/transport/test_device.rb index ad3a04f6..7974f1d4 100644 --- a/spec/fixtures/test_module/lib/puppet/transport/test_device.rb +++ b/spec/fixtures/test_module/lib/puppet/transport/test_device.rb @@ -1,19 +1,19 @@ module Puppet::Transport # a transport for a test_device class TestDevice - def initialize(connection_info); + def initialize(_context, connection_info); puts connection_info end - def facts + def facts(_context) { 'foo' => 'bar' } end - def verify + def verify(_context) return true end - def close + def close(_context) return end end diff --git a/spec/puppet/resource_api/base_context_spec.rb b/spec/puppet/resource_api/base_context_spec.rb index 34869f75..1c85129d 100644 --- a/spec/puppet/resource_api/base_context_spec.rb +++ b/spec/puppet/resource_api/base_context_spec.rb @@ -13,10 +13,17 @@ def send_log(log, msg) TestContext.new(definition) end - let(:definition) { { name: 'some_resource', attributes: { name: { type: 'String', desc: 'message' } }, features: feature_support } } + let(:definition_hash) { { name: 'some_resource', attributes: { name: { type: 'String', desc: 'message' } }, features: feature_support } } + let(:definition) { Puppet::ResourceApi::TypeDefinition.new(definition_hash) } let(:feature_support) { [] } - it { expect { described_class.new(nil) }.to raise_error ArgumentError, %r{BaseContext requires definition to be a Hash} } + it { expect { described_class.new(nil) }.to raise_error ArgumentError, %r{BaseContext requires definition to be a child of Puppet::ResourceApi::BaseTypeDefinition} } + describe 'legacy hash definition support' do + let(:definition) { definition_hash } + + it { expect { context }.not_to raise_error } + it { expect(context.type.name).to eq 'some_resource' } + end describe '#failed?' do it('defaults to false') { is_expected.not_to be_failed } diff --git a/spec/puppet/resource_api/transport/wrapper_spec.rb b/spec/puppet/resource_api/transport/wrapper_spec.rb index f2692b19..b6652424 100644 --- a/spec/puppet/resource_api/transport/wrapper_spec.rb +++ b/spec/puppet/resource_api/transport/wrapper_spec.rb @@ -37,12 +37,15 @@ describe '#facts' do context 'when called' do let(:instance) { described_class.new('wibble', {}) } + let(:context) { instance_double(Puppet::ResourceApi::PuppetContext, 'context') } let(:facts) { { 'foo' => 'bar' } } let(:transport) { instance_double(Puppet::Transport::TestDevice, 'transport') } it 'will return the facts provided by the transport' do allow(Puppet::ResourceApi::Transport).to receive(:connect).and_return(transport) - allow(transport).to receive(:facts).and_return(facts) + allow(Puppet::ResourceApi::Transport).to receive(:list).and_return(schema: :dummy) + allow(Puppet::ResourceApi::PuppetContext).to receive(:new).and_return(context) + allow(transport).to receive(:facts).with(context).and_return(facts) expect(instance.facts).to eq(facts) end @@ -53,12 +56,13 @@ context 'when the transport can handle the method' do let(:instance) { described_class.new('wibble', {}) } let(:transport) { instance_double(Puppet::Transport::TestDevice, 'transport') } + let(:context) { instance_double(Puppet::ResourceApi::PuppetContext, 'context') } it 'will return the facts provided by the transport' do allow(Puppet::ResourceApi::Transport).to receive(:connect).and_return(transport) expect(transport).to receive(:close) - instance.close + instance.close(context) end end diff --git a/spec/puppet/resource_api/transport_spec.rb b/spec/puppet/resource_api/transport_spec.rb index 3208900d..c9a748a9 100644 --- a/spec/puppet/resource_api/transport_spec.rb +++ b/spec/puppet/resource_api/transport_spec.rb @@ -11,76 +11,118 @@ Puppet.debug = true end - context 'when registering a schema with missing keys' do - it { expect { described_class.register([]) }.to raise_error(Puppet::DevError, %r{requires a hash as schema}) } - it { expect { described_class.register({}) }.to raise_error(Puppet::DevError, %r{requires a `:name`}) } - it { expect { described_class.register(name: 'no connection info', desc: 'some description') }.to raise_error(Puppet::DevError, %r{requires `:connection_info`}) } - it { expect { described_class.register(name: 'no description') }.to raise_error(Puppet::DevError, %r{requires `:desc`}) } - it { expect { described_class.register(name: 'no hash attributes', desc: 'some description', connection_info: []) }.to raise_error(Puppet::DevError, %r{`:connection_info` must be a hash, not}) } + after(:each) do + # reset registered transports between tests to reduce cross-test poisoning + described_class.instance_variable_set(:@transports, nil) end - context 'when registering a minimal transport' do - let(:schema) { { name: 'minimal', desc: 'a minimal connection', connection_info: {} } } + describe '#register(schema)' do + context 'when registering a schema with missing keys' do + it { expect { described_class.register([]) }.to raise_error(Puppet::DevError, %r{requires a hash as schema}) } + it { expect { described_class.register({}) }.to raise_error(Puppet::DevError, %r{requires a `:name`}) } + it { expect { described_class.register(name: 'no connection info', desc: 'some description') }.to raise_error(Puppet::DevError, %r{requires `:connection_info`}) } + it { expect { described_class.register(name: 'no description') }.to raise_error(Puppet::DevError, %r{requires `:desc`}) } + it { expect { described_class.register(name: 'no hash attributes', desc: 'some description', connection_info: []) }.to raise_error(Puppet::DevError, %r{`:connection_info` must be a hash, not}) } + end + + context 'when registering a minimal transport' do + let(:schema) { { name: 'minimal', desc: 'a minimal connection', connection_info: {} } } - it { expect { described_class.register(schema) }.not_to raise_error } + it { expect { described_class.register(schema) }.not_to raise_error } - context 'when re-registering a transport' do - it { expect { described_class.register(schema) }.to raise_error(Puppet::DevError, %r{`minimal` is already registered}) } + context 'when re-registering a transport' do + it { + described_class.register(schema) + expect { described_class.register(schema) }.to raise_error(Puppet::DevError, %r{`minimal` is already registered}) + } + end end - end - context 'when registering a transport' do - let(:schema) do - { - name: 'a_remote_thing', - desc: 'basic transport', - connection_info: { - host: { - type: 'String', - desc: 'the host ip address or hostname', - }, - user: { - type: 'String', - desc: 'the user to connect as', + context 'when registering a transport' do + let(:schema) do + { + name: 'a_remote_thing', + desc: 'basic transport', + connection_info: { + host: { + type: 'String', + desc: 'the host ip address or hostname', + }, + user: { + type: 'String', + desc: 'the user to connect as', + }, + password: { + type: 'String', + sensitive: true, + desc: 'the password to make the connection', + }, }, - 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 + end + + context 'when registering a transport with a bad type' do + let(:schema) do + { + name: 'a_bad_thing', + desc: 'basic transport', + connection_info: { + host: { + type: 'garbage', + desc: 'the host ip address or hostname', + }, }, - }, + } + end + + it { + expect { described_class.register(schema) }.to raise_error( + Puppet::DevError, %r{ is not a valid type specification} + ) } end + end - it 'adds to the transports register' do - expect { described_class.register(schema) }.not_to raise_error + describe '#list' do + subject { described_class.list } + + context 'with no transports registered' do + it { is_expected.to eq({}) } end - end - context 'when registering a transport with a bad type' do - let(:schema) do - { - name: 'a_bad_thing', - desc: 'basic transport', - connection_info: { - host: { - type: 'garbage', - desc: 'the host ip address or hostname', + context 'with a transport registered' do + let(:schema) do + { + name: 'test_target', + desc: 'a basic transport', + connection_info: { + host: { + type: 'String', + desc: 'the host ip address or hostname', + }, }, - }, - } - end + } + end - it { - expect { described_class.register(schema) }.to raise_error( - Puppet::DevError, %r{ is not a valid type specification} - ) - } + before(:each) do + described_class.register(schema) + end + + it { expect(described_class.list['test_target'].definition).to eq schema } + it 'returns a new object' do + expect(described_class.list['test_target'].definition.object_id).not_to eq schema.object_id + end + end end - context 'when connecting to a transport' do + describe '#connect(name, connection_info)', agent_test: true do let(:name) { 'test_target' } - let(:connection_info) do + let(:schema) do { name: 'test_target', desc: 'a basic transport', @@ -94,54 +136,63 @@ end context 'when the transport file does not exist' do - it 'throws a DevError' do - expect(described_class).to receive(:validate).with(name, connection_info) - expect { described_class.connect(name, connection_info) }.to raise_error Puppet::DevError, - %r{Cannot load transport for `test_target`} + it 'throws a LoadError' do + expect(described_class).to receive(:validate).with(name, host: 'example.com') + expect { described_class.connect(name, host: 'example.com') }.to raise_error LoadError, %r{(no such file to load|cannot load such file) -- puppet/transport/test_target} end end context 'when the transport file does exist' do context 'with an incorrectly defined transport' do - it 'throws a DevError' do - expect(described_class).to receive(:validate).with(name, connection_info) + it 'throws a NameError' do + described_class.register(schema) + + expect(described_class).to receive(:validate).with(name, host: 'example.com') expect(described_class).to receive(:require).with('puppet/transport/test_target') - expect { described_class.connect(name, connection_info) }.to raise_error Puppet::DevError, - %r{uninitialized constant (Puppet::Transport|TestTarget)} + expect { described_class.connect(name, host: 'example.com') }.to raise_error NameError, + %r{uninitialized constant (Puppet::Transport|TestTarget)} end end context 'with a correctly defined transport' do let(:test_target) { double('Puppet::Transport::TestTarget') } # rubocop:disable RSpec/VerifiedDoubles + let(:context) { instance_double(Puppet::ResourceApi::PuppetContext, 'context') } it 'loads initiates the class successfully' do + described_class.register(schema) + + allow(described_class).to receive(:require).with('puppet/resource_api/puppet_context').and_call_original expect(described_class).to receive(:require).with('puppet/transport/test_target') - expect(described_class).to receive(:validate).with(name, connection_info) + expect(described_class).to receive(:validate).with(name, host: 'example.com') + expect(Puppet::ResourceApi::PuppetContext).to receive(:new).with(kind_of(Puppet::ResourceApi::TransportSchemaDef)).and_return(context) stub_const('Puppet::Transport::TestTarget', test_target) - expect(test_target).to receive(:new).with(connection_info) + expect(test_target).to receive(:new).with(context, host: 'example.com') - described_class.connect(name, connection_info) + described_class.connect(name, host: 'example.com') end end end end - describe '#self.validate' do + describe '#validate(name, connection_info)', agent_test: true do context 'when the transport being validated has not be registered' do - it { expect { described_class.validate('wibble', {}) }.to raise_error Puppet::DevError, %r{Transport for `wibble` not registered} } + it { expect { described_class.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 been registered' do let(:schema) { { name: 'validate', desc: 'a minimal connection', connection_info: {} } } + let(:schema_def) { instance_double('Puppet::ResourceApi::TransportSchemaDef', 'schema_def') } + + it 'validates the connection_info' do + allow(Puppet::ResourceApi::TransportSchemaDef).to receive(:new).with(schema).and_return(schema_def) - it 'continues to validate the connection_info' do - # rubocop:disable RSpec/AnyInstance - expect_any_instance_of(Puppet::ResourceApi::TransportSchemaDef).to receive(:check_schema).with({}) - expect_any_instance_of(Puppet::ResourceApi::TransportSchemaDef).to receive(:validate).with({}) - # rubocop:enable RSpec/AnyInstance described_class.register(schema) - described_class.validate('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') end end end