From 623af81a656d2207ef0ea2557b9a716a40c21803 Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Fri, 25 May 2018 15:42:34 -0500 Subject: [PATCH 01/19] WIP: Rework network_route provider Signed-off-by: David Hollinger --- .sync.yml | 2 + Gemfile | 2 + .../provider/network_route/network_route.rb | 60 ++++++++ .../redhat.rb | 0 .../routes.rb | 0 lib/puppet/type/network_route.rb | 141 ++++++++---------- lib/puppet/type/network_route_old.rb | 81 ++++++++++ .../network_route/network_route_spec.rb | 49 ++++++ spec/unit/puppet/type/network_route_spec.rb | 8 + 9 files changed, 262 insertions(+), 81 deletions(-) create mode 100644 lib/puppet/provider/network_route/network_route.rb rename lib/puppet/provider/{network_route => network_route_old}/redhat.rb (100%) rename lib/puppet/provider/{network_route => network_route_old}/routes.rb (100%) create mode 100644 lib/puppet/type/network_route_old.rb create mode 100644 spec/unit/puppet/provider/network_route/network_route_spec.rb create mode 100644 spec/unit/puppet/type/network_route_spec.rb diff --git a/.sync.yml b/.sync.yml index 0fee9dfd..65f7cb48 100644 --- a/.sync.yml +++ b/.sync.yml @@ -6,6 +6,8 @@ spec/spec_helper.rb: spec_overrides: "require 'spec_helper_methods'" Gemfile: optional: + ':development': + - gem: 'puppet-resource_api' ':test': - gem: 'ipaddress' - gem: 'rspec-its' diff --git a/Gemfile b/Gemfile index b96cc2ad..65ac649f 100644 --- a/Gemfile +++ b/Gemfile @@ -31,6 +31,7 @@ group :test do gem 'simplecov-console', :require => false gem 'parallel_tests', :require => false gem 'ipaddress', :require => false + gem 'net-ip', :require => false gem 'rspec-its', :require => false end @@ -39,6 +40,7 @@ group :development do gem 'travis-lint', :require => false gem 'guard-rake', :require => false gem 'overcommit', '>= 0.39.1', :require => false + gem 'puppet-resource_api', :require => false end group :system_tests do diff --git a/lib/puppet/provider/network_route/network_route.rb b/lib/puppet/provider/network_route/network_route.rb new file mode 100644 index 00000000..eff3d65a --- /dev/null +++ b/lib/puppet/provider/network_route/network_route.rb @@ -0,0 +1,60 @@ +require 'net/ip' +# require_relative '../../../puppet_x/voxpupuli/utils' +require 'puppet/resource_api/simple_provider' + +# Implementation for the network_route type using the Resource API. +class Puppet::Provider::NetworkRoute::NetworkRoute < Puppet::ResourceApi::SimpleProvider + # include PuppetX::FileMapper + + def routes_list + routes = [] + Net::IP.routes.each do |route| + routes.push(route.instance_variables.each_with_object({}) { |var, hash| hash[var.to_s.delete("@")] = route.instance_variable_get(var) }) + end + routes + end + + def get(_context) + routes_list.map do |route| + default = if route['prefix'] == 'default' + true + else + false + end + + { + ensure: 'present', + prefix: route['prefix'], + default_route: default, + gateway: route['via'], + interface: route['dev'], + metric: route['metric'], + table: route['table'], + source: route['src'], + scope: route['scope'], + protocol: route['proto'], + mtu: route['mtu'] + }.compact! + end + end + + def puppet_munge(should) + should.delete(:ensure) + if should[:default_route] + should[:prefix] = 'default' + should.delete(:default_route) + should.delete(:prefix) + else + should[:prefix] = should.delete(:prefix) + end + should[:via] = should.delete(:gateway) if should[:gateway] + should[:dev] = should.delete(:interface) if should[:interface] + should[:metric] = should.delete(:metric) + should[:table] = should.delete(:table) + should[:src] = should.delete(:source) if should[:source] + should[:scope] = should.delete(:scope) + should[:proto] = should.delete(:protocol) + should[:mtu] = should.delete(:mtu) if should[:mtu] + should + end +end diff --git a/lib/puppet/provider/network_route/redhat.rb b/lib/puppet/provider/network_route_old/redhat.rb similarity index 100% rename from lib/puppet/provider/network_route/redhat.rb rename to lib/puppet/provider/network_route_old/redhat.rb diff --git a/lib/puppet/provider/network_route/routes.rb b/lib/puppet/provider/network_route_old/routes.rb similarity index 100% rename from lib/puppet/provider/network_route/routes.rb rename to lib/puppet/provider/network_route_old/routes.rb diff --git a/lib/puppet/type/network_route.rb b/lib/puppet/type/network_route.rb index 7dd06da4..54634407 100644 --- a/lib/puppet/type/network_route.rb +++ b/lib/puppet/type/network_route.rb @@ -1,81 +1,60 @@ -require 'ipaddr' -require_relative '../../puppet_x/voxpupuli/utils.rb' - -Puppet::Type.newtype(:network_route) do - @doc = 'Manage non-volatile route configuration information' - - include PuppetX::Voxpupuli::Utils - - ensurable - - newparam(:name) do - isnamevar - desc 'The name of the network route' - end - - newproperty(:network) do - isrequired - desc 'The target network address' - validate do |value| - unless value == 'default' - a = PuppetX::Voxpupuli::Utils.try { IPAddr.new(value) } - raise("Invalid value for parameter 'network': #{value}") unless a - end - end - end - - newproperty(:netmask) do - isrequired - desc 'The subnet mask to apply to the route' - - validate do |value| - unless value.length <= 3 || PuppetX::Voxpupuli::Utils.try { IPAddr.new(value) } - raise("Invalid value for parameter 'netmask': #{value}") - end - end - - munge do |value| - # '255.255.255.255'.to_i will return 255, so we try to convert it back: - if value.to_i.to_s == value - # what are the chances someone is using /16 for their IPv6 network? - addr = value.to_i <= 32 ? '255.255.255.255' : 'ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff' - IPAddr.new(addr).mask(value.strip.to_i).to_s - elsif PuppetX::Voxpupuli::Utils.try { IPAddr.new(value).ipv6? } - IPAddr.new('ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff').mask(value).to_s - elsif PuppetX::Voxpupuli::Utils.try { IPAddr.new(value).ipv4? } - IPAddr.new('255.255.255.255').mask(value).to_s - else - raise("Invalid value for parameter 'netmask': #{value}") - end - end - end - - newproperty(:gateway) do - isrequired - desc 'The gateway to use for the route' - - validate do |value| - begin - IPAddr.new(value) - rescue ArgumentError - raise("Invalid value for parameter 'gateway': #{value}") - end - end - end - - newproperty(:interface) do - isrequired - desc 'The interface to use for the route' - end - - # `:options` provides an arbitrary passthrough for provider properties, so - # that provider specific behavior doesn't clutter up the main type but still - # allows for more powerful actions to be taken. - newproperty(:options, required_features: :provider_options) do - desc 'Provider specific options to be passed to the provider' - - validate do |value| - raise ArgumentError, "#{self.class} requires a string for the options property" unless value.is_a?(String) - end - end -end +require 'puppet/resource_api' + +Puppet::ResourceApi.register_type( + name: 'network_route', + docs: <<-EOS, + Manage non-volatile route configuration information. + EOS + attributes: { + ensure: { + type: 'Enum[present, absent]', + desc: 'Whether the network route should be present or absent on the target system.', + default: 'present', + }, + prefix: { + type: 'String', + desc: 'The destination prefix/network of the route.', + behaviour: :namevar, + }, + default_route: { + type: 'Optional[Boolean]', + desc: 'Whether the route is default or not.', + }, + gateway: { + type: 'Optional[String]', + desc: 'The gateway to use for the route.', + }, + interface: { + type: 'Optional[String]', + desc: 'The interface to use for the route.', + }, + metric: { + type: 'String', + desc: 'preference value of the route. NUMBER is an arbitrary 32bit number.', + default: '100', + }, + table: { + type: 'String', + desc: 'table to add this route.', + default: 'local', + }, + source: { + type: 'Optional[String]', + desc: 'the source address to prefer using when sending to the destinations covered by route prefix.', + }, + scope: { + type: 'Enum["global", "nowhere", "host", "link", "site"]', + desc: 'scope of the destinations covered by the route prefix.', + default: 'global', + }, + protocol: { + type: 'String', + desc: 'routing protocol identifier of this route.', + default: 'boot', + }, + mtu: { + type: 'Optional[String]', + desc: 'the MTU along the path to destination.', + }, + }, +) diff --git a/lib/puppet/type/network_route_old.rb b/lib/puppet/type/network_route_old.rb new file mode 100644 index 00000000..03289264 --- /dev/null +++ b/lib/puppet/type/network_route_old.rb @@ -0,0 +1,81 @@ +require 'ipaddr' +require_relative '../../puppet_x/voxpupuli/utils.rb' + +Puppet::Type.newtype(:network_route) do + @doc = 'Manage non-volatile route configuration information' + + include PuppetX::Voxpupuli::Utils + + ensurable + + newparam(:name) do + isnamevar + desc 'The name of the network route' + end + + newproperty(:network) do + isrequired + desc 'The target network address' + validate do |value| + unless value == 'default' + a = PuppetX::Voxpupuli::Utils.try { IPAddr.new(value) } + raise("Invalid value for network: #{value}") unless a + end + end + end + + newproperty(:netmask) do + isrequired + desc 'The subnet mask to apply to the route' + + validate do |value| + unless value.length <= 3 || PuppetX::Voxpupuli::Utils.try { IPAddr.new(value) } + raise("Invalid value for argument netmask: #{value}") + end + end + + munge do |value| + # '255.255.255.255'.to_i will return 255, so we try to convert it back: + if value.to_i.to_s == value + # what are the chances someone is using /16 for their IPv6 network? + addr = value.to_i <= 32 ? '255.255.255.255' : 'ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff' + IPAddr.new(addr).mask(value.strip.to_i).to_s + elsif PuppetX::Voxpupuli::Utils.try { IPAddr.new(value).ipv6? } + IPAddr.new('ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff').mask(value).to_s + elsif PuppetX::Voxpupuli::Utils.try { IPAddr.new(value).ipv4? } + IPAddr.new('255.255.255.255').mask(value).to_s + else + raise("Invalid value for argument netmask: #{value}") + end + end + end + + newproperty(:gateway) do + isrequired + desc 'The gateway to use for the route' + + validate do |value| + begin + IPAddr.new(value) + rescue ArgumentError + raise("Invalid value for gateway: #{value}") + end + end + end + + newproperty(:interface) do + isrequired + desc 'The interface to use for the route' + end + + # `:options` provides an arbitrary passthrough for provider properties, so + # that provider specific behavior doesn't clutter up the main type but still + # allows for more powerful actions to be taken. + newproperty(:options, required_features: :provider_options) do + desc 'Provider specific options to be passed to the provider' + + validate do |value| + raise ArgumentError, "#{self.class} requires a string for the options property" unless value.is_a?(String) + end + end +end diff --git a/spec/unit/puppet/provider/network_route/network_route_spec.rb b/spec/unit/puppet/provider/network_route/network_route_spec.rb new file mode 100644 index 00000000..3c2dd98a --- /dev/null +++ b/spec/unit/puppet/provider/network_route/network_route_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +ensure_module_defined('Puppet::Provider::NetworkRoute') +require 'puppet/provider/network_route/network_route' + +RSpec.describe Puppet::Provider::NetworkRoute::NetworkRoute do + subject(:provider) { described_class.new } + + let(:context) { instance_double('Puppet::ResourceApi::BaseContext', 'context') } + + describe '#get' do + it 'processes resources' do + expect(provider.get(context)).to eq [ + { + name: 'foo', + ensure: 'present', + }, + { + name: 'bar', + ensure: 'present', + }, + ] + end + end + + describe 'create(context, name, should)' do + it 'creates the resource' do + expect(context).to receive(:notice).with(%r{\ACreating 'a'}) + + provider.create(context, 'a', name: 'a', ensure: 'present') + end + end + + describe 'update(context, name, should)' do + it 'updates the resource' do + expect(context).to receive(:notice).with(%r{\AUpdating 'foo'}) + + provider.update(context, 'foo', name: 'foo', ensure: 'present') + end + end + + describe 'delete(context, name, should)' do + it 'deletes the resource' do + expect(context).to receive(:notice).with(%r{\ADeleting 'foo'}) + + provider.delete(context, 'foo') + end + end +end diff --git a/spec/unit/puppet/type/network_route_spec.rb b/spec/unit/puppet/type/network_route_spec.rb new file mode 100644 index 00000000..29a7bb8e --- /dev/null +++ b/spec/unit/puppet/type/network_route_spec.rb @@ -0,0 +1,8 @@ +require 'spec_helper' +require 'puppet/type/network_route' + +RSpec.describe 'the network_route type' do + it 'loads' do + expect(Puppet::Type.type(:network_route)).not_to be_nil + end +end From c245a977e17f8c563e0b8726481d60a59c4fcf94 Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Wed, 30 May 2018 14:26:40 -0500 Subject: [PATCH 02/19] Remove simple_provider --- lib/puppet/provider/network_route/network_route.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/puppet/provider/network_route/network_route.rb b/lib/puppet/provider/network_route/network_route.rb index eff3d65a..3e93440c 100644 --- a/lib/puppet/provider/network_route/network_route.rb +++ b/lib/puppet/provider/network_route/network_route.rb @@ -3,7 +3,7 @@ require 'puppet/resource_api/simple_provider' # Implementation for the network_route type using the Resource API. -class Puppet::Provider::NetworkRoute::NetworkRoute < Puppet::ResourceApi::SimpleProvider +class Puppet::Provider::NetworkRoute::NetworkRoute # include PuppetX::FileMapper def routes_list From 33cc39a56ede9a1afc3c26cf469e36dfb4fc006b Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Wed, 30 May 2018 16:38:38 -0500 Subject: [PATCH 03/19] Fully functioning provider for network_route --- .../provider/network_route/network_route.rb | 44 +++++++++++++++++-- lib/puppet/type/network_route.rb | 8 ++-- 2 files changed, 43 insertions(+), 9 deletions(-) diff --git a/lib/puppet/provider/network_route/network_route.rb b/lib/puppet/provider/network_route/network_route.rb index 3e93440c..fad57d3f 100644 --- a/lib/puppet/provider/network_route/network_route.rb +++ b/lib/puppet/provider/network_route/network_route.rb @@ -9,7 +9,7 @@ class Puppet::Provider::NetworkRoute::NetworkRoute def routes_list routes = [] Net::IP.routes.each do |route| - routes.push(route.instance_variables.each_with_object({}) { |var, hash| hash[var.to_s.delete("@")] = route.instance_variable_get(var) }) + routes.push(route.to_h) end routes end @@ -43,18 +43,54 @@ def puppet_munge(should) if should[:default_route] should[:prefix] = 'default' should.delete(:default_route) - should.delete(:prefix) else should[:prefix] = should.delete(:prefix) end should[:via] = should.delete(:gateway) if should[:gateway] should[:dev] = should.delete(:interface) if should[:interface] should[:metric] = should.delete(:metric) - should[:table] = should.delete(:table) + should[:table] = should.delete(:table) if should[:table] should[:src] = should.delete(:source) if should[:source] - should[:scope] = should.delete(:scope) + should[:scope] = should.delete(:scope) if should[:scope] should[:proto] = should.delete(:protocol) should[:mtu] = should.delete(:mtu) if should[:mtu] should end + + def set(context, changes) + changes.each do |name, change| + is = change.key?(:is) ? change[:is] : get_single(name) + should = change[:should] + + is = { prefix: name, ensure: 'absent' } if is.nil? + should = { prefix: name, ensure: 'absent' } if should.nil? + + if is[:ensure].to_s == 'absent' && should[:ensure].to_s == 'present' + create(context, name, should) + elsif is[:ensure] == should[:ensure] && is != should + update(context, name, should) + elsif is[:ensure].to_s == 'present' && should[:ensure].to_s == 'absent' + delete(context, name, should) + end + end + end + + def create(context, name, should) + puppet_munge(should) + route = Net::IP::Route.new(should) + Net::IP.routes.add(route) + end + + def update(context, name, should) + puppet_munge(should) + route = Net::IP::Route.new(should) + Net::IP.routes.flush(route.prefix) + Net::IP.routes.add(route) + end + + def delete(context, name, should) + puppet_munge(should) + route = Net::IP::Route.new(should) + Net::IP.routes.flush(route.prefix) + end end diff --git a/lib/puppet/type/network_route.rb b/lib/puppet/type/network_route.rb index 54634407..605beed6 100644 --- a/lib/puppet/type/network_route.rb +++ b/lib/puppet/type/network_route.rb @@ -34,23 +34,21 @@ default: '100', }, table: { - type: 'String', + type: 'Optional[String]', desc: 'table to add this route.', - default: 'local', }, source: { type: 'Optional[String]', desc: 'the source address to prefer using when sending to the destinations covered by route prefix.', }, scope: { - type: 'Enum["global", "nowhere", "host", "link", "site"]', + type: 'Optional[Enum["global", "nowhere", "host", "link", "site"]]', desc: 'scope of the destinations covered by the route prefix.', - default: 'global', }, protocol: { type: 'String', desc: 'routing protocol identifier of this route.', - default: 'boot', + default: 'static', }, mtu: { type: 'Optional[String]', From da5e2608119cb43a743d524bedacfe0e3a567025 Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Wed, 30 May 2018 23:32:24 -0500 Subject: [PATCH 04/19] Convert string hash keys to symbolized keys --- .../provider/network_route/network_route.rb | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/puppet/provider/network_route/network_route.rb b/lib/puppet/provider/network_route/network_route.rb index fad57d3f..240edd7a 100644 --- a/lib/puppet/provider/network_route/network_route.rb +++ b/lib/puppet/provider/network_route/network_route.rb @@ -16,7 +16,7 @@ def routes_list def get(_context) routes_list.map do |route| - default = if route['prefix'] == 'default' + default = if route[:prefix] == 'default' true else false @@ -24,16 +24,16 @@ def get(_context) { ensure: 'present', - prefix: route['prefix'], + prefix: route[:prefix], default_route: default, - gateway: route['via'], - interface: route['dev'], - metric: route['metric'], - table: route['table'], - source: route['src'], - scope: route['scope'], - protocol: route['proto'], - mtu: route['mtu'] + gateway: route[:via], + interface: route[:dev], + metric: route[:metric], + table: route[:table], + source: route[:src], + scope: route[:scope], + protocol: route[:proto], + mtu: route[:mtu] }.compact! end end From f41d8a9c4e168cdd161f5266972057b7d998c12a Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Mon, 11 Jun 2018 16:58:53 -0500 Subject: [PATCH 05/19] Add network::route defined type --- manifests/route.pp | 61 +++++++++++++++++++++++++++++++++++++ spec/classes/route_spec.rb | 11 +++++++ templates/routes/redhat.erb | 23 ++++++++++++++ 3 files changed, 95 insertions(+) create mode 100644 manifests/route.pp create mode 100644 spec/classes/route_spec.rb create mode 100644 templates/routes/redhat.erb diff --git a/manifests/route.pp b/manifests/route.pp new file mode 100644 index 00000000..6fce5a22 --- /dev/null +++ b/manifests/route.pp @@ -0,0 +1,61 @@ +# Deploys a network route to the system using ip route +# and adds the route to a file for persistence. +# +# @summary A short summary of the purpose of this class +# +# @example +# include network::route +define network::route( + Stdlib::Ipv4 $network, + Stdlib::Ipv4 $netmask, + Enum['present', 'absent'] $ensure = 'present', + Boolean $default_route = false, + String $metric = '100', + String $protocol = 'static', + Optional[Stdlib::Ipv4] $gateway, + Optional[String] $interface, + Optional[String] $table, + Optional[Stdlib::Ipv4] $source, + Optional[Enum['global', 'nowhere', 'host', 'link', 'site']] $scope, + Optional[String] $mtu, +) { + $route_file = case $facts['os']['name'] { + 'Debian': { + '/etc/network/routes' + } + 'Ubuntu': { + if $facts['os']['release']['full'] == '18.04' { + '/etc/network/routes' + } else { + '/etc/network/routes' + } + } + /RedHat|CentOS/: { + "/etc/sysconfig/network-scripts/route-${interface}" + } + default: { + fail("Network::Route is not compatible with ${facts['os']['family']}!") + } + } + + file { $route_file: + ensure => $ensure, + owner => 'root', + group => 'root', + mode => '0755', + content => template("network/routes/${facts['os']['family'].downcase}.erb"), + } + + network_route { "${network}/${netmask}": + ensure => $ensure, + default_route => $default_route, + gateway => $gateway, + interface => $interface, + metric => $metric, + table => $table, + source => $source, + scope => $scope, + protocol => $protocol, + mtu => $mtu, + } +} diff --git a/spec/classes/route_spec.rb b/spec/classes/route_spec.rb new file mode 100644 index 00000000..6ac6f26c --- /dev/null +++ b/spec/classes/route_spec.rb @@ -0,0 +1,11 @@ +require 'spec_helper' + +describe 'network::route' do + on_supported_os.each do |os, os_facts| + context "on #{os}" do + let(:facts) { os_facts } + + it { is_expected.to compile } + end + end +end diff --git a/templates/routes/redhat.erb b/templates/routes/redhat.erb new file mode 100644 index 00000000..0a1909e6 --- /dev/null +++ b/templates/routes/redhat.erb @@ -0,0 +1,23 @@ +<% require 'net/ip' + route = { + prefix: @prefix, + proto: @protocol, + metric: @metric, + dev: @interface, + via: @gateway, + src: @source, + scope: @scope, + mtu: @mtu, + table: @table, + } + + route.each do |k, v| + if v.nil? + route.delete(k) + end + end + + new_route = Net::IP::Route.new(new_route) +%> + +<%= new_route.prefix %> <%= new_route.to_params %> \ No newline at end of file From ae6d6d71da52578cac82344fe4c6139622508c62 Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Mon, 16 Jul 2018 22:59:32 -0500 Subject: [PATCH 06/19] Remove legacy network route resources --- .../provider/network_route_old/redhat.rb | 104 --------------- .../provider/network_route_old/routes.rb | 118 ------------------ lib/puppet/type/network_route_old.rb | 81 ------------ 3 files changed, 303 deletions(-) delete mode 100644 lib/puppet/provider/network_route_old/redhat.rb delete mode 100644 lib/puppet/provider/network_route_old/routes.rb delete mode 100644 lib/puppet/type/network_route_old.rb diff --git a/lib/puppet/provider/network_route_old/redhat.rb b/lib/puppet/provider/network_route_old/redhat.rb deleted file mode 100644 index 536a946f..00000000 --- a/lib/puppet/provider/network_route_old/redhat.rb +++ /dev/null @@ -1,104 +0,0 @@ -require 'ipaddr' -require 'puppetx/filemapper' - -Puppet::Type.type(:network_route).provide(:redhat) do - # RHEL network_route routes provider. - # - # This provider uses the filemapper mixin to map the routes file to a - # collection of network_route providers, and back. - # - # @see https://access.redhat.com/knowledge/docs/en-US/Red_Hat_Enterprise_Linux/6/html/Deployment_Guide/s1-networkscripts-static-routes.html - - include PuppetX::FileMapper - - desc 'RHEL style routes provider' - - confine osfamily: :redhat - defaultfor osfamily: :redhat - - has_feature :provider_options - - def select_file - "/etc/sysconfig/network-scripts/route-#{@resource[:interface]}" - end - - def self.target_files - Dir['/etc/sysconfig/network-scripts/route-*'] - end - - def self.parse_file(_filename, contents) - routes = [] - - lines = contents.split("\n") - lines.each do |line| - # Strip off any trailing comments - line.sub!(%r{#.*$}, '') - - if line =~ %r{^\s*#|^\s*$} - # Ignore comments and blank lines - next - end - - route = line.split(' ', 6) - if route.length < 4 - raise Puppet::Error, 'Malformed redhat route file, cannot instantiate network_route resources' - end - - new_route = {} - - new_route[:gateway] = route[2] - new_route[:interface] = route[4] - new_route[:options] = route[5] if route[5] - - if route[0] == 'default' - cidr_target = 'default' - - new_route[:name] = cidr_target - new_route[:network] = 'default' - new_route[:netmask] = '0.0.0.0' - else - # use the CIDR version of the target as :name - network, netmask = route[0].split('/') - cidr_target = "#{network}/#{IPAddr.new(netmask).to_i.to_s(2).count('1')}" - - new_route[:name] = cidr_target - new_route[:network] = network - new_route[:netmask] = netmask - end - - routes << new_route - end - - routes - end - - # Generate an array of sections - def self.format_file(_filename, providers) - contents = [] - contents << header - # Build routes - providers.sort_by(&:name).each do |provider| - [:network, :netmask, :gateway, :interface].each do |prop| - raise Puppet::Error, "#{provider.name} is missing the required parameter '#{prop}'." if provider.send(prop).nil? - end - contents << if provider.network == 'default' - "#{provider.network} via #{provider.gateway} dev #{provider.interface}" - else - "#{provider.network}/#{provider.netmask} via #{provider.gateway} dev #{provider.interface}" - end - contents << (provider.options == :absent ? "\n" : " #{provider.options}\n") - end - contents.join - end - - def self.header - str = <<-HEADER -# HEADER: This file is being managed by puppet. Changes to -# HEADER: routes that are not being managed by puppet will persist; -# HEADER: however changes to routes that are being managed by puppet will -# HEADER: be overwritten. In addition, file order is NOT guaranteed. -# HEADER: Last generated at: #{Time.now} -HEADER - str - end -end diff --git a/lib/puppet/provider/network_route_old/routes.rb b/lib/puppet/provider/network_route_old/routes.rb deleted file mode 100644 index 290bb4d7..00000000 --- a/lib/puppet/provider/network_route_old/routes.rb +++ /dev/null @@ -1,118 +0,0 @@ -require 'ipaddr' -require 'puppetx/filemapper' - -Puppet::Type.type(:network_route).provide(:routes) do - # Debian network_route routes provider. - # - # This provider uses the filemapper mixin to map the routes file to a - # collection of network_route providers, and back. - # - # @see http://wiki.debian.org/NetworkConfiguration - # @see http://packages.debian.org/squeeze/ifupdown-extras - - include PuppetX::FileMapper - - desc 'Debian routes style provider' - - confine osfamily: :debian - - # $ dpkg -S /etc/network/if-up.d/20static-routes - # ifupdown-extra: /etc/network/if-up.d/20static-routes - confine exists: '/etc/network/if-up.d/20static-routes' - - defaultfor osfamily: :debian - - has_feature :provider_options - - def select_file - '/etc/network/routes' - end - - def self.target_files - ['/etc/network/routes'] - end - - class MalformedRoutesError < Puppet::Error - def initialize(msg = nil) - msg = 'Malformed debian routes file; cannot instantiate network_route resources' if msg.nil? - super - end - end - - def self.raise_malformed - @failed = true - raise MalformedRoutesError - end - - def self.parse_file(_filename, contents) - # Build out an empty hash for new routes for storing their configs. - route_hash = Hash.new do |hash, key| - hash[key] = {} - hash[key][:name] = key - hash[key] - end - - lines = contents.split("\n") - lines.each do |line| - # Strip off any trailing comments - line.sub!(%r{#.*$}, '') - - if line =~ %r{^\s*#|^\s*$} - # Ignore comments and blank lines - next - end - - route = line.split(' ', 5) - - raise_malformed if route.length < 4 - - if route[0] == 'default' - name = 'default' - route_hash[name][:network] = 'default' - route_hash[name][:netmask] = '0.0.0.0' - else - # use the CIDR version of the target as :name - name = "#{route[0]}/#{IPAddr.new(route[1]).to_i.to_s(2).count('1')}" - route_hash[name][:network] = route[0] - route_hash[name][:netmask] = route[1] - end - route_hash[name][:gateway] = route[2] - route_hash[name][:interface] = route[3] - route_hash[name][:options] = route[4] if route[4] - end - - route_hash.values - end - - # Generate an array of sections - def self.format_file(_filename, providers) - contents = [] - contents << header - - # Build routes - providers.sort_by(&:name).each do |provider| - raise Puppet::Error, "#{provider.name} is missing the required parameter 'network'." if provider.network.nil? - raise Puppet::Error, "#{provider.name} is missing the required parameter 'netmask'." if provider.netmask.nil? - raise Puppet::Error, "#{provider.name} is missing the required parameter 'gateway'." if provider.gateway.nil? - raise Puppet::Error, "#{provider.name} is missing the required parameter 'interface'." if provider.interface.nil? - - netmask = (provider.name == 'default' ? '0.0.0.0' : provider.netmask) - - contents << "#{provider.network} #{netmask} #{provider.gateway} #{provider.interface}" - contents << (provider.options == :absent ? "\n" : " #{provider.options}\n") - end - - contents.join - end - - def self.header - str = <<-HEADER -# HEADER: This file is being managed by puppet. Changes to -# HEADER: routes that are not being managed by puppet will persist; -# HEADER: however changes to routes that are being managed by puppet will -# HEADER: be overwritten. In addition, file order is NOT guaranteed. -# HEADER: Last generated at: #{Time.now} -HEADER - str - end -end diff --git a/lib/puppet/type/network_route_old.rb b/lib/puppet/type/network_route_old.rb deleted file mode 100644 index 03289264..00000000 --- a/lib/puppet/type/network_route_old.rb +++ /dev/null @@ -1,81 +0,0 @@ -require 'ipaddr' -require_relative '../../puppet_x/voxpupuli/utils.rb' - -Puppet::Type.newtype(:network_route) do - @doc = 'Manage non-volatile route configuration information' - - include PuppetX::Voxpupuli::Utils - - ensurable - - newparam(:name) do - isnamevar - desc 'The name of the network route' - end - - newproperty(:network) do - isrequired - desc 'The target network address' - validate do |value| - unless value == 'default' - a = PuppetX::Voxpupuli::Utils.try { IPAddr.new(value) } - raise("Invalid value for network: #{value}") unless a - end - end - end - - newproperty(:netmask) do - isrequired - desc 'The subnet mask to apply to the route' - - validate do |value| - unless value.length <= 3 || PuppetX::Voxpupuli::Utils.try { IPAddr.new(value) } - raise("Invalid value for argument netmask: #{value}") - end - end - - munge do |value| - # '255.255.255.255'.to_i will return 255, so we try to convert it back: - if value.to_i.to_s == value - # what are the chances someone is using /16 for their IPv6 network? - addr = value.to_i <= 32 ? '255.255.255.255' : 'ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff' - IPAddr.new(addr).mask(value.strip.to_i).to_s - elsif PuppetX::Voxpupuli::Utils.try { IPAddr.new(value).ipv6? } - IPAddr.new('ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff').mask(value).to_s - elsif PuppetX::Voxpupuli::Utils.try { IPAddr.new(value).ipv4? } - IPAddr.new('255.255.255.255').mask(value).to_s - else - raise("Invalid value for argument netmask: #{value}") - end - end - end - - newproperty(:gateway) do - isrequired - desc 'The gateway to use for the route' - - validate do |value| - begin - IPAddr.new(value) - rescue ArgumentError - raise("Invalid value for gateway: #{value}") - end - end - end - - newproperty(:interface) do - isrequired - desc 'The interface to use for the route' - end - - # `:options` provides an arbitrary passthrough for provider properties, so - # that provider specific behavior doesn't clutter up the main type but still - # allows for more powerful actions to be taken. - newproperty(:options, required_features: :provider_options) do - desc 'Provider specific options to be passed to the provider' - - validate do |value| - raise ArgumentError, "#{self.class} requires a string for the options property" unless value.is_a?(String) - end - end -end From 1f7ea322f942e6689c266148af8fbd76b98a2b4b Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Sat, 21 Jul 2018 22:12:53 -0500 Subject: [PATCH 07/19] Initial test working but needs mocks --- .../network_route/network_route_spec.rb | 89 +++++++++++++------ 1 file changed, 62 insertions(+), 27 deletions(-) diff --git a/spec/unit/puppet/provider/network_route/network_route_spec.rb b/spec/unit/puppet/provider/network_route/network_route_spec.rb index 3c2dd98a..d8d1c49f 100644 --- a/spec/unit/puppet/provider/network_route/network_route_spec.rb +++ b/spec/unit/puppet/provider/network_route/network_route_spec.rb @@ -1,6 +1,7 @@ require 'spec_helper' -ensure_module_defined('Puppet::Provider::NetworkRoute') +# nsure_module_defined('Puppet::Provider::NetworkRoute') +module Puppet::Provider::NetworkRoute; end require 'puppet/provider/network_route/network_route' RSpec.describe Puppet::Provider::NetworkRoute::NetworkRoute do @@ -11,39 +12,73 @@ describe '#get' do it 'processes resources' do expect(provider.get(context)).to eq [ - { - name: 'foo', - ensure: 'present', - }, - { - name: 'bar', - ensure: 'present', - }, + {:ensure=>"present", + :prefix=>"default", + :default_route=>true, + :gateway=>"10.155.255.1", + :interface=>"wlp3s0", + :metric=>"600", + :protocol=>"dhcp"}, + {:ensure=>"present", + :prefix=>"10.155.255.0/24", + :default_route=>false, + :interface=>"wlp3s0", + :metric=>"600", + :source=>"10.155.255.110", + :scope=>"link", + :protocol=>"kernel"}, + {:ensure=>"present", + :prefix=>"169.254.0.0/16", + :default_route=>false, + :interface=>"virbr0", + :metric=>"1000", + :scope=>"link"}, + {:ensure=>"present", + :prefix=>"172.17.0.0/16", + :default_route=>false, + :interface=>"docker0", + :source=>"172.17.0.1", + :scope=>"link", + :protocol=>"kernel"}, + {:ensure=>"present", + :prefix=>"172.18.0.0/16", + :default_route=>false, + :interface=>"br-39a722eeac35", + :source=>"172.18.0.1", + :scope=>"link", + :protocol=>"kernel"}, + {:ensure=>"present", + :prefix=>"192.168.122.0/24", + :default_route=>false, + :interface=>"virbr0", + :source=>"192.168.122.1", + :scope=>"link", + :protocol=>"kernel"} ] end end - describe 'create(context, name, should)' do - it 'creates the resource' do - expect(context).to receive(:notice).with(%r{\ACreating 'a'}) + # describe 'create(context, name, should)' do + # it 'creates the resource' do + # expect(context).to receive(:notice).with(%r{\ACreating 'a'}) - provider.create(context, 'a', name: 'a', ensure: 'present') - end - end + # provider.create(context, 'a', name: 'a', ensure: 'present') + # end + # end - describe 'update(context, name, should)' do - it 'updates the resource' do - expect(context).to receive(:notice).with(%r{\AUpdating 'foo'}) + # describe 'update(context, name, should)' do + # it 'updates the resource' do + # expect(context).to receive(:notice).with(%r{\AUpdating 'foo'}) - provider.update(context, 'foo', name: 'foo', ensure: 'present') - end - end + # provider.update(context, 'foo', name: 'foo', ensure: 'present') + # end + # end - describe 'delete(context, name, should)' do - it 'deletes the resource' do - expect(context).to receive(:notice).with(%r{\ADeleting 'foo'}) + # describe 'delete(context, name, should)' do + # it 'deletes the resource' do + # expect(context).to receive(:notice).with(%r{\ADeleting 'foo'}) - provider.delete(context, 'foo') - end - end + # provider.delete(context, 'foo') + # end + # end end From 36ba373e10dd0b65ff062f712473951041221382 Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Tue, 14 Aug 2018 16:55:41 -0500 Subject: [PATCH 08/19] Add new type tests --- lib/puppet/type/network_route.rb | 2 +- spec/unit/puppet/type/network_route_spec.rb | 178 ++++++++++++++++++++ 2 files changed, 179 insertions(+), 1 deletion(-) diff --git a/lib/puppet/type/network_route.rb b/lib/puppet/type/network_route.rb index 605beed6..79997936 100644 --- a/lib/puppet/type/network_route.rb +++ b/lib/puppet/type/network_route.rb @@ -46,7 +46,7 @@ desc: 'scope of the destinations covered by the route prefix.', }, protocol: { - type: 'String', + type: 'Enum["static", "redirect", "kernel", "boot", "ra"]', desc: 'routing protocol identifier of this route.', default: 'static', }, diff --git a/spec/unit/puppet/type/network_route_spec.rb b/spec/unit/puppet/type/network_route_spec.rb index 29a7bb8e..c38ee02b 100644 --- a/spec/unit/puppet/type/network_route_spec.rb +++ b/spec/unit/puppet/type/network_route_spec.rb @@ -5,4 +5,182 @@ it 'loads' do expect(Puppet::Type.type(:network_route)).not_to be_nil end + + context 'prefix is default' do + let(:resource) do + Puppet::Type.type('network_route').new( + prefix: 'default', + default_route: true, + ) + end + + it 'prefix is set' do + expect(resource[:prefix]).to eq 'default' + end + + it 'name is set to prefix' do + expect(resource[:name]).to eq 'default' + end + + it 'default_route should be true' do + expect(resource[:default_route]).to eq :true + end + + it 'metric should be 100' do + expect(resource[:metric]).to eq '100' + end + + it 'protocol should be static' do + expect(resource[:protocol]).to eq 'static' + end + end + + context 'with a non-default route' do + let(:resource) do + Puppet::Type.type('network_route').new( + prefix: '10.155.255.0/24', + default_route: false, + metric: '600', + source: '10.155.255.110' + ) + end + + it 'prefix is set to network address' do + expect(resource[:prefix]).to eq '10.155.255.0/24' + end + + it 'default_route is false' do + expect(resource[:default_route]).to eq :false + end + + it 'metric is set to 600' do + expect(resource[:metric]).to eq '600' + end + + it 'source is set to 10.155.255.110' do + expect(resource[:source]).to eq '10.155.255.110' + end + end + + context 'with gateway set' do + let(:resource) do + Puppet::Type.type('network_route').new( + prefix: 'default', + gateway: '10.155.255.1' + ) + end + + it 'gateway should be 10.155.255.1' do + expect(resource[:gateway]).to eq '10.155.255.1' + end + end + + context 'with interface set' do + let(:resource) do + Puppet::Type.type('network_route').new( + prefix: 'default', + interface: 'eth0' + ) + end + + it 'interface should be eth0' do + expect(resource[:interface]).to eq 'eth0' + end + end + + context 'with metric set' do + let(:resource) do + Puppet::Type.type('network_route').new( + prefix: 'default', + metric: '400' + ) + end + + it 'metric should be 400' do + expect(resource[:metric]).to eq '400' + end + end + + context 'with table set' do + let(:resource) do + Puppet::Type.type('network_route').new( + prefix: 'default', + table: 'main' + ) + end + + it 'table should be main' do + expect(resource[:table]).to eq 'main' + end + end + + context 'with source set' do + let(:resource) do + Puppet::Type.type('network_route').new( + prefix: 'default', + source: '10.155.255.10' + ) + end + + it 'source should be 10.155.255.10' do + expect(resource[:source]).to eq '10.155.255.10' + end + end + + context 'with scope set' do + let(:resource) do + Puppet::Type.type('network_route').new( + prefix: 'default', + scope: 'link' + ) + end + + it 'scope should be link' do + expect(resource[:scope]).to eq 'link' + end + end + + context 'with protocol set' do + let(:resource) do + Puppet::Type.type('network_route').new( + prefix: 'default', + protocol: 'boot' + ) + end + + it 'protocol should be boot' do + expect(resource[:protocol]).to eq 'boot' + end + end + + context 'with mtu set' do + let(:resource) do + Puppet::Type.type('network_route').new( + prefix: 'default', + mtu: '1500' + ) + end + + it 'mtu should be 1500' do + expect(resource[:mtu]).to eq '1500' + end + end + + context 'with invalid scope' do + it 'should raise an error' do + expect { Puppet::Type.type('network_route').new( + prefix: 'default', + scope: 'fail' + )}.to raise_error(Puppet::ResourceError) + end + end + + context 'with invalid protocol' do + it 'should raise an error' do + expect { Puppet::Type.type('network_route').new( + prefix: 'default', + protocol: 'fail' + )}.to raise_error(Puppet::ResourceError) + end + end end From 73d686d9d778aa27a586026995b5b24178e84b83 Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Wed, 15 Aug 2018 12:54:21 -0500 Subject: [PATCH 09/19] Add network_route provider #get test --- .../network_route/network_route_spec.rb | 84 +++++++++---------- 1 file changed, 40 insertions(+), 44 deletions(-) diff --git a/spec/unit/puppet/provider/network_route/network_route_spec.rb b/spec/unit/puppet/provider/network_route/network_route_spec.rb index d8d1c49f..cc868d08 100644 --- a/spec/unit/puppet/provider/network_route/network_route_spec.rb +++ b/spec/unit/puppet/provider/network_route/network_route_spec.rb @@ -8,56 +8,52 @@ module Puppet::Provider::NetworkRoute; end subject(:provider) { described_class.new } let(:context) { instance_double('Puppet::ResourceApi::BaseContext', 'context') } + let(:route) do + [ + { + prefix: 'default', + via: '10.0.2.2', + dev: 'enp0s3', + proto: 'dhcp', + metric: '100' + } + ] + end + let(:network_route) do + [ + { + default_route: true, + ensure: "present", + gateway: "10.0.2.2", + interface: "enp0s3", + metric: "100", + prefix: "default", + protocol: "dhcp" + } + ] + end describe '#get' do + before(:each) do + allow(provider).to receive(:routes_list).and_return(route) # rubocop:disable RSpec/SubjectStub + end + it 'processes resources' do - expect(provider.get(context)).to eq [ - {:ensure=>"present", - :prefix=>"default", - :default_route=>true, - :gateway=>"10.155.255.1", - :interface=>"wlp3s0", - :metric=>"600", - :protocol=>"dhcp"}, - {:ensure=>"present", - :prefix=>"10.155.255.0/24", - :default_route=>false, - :interface=>"wlp3s0", - :metric=>"600", - :source=>"10.155.255.110", - :scope=>"link", - :protocol=>"kernel"}, - {:ensure=>"present", - :prefix=>"169.254.0.0/16", - :default_route=>false, - :interface=>"virbr0", - :metric=>"1000", - :scope=>"link"}, - {:ensure=>"present", - :prefix=>"172.17.0.0/16", - :default_route=>false, - :interface=>"docker0", - :source=>"172.17.0.1", - :scope=>"link", - :protocol=>"kernel"}, - {:ensure=>"present", - :prefix=>"172.18.0.0/16", - :default_route=>false, - :interface=>"br-39a722eeac35", - :source=>"172.18.0.1", - :scope=>"link", - :protocol=>"kernel"}, - {:ensure=>"present", - :prefix=>"192.168.122.0/24", - :default_route=>false, - :interface=>"virbr0", - :source=>"192.168.122.1", - :scope=>"link", - :protocol=>"kernel"} - ] + expect(provider.get(context)).to eq( + [{:default_route=>true, + :ensure=>"present", + :gateway=>"10.0.2.2", + :interface=>"enp0s3", + :metric=>"100", + :prefix=>"default", + :protocol=>"dhcp"}] + ) end end + describe '#puppet_munge(should)' do + end + # describe 'create(context, name, should)' do # it 'creates the resource' do # expect(context).to receive(:notice).with(%r{\ACreating 'a'}) From 81450b68d4a530404e02bc11d2d41d1d370a85cd Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Wed, 15 Aug 2018 16:30:39 -0500 Subject: [PATCH 10/19] Add puppet_munge test to provider tests --- .../network_route/network_route_spec.rb | 45 ++++++++++++------- 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/spec/unit/puppet/provider/network_route/network_route_spec.rb b/spec/unit/puppet/provider/network_route/network_route_spec.rb index cc868d08..f7de630d 100644 --- a/spec/unit/puppet/provider/network_route/network_route_spec.rb +++ b/spec/unit/puppet/provider/network_route/network_route_spec.rb @@ -23,16 +23,32 @@ module Puppet::Provider::NetworkRoute; end [ { default_route: true, - ensure: "present", - gateway: "10.0.2.2", - interface: "enp0s3", - metric: "100", - prefix: "default", - protocol: "dhcp" + ensure: 'present', + gateway: '10.0.2.2', + interface: 'enp0s3', + metric: '100', + prefix: 'default', + protocol: 'dhcp' } ] end + describe '#puppet_munge(should)' do + let(:should) { network_route[0] } + + it 'should parse network_route into iproute2 keys' do + expect(provider.puppet_munge(should)).to eq( + { + dev: 'enp0s3', + metric: '100', + prefix: 'default', + proto: 'dhcp', + via: '10.0.2.2', + } + ) + end + end + describe '#get' do before(:each) do allow(provider).to receive(:routes_list).and_return(route) # rubocop:disable RSpec/SubjectStub @@ -40,20 +56,17 @@ module Puppet::Provider::NetworkRoute; end it 'processes resources' do expect(provider.get(context)).to eq( - [{:default_route=>true, - :ensure=>"present", - :gateway=>"10.0.2.2", - :interface=>"enp0s3", - :metric=>"100", - :prefix=>"default", - :protocol=>"dhcp"}] + [{default_route: true, + ensure: 'present', + gateway: '10.0.2.2', + interface: 'enp0s3', + metric: '100', + prefix: 'default', + protocol: 'dhcp'}] ) end end - describe '#puppet_munge(should)' do - end - # describe 'create(context, name, should)' do # it 'creates the resource' do # expect(context).to receive(:notice).with(%r{\ACreating 'a'}) From 9241848ae6be9ccb9241fb8e75f7a0494a72d1a0 Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Wed, 15 Aug 2018 17:06:57 -0500 Subject: [PATCH 11/19] WIP - add create tests --- .../network_route/network_route_spec.rb | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/spec/unit/puppet/provider/network_route/network_route_spec.rb b/spec/unit/puppet/provider/network_route/network_route_spec.rb index f7de630d..173732da 100644 --- a/spec/unit/puppet/provider/network_route/network_route_spec.rb +++ b/spec/unit/puppet/provider/network_route/network_route_spec.rb @@ -67,13 +67,24 @@ module Puppet::Provider::NetworkRoute; end end end - # describe 'create(context, name, should)' do - # it 'creates the resource' do - # expect(context).to receive(:notice).with(%r{\ACreating 'a'}) + describe 'create(context, name, should)' do + let(:should) { route[0] } - # provider.create(context, 'a', name: 'a', ensure: 'present') - # end - # end + before(:each) do + #allow(Net::IP::Route).to receive(:new).with(should).and_return(:new_route) + #allow(Net::IP).to receive_message_chain("routes.new").with(new_route).and_return(nil) + allow(provider).to receive(:puppet_munge).with(network_route[0]).and_return(should) + end + + it 'creates the resource' do + # expect(context).to receive(:notice).with(%r{\ACreating 'a'}) + + # provider.create(context, 'a', name: 'a', ensure: 'present') + expect(Net::IP::Route).to receive(:new).with(should).and_return('') + + provider.create(context, 'default', network_route[0]) + end + end # describe 'update(context, name, should)' do # it 'updates the resource' do From 8e53ffb14e2866f0dc636abf5b9382985b9bc3af Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Thu, 16 Aug 2018 15:30:43 -0500 Subject: [PATCH 12/19] Finish unit tests for network_route provider --- .../provider/network_route/network_route.rb | 1 - .../network_route/network_route_spec.rb | 57 +++++++++++-------- 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/lib/puppet/provider/network_route/network_route.rb b/lib/puppet/provider/network_route/network_route.rb index 240edd7a..5e971c0c 100644 --- a/lib/puppet/provider/network_route/network_route.rb +++ b/lib/puppet/provider/network_route/network_route.rb @@ -1,5 +1,4 @@ require 'net/ip' -# require_relative '../../../puppet_x/voxpupuli/utils' require 'puppet/resource_api/simple_provider' # Implementation for the network_route type using the Resource API. diff --git a/spec/unit/puppet/provider/network_route/network_route_spec.rb b/spec/unit/puppet/provider/network_route/network_route_spec.rb index 173732da..e0967090 100644 --- a/spec/unit/puppet/provider/network_route/network_route_spec.rb +++ b/spec/unit/puppet/provider/network_route/network_route_spec.rb @@ -8,6 +8,14 @@ module Puppet::Provider::NetworkRoute; end subject(:provider) { described_class.new } let(:context) { instance_double('Puppet::ResourceApi::BaseContext', 'context') } + let(:routes) { instance_double('Net::IP::Route::Collection', 'routes') } + let(:netiproute) { instance_double('Net::IP::Route', prefix: 'route') } + + before(:each) do + allow(Net::IP::Route).to receive(:new).with('should').and_return(netiproute) + allow(Net::IP::Route::Collection).to receive(:new).with('main').and_return(routes) + end + let(:route) do [ { @@ -67,38 +75,37 @@ module Puppet::Provider::NetworkRoute; end end end - describe 'create(context, name, should)' do - let(:should) { route[0] } - + describe '#create(context, name, should)' do before(:each) do - #allow(Net::IP::Route).to receive(:new).with(should).and_return(:new_route) - #allow(Net::IP).to receive_message_chain("routes.new").with(new_route).and_return(nil) - allow(provider).to receive(:puppet_munge).with(network_route[0]).and_return(should) + allow(provider).to receive(:puppet_munge).with('should').and_return('munged') end - - it 'creates the resource' do - # expect(context).to receive(:notice).with(%r{\ACreating 'a'}) - - # provider.create(context, 'a', name: 'a', ensure: 'present') - expect(Net::IP::Route).to receive(:new).with(should).and_return('') - provider.create(context, 'default', network_route[0]) + it 'creates the resource' do + expect(routes).to receive(:add).with(netiproute) + provider.create(context, 'default', 'should') end end - # describe 'update(context, name, should)' do - # it 'updates the resource' do - # expect(context).to receive(:notice).with(%r{\AUpdating 'foo'}) + describe '#update(context, name, should)' do + before(:each) do + allow(provider).to receive(:puppet_munge).with('should').and_return('munged') + end - # provider.update(context, 'foo', name: 'foo', ensure: 'present') - # end - # end + it 'updates the resource' do + expect(routes).to receive(:flush).with(netiproute.prefix) + expect(routes).to receive(:add).with(netiproute) + provider.update(context, 'default', 'should') + end + end - # describe 'delete(context, name, should)' do - # it 'deletes the resource' do - # expect(context).to receive(:notice).with(%r{\ADeleting 'foo'}) + describe 'delete(context, name, should)' do + before(:each) do + allow(provider).to receive(:puppet_munge).with('should').and_return('munged') + end - # provider.delete(context, 'foo') - # end - # end + it 'deletes the resource' do + expect(routes).to receive(:flush).with(netiproute.prefix) + provider.delete(context, 'default', 'should') + end + end end From 5caf0090af64e19c62ccbaf6901dab32792ef990 Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Thu, 16 Aug 2018 21:04:09 -0500 Subject: [PATCH 13/19] Remove network facts as they cover much of the same ground as facter 3 --- lib/facter/network.rb | 68 ------------------------------------------- manifests/route.pp | 4 +-- 2 files changed, 2 insertions(+), 70 deletions(-) delete mode 100644 lib/facter/network.rb diff --git a/lib/facter/network.rb b/lib/facter/network.rb deleted file mode 100644 index 136d62da..00000000 --- a/lib/facter/network.rb +++ /dev/null @@ -1,68 +0,0 @@ -require 'facter' -require 'open-uri' -require 'timeout' - -# Facter 3 has new facts that we can use instead of trying to do it -# ourselves -def facter_3 - facter_version = Gem::Version.new(Facter.version) - version3 = Gem::Version.new('3.0.0') - facter_version >= version3 -end - -# Gateway -# Expected output: The ip address of the nexthop/default router -Facter.add('network_nexthop_ip') do - my_gw = nil - confine :kernel => :linux # rubocop:disable Style/HashSyntax - setcode do - gw_address = Facter::Util::Resolution.exec('ip route show 0/0') - # not all network configurations will have a nexthop. - # the ip tool expresses the presence of a nexthop with the word 'via' - if gw_address.include? ' via ' - my_gw = gw_address.split(%r{\s+})[2].match(%r{^(?:[0-9]{1,3}\.){3}[0-9]{1,3}$}).to_s - end - my_gw - end -end - -# Primary interface -# Expected output: The specific interface name that the node uses to communicate with the nexthop -Facter.add('network_primary_interface') do - confine :kernel => :linux # rubocop:disable Style/HashSyntax - setcode do - next Facter.fact(:networking).value['primary'] if facter_3 - gw_address = Facter::Util::Resolution.exec('ip route show 0/0') - # not all network configurations will have a nexthop. - # the ip tool expresses the presence of a nexthop with the word 'via' - if gw_address.include? ' via ' - my_gw = gw_address.split(%r{\s+})[2].match(%r{^(?:[0-9]{1,3}\.){3}[0-9]{1,3}$}).to_s - fun = Facter::Util::Resolution.exec("ip route get #{my_gw}").split("\n")[0] - # some network configurations simply have a link that all interactions are abstracted through - elsif gw_address.include? 'scope link' - # since we have no default route ip to determine where to send 'traffic not otherwise explicitly routed' - # lets just use 8.8.8.8 as far as a route goes. - fun = Facter::Util::Resolution.exec('ip route get 8.8.8.8').split("\n")[0] - end - fun.split(%r{dev\s+([^\s]*)\s+src\s+([^\s]*)})[1].to_s - end -end - -# Primary IP -# Expected output: The ipaddress configured on the interface that communicates with the nexthop -Facter.add('network_primary_ip') do - confine :kernel => :linux # rubocop:disable Style/HashSyntax - setcode do - next Facter.fact(:networking).value['ip'] if facter_3 - gw_address = Facter::Util::Resolution.exec('ip route show 0/0') - if gw_address.include? ' via ' - my_gw = gw_address.split(%r{\s+})[2].match(%r{^(?:[0-9]{1,3}\.){3}[0-9]{1,3}$}).to_s - fun = Facter::Util::Resolution.exec("ip route get #{my_gw}").split("\n")[0] - elsif gw_address.include? 'scope link' - # since we have no default route ip to determine where to send 'traffic not otherwise explicitly routed' - # lets just use 8.8.8.8 as far as a route goes and grab our IP from there. - fun = Facter::Util::Resolution.exec('ip route get 8.8.8.8').split("\n")[0] - end - fun.split(%r{dev\s+([^\s]*)\s+src\s+([^\s]*)})[2].to_s - end -end diff --git a/manifests/route.pp b/manifests/route.pp index 6fce5a22..45b9137d 100644 --- a/manifests/route.pp +++ b/manifests/route.pp @@ -31,9 +31,9 @@ } } /RedHat|CentOS/: { - "/etc/sysconfig/network-scripts/route-${interface}" + "/etc/sysconfig/network-scripts/route-${interface}" } - default: { + default: { fail("Network::Route is not compatible with ${facts['os']['family']}!") } } From 4e5937d0f1f8ec8c107acd9f174b21d550145c30 Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Thu, 16 Aug 2018 21:13:31 -0500 Subject: [PATCH 14/19] Remove old unused tests --- spec/classes/route_spec.rb | 11 - spec/unit/facts/network_nexthop_ip_spec.rb | 40 ---- .../facts/network_primary_interface_spec.rb | 51 ----- spec/unit/facts/network_primary_ip_spec.rb | 52 ----- .../provider/network_route/redhat_spec.rb | 171 --------------- .../provider/network_route/routes_spec.rb | 194 ------------------ spec/unit/type/network_route_spec.rb | 74 ------- 7 files changed, 593 deletions(-) delete mode 100644 spec/classes/route_spec.rb delete mode 100644 spec/unit/facts/network_nexthop_ip_spec.rb delete mode 100644 spec/unit/facts/network_primary_interface_spec.rb delete mode 100644 spec/unit/facts/network_primary_ip_spec.rb delete mode 100644 spec/unit/provider/network_route/redhat_spec.rb delete mode 100644 spec/unit/provider/network_route/routes_spec.rb delete mode 100644 spec/unit/type/network_route_spec.rb diff --git a/spec/classes/route_spec.rb b/spec/classes/route_spec.rb deleted file mode 100644 index 6ac6f26c..00000000 --- a/spec/classes/route_spec.rb +++ /dev/null @@ -1,11 +0,0 @@ -require 'spec_helper' - -describe 'network::route' do - on_supported_os.each do |os, os_facts| - context "on #{os}" do - let(:facts) { os_facts } - - it { is_expected.to compile } - end - end -end diff --git a/spec/unit/facts/network_nexthop_ip_spec.rb b/spec/unit/facts/network_nexthop_ip_spec.rb deleted file mode 100644 index 471664eb..00000000 --- a/spec/unit/facts/network_nexthop_ip_spec.rb +++ /dev/null @@ -1,40 +0,0 @@ -require 'spec_helper' - -describe 'network_nexthop_ip' do - before do - Facter.fact(:kernel).stubs(:value).returns('linux') - end - context 'on a Linux host' do - before do - Facter::Util::Resolution.stubs(:exec).with('ip route show 0/0').returns('default via 1.2.3.4 dev eth0') - end - it 'execs ip and determine the next hop' do - expect(Facter.fact(:network_nexthop_ip).value).to eq('1.2.3.4') - end - end - context 'on an OpenVZ VM' do - before do - Facter.clear - Facter.fact(:kernel).stubs(:value).returns('linux') - Facter.fact(:virtual).stubs(:value).returns('openvz') - Facter::Util::Resolution.stubs(:exec) - end - context 'which has the default route via a veth device' do - before do - Facter.fact(:macaddress).stubs(:value).returns(nil) - Facter::Util::Resolution.stubs(:exec).with('ip route show 0/0').returns('default via 1.2.3.4 dev eth0') - end - it 'execs ip and determine the next hop' do - expect(Facter.fact(:network_nexthop_ip).value).to eq('1.2.3.4') - end - end - context 'with only venet interfaces' do - before do - Facter::Util::Resolution.stubs(:exec).with('ip route show 0/0').returns('default dev venet0 scope link') - end - it 'does not exist' do - expect(Facter.fact(:network_nexthop_ip).value).to be_nil - end - end - end -end diff --git a/spec/unit/facts/network_primary_interface_spec.rb b/spec/unit/facts/network_primary_interface_spec.rb deleted file mode 100644 index 90d5f01f..00000000 --- a/spec/unit/facts/network_primary_interface_spec.rb +++ /dev/null @@ -1,51 +0,0 @@ -require 'spec_helper' - -describe 'network_primary_interface' do - before do - Facter.clear - Facter.fact(:kernel).stubs(:value).returns('linux') - end - context 'on a Linux host with Facter 2.x' do - before do - Facter::Util::Resolution.stubs(:exec).with('ip route show 0/0').returns('default via 1.2.3.4 dev eth0') - Facter::Util::Resolution.stubs(:exec).with('ip route get 1.2.3.4').returns('1.2.3.4 dev eth0 src 1.2.3.99\n - cache mtu 1500 advmss 1460 hoplimit 64') - end - it 'execs ip and determines the primary interface' do - expect(Facter.fact(:network_primary_interface).value).to eq('eth0') - end - end - context 'on an OpenVZ VM with Facter 2.x' do - before do - Facter.stubs(:version).returns('2.4.6') - Facter.fact(:kernel).stubs(:value).returns('linux') - Facter.fact(:virtual).stubs(:value).returns('openvz') - Facter::Util::Resolution.stubs(:exec) - end - context 'with only venet devices' do - before do - Facter::Util::Resolution.stubs(:exec).with('ip route show 0/0').returns('default dev venet0 scope link') - Facter::Util::Resolution.stubs(:exec).with('ip route get 8.8.8.8').returns('8.8.8.8 dev venet0 src 1.2.3.99\n - cache mtu 1500 advmss 1460 hoplimit 64') - end - it 'execs ip and determines the primary interface' do - expect(Facter.fact(:network_primary_interface).value).to eq('venet0') - end - end - end - context 'on a Linux host with Facter 3' do - before do - Facter::Util::Resolution.expects(:exec).never - interface_fact = Facter.fact(:network_primary_interface) - networking_fact = Facter::Util::Fact.new(:networking) - networking_fact.expects(:value).returns('primary' => 'eth1') - Facter.expects(:fact).with(:networking).returns(networking_fact) - Facter.expects(:fact).with(:network_primary_interface).returns(interface_fact) - end - it 'uses the built-in facts to determine the primary interface' do - # (rski) For some reason with ruby 1.9.3 this doesn't work in the before block - Facter.stubs(:version).returns('3.0.0') - expect(Facter.fact(:network_primary_interface).value).to eq('eth1') - end - end -end diff --git a/spec/unit/facts/network_primary_ip_spec.rb b/spec/unit/facts/network_primary_ip_spec.rb deleted file mode 100644 index 86fab556..00000000 --- a/spec/unit/facts/network_primary_ip_spec.rb +++ /dev/null @@ -1,52 +0,0 @@ -require 'spec_helper' - -describe 'network_primary_ip' do - before do - Facter.clear - Facter.fact(:kernel).stubs(:value).returns('linux') - end - context 'on a Linux host with Facter 2.x' do - before do - Facter.stubs(:version).returns('2.4.6') - Facter::Util::Resolution.stubs(:exec).with('ip route show 0/0').returns('default via 1.2.3.4 dev eth0').once - Facter::Util::Resolution.stubs(:exec).with('ip route get 1.2.3.4').returns("1.2.3.4 dev eth0 src 1.2.3.99\n - cache mtu 1500 advmss 1460 hoplimit 64").once - end - it 'execs ip and determine the primary ip address' do - expect(Facter.fact(:network_primary_ip).value).to eq('1.2.3.99') - end - end - context 'on an OpenVZ VM with Facter 2.x' do - before do - Facter.clear - Facter.stubs(:version).returns('2.4.6') - Facter.fact(:virtual).stubs(:value).returns('openvz') - Facter::Util::Resolution.stubs(:exec) - end - context 'with only venet devices' do - before do - Facter::Util::Resolution.stubs(:exec).with('ip route show 0/0').returns('default dev venet0 scope link') - Facter::Util::Resolution.stubs(:exec).with('ip route get 8.8.8.8').returns("8.8.8.8 dev venet0 src 1.2.3.99\n - cache mtu 1500 advmss 1460 hoplimit 64") - end - it 'execs ip and determine the primary ip address' do - expect(Facter.fact(:network_primary_ip).value).to eq('1.2.3.99') - end - end - end - context 'on a Linux host with Facter 3.x' do - before do - Facter::Util::Resolution.expects(:exec).never - ip_fact = Facter.fact(:network_primary_ip) - networking_fact = Facter::Util::Fact.new(:networking) - networking_fact.expects(:value).returns('ip' => '2.3.4.5') - Facter.expects(:fact).with(:networking).returns(networking_fact) - Facter.expects(:fact).with(:network_primary_ip).returns(ip_fact) - end - it 'uses the built-in facts to resolve the primary ip address' do - # (rski) For some reason this doesn't work with ruby 1.9.3 - Facter.stubs(:version).returns('3.0.0') - expect(Facter.fact(:network_primary_ip).value).to eq('2.3.4.5') - end - end -end diff --git a/spec/unit/provider/network_route/redhat_spec.rb b/spec/unit/provider/network_route/redhat_spec.rb deleted file mode 100644 index 85d61d9b..00000000 --- a/spec/unit/provider/network_route/redhat_spec.rb +++ /dev/null @@ -1,171 +0,0 @@ -require 'spec_helper' - -describe Puppet::Type.type(:network_route).provider(:redhat) do - def fixture_data(file) - basedir = File.join(PROJECT_ROOT, 'spec', 'fixtures', 'provider', 'network_route', 'redhat') - File.read(File.join(basedir, file)) - end - - describe 'when parsing' do - describe 'a simple well formed file' do - let(:data) { described_class.parse_file('', fixture_data('simple_routes')) } - - it 'parses out normal ipv4 network routes' do - expect(data.find { |h| h[:name] == '172.17.67.0/30' }).to eq( - name: '172.17.67.0/30', - network: '172.17.67.0', - netmask: '255.255.255.252', - gateway: '172.18.6.2', - interface: 'vlan200' - ) - end - it 'parses out ipv6 network routes' do - expect(data.find { |h| h[:name] == '2a01:4f8:211:9d5:53::/96' }).to eq( - name: '2a01:4f8:211:9d5:53::/96', - network: '2a01:4f8:211:9d5:53::', - netmask: 'ffff:ffff:ffff:ffff:ffff:ffff::', - gateway: '2a01:4f8:211:9d5::2', - interface: 'vlan200' - ) - end - - it 'parses out default routes' do - expect(data.find { |h| h[:name] == 'default' }).to eq( - name: 'default', - network: 'default', - netmask: '0.0.0.0', - gateway: '10.0.0.1', - interface: 'eth1' - ) - end - end - - describe 'an advanced, well formed file' do - let :data do - described_class.parse_file('', fixture_data('advanced_routes')) - end - - it 'parses out normal ipv4 network routes' do - expect(data.find { |h| h[:name] == '2a01:4f8:211:9d5:53::/96' }).to eq( - name: '2a01:4f8:211:9d5:53::/96', - network: '2a01:4f8:211:9d5:53::', - netmask: 'ffff:ffff:ffff:ffff:ffff:ffff::', - gateway: '2a01:4f8:211:9d5::2', - interface: 'vlan200', - options: 'table 200' - ) - end - - it 'parses out normal ipv6 network routes' do - expect(data.find { |h| h[:name] == '172.17.67.0/30' }).to eq( - name: '172.17.67.0/30', - network: '172.17.67.0', - netmask: '255.255.255.252', - gateway: '172.18.6.2', - interface: 'vlan200', - options: 'table 200' - ) - end - end - - describe 'an invalid file' do - it 'fails' do - expect do - described_class.parse_file('', "192.168.1.1/30 via\n") - end.to raise_error(%r{Malformed redhat route file}) - end - end - end - - describe 'when formatting' do - let :route1_provider do - stub( - 'route1_provider', - name: '172.17.67.0/30', - network: '172.17.67.0', - netmask: '30', - gateway: '172.18.6.2', - interface: 'vlan200', - options: 'table 200' - ) - end - - let :route2_provider do - stub( - 'lo_provider', - name: '172.28.45.0/30', - network: '172.28.45.0', - netmask: '30', - gateway: '172.18.6.2', - interface: 'eth0', - options: 'table 200' - ) - end - - let :defaultroute_provider do - stub( - 'defaultroute_provider', - name: 'default', - network: 'default', - netmask: '', - gateway: '10.0.0.1', - interface: 'eth1', - options: 'table 200' - ) - end - - let :nooptions_provider do - stub( - 'nooptions_provider', - name: 'default', - network: 'default', - netmask: '', - gateway: '10.0.0.1', - interface: 'eth2', - options: :absent - ) - end - - let :content do - described_class.format_file('', [route1_provider, route2_provider, defaultroute_provider, nooptions_provider]) - end - - describe 'writing the route line' do - describe 'For standard (non-default) routes' do - it 'writes a single line for the route' do - expect(content.scan(%r{^172.17.67.0\/30 .*$}).length).to eq(1) - end - - it 'writes 7 fields' do - expect(content.scan(%r{^172.17.67.0\/30 .*$}).first.split(' ').length).to eq(7) - end - - it 'has the correct fields appended' do - expect(content.scan(%r{^172.17.67.0\/30 .*$}).first).to include('172.17.67.0/30 via 172.18.6.2 dev vlan200 table 200') - end - - it 'fails if the netmask property is not defined' do - route2_provider.unstub(:netmask) - route2_provider.stubs(:netmask).returns nil - expect { content }.to raise_exception(%r{is missing the required parameter 'netmask'}) - end - - it 'fails if the gateway property is not defined' do - route2_provider.unstub(:gateway) - route2_provider.stubs(:gateway).returns nil - expect { content }.to raise_exception(%r{is missing the required parameter 'gateway'}) - end - end - end - - describe 'for default routes' do - it 'has the correct fields appended' do - expect(content.scan(%r{^default .*$}).first).to include('default via 10.0.0.1 dev eth1') - end - - it 'does not contain the word absent when no options are defined' do - expect(content).not_to match(%r{absent}) - end - end - end -end diff --git a/spec/unit/provider/network_route/routes_spec.rb b/spec/unit/provider/network_route/routes_spec.rb deleted file mode 100644 index 52d298dc..00000000 --- a/spec/unit/provider/network_route/routes_spec.rb +++ /dev/null @@ -1,194 +0,0 @@ -require 'spec_helper' - -describe Puppet::Type.type(:network_route).provider(:routes) do - def fixture_data(file) - basedir = File.join(PROJECT_ROOT, 'spec', 'fixtures', 'provider', 'network_route', 'routes_spec') - File.read(File.join(basedir, file)) - end - - describe 'when parsing' do - it 'parses out simple ipv4 iface lines' do - fixture = fixture_data('simple_routes') - data = described_class.parse_file('', fixture) - - expect(data.find { |h| h[:name] == '172.17.67.0/24' }).to eq( - name: '172.17.67.0/24', - network: '172.17.67.0', - netmask: '255.255.255.0', - gateway: '172.18.6.2', - interface: 'vlan200' - ) - end - - it 'names default routes "default" and have a 0.0.0.0 netmask' do - fixture = fixture_data('simple_routes') - data = described_class.parse_file('', fixture) - - expect(data.find { |h| h[:name] == 'default' }).to eq( - name: 'default', - network: 'default', - netmask: '0.0.0.0', - gateway: '172.18.6.2', - interface: 'vlan200' - ) - end - - it 'parses out simple ipv6 iface lines' do - fixture = fixture_data('simple_routes') - data = described_class.parse_file('', fixture) - - expect(data.find { |h| h[:name] == '2a01:4f8:211:9d5:53::/96' }).to eq( - name: '2a01:4f8:211:9d5:53::/96', - network: '2a01:4f8:211:9d5:53::', - netmask: 'ffff:ffff:ffff:ffff:ffff:ffff::', - gateway: '2a01:4f8:211:9d5::2', - interface: 'vlan200' - ) - end - - it 'parses out advanced routes' do - fixture = fixture_data('advanced_routes') - data = described_class.parse_file('', fixture) - - expect(data.find { |h| h[:name] == '172.17.67.0/24' }).to eq( - name: '172.17.67.0/24', - network: '172.17.67.0', - netmask: '255.255.255.0', - gateway: '172.18.6.2', - interface: 'vlan200', - options: 'table 200' - ) - end - it 'parses out advanced ipv6 iface lines' do - fixture = fixture_data('advanced_routes') - data = described_class.parse_file('', fixture) - - expect(data.find { |h| h[:name] == '2a01:4f8:211:9d5:53::/96' }).to eq( - name: '2a01:4f8:211:9d5:53::/96', - network: '2a01:4f8:211:9d5:53::', - netmask: 'ffff:ffff:ffff:ffff:ffff:ffff::', - gateway: '2a01:4f8:211:9d5::2', - interface: 'vlan200', - options: 'table 200' - ) - end - - describe 'when reading an invalid routes file' do - it 'with missing options should fail' do - expect do - described_class.parse_file('', "192.168.1.1 255.255.255.0 172.16.0.1\n") - end.to raise_error(%r{Malformed debian routes file}) - end - end - end - - describe 'when formatting' do - let :route1_provider do - stub( - 'route1_provider', - name: '172.17.67.0', - network: '172.17.67.0', - netmask: '255.255.255.0', - gateway: '172.18.6.2', - interface: 'vlan200', - options: 'table 200' - ) - end - - let :route2_provider do - stub( - 'lo_provider', - name: '172.28.45.0', - network: '172.28.45.0', - netmask: '255.255.255.0', - gateway: '172.18.6.2', - interface: 'eth0', - options: 'table 200' - ) - end - - let :content do - described_class.format_file('', [route1_provider, route2_provider]) - end - - describe 'writing the route line' do - it 'writes a single line for the route' do - expect(content.scan(%r{^172.17.67.0 .*$}).length).to eq(1) - end - it 'writes all 6 fields' do - expect(content.scan(%r{^172.17.67.0 .*$}).first.split(' ').length).to eq(6) - end - - it 'has the correct fields appended' do - expect(content.scan(%r{^172.17.67.0 .*$}).first).to include('172.17.67.0 255.255.255.0 172.18.6.2 vlan200 table 200') - end - - it 'fails if the netmask property is not defined' do - route2_provider.unstub(:netmask) - route2_provider.stubs(:netmask).returns nil - expect { content }.to raise_exception(%r{is missing the required parameter 'netmask'}) - end - - it 'fails if the gateway property is not defined' do - route2_provider.unstub(:gateway) - route2_provider.stubs(:gateway).returns nil - expect { content }.to raise_exception(%r{is missing the required parameter 'gateway'}) - end - end - end - describe 'when formatting simple files' do - let :route1_provider do - stub( - 'route1_provider', - name: '172.17.67.0', - network: '172.17.67.0', - netmask: '255.255.255.0', - gateway: '172.18.6.2', - interface: 'vlan200', - options: :absent - ) - end - - let :route2_provider do - stub( - 'lo_provider', - name: '172.28.45.0', - network: '172.28.45.0', - netmask: '255.255.255.0', - gateway: '172.18.6.2', - interface: 'eth0', - options: :absent - ) - end - - let :content do - described_class.format_file('', [route1_provider, route2_provider]) - end - - describe 'writing the route line' do - it 'writes a single line for the route' do - expect(content.scan(%r{^172.17.67.0 .*$}).length).to eq(1) - end - - it 'writes only fields' do - expect(content.scan(%r{^172.17.67.0 .*$}).first.split(' ').length).to eq(4) - end - - it 'has the correct fields appended' do - expect(content.scan(%r{^172.17.67.0 .*$}).first).to include('172.17.67.0 255.255.255.0 172.18.6.2 vlan200') - end - - it 'fails if the netmask property is not defined' do - route2_provider.unstub(:netmask) - route2_provider.stubs(:netmask).returns nil - expect { content }.to raise_exception(%r{is missing the required parameter 'netmask'}) - end - - it 'fails if the gateway property is not defined' do - route2_provider.unstub(:gateway) - route2_provider.stubs(:gateway).returns nil - expect { content }.to raise_exception(%r{is missing the required parameter 'gateway'}) - end - end - end -end diff --git a/spec/unit/type/network_route_spec.rb b/spec/unit/type/network_route_spec.rb deleted file mode 100644 index a1ca56cd..00000000 --- a/spec/unit/type/network_route_spec.rb +++ /dev/null @@ -1,74 +0,0 @@ -require 'spec_helper' - -describe Puppet::Type.type(:network_route) do - before do - provider_class = stub 'provider class', name: 'fake', suitable?: true, supports_parameter?: true - provider_class.stubs(:new) - - Puppet::Type.type(:network_route).stubs(:defaultprovider).returns provider_class - Puppet::Type.type(:network_route).stubs(:provider).returns provider_class - end - - describe 'when validating the attribute' do - describe :name do # rubocop:disable RSpec/DescribeSymbol - it { expect(Puppet::Type.type(:network_route).attrtype(:name)).to eq(:param) } - end - - [:ensure, :network, :netmask, :gateway, :interface, :options].each do |property| - describe property do - it { expect(Puppet::Type.type(:network_route).attrtype(property)).to eq(:property) } - end - end - - it 'use the name parameter as the namevar' do - expect(Puppet::Type.type(:network_route).key_attributes).to eq([:name]) - end - - describe 'ensure' do - it 'is an ensurable value' do - expect(Puppet::Type.type(:network_route).propertybyname(:ensure).ancestors).to be_include(Puppet::Property::Ensure) - end - end - end - - describe 'when validating the attribute value' do - describe 'network' do - it 'validates the network as an IP address' do - expect do - Puppet::Type.type(:network_route).new(name: '192.168.1.0/24', network: 'not an ip address', netmask: '255.255.255.0', gateway: '23.23.23.42', interface: 'eth0') - end.to raise_error(%r{Invalid value for parameter 'network'}) - end - end - - describe 'netmask' do - it 'fails if an invalid netmask is used' do - expect do - Puppet::Type.type(:network_route).new(name: '192.168.1.0/24', network: '192.168.1.0', netmask: 'This is clearly not a netmask', gateway: '23.23.23.42', interface: 'eth0') - end.to raise_error(%r{Invalid value for parameter 'netmask'}) - end - - it 'converts netmasks of the CIDR form' do - r = Puppet::Type.type(:network_route).new(name: '192.168.1.0/24', network: '192.168.1.0', netmask: '24', gateway: '23.23.23.42', interface: 'eth0') - expect(r[:netmask]).to eq('255.255.255.0') - end - - it 'converts IPv6 netmasks of the CIDR form' do - r = Puppet::Type.type(:network_route).new(name: 'lxd bridge', network: 'fd58:281b:6eef:eb3d::', netmask: '64', gateway: 'fd58:281b:6eef:eb3d::1', interface: 'lxdbr0') - expect(r[:netmask]).to eq('ffff:ffff:ffff:ffff::') - end - - it 'converts netmasks of the expanded netmask form' do - r = Puppet::Type.type(:network_route).new(name: '192.168.1.0/24', network: '192.168.1.0', netmask: '255.255.128.0', gateway: '23.23.23.42', interface: 'eth0') - expect(r[:netmask]).to eq('255.255.128.0') - end - end - - describe 'gateway' do - it 'validates as an IP address' do - expect do - Puppet::Type.type(:network_route).new(name: '192.168.1.0/24', network: '192.168.1.0', netmask: '255.255.255.0', gateway: 'not an ip address', interface: 'eth0') - end.to raise_error(%r{Invalid value for parameter 'gateway'}) - end - end - end -end From f677bd9ff4a94138a3d5baa05c66235d5233a2fd Mon Sep 17 00:00:00 2001 From: David Hollinger Date: Fri, 17 Aug 2018 14:26:43 -0500 Subject: [PATCH 15/19] Puppet Strings update v1 --- lib/puppet/type/network_route.rb | 2 +- manifests/init.pp | 66 +++++++++++--------------------- manifests/route.pp | 46 ++++++++++++++++++++-- 3 files changed, 67 insertions(+), 47 deletions(-) diff --git a/lib/puppet/type/network_route.rb b/lib/puppet/type/network_route.rb index 79997936..b4ff9ed3 100644 --- a/lib/puppet/type/network_route.rb +++ b/lib/puppet/type/network_route.rb @@ -46,7 +46,7 @@ desc: 'scope of the destinations covered by the route prefix.', }, protocol: { - type: 'Enum["static", "redirect", "kernel", "boot", "ra"]', + type: 'Enum["static", "redirect", "kernel", "boot", "ra", "dhcp"]', desc: 'routing protocol identifier of this route.', default: 'static', }, diff --git a/manifests/init.pp b/manifests/init.pp index dd57221c..75242e21 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -1,58 +1,38 @@ -# = Class: network +# Manage network configuration and routing on Linux systems with the +# network_route and network_config resources. This also installs the +# packages and gems needed to manage these settings. # -# Install the packages and gems required by the network_route and network_config resources +# @summary Manage linux network configuration and routes # -# == Parameters +# @author Vox Pupuli # -# [*ifupdown_extra*] +# @example +# include network # -# The name of the ifupdown-extra package +# @param ifupdown_extra +# The name of the ifupdown-extra package # -# Default: ifupdown-extra +# @param ifupdown_extra_provider +# The provider of the ifupdown-extra package # -# [*ifupdown_extra_provider*] +# @param manage_ifupdown_extra +# Whether this class should manage the ifupdown-extra package # -# The provider of the ifupdown-extra package +# @param ensure_ifupdown_extra +# What state the ifupdown-extra package should be in # -# Default: undef +# @param ipaddress +# The name of the ipaddress gems # -# [*manage_ifupdown_extra*] +# @param ipaddress_provider +# The provider of the ipaddress gem # -# Whether this class should manage the ifupdown-extra package +# @param manage_ipaddress +# Whether this class should manage the ipaddress gem # -# Default: true +# @param ensure_ipaddress +# What state the ifupdown-extra package should be in # -# [*ensure_ifupdown_extra*] -# -# What state the ifupdown-extra package should be in -# -# Default: present -# -# [*ipaddress*] -# -# The name of the ipaddress gems -# -# Default: ipaddress -# -# [*ipaddress_provider*] -# -# The provider of the ipaddress gem -# -# Default: gem -# -# [*manage_ipaddress*] -# -# Whether this class should manage the ipaddress gem -# -# Default: true -# -# [*ensure_ipaddress*] -# -# What state the ifupdown-extra package should be in -# -# Default: present -# - class network( $ifupdown_extra = 'ifupdown-extra', $ifupdown_extra_provider = undef, diff --git a/manifests/route.pp b/manifests/route.pp index 45b9137d..aa08518b 100644 --- a/manifests/route.pp +++ b/manifests/route.pp @@ -1,17 +1,53 @@ # Deploys a network route to the system using ip route # and adds the route to a file for persistence. # -# @summary A short summary of the purpose of this class +# @summary Manages network routes in both the table and persistent file +# +# @author Vox Pupuli # # @example -# include network::route +# network::route { '192.168.232.0/24': +# ensure => present +# network => '192.168.232.0', +# netmask => '24', +# gateway => '192.168.100.1', +# } +# +# @param network +# IPv4 Network or Host ip address the route is pointing to. +# @param netmask +# IPv4 CIDR address for the network/host destination route. If using host, +# then the netmask should be '32' +# @param ensure +# Ensure the route exists (or not) +# @param default_route +# Is the route the default route? +# @param metric +# Set the routing metric. +# @param protocol +# Set the iproute2 protocol. +# Valid values are: 'static', 'dhcp', 'redirect', 'kernel', 'boot', 'ra' +# @param gateway +# The network gateway to route through. +# @param interface +# The Network Interface to route through. +# @param table +# The iproute2 routing table to apply the route to. +# @param source +# The source IP for which to apply the route to. +# @param scope +# The iproute2 scope to apply. +# Valid values: 'global', 'nowhere', 'host', 'link', 'site' +# @param mtu +# The MTU to apply to the route. +# define network::route( Stdlib::Ipv4 $network, Stdlib::Ipv4 $netmask, Enum['present', 'absent'] $ensure = 'present', Boolean $default_route = false, String $metric = '100', - String $protocol = 'static', + Enum['static', 'redirect', 'kernel', 'boot', 'ra', 'dhcp'] $protocol = 'static', Optional[Stdlib::Ipv4] $gateway, Optional[String] $interface, Optional[String] $table, @@ -19,6 +55,10 @@ Optional[Enum['global', 'nowhere', 'host', 'link', 'site']] $scope, Optional[String] $mtu, ) { + if $gateway == undef && $interface == undef { + fail('Route must have a gateway or interface!') + } + $route_file = case $facts['os']['name'] { 'Debian': { '/etc/network/routes' From e412732df264eb978fb0ea9f33776cb34a8a6dd9 Mon Sep 17 00:00:00 2001 From: Keith Ward Date: Sun, 8 Dec 2019 22:15:17 +0000 Subject: [PATCH 16/19] Fixup syntax of route.pp --- manifests/route.pp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/manifests/route.pp b/manifests/route.pp index aa08518b..67c1050e 100644 --- a/manifests/route.pp +++ b/manifests/route.pp @@ -55,7 +55,8 @@ Optional[Enum['global', 'nowhere', 'host', 'link', 'site']] $scope, Optional[String] $mtu, ) { - if $gateway == undef && $interface == undef { + + if $gateway == undef and $interface == undef { fail('Route must have a gateway or interface!') } From fe44ebcd31e3f5810f4108e7f171d4235670084d Mon Sep 17 00:00:00 2001 From: Keith Ward Date: Mon, 9 Dec 2019 00:56:16 +0000 Subject: [PATCH 17/19] Clear out some of the low hangingn rubocop errors --- .../provider/network_route/network_route.rb | 6 +-- lib/puppet/type/network_route.rb | 26 +++++----- .../network_route/network_route_spec.rb | 48 +++++++++---------- spec/unit/puppet/type/network_route_spec.rb | 26 +++++----- 4 files changed, 54 insertions(+), 52 deletions(-) diff --git a/lib/puppet/provider/network_route/network_route.rb b/lib/puppet/provider/network_route/network_route.rb index 5e971c0c..cfb321ad 100644 --- a/lib/puppet/provider/network_route/network_route.rb +++ b/lib/puppet/provider/network_route/network_route.rb @@ -74,20 +74,20 @@ def set(context, changes) end end - def create(context, name, should) + def create(_context, _name, should) puppet_munge(should) route = Net::IP::Route.new(should) Net::IP.routes.add(route) end - def update(context, name, should) + def update(_context, _name, should) puppet_munge(should) route = Net::IP::Route.new(should) Net::IP.routes.flush(route.prefix) Net::IP.routes.add(route) end - def delete(context, name, should) + def delete(_context, _name, should) puppet_munge(should) route = Net::IP::Route.new(should) Net::IP.routes.flush(route.prefix) diff --git a/lib/puppet/type/network_route.rb b/lib/puppet/type/network_route.rb index b4ff9ed3..4997b19e 100644 --- a/lib/puppet/type/network_route.rb +++ b/lib/puppet/type/network_route.rb @@ -9,50 +9,50 @@ ensure: { type: 'Enum[present, absent]', desc: 'Whether the network route should be present or absent on the target system.', - default: 'present', + default: 'present' }, prefix: { type: 'String', desc: 'The destination prefix/network of the route.', - behaviour: :namevar, + behaviour: :namevar }, default_route: { type: 'Optional[Boolean]', - desc: 'Whether the route is default or not.', + desc: 'Whether the route is default or not.' }, gateway: { type: 'Optional[String]', - desc: 'The gateway to use for the route.', + desc: 'The gateway to use for the route.' }, interface: { type: 'Optional[String]', - desc: 'The interface to use for the route.', + desc: 'The interface to use for the route.' }, metric: { type: 'String', desc: 'preference value of the route. NUMBER is an arbitrary 32bit number.', - default: '100', + default: '100' }, table: { type: 'Optional[String]', - desc: 'table to add this route.', + desc: 'table to add this route.' }, source: { type: 'Optional[String]', - desc: 'the source address to prefer using when sending to the destinations covered by route prefix.', + desc: 'the source address to prefer using when sending to the destinations covered by route prefix.' }, scope: { type: 'Optional[Enum["global", "nowhere", "host", "link", "site"]]', - desc: 'scope of the destinations covered by the route prefix.', + desc: 'scope of the destinations covered by the route prefix.' }, protocol: { type: 'Enum["static", "redirect", "kernel", "boot", "ra", "dhcp"]', desc: 'routing protocol identifier of this route.', - default: 'static', + default: 'static' }, mtu: { type: 'Optional[String]', - desc: 'the MTU along the path to destination.', - }, - }, + desc: 'the MTU along the path to destination.' + } + } ) diff --git a/spec/unit/puppet/provider/network_route/network_route_spec.rb b/spec/unit/puppet/provider/network_route/network_route_spec.rb index e0967090..b6bf07bf 100644 --- a/spec/unit/puppet/provider/network_route/network_route_spec.rb +++ b/spec/unit/puppet/provider/network_route/network_route_spec.rb @@ -11,11 +11,6 @@ module Puppet::Provider::NetworkRoute; end let(:routes) { instance_double('Net::IP::Route::Collection', 'routes') } let(:netiproute) { instance_double('Net::IP::Route', prefix: 'route') } - before(:each) do - allow(Net::IP::Route).to receive(:new).with('should').and_return(netiproute) - allow(Net::IP::Route::Collection).to receive(:new).with('main').and_return(routes) - end - let(:route) do [ { @@ -41,42 +36,45 @@ module Puppet::Provider::NetworkRoute; end ] end + before do + allow(Net::IP::Route).to receive(:new).with('should').and_return(netiproute) + allow(Net::IP::Route::Collection).to receive(:new).with('main').and_return(routes) + end + describe '#puppet_munge(should)' do let(:should) { network_route[0] } - it 'should parse network_route into iproute2 keys' do - expect(provider.puppet_munge(should)).to eq( - { - dev: 'enp0s3', - metric: '100', - prefix: 'default', - proto: 'dhcp', - via: '10.0.2.2', - } + it 'parses network_route into iproute2 keys' do + expect(provider.puppet_munge(is_expected.to)).to eq( + dev: 'enp0s3', + metric: '100', + prefix: 'default', + proto: 'dhcp', + via: '10.0.2.2' ) end end describe '#get' do - before(:each) do + before do allow(provider).to receive(:routes_list).and_return(route) # rubocop:disable RSpec/SubjectStub end it 'processes resources' do expect(provider.get(context)).to eq( - [{default_route: true, - ensure: 'present', - gateway: '10.0.2.2', - interface: 'enp0s3', - metric: '100', - prefix: 'default', - protocol: 'dhcp'}] + [{ default_route: true, + ensure: 'present', + gateway: '10.0.2.2', + interface: 'enp0s3', + metric: '100', + prefix: 'default', + protocol: 'dhcp' }] ) end end describe '#create(context, name, should)' do - before(:each) do + before do allow(provider).to receive(:puppet_munge).with('should').and_return('munged') end @@ -87,7 +85,7 @@ module Puppet::Provider::NetworkRoute; end end describe '#update(context, name, should)' do - before(:each) do + before do allow(provider).to receive(:puppet_munge).with('should').and_return('munged') end @@ -99,7 +97,7 @@ module Puppet::Provider::NetworkRoute; end end describe 'delete(context, name, should)' do - before(:each) do + before do allow(provider).to receive(:puppet_munge).with('should').and_return('munged') end diff --git a/spec/unit/puppet/type/network_route_spec.rb b/spec/unit/puppet/type/network_route_spec.rb index c38ee02b..b133bdc1 100644 --- a/spec/unit/puppet/type/network_route_spec.rb +++ b/spec/unit/puppet/type/network_route_spec.rb @@ -10,7 +10,7 @@ let(:resource) do Puppet::Type.type('network_route').new( prefix: 'default', - default_route: true, + default_route: true ) end @@ -167,20 +167,24 @@ end context 'with invalid scope' do - it 'should raise an error' do - expect { Puppet::Type.type('network_route').new( - prefix: 'default', - scope: 'fail' - )}.to raise_error(Puppet::ResourceError) + it 'raises an error' do + expect do + Puppet::Type.type('network_route').new( + prefix: 'default', + scope: 'fail' + ) + end.to raise_error(Puppet::ResourceError) end end context 'with invalid protocol' do - it 'should raise an error' do - expect { Puppet::Type.type('network_route').new( - prefix: 'default', - protocol: 'fail' - )}.to raise_error(Puppet::ResourceError) + it 'raises an error' do + expect do + Puppet::Type.type('network_route').new( + prefix: 'default', + protocol: 'fail' + ) + end.to raise_error(Puppet::ResourceError) end end end From e184f337afd82b9b1264eba55116d45aa4a0928a Mon Sep 17 00:00:00 2001 From: Keith Ward Date: Mon, 9 Dec 2019 01:24:19 +0000 Subject: [PATCH 18/19] Add puppet resource API to Gemfile/test --- .sync.yml | 1 + Gemfile | 1 + 2 files changed, 2 insertions(+) diff --git a/.sync.yml b/.sync.yml index 65f7cb48..e83307ef 100644 --- a/.sync.yml +++ b/.sync.yml @@ -11,3 +11,4 @@ Gemfile: ':test': - gem: 'ipaddress' - gem: 'rspec-its' + - gem: 'puppet-resource_api' diff --git a/Gemfile b/Gemfile index 65ac649f..3a49261f 100644 --- a/Gemfile +++ b/Gemfile @@ -33,6 +33,7 @@ group :test do gem 'ipaddress', :require => false gem 'net-ip', :require => false gem 'rspec-its', :require => false + gem 'puppet-resource_api', :require => false end group :development do From d6acbe3ee1193490a3644f99718ecfc8e3e8c473 Mon Sep 17 00:00:00 2001 From: Keith Ward Date: Tue, 10 Dec 2019 01:05:58 +0000 Subject: [PATCH 19/19] Fixup overzealous rubocop --- spec/unit/puppet/provider/network_route/network_route_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/unit/puppet/provider/network_route/network_route_spec.rb b/spec/unit/puppet/provider/network_route/network_route_spec.rb index b6bf07bf..3f9fc0c0 100644 --- a/spec/unit/puppet/provider/network_route/network_route_spec.rb +++ b/spec/unit/puppet/provider/network_route/network_route_spec.rb @@ -45,7 +45,7 @@ module Puppet::Provider::NetworkRoute; end let(:should) { network_route[0] } it 'parses network_route into iproute2 keys' do - expect(provider.puppet_munge(is_expected.to)).to eq( + expect(provider.puppet_munge(should)).to eq( dev: 'enp0s3', metric: '100', prefix: 'default',