From 39baaae8d8c315f34f32a28bdf32f37e8fbbedff Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Wed, 4 Apr 2018 12:21:34 +0200 Subject: [PATCH 1/7] implement generic handler for not yet known records (bsc#1086526) --- .gitignore | 1 + src/lib/cfa/ntp_conf.rb | 36 +++++++++++++++++++++++++---- test/fixtures/autoyast/autoinst.xml | 6 +++++ test/ntp_client_test.rb | 7 +++++- test/test_helper.rb | 2 ++ 5 files changed, 47 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index a8f17109..e5980009 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,4 @@ tmp.* /.yardoc /coverage test/data/scr_root/var/lib/YaST2/file_checksums.ycp +test/data/scr_root/etc/sysconfig/ diff --git a/src/lib/cfa/ntp_conf.rb b/src/lib/cfa/ntp_conf.rb index 2a09a9de..53c11900 100755 --- a/src/lib/cfa/ntp_conf.rb +++ b/src/lib/cfa/ntp_conf.rb @@ -1,3 +1,4 @@ +require "yast/logger" require "cfa/base_model" require "cfa/augeas_parser" require "cfa/matcher" @@ -209,7 +210,9 @@ def reset_cache def record_entries return @record_entries if @record_entries matcher = Matcher.new do |k, _v| - RECORD_ENTRIES.include?(k.gsub("[]", "")) + # we want to get all content except comments, as now we can process + # even not yet known options with MiscRecord + k !~ /#comment/ end @record_entries = @augeas_tree.select(matcher).map do |e| Record.new_from_augeas(e) @@ -228,6 +231,7 @@ def record_entries # for its options in the AugeasElement. For each one, # a Record subclass is created. class Record + include ::Yast::Logger # Creates the corresponding subclass object according # to its AugeasElement key. # @param [String] key @@ -239,8 +243,22 @@ def self.new_from_augeas(augeas_entry) # @param [string] key def self.record_class(key) entry_type = key.gsub("[]", "") - record_class = "::CFA::NtpConf::#{entry_type.capitalize}Record" - Kernel.const_get(record_class) + record_class = "#{entry_type.capitalize}Record" + if CFA::NtpConf.constants.include?(record_class.to_sym) + CFA::NtpConf.const_get(record_class.to_sym) + else + # it is not a predefined record type, so use a generic misc record + res = Class.new(MiscRecord) + # and specify there its augeas type + res.send(:define_method, :augeas_type) { entry_type + "[]" } + res + end + end + + # default implementation just return constant AUGEAS_KEY, but can be + # redefined + def augeas_type + self.class.const_get("AUGEAS_KEY") end def initialize(augeas = nil) @@ -320,7 +338,7 @@ def raw_options=(raw_options) protected def create_augeas - { key: self.class.const_get("AUGEAS_KEY"), value: nil } + { key: augeas_type, value: nil } end def tree_value? @@ -361,6 +379,16 @@ def options_matcher end end + # class to represent generic non-specific key + class MiscRecord < Record + def options + [] + end + + def options=(_value) + end + end + # class to represent a ntp command record entry. There is a # subclass for server, peer, broadcast and broacastclient. class CommandRecord < Record diff --git a/test/fixtures/autoyast/autoinst.xml b/test/fixtures/autoyast/autoinst.xml index 3c7c1901..e0246fa5 100644 --- a/test/fixtures/autoyast/autoinst.xml +++ b/test/fixtures/autoyast/autoinst.xml @@ -29,6 +29,12 @@ iburst server + +
panic 0
+ + + tinker + diff --git a/test/ntp_client_test.rb b/test/ntp_client_test.rb index 958f813f..5260d941 100644 --- a/test/ntp_client_test.rb +++ b/test/ntp_client_test.rb @@ -1,5 +1,6 @@ require_relative "test_helper" +require "yast2/target_file" require "fileutils" require "cfa/memory_file" require "cfa/ntp_conf" @@ -61,7 +62,7 @@ end it "reads the list of peers" do - expect(subject.ntp_records.size).to eq 4 + expect(subject.ntp_records.size).to eq 5 end it "reads the list of restricts" do @@ -89,6 +90,10 @@ it "reads sync intervall" do expect(subject.sync_interval).to eq 15 end + + it "does not crash during write" do + subject.Write + end end context "with an empty AutoYaST configuration" do diff --git a/test/test_helper.rb b/test/test_helper.rb index 81c96afc..0fe30708 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -4,6 +4,8 @@ require "yast/rspec" require "yaml" +ENV["LC_ALL"] = "en_US.utf-8" + RSpec.configure do |config| config.mock_with :rspec do |mocks| # If you misremember a method name both in code and in tests, From d47673df536f8e313d98234b6f3e8b80b37ed1c9 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Mon, 16 Apr 2018 15:00:16 +0200 Subject: [PATCH 2/7] add test to reproduce problem with write --- test/ntp_client_test.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/ntp_client_test.rb b/test/ntp_client_test.rb index 5260d941..fb5fed2c 100644 --- a/test/ntp_client_test.rb +++ b/test/ntp_client_test.rb @@ -91,8 +91,10 @@ expect(subject.sync_interval).to eq 15 end - it "does not crash during write" do - subject.Write + it "following write succeed" do + puts subject.ntp_records.inspect + expect(Yast::Report).to_not receive(:Error) + expect(subject.Write).to eq true end end From ac38bf75920f9169d10884635b9f650e0aeb7faa Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Mon, 16 Apr 2018 15:32:45 +0200 Subject: [PATCH 3/7] Drop generic client as it does not work with variety of options and more strict augeas checking. Instead add just tinker type. --- src/lib/cfa/ntp_conf.rb | 32 ++++++++++++++------------------ test/ntp_client_test.rb | 3 ++- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/src/lib/cfa/ntp_conf.rb b/src/lib/cfa/ntp_conf.rb index 53c11900..0fcdcd7e 100755 --- a/src/lib/cfa/ntp_conf.rb +++ b/src/lib/cfa/ntp_conf.rb @@ -40,6 +40,7 @@ class NtpConf < ::CFA::BaseModel manycastclient fudge restrict + tinker driftfile logfile keys @@ -210,9 +211,7 @@ def reset_cache def record_entries return @record_entries if @record_entries matcher = Matcher.new do |k, _v| - # we want to get all content except comments, as now we can process - # even not yet known options with MiscRecord - k !~ /#comment/ + RECORD_ENTRIES.include?(k.gsub("[]", "")) end @record_entries = @augeas_tree.select(matcher).map do |e| Record.new_from_augeas(e) @@ -247,11 +246,7 @@ def self.record_class(key) if CFA::NtpConf.constants.include?(record_class.to_sym) CFA::NtpConf.const_get(record_class.to_sym) else - # it is not a predefined record type, so use a generic misc record - res = Class.new(MiscRecord) - # and specify there its augeas type - res.send(:define_method, :augeas_type) { entry_type + "[]" } - res + raise "Unsupported entry #{key}." end end @@ -379,16 +374,6 @@ def options_matcher end end - # class to represent generic non-specific key - class MiscRecord < Record - def options - [] - end - - def options=(_value) - end - end - # class to represent a ntp command record entry. There is a # subclass for server, peer, broadcast and broacastclient. class CommandRecord < Record @@ -421,6 +406,17 @@ def add_option(key, option) end end + # class to represent generic non-specific key + class TinkerRecord < CommandRecord + AUGEAS_KEY = "tinker[]".freeze + # Tinker commands place values as options in subtree + def value=(val) + self.options=(val) + super(nil) + end + end + + # class to represent a driftfile entry. # For example: # driftfile /var/lib/ntp/drift/ntp.drift diff --git a/test/ntp_client_test.rb b/test/ntp_client_test.rb index fb5fed2c..36725056 100644 --- a/test/ntp_client_test.rb +++ b/test/ntp_client_test.rb @@ -92,7 +92,8 @@ end it "following write succeed" do - puts subject.ntp_records.inspect + allow(subject).to receive(:update_netconfig).and_return(true) + allow(subject).to receive(:check_service).and_return(true) expect(Yast::Report).to_not receive(:Error) expect(subject.Write).to eq true end From a1fec1d08a0d193cf3c6f7ef5d0742cf6af3fd3a Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Mon, 16 Apr 2018 15:43:08 +0200 Subject: [PATCH 4/7] more test mocking. It failing when run in ibs --- test/ntp_client_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ntp_client_test.rb b/test/ntp_client_test.rb index 36725056..f007359c 100644 --- a/test/ntp_client_test.rb +++ b/test/ntp_client_test.rb @@ -92,7 +92,7 @@ end it "following write succeed" do - allow(subject).to receive(:update_netconfig).and_return(true) + allow(subject).to receive(:write_and_update_policy).and_return(true) allow(subject).to receive(:check_service).and_return(true) expect(Yast::Report).to_not receive(:Error) expect(subject.Write).to eq true From 3d8aa026a26c2c631fb75ff285041250522ab73e Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Mon, 16 Apr 2018 15:46:23 +0200 Subject: [PATCH 5/7] make rubocop happy --- src/lib/cfa/ntp_conf.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/lib/cfa/ntp_conf.rb b/src/lib/cfa/ntp_conf.rb index 0fcdcd7e..17f96290 100755 --- a/src/lib/cfa/ntp_conf.rb +++ b/src/lib/cfa/ntp_conf.rb @@ -411,12 +411,11 @@ class TinkerRecord < CommandRecord AUGEAS_KEY = "tinker[]".freeze # Tinker commands place values as options in subtree def value=(val) - self.options=(val) + self.options = val super(nil) end end - # class to represent a driftfile entry. # For example: # driftfile /var/lib/ntp/drift/ntp.drift From ab99cd0602127ca4a88a63b771b529607f796767 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 17 Apr 2018 08:54:26 +0200 Subject: [PATCH 6/7] fix Tinker record to really write all options --- src/lib/cfa/ntp_conf.rb | 40 ++++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/src/lib/cfa/ntp_conf.rb b/src/lib/cfa/ntp_conf.rb index 17f96290..b40b8337 100755 --- a/src/lib/cfa/ntp_conf.rb +++ b/src/lib/cfa/ntp_conf.rb @@ -406,16 +406,6 @@ def add_option(key, option) end end - # class to represent generic non-specific key - class TinkerRecord < CommandRecord - AUGEAS_KEY = "tinker[]".freeze - # Tinker commands place values as options in subtree - def value=(val) - self.options = val - super(nil) - end - end - # class to represent a driftfile entry. # For example: # driftfile /var/lib/ntp/drift/ntp.drift @@ -474,6 +464,36 @@ def options_matcher end end + # class to represent tinker misc option - https://www.eecis.udel.edu/~mills/ntp/html/miscopt.html#tinker + class TinkerRecord < CommandRecord + AUGEAS_KEY = "tinker[]".freeze + + def value=(val) + values = val.to_s.split("\s") + map = Hash[*values] + ensure_tree_value + tree_value.tree.delete("key") + tree_value.tree.delete("key[]") + # fix writting order as key have to be before trailing comment + any_matcher = CFA::Matcher.new { true } + placer = CFA::BeforePlacer.new(any_matcher) + map.each_pair.reverse_each { |key, value| tree_value.tree.add(key, value, placer) } + end + + def value + return [] unless tree_value? + key_matcher = CFA::Matcher.new { |k, _v| k == "key" || k == "key[]" } + keys = tree_value.tree.select(key_matcher) + keys.map { |option| option[:value] }.join(" ") + end + + # overwrite options matcher as Tinker is stored in autoyast as address and not options + def options_matcher + # no options, everything is in value + Matcher.new { false } + end + end + # class to represent a requestkey entry. # For example: # requestkey 1 From 2478b1308c9ef349d824fd2283195322db909029 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 17 Apr 2018 09:04:19 +0200 Subject: [PATCH 7/7] changes --- package/yast2-ntp-client.changes | 6 ++++++ package/yast2-ntp-client.spec | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/package/yast2-ntp-client.changes b/package/yast2-ntp-client.changes index 852719eb..a492b2b7 100644 --- a/package/yast2-ntp-client.changes +++ b/package/yast2-ntp-client.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Tue Apr 17 07:03:46 UTC 2018 - jreidinger@suse.com + +- Add support for writting Tinker record via autoyast (bsc#1086526) +- 3.2.16 + ------------------------------------------------------------------- Wed Oct 11 08:40:26 UTC 2017 - mvidner@suse.com diff --git a/package/yast2-ntp-client.spec b/package/yast2-ntp-client.spec index dbcefb7c..8c7da651 100644 --- a/package/yast2-ntp-client.spec +++ b/package/yast2-ntp-client.spec @@ -17,7 +17,7 @@ Name: yast2-ntp-client -Version: 3.2.15 +Version: 3.2.16 Release: 0 Summary: YaST2 - NTP Client Configuration License: GPL-2.0+