[SLE-7188] Default to SUSE NTP servers

package/yast2-ntp-client.changes

@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Wed Feb 19 13:24:06 UTC 2020 - Imobach Gonzalez Sosa <igonzalezsosa@suse.com>
+
+- Rely on the new Y2Network::NtpServer class (jsc#SLE-7188).
+- 4.2.8
+
 -------------------------------------------------------------------
 Thu Jan 23 12:58:04 UTC 2020 - Steffen Winterfeldt <snwint@suse.com>
 

package/yast2-ntp-client.spec

@@ -17,7 +17,7 @@
 
 
 Name:           yast2-ntp-client
-Version:        4.2.7
+Version:        4.2.8
 Release:        0
 Summary:        YaST2 - NTP Client Configuration
 License:        GPL-2.0-or-later
@@ -39,15 +39,17 @@ BuildRequires:  yast2-devtools >= 4.2.2
 BuildRequires:  rubygem(%rb_default_ruby_abi:cfa) >= 0.6.0
 BuildRequires:  rubygem(%rb_default_ruby_abi:rspec)
 BuildRequires:  rubygem(%rb_default_ruby_abi:yast-rake)
+# Y2Network::NtpServer
+BuildRequires:  yast2-network >= 4.2.55
 
 # proper acting TargetFile when scr is switched
 Requires:       augeas-lenses
 # cwm/popup
 Requires:       yast2 >= 4.1.15
 Requires:       yast2-country-data
 # needed for network/config agent
-# Yast::Lan.dhcp_ntp_servers
-Requires:       yast2-network >= 4.1.17
+# Y2Network::NtpServer
+Requires:       yast2-network >= 4.2.55
 Requires:       yast2-ruby-bindings >= 1.0.0
 Requires:       rubygem(%rb_default_ruby_abi:cfa) >= 0.6.0
 

src/clients/ntp-client_proposal.rb

@@ -491,15 +491,20 @@ def configured_ntp_items
     # @return [Array<Yast::Term>] ntp address Item
     def timezone_ntp_items
       timezone_country = Timezone.GetCountryForTimezone(Timezone.timezone)
-      NtpClient.GetNtpServersByCountry(timezone_country, true)
+      servers = NtpClient.country_ntp_servers(timezone_country)
+      # Select the first occurrence of pool.ntp.org as the default option (bnc#940881)
+      selected = servers.find { |s| s.hostname.end_with?("pool.ntp.org") }
+      servers.map do |server|
+        Item(Id(server.hostname), server.hostname, server.hostname == selected)
+      end
     end
 
     # List of dhcp ntp servers Yast::Term items with the ntp address ID and
     # label
     #
     # @return [Array<Yast::Term>] ntp address table Item
     def dhcp_ntp_items
-      NtpClient.dhcp_ntp_servers.map { |s| Item(Id(s), s) }
+      NtpClient.dhcp_ntp_servers.map { |s| Item(Id(s.hostname), s.hostname) }
     end
 
     # List of ntp servers Yast::Term items with the ntp address ID and label

src/lib/y2ntp_client/widgets/pool_widgets.rb

@@ -308,8 +308,15 @@ def help
 
     private
 
+      # Returns the country for the given address
+      #
+      # FIXME: if the UI used a proper object instead of just a string, this
+      # method will not be needed.
+      #
+      # @param address [String] Server address
       def country_for(address)
-        Yast::NtpClient.GetNtpServers.fetch(address, {})["country"]
+        server = Yast::NtpClient.public_ntp_servers.find { |s| s.hostname == address }
+        server ? server.country : nil
       end
     end
 
@@ -382,7 +389,7 @@ def label
 
       # macro seeItemsSelection
       def items
-        ntp_servers.map { |s, v| [s, "#{s} (#{v["country"]})"] }
+        ntp_servers.map { |s| [s.hostname, "#{s.hostname} (#{s.country})"] }
       end
 
       def refresh(country)
@@ -393,10 +400,10 @@ def refresh(country)
     private
 
       def ntp_servers
-        servers = Yast::NtpClient.GetNtpServers
+        servers = Yast::NtpClient.public_ntp_servers
         return servers if @country.to_s.empty?
 
-        servers.find_all { |_s, v| v["country"] == @country }
+        servers.select { |s| s.country == @country }
       end
     end
   end

src/modules/NtpClient.rb

@@ -14,6 +14,7 @@
 require "ui/text_helpers"
 require "erb"
 require "yast2/systemctl"
+require "y2network/ntp_server"
 
 module Yast
   class NtpClientClass < Module
@@ -193,7 +194,35 @@ def MakePoolRecord(country_code, location)
       }
     end
 
