Merge pull request #1036 from mchf/hostname-fixes

Fixed validation of hostname

package/yast2-network.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Fri Feb  7 17:31:31 UTC 2020 - Michal Filka <mfilka@suse.com>
+
+- bsc#1162271
+  - fixed validation of hostname
+- 4.2.54
+
 -------------------------------------------------------------------
 Fri Feb  7 15:50:13 UTC 2020 - Knut Anderssen <kanderssen@suse.com>
 

package/yast2-network.spec

@@ -17,7 +17,7 @@
 
 
 Name:           yast2-network
-Version:        4.2.53
+Version:        4.2.54
 Release:        0
 Summary:        YaST2 - Network Configuration
 License:        GPL-2.0-only

src/include/network/services/dns.rb

@@ -19,11 +19,7 @@
 # you may find current contact information at www.novell.com
 #
 # **************************************************************************
-# File:  include/network/lan/dialogs.ycp
-# Package:  Network configuration
-# Summary:  Summary, overview and IO dialogs for network cards config
-# Authors:  Michal Svec <msvec@suse.cz>
-#
+
 module Yast
   module NetworkServicesDnsInclude
     # CWM wants id-value pairs
@@ -61,10 +57,12 @@ def initialize_network_services_dns(include_target)
       # are lists and their widgets are suffixed.
       @hn_settings = {}
 
