Use control.xml to set DNS.dhcp_hostname (bnc#870896)

In order to completely close the bug, changes in control.xml for both SLED and
SLES are needed.

src/lib/network/network_autoconfiguration.rb

@@ -83,8 +83,10 @@ def configure_dns
       DNS.Read # handles NetworkConfig too
       DNS.ProposeHostname # generate random hostname, if none known so far
 
-      # propose settings
-      DNS.dhcp_hostname = !Arch.is_laptop
+      # FIXME after SLE12: DNS.default_dhcp_hostname should be private (setting
+      # default values is not something for an API), but that would need some
+      # refactoring of this part.
+      DNS.dhcp_hostname = DNS.default_dhcp_hostname
 
       # get default value, from control.xml
       DNS.write_hostname = DNS.DefaultWriteHostname

src/modules/DNS.rb

@@ -43,6 +43,7 @@ def main
       Yast.import "UI"
       textdomain "network"
 
+      Yast.import "Arch"
       Yast.import "NetHwDetection"
       Yast.import "Hostname"
       Yast.import "IP"
@@ -228,6 +229,24 @@ def DefaultWriteHostname
       whth
     end
 
+    # Default value for #dhcp_hostname based on ProductFeatures and Arch
+    #
+    # @return [Boolean] value set in features or, if none is set, false just
+    #                   for laptops
+    def default_dhcp_hostname
+      # ProductFeatures.GetBooleanFeature returns false either if the value is
+      # false or if it's missing, so let's discard the later case calling
+      # ProductFeatures.GetFeature first
+      feature_index = ["globals", "dhclient_set_hostname"]
+      feature = ProductFeatures.GetFeature(*feature_index)
+      # No value for the feature
+      if feature.nil? || (feature.respond_to?(:empty?) && feature.empty?)
+        !Arch.is_laptop
+      else
+        ProductFeatures.GetBooleanFeature(*feature_index)
+      end
+    end
+
     def ReadHostname
       fqhostname = ""
       # In installation (standard, or AutoYaST one), prefer /etc/install.inf
@@ -445,7 +464,7 @@ def Write
     # @return true if success
     def Import(settings)
       settings = deep_copy(settings)
-      @dhcp_hostname = Ops.get_boolean(settings, "dhcp_hostname", false)
+      @dhcp_hostname = settings.fetch("dhcp_hostname") { default_dhcp_hostname }
       #if not defined, set to 'auto'
       @resolv_conf_policy = Ops.get_string(
         settings,

test/data/dhcp_hostname_false.xml

@@ -0,0 +1,36 @@
+<?xml version="1.0"?>
+<productDefines xmlns="http://www.suse.com/1.0/yast2ns" xmlns:config="http://www.suse.com/1.0/configns">
+
+<!--
+Work around for the text domain
+textdomain="control"
+-->
+
+    <textdomain>control</textdomain>
+
+    <globals>
+        <additional_kernel_parameters/>
+        <enable_autologin config:type="boolean">false</enable_autologin>
+        <write_hostname_to_hosts config:type="boolean">true</write_hostname_to_hosts>
+        <dhclient_set_hostname config:type="boolean">false</dhclient_set_hostname>
+    </globals>
+
+    <software>
+        <delete_old_packages config:type="boolean">true</delete_old_packages>
+    </software>
+
+    <partitioning>
+        <try_separate_home config:type="boolean">true</try_separate_home>
+    </partitioning>
+
+    <network>
+        <force_static_ip config:type="boolean">false</force_static_ip>
+        <network_manager>always</network_manager>
+        <startmode>auto</startmode>
+    </network>
+
+    <texts>
+        <dummy_desktop><label/></dummy_desktop>
+    </texts>
+
+</productDefines>

test/data/dhcp_hostname_nil.xml

@@ -0,0 +1,35 @@
+<?xml version="1.0"?>
+<productDefines xmlns="http://www.suse.com/1.0/yast2ns" xmlns:config="http://www.suse.com/1.0/configns">
+
+<!--
+Work around for the text domain
+textdomain="control"
+-->
+
+    <textdomain>control</textdomain>
+
+    <globals>
+        <additional_kernel_parameters/>
+        <enable_autologin config:type="boolean">false</enable_autologin>
+        <write_hostname_to_hosts config:type="boolean">true</write_hostname_to_hosts>
+    </globals>
+
+    <software>
+        <delete_old_packages config:type="boolean">true</delete_old_packages>
+    </software>
+
+    <partitioning>
+        <try_separate_home config:type="boolean">true</try_separate_home>
+    </partitioning>
+
+    <network>
+        <force_static_ip config:type="boolean">false</force_static_ip>
+        <network_manager>always</network_manager>
+        <startmode>auto</startmode>
+    </network>
+
+    <texts>
+        <dummy_desktop><label/></dummy_desktop>
+    </texts>
+
+</productDefines>

test/data/dhcp_hostname_true.xml

@@ -0,0 +1,36 @@
+<?xml version="1.0"?>
+<productDefines xmlns="http://www.suse.com/1.0/yast2ns" xmlns:config="http://www.suse.com/1.0/configns">
+
+<!--
+Work around for the text domain
+textdomain="control"
+-->
+
+    <textdomain>control</textdomain>
+
+    <globals>
+        <additional_kernel_parameters/>
+        <enable_autologin config:type="boolean">false</enable_autologin>
+        <write_hostname_to_hosts config:type="boolean">true</write_hostname_to_hosts>
+        <dhclient_set_hostname config:type="boolean">true</dhclient_set_hostname>
+    </globals>
+
+    <software>
+        <delete_old_packages config:type="boolean">true</delete_old_packages>
+    </software>
+
+    <partitioning>
+        <try_separate_home config:type="boolean">true</try_separate_home>
+    </partitioning>
+
+    <network>
+        <force_static_ip config:type="boolean">false</force_static_ip>
+        <network_manager>always</network_manager>
+        <startmode>auto</startmode>
+    </network>
+
+    <texts>
+        <dummy_desktop><label/></dummy_desktop>
+    </texts>
+
+</productDefines>

test/dns_test.rb

@@ -0,0 +1,111 @@
+#!/usr/bin/env rspec
+
+ENV["Y2DIR"] = File.expand_path("../../src", __FILE__)
+
+require "yast"
+require_relative "SCRStub"
+
+module Yast
+  RSpec.configure do |c|
+    c.include SCRStub
+  end
+
+  import "Arch"
+  import "DNS"
+  import "ProductControl"
+
+  describe DNS do
+    describe ".default_dhcp_hostname" do
+      before do
+        allow(Arch).to receive(:is_laptop).and_return laptop
+        ProductControl.ReadControlFile(File.join(SCRStub::DATA_PATH, control_file))
+      end
+
+      context "with dhcp_hostname=true in control file" do
+        let(:control_file) { "dhcp_hostname_true.xml" }
+
+        context "in a laptop" do
+          let(:laptop) { true }
+
+          it "returns the value from product features" do
+            expect(DNS.default_dhcp_hostname).to eql(true)
+          end
+        end
+
+        context "in a workstation" do
+          let(:laptop) { false }
+
+          it "returns the value from product features" do
+            expect(DNS.default_dhcp_hostname).to eql(true)
+          end
+        end
+      end
+
+      context "with dhcp_hostname=false in control file" do
+        let(:control_file) { "dhcp_hostname_false.xml" }
+
+        context "in a laptop" do
+          let(:laptop) { true }
+
+          it "returns the value from product features" do
+            expect(DNS.default_dhcp_hostname).to eql(false)
+          end
+        end
+
+        context "in a workstation" do
+          let(:laptop) { false }
+
+          it "returns the value from product features" do
+            expect(DNS.default_dhcp_hostname).to eql(false)
+          end
+        end
+      end
+
+      context "without dhcp_hostname in control file" do
+        let(:control_file) { "dhcp_hostname_nil.xml" }
+
+        context "in a laptop" do
+          let(:laptop) { true }
+
+          it "returns false" do
+            expect(DNS.default_dhcp_hostname).to eql(false)
+          end
+        end
+
+        context "in a workstation" do
+          let(:laptop) { false }
+
+          it "returns true" do
+            expect(DNS.default_dhcp_hostname).to eql(true)
+          end
+        end
+      end
+    end
+
+    describe ".Import" do
+      context "with present dhcp_hostname and write_hostname" do
+        let(:settings) { {"hostname" => "host", "dhcp_hostname" => true, "write_hostname" => true} }
+
+        it "honors the provided values" do
+          expect(DNS).to_not receive(:DefaultWriteHostname)
+          expect(DNS).to_not receive(:default_dhcp_hostname)
+          DNS.Import(settings)
+          expect(DNS.write_hostname).to eql(true)
+          expect(DNS.dhcp_hostname).to eql(true)
+        end
+      end
+
+      context "with missing dhcp_hostname and write_hostname" do
+        let(:settings) { {"hostname" => "host"} }
+
+        it "relies on proper methods to get default values" do
+          expect(DNS).to receive(:DefaultWriteHostname).and_return false
+          expect(DNS).to receive(:default_dhcp_hostname).and_return false
+          DNS.Import(settings)
+          expect(DNS.write_hostname).to eql(false)
+          expect(DNS.dhcp_hostname).to eql(false)
+        end
+      end
+    end
+  end
+end