+    # Returns the know ntp servers
+    #
+    # @return [Array<Y2Network::NtpServer>] Known NTP servers
+    def public_ntp_servers
+      update_ntp_servers! if @ntp_servers.nil?
+      @ntp_servers.values.map do |srv|
+        Y2Network::NtpServer.new(
+          srv["address"], country: srv["country"], location: srv["location"]
+        )
+      end
+    end
+
+    # Returns the NTP servers for the given country
+    #
+    # @param country [String] Country code
+    # @return [Array<Y2Network::NtpServer>] NTP servers for the given country
+    def country_ntp_servers(country)
+      normalized_country = country.upcase
+      servers = public_ntp_servers.select { |s| s.country.upcase == normalized_country }
+      # bnc#458917 add country, in case data/country.ycp does not have it
+      country_server = make_country_ntp_server(country)
+      unless servers.map(&:hostname).include?(country_server.hostname)
+        servers << country_server
+      end
+      servers
+    end
+
     # Get the list of known NTP servers
+    # @deprecated Use public_ntp_servers instead
     # @return a list of known NTP servers
     def GetNtpServers
       update_ntp_servers! if @ntp_servers.nil?
@@ -219,9 +248,11 @@ def GetCountryNames
     end
 
     # Get list of public NTP servers for a country
+    #
     # @param [String] country two-letter country code
     # @param [Boolean] terse_output display additional data (location etc.)
     # @return [Array] of servers (usable as combo-box items)
+    # @deprecated Use public_ntp_servers_by_country instead
     def GetNtpServersByCountry(country, terse_output)
       country_names = {}
       servers = GetNtpServers()
@@ -593,7 +624,7 @@ def AutoPackages
     # Convenience method to obtain the list of ntp servers proposed by DHCP
     # @see https://www.rubydoc.info/github/yast/yast-network/Yast/LanClass:${0}
     def dhcp_ntp_servers
-      Yast::Lan.dhcp_ntp_servers
+      Yast::Lan.dhcp_ntp_servers.map { |s| Y2Network::NtpServer.new(s) }
     end
 
     publish variable: :AbortFunction, type: "boolean ()"
@@ -919,6 +950,7 @@ def country_server_label(location = "", country = "")
     # @return [Array <Hash>] pool records for given countries
     def pool_servers_for(known_countries)
       known_countries.map do |short_country, country_name|
+        # bnc#458917 add country, in case data/country.ycp does not have it
         MakePoolRecord(short_country, country_name)
       end
     end
@@ -945,6 +977,15 @@ def unsupported_error(unsupported)
 
       Yast::Report.Error(wrap_text(msg, width - 4))
     end
+
+    # Pool server for the given country
+    #
+    # @param country [String] Country code
+    # @return [Y2Network::NtpServer]
+    def make_country_ntp_server(country)
+      record = MakePoolRecord(country, "")
+      Y2Network::NtpServer.new(record["address"], country: record["country"])
+    end
   end
 
   NtpClient = NtpClientClass.new

test/data/ntp_servers_sample.yml

@@ -0,0 +1,16 @@
+---
+
+- access_policy: open access
+  address: ntp.cgi.cz
+  country: CZ
+  exact_location: Prague, The Czech Republic
+  location: Czech Republic
+  stratum: '2'
+  synchronization: NTP V4 secondary (stratum 2), PC/FreebSD
+
+- address: tick.fh-augsburg.de
+  country: DE
+  exact_location: Augsburg University of Applied Sciences (FH), Augsburg, Bavaria,
+    Germany
+  stratum: '2'
+  synchronization: NTP V3 secondary (stratum 2), i486/Linux

test/ntp_client_proposal_test.rb

@@ -13,6 +13,51 @@
     client
   end
 