+      # TODO: It would be nice to display also transient hostname here - e.g. that
+      # one received from dhcp
       @widget_descr_dns = {
         "HOSTNAME"        => {
           "widget"            => :textentry,
-          "label"             => Label.HostName,
+          "label"             => _("Static H&ostname"),
           "opt"               => [],
           "help"              => Ops.get_string(@help, "hostname_global", ""),
           "valid_chars"       => Hostname.ValidChars,
@@ -431,17 +429,13 @@ def HandleResolverData(key, _event)
     # Validator for hostname, no_popup
     # @param key [String] the widget being validated
     # @param _event [Hash] the event being handled
-    # @return whether valid
+    # @return [Boolean] whether the current static hostname is valid or not
     def ValidateHostname(key, _event)
-      dhn = dhcp? && use_dhcp_hostname?
-      # If the names are set by dhcp, the user may enter backup values
-      # here - N#28427. That is, host and domain name are optional then.
-      # For static config, they are mandatory.
-      value = Convert.to_string(UI.QueryWidget(Id(key), :Value))
-
-      return Hostname.Check(value) if !dhn || value != ""
+      value = UI.QueryWidget(Id(key), :Value).to_s
 
-      true
+      # 1) empty hostname is allowed - /etc/hostname gets cleared in such case
+      # 2) FQDN is allowed
+      value.empty? || Hostname.Check(value.tr(".", ""))
     end
 
     # Validator for the search list

src/lib/y2network/sysconfig/hostname_writer.rb

@@ -73,8 +73,15 @@ def update_sysconfig_dhcp(hostname, old_hostname)
       #
       # @param hostname [Y2Network::Hostname] Hostname configuration
       def update_hostname(hostname)
-        Yast::Execute.on_target!("/usr/bin/hostname", hostname.hostname.split(".")[0])
-        Yast::SCR.Write(Yast::Path.new(".target.string"), HOSTNAME_PATH, "#{hostname.hostname}\n")
+        hostname = hostname.static
+        # 1) when user asked for erasing hostname from /etc/hostname, we keep runtime as it is
+        # 2) we will write whatever user wants even FQDN - no changes under the hood
+        Yast::Execute.on_target!("/usr/bin/hostname", hostname) if !hostname.empty?
+        Yast::SCR.Write(
+          Yast::Path.new(".target.string"),
+          HOSTNAME_PATH,
+          hostname.empty? ? "" : hostname + "\n"
+        )
       end
     end
   end

test/dns_service_test.rb

@@ -0,0 +1,60 @@
+#!/usr/bin/env rspec
+
+# Copyright (c) [2020] SUSE LLC
+#
+# All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or modify it
+# under the terms of version 2 of the GNU General Public License as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+# more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program; if not, contact SUSE LLC.
+#
+# To contact SUSE LLC about this file by physical or electronic mail, you may
+# find current contact information at www.suse.com.
+
+require_relative "test_helper"
+
+Yast.import "UI"
+
+class DummyDnsService < Yast::Module
+  def initialize
+    Yast.include self, "network/services/dns.rb"
+  end
+end
+
+describe "NetworkServicesDnsInclude" do
+  subject { DummyDnsService.new }
+
+  describe "#ValidateHostname" do
+    it "allows empty hostname" do
+      allow(Yast::UI).to receive(:QueryWidget).and_return("")
+
+      expect(subject.ValidateHostname("", {})).to be true
+    end
+
+    it "allows valid characters in hostname" do
+      allow(Yast::UI).to receive(:QueryWidget).and_return("sles")
+
+      expect(subject.ValidateHostname("", {})).to be true
+    end
+
+    it "allows FQDN hostname if user asks for it" do
+      allow(Yast::UI).to receive(:QueryWidget).and_return("sles.suse.de")
+
+      expect(subject.ValidateHostname("", {})).to be true
+    end
+
+    it "disallows invalid characters in hostname" do
+      allow(Yast::UI).to receive(:QueryWidget).and_return("suse_sles")
+
+      expect(subject.ValidateHostname("", {})).to be false
+    end
+  end
+end

test/y2network/hostname_writer_test.rb

@@ -0,0 +1,91 @@
+# Copyright (c) [2020] SUSE LLC
+#
+# All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or modify it
+# under the terms of version 2 of the GNU General Public License as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+# more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program; if not, contact SUSE LLC.
+#
+# To contact SUSE LLC about this file by physical or electronic mail, you may
+# find current contact information at www.suse.com.
+require_relative "../test_helper"
+require "y2network/sysconfig/hostname_writer"
+require "y2network/hostname"
+
+describe Y2Network::Sysconfig::HostnameWriter do
+  subject { Y2Network::Sysconfig::HostnameWriter.new }
+
+  describe ".write" do
+    let(:hostname_container) do
+      instance_double(
+        Y2Network::Hostname,
+        static:         hostname,
+        dhcp_hostname:  false,
+        save_hostname?: true
+      )
+    end
+
+    let(:old_hostname_container) do
+      instance_double(
+        Y2Network::Hostname,
+        static:         "old#{hostname}",
+        dhcp_hostname:  false,
+        save_hostname?: true
+      )
+    end
+
+    before(:each) do
+      allow(subject).to receive(:update_sysconfig_dhcp).and_return(nil)
+    end
+
+    context "when updating hostname" do
+      let(:hostname) { "hostname" }
+
+      it "updates system with the new hostname" do
+        expect(Yast::Execute)
+          .to receive(:on_target!)
+          .with("/usr/bin/hostname", hostname)
+        expect(Yast::SCR)
+          .to receive(:Write)
+          .with(anything, anything, /#{hostname}/)
+
+        subject.write(hostname_container, old_hostname_container)
+      end
+    end
+
+    context "when deleting hostname" do
+      let(:hostname) { "" }
+
+      it "updates system with the new hostname" do
+        expect(Yast::Execute)
+          .not_to receive(:on_target!)
+        expect(Yast::SCR)
+          .to receive(:Write)
+          .with(anything, anything, /#{hostname}/)
+
+        subject.write(hostname_container, old_hostname_container)
+      end
+    end
+
+    context "when no change in hostname" do
+      let(:hostname) { "hostname" }
+
+      it "does not try to update anything" do
+        expect(Yast::Execute)
+          .not_to receive(:on_target!)
+        expect(Yast::SCR)
+          .not_to receive(:Write)
+
+        subject.write(hostname_container, hostname_container)
+      end
+    end
+  end
+end