From be83af35c5d816548552a4375d29c9e1a54fcbba Mon Sep 17 00:00:00 2001 From: Trevor Bramble Date: Thu, 22 Mar 2018 09:58:22 -0700 Subject: [PATCH] Revise /etc/hosts for correctness and clarity (#2863) * Clean up test data, correct parse error handling * Use functional pipeline to avoid need for conditional clauses and clarify the intent of the comment parsing. * Extract magic strings to constants * Remove code and tests now covered by FileReader Co-authored-by: Trevor Bramble Co-authored-by: Paul Welch Signed-off-by: Trevor Bramble --- lib/resources/etc_hosts.rb | 57 +++++++++++---------------- test/helper.rb | 1 + test/unit/resources/etc_hosts_test.rb | 19 +++++++-- 3 files changed, 40 insertions(+), 37 deletions(-) diff --git a/lib/resources/etc_hosts.rb b/lib/resources/etc_hosts.rb index a98600ba8..0ab9c23bb 100644 --- a/lib/resources/etc_hosts.rb +++ b/lib/resources/etc_hosts.rb @@ -23,53 +23,44 @@ class EtcHosts < Inspec.resource(1) include CommentParser include FileReader + DEFAULT_UNIX_PATH = '/etc/hosts'.freeze + DEFAULT_WINDOWS_PATH = 'C:\windows\system32\drivers\etc\hosts'.freeze + def initialize(hosts_path = nil) - @conf_path = hosts_path || default_hosts_file_path - @content = nil - @params = nil - read_content + content = read_file_content(hosts_path || default_hosts_file_path) + + @params = parse_conf(content.lines) end - filter = FilterTable.create - filter.add_accessor(:where) - .add_accessor(:entries) - .add(:ip_address, field: 'ip_address') - .add(:primary_name, field: 'primary_name') - .add(:all_host_names, field: 'all_host_names') - - filter.connect(self, :params) + FilterTable.create + .add_accessor(:where) + .add_accessor(:entries) + .add(:ip_address, field: 'ip_address') + .add(:primary_name, field: 'primary_name') + .add(:all_host_names, field: 'all_host_names') + .connect(self, :params) private def default_hosts_file_path - inspec.os.windows? ? 'C:\windows\system32\drivers\etc\hosts' : '/etc/hosts' + inspec.os.windows? ? DEFAULT_WINDOWS_PATH : DEFAULT_UNIX_PATH end - def read_content - @content = '' - @params = {} - @content = read_file(@conf_path) - @params = parse_conf(@content) + def parse_conf(lines) + lines.reject(&:empty?).reject(&comment?).map(&parse_data).map(&format_data) end - def parse_conf(content) - content.map do |line| - data, = parse_comment_line(line, comment_char: '#', standalone_comments: false) - parse_line(data) unless data == '' - end.compact + def comment? + parse_options = { comment_char: '#', standalone_comments: false } + + ->(data) { parse_comment_line(data, parse_options).first.empty? } end - def parse_line(line) - line_parts = line.split - return nil unless line_parts.length >= 2 - { - 'ip_address' => line_parts[0], - 'primary_name' => line_parts[1], - 'all_host_names' => line_parts[1..-1], - } + def parse_data + ->(data) { [data.split[0], data.split[1], data.split[1..-1]] } end - def read_file(conf_path = @conf_path) - read_file_content(conf_path).lines + def format_data + ->(data) { %w{ip_address primary_name all_host_names}.zip(data).to_h } end end diff --git a/test/helper.rb b/test/helper.rb index 1994361b0..240b6f666 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -177,6 +177,7 @@ class MockLoader '/etc/postgresql/9.5/main' => mockfile.call('9.5.main'), '/var/lib/postgresql/9.5/main' => mockfile.call('var.9.5.main'), '/etc/hosts' => mockfile.call('hosts'), + '/etc/hosts_empty' => emptyfile.call, 'C:\windows\system32\drivers\etc\hosts' => mockfile.call('hosts'), '/etc/fstab' => mockfile.call('fstab'), 'fstab_no_home' => mockfile.call('fstab_no_home'), diff --git a/test/unit/resources/etc_hosts_test.rb b/test/unit/resources/etc_hosts_test.rb index 3e7412c39..735410eab 100644 --- a/test/unit/resources/etc_hosts_test.rb +++ b/test/unit/resources/etc_hosts_test.rb @@ -1,25 +1,36 @@ # encoding: utf-8 -# author: Matthew Dromazos require 'helper' require 'inspec/resource' describe 'Inspec::Resources::EtcHosts' do let(:resource) { load_resource('etc_hosts') } + + let(:all_v4_hosts) do + %W{localhost localhost.localdomain localhost4 localhost4.localdomain4} + end + + let(:all_v6_hosts) do + %W{localhost localhost.localdomain localhost6 localhost6.localdomain6} + end + it 'Verify etc_hosts filtering by `ip_address`' do entries = resource.where { ip_address == '127.0.0.1' } _(entries.primary_name).must_equal ['localhost'] - _(entries.all_host_names).must_equal [['localhost', 'localhost.localdomain', 'localhost4', 'localhost4.localdomain4']] + _(entries.all_host_names).must_equal [all_v4_hosts] end it 'Verify etc_hosts filtering by `canonical_hostname`' do entries = resource.where { primary_name == 'localhost' } _(entries.ip_address).must_equal ['127.0.0.1', '::1'] - _(entries.all_host_names).must_equal [['localhost', 'localhost.localdomain', 'localhost4', 'localhost4.localdomain4'], ['localhost', 'localhost.localdomain', 'localhost6', 'localhost6.localdomain6']] + _(entries.all_host_names).must_equal [all_v4_hosts, all_v6_hosts] end it 'Verify etc_hosts filtering by `all_host_names`' do - entries = resource.where { all_host_names == ['localhost', 'localhost.localdomain', 'localhost4', 'localhost4.localdomain4'] } + # direct reference all_v4_hosts fail in filter scope + expected_hosts = all_v4_hosts + + entries = resource.where { all_host_names == expected_hosts } _(entries.ip_address).must_equal ['127.0.0.1'] _(entries.primary_name).must_equal ['localhost'] end