+  describe "#MakeProposal" do
+    let(:dhcp_ntp_servers) { [] }
+
+    before do
+      allow(Yast::Lan).to receive(:dhcp_ntp_servers)
+        .and_return(dhcp_ntp_servers)
+     allow(Yast::Directory).to receive(:find_data_file).and_call_original
+     allow(Yast::Directory).to receive(:find_data_file).with("ntp_servers.yml")
+       .and_return(DATA_PATH.join("ntp_servers_sample.yml").to_s)
+     allow(Yast::Timezone).to receive(:timezone).and_return("Europe/Berlin")
+     allow(Yast::Timezone).to receive(:GetCountryForTimezone)
+       .with("Europe/Berlin").and_return("de")
+    end
+
+    context "when NTP servers were found via DHCP" do
+      let(:dhcp_ntp_servers) { ["test.example.net"] }
+
+      it "proposes only the found servers" do
+        expect(Yast::UI).to receive(:ChangeWidget) do |*args|
+          items = args.last
+          hostnames = items.map { |i| i[1] }
+          expect(hostnames).to eq(
+            ["test.example.net"]
+          )
+        end
+        subject.MakeProposal
+      end
+    end
+
+    context "when no NTP server were found via DHCP" do
+      let(:dhcp_ntp_servers) { [] }
+
+      it "proposes the known public servers for the current timezone" do
+        expect(Yast::UI).to receive(:ChangeWidget) do |*args|
+          items = args.last
+          hostnames = items.map { |i| i[1] }
+          expect(hostnames).to eq(
+            ["tick.fh-augsburg.de", "de.pool.ntp.org"]
+          )
+        end
+        subject.MakeProposal
+      end
+    end
+  end
+
   describe "#Write" do
     let(:ntp_server) { "fake.pool.ntp.org" }
     let(:write_only) { false }

test/ntp_client_test.rb

@@ -340,6 +340,38 @@
     end
   end
 
+  describe "#public_ntp_servers" do
+    before do
+      allow(subject).to receive(:GetAllKnownCountries)
+        .and_return("DE" => "Germany", "CZ" => "Czech Republic")
+      allow(Yast::Directory).to receive(:find_data_file).with("ntp_servers.yml")
+        .and_return(DATA_PATH.join("ntp_servers_sample.yml").to_s)
+    end
+
+    it "returns the list of public NTP servers including the default one for each country" do
+      servers = subject.public_ntp_servers
+      expect(servers.map(&:hostname)).to eq(
+        ["ntp.cgi.cz", "tick.fh-augsburg.de", "de.pool.ntp.org", "cz.pool.ntp.org"]
+      )
+    end
+  end
+
+  describe "#country_ntp_servers" do
+    before do
+      allow(subject).to receive(:GetAllKnownCountries)
+        .and_return("DE" => "Germany", "CZ" => "Czech Republic")
+      allow(Yast::Directory).to receive(:find_data_file).with("ntp_servers.yml")
+        .and_return(DATA_PATH.join("ntp_servers_sample.yml").to_s)
+    end
+
+    it "returns the list of public NTP servers for the given country" do
+      servers = subject.country_ntp_servers("DE")
+      expect(servers.map(&:hostname)).to eq(
+        ["tick.fh-augsburg.de", "de.pool.ntp.org"]
+      )
+    end
+  end
+
   describe "#MakePoolRecord" do
     let(:record) do
       {

test/test_helper.rb

@@ -4,6 +4,10 @@
 require "yast"
 require "yast/rspec"
 require "yaml"
+require "pathname"
+
+TESTS_PATH = Pathname.new(File.dirname(__FILE__))
+DATA_PATH = TESTS_PATH.join("data")
 
 RSpec.configure do |config|
   config.mock_with :rspec do |mocks|
@@ -18,12 +22,13 @@
 
 # stub module to prevent its Import
 # Useful for modules from different yast packages, to avoid build dependencies
-def stub_module(name)
-  Yast.const_set(name.to_sym, Class.new { def self.fake_method; end })
+def stub_module(name, fake_class = nil)
+  fake_class = Class.new { def self.fake_method; end } if fake_class.nil?
+  Yast.const_set name.to_sym, fake_class
 end
 
 # stub classes from other modules to speed up a build
-stub_module("Lan")
+stub_module("Lan", Class.new { def dhcp_ntp_servers; []; end })
 stub_module("Language")
 stub_module("Pkg")
 stub_module("PackageCallbacks")