From 39baaae8d8c315f34f32a28bdf32f37e8fbbedff Mon Sep 17 00:00:00 2001
From: Josef Reidinger <jreidinger@suse.cz>
Date: Wed, 4 Apr 2018 12:21:34 +0200
Subject: [PATCH 1/2] 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 @@
         <options>iburst</options>
         <type>server</type>
       </peer>
+      <peer>
+        <address>panic 0</address>
+        <comment/>
+        <options/>
+        <type>tinker</type>
+      </peer>
     </peers>
     <restricts config:type="list">
       <restrict>
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 <jreidinger@suse.cz>
Date: Mon, 16 Apr 2018 15:00:16 +0200
Subject: [PATCH 2/2] 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