Merge bd93549 into 1d2eef2

package/yast2-network.changes

@@ -1,3 +1,17 @@
+-------------------------------------------------------------------
+Wed Jul  3 16:40:45 UTC 2019 - Imobach Gonzalez Sosa <igonzalezsosa@suse.com>
+
+- The 'lan' client returns the correct value (bsc#1140199).
+- 4.2.8
+
+-------------------------------------------------------------------
+Wed Jul  3 13:18:05 UTC 2019 - Knut Anderssen <kanderssen@suse.com>
+
+- bsc#1137346
+  - CLI: Report an error instead of raising an exception when the
+    device type is not provided and there is no way to infer it
+    from the given options.
+
 -------------------------------------------------------------------
 Fri Jun 14 13:14:05 UTC 2019 - Knut Anderssen <kanderssen@suse.com>
 

package/yast2-network.spec

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

src/clients/lan.rb

@@ -92,7 +92,8 @@ def main
             "example" => [
               "yast lan add name=vlan50 ethdevice=eth0 bootproto=dhcp",
               "yast lan add name=br0 bridge_ports=eth0 eth1 bootproot=dhcp",
-              "yast lan add name=bond0 slaves=eth0 eth1 bootproto=dhcp"
+              "yast lan add name=bond0 slaves=eth0 eth1 bootproto=dhcp",
+              "yast lan add name=dummy0 type=dummy ip=10.0.0.100"
             ]
           },
           "edit"   => {
@@ -205,7 +206,7 @@ def main
 
       # EOF
 
-      nil
+      @ret
     end
   end
 end

src/include/network/lan/cmdline.rb

@@ -182,41 +182,35 @@ def ListHandler(options)
     # Handler for action "add"
     # @param [Hash{String => String}] options action options
     def AddHandler(options)
-      options = deep_copy(options)
       LanItems.AddNew
       Lan.Add
-      Ops.set(
-        LanItems.Items,
-        [LanItems.current, "ifcfg"],
-        Ops.get(options, "name", "")
-      )
-      LanItems.type = options["type"]
-      raise "Device type is mandatory." if !LanItems.type
-
-      if LanItems.type == "bond"
-        LanItems.bond_slaves = Builtins.splitstring(
-          Ops.get(options, "slaves", ""),
-          " "
-        )
-      end
-      if LanItems.type == "vlan"
-        LanItems.vlan_etherdevice = Ops.get(options, "ethdevice", "")
+      LanItems.Items[LanItems.current]["ifcfg"] = options.fetch("name", "")
+      LanItems.type = options.fetch("type", infered_type(options))
+      if LanItems.type.empty?
+        Report.Error(_("The device type is mandatory."))
+        return false
       end
-      if LanItems.type == "br"
-        LanItems.bridge_ports = Ops.get(options, "bridge_ports", "")
+
+      case LanItems.type
+      when "bond"
+        LanItems.bond_slaves = options.fetch("slaves", "").split(" ")
+      when "vlan"
+        LanItems.vlan_etherdevice = options.fetch("ethdevice", "")
+      when "br"
+        LanItems.bridge_ports = options.fetch("bridge_ports", "")
       end
 
-      LanItems.bootproto = Ops.get(options, "bootproto", "none")
-      if !Builtins.contains(["none", "static", "dhcp"], LanItems.bootproto)
+      LanItems.bootproto = options.fetch("bootproto", "none")
+      unless ["none", "static", "dhcp"].include? LanItems.bootproto
         Report.Error(_("Impossible value for bootproto."))
         return false
       end
 
-      LanItems.ipaddr = Ops.get(options, "ip", "")
-      LanItems.prefix = Ops.get(options, "prefix", "")
-      LanItems.netmask = Ops.get(options, "netmask", "255.255.255.0")
-      LanItems.startmode = Ops.get(options, "startmode", "auto")
-      if !Builtins.contains(["auto", "ifplugd", "nfsroot"], LanItems.startmode)
+      LanItems.ipaddr = options.fetch("ip", "")
+      LanItems.prefix = options.fetch("prefix", "")
+      LanItems.netmask = options.fetch("netmask", "255.255.255.0")
+      LanItems.startmode = options.fetch("startmode", "auto")
+      unless ["auto", "ifplugd", "nfsroot"].include? LanItems.startmode
         Report.Error(_("Impossible value for startmode."))
         return false
       end
@@ -321,5 +315,20 @@ def DeleteHandler(options)
 
       true
     end
+
+  private
+
+    # Return the infered type from the given options or an empty string if no
+    # one infered.
+    #
+    # @param options [Hash{String => String}] action options
+    # @return [String] infered device type; an empty string if not infered
+    def infered_type(options)
+      return "bond" if options.include? "slaves"
+      return "vlan" if options.include? "ethdevice"
+      return "br"   if options.include? "bridge_ports"
+
+      ""
+    end
   end
 end

test/cmdline_test.rb

@@ -21,4 +21,62 @@ def initialize
       expect(subject.ShowHandler("id" => "0")).to eq true
     end
   end
+
+  describe "AddHandler" do
+    let(:options) { { "name" => "vlan0", "ethdevice" => "eth0", "bootproto" => "dhcp" } }
+
+    before do
+      allow(Yast::Report).to receive(:Error)
+      allow(Yast::LanItems).to receive(:Commit)
+    end
+
+    context "when called without type" do
+      let(:no_type_options) { options.reject { |k| k == "ethdevice" } }
+
+      context "and it cannot be infered from the given options" do
+        it "reports an error" do
+          expect(Yast::Report).to receive(:Error)
+          subject.AddHandler(no_type_options)
+        end
+
+        it "returns false" do
+          expect(subject.AddHandler(no_type_options)).to eq false
+        end
+      end
+    end
+
+    context "when startmode is given" do
+      context "but with an invalid option" do
+        it "reports an error" do
+          expect(Yast::Report).to receive(:Error)
+          subject.AddHandler(options.merge("startmode" => "wrong"))
+        end
+
+        it "returns false" do
+          expect(subject.AddHandler(options.merge("startmode" => "wrong"))).to eq false
+        end
+      end
+    end
+
+    xcontext "when a valid configuration is providen" do
+      before do
+        allow(subject).to receive(:ListHandler)
+      end
+
+      it "commits the new configuration" do
+        expect(Yast::LanItems).to receive(:Commit)
+        subject.AddHandler(options)
+      end
+
+      it "lists the final configuration" do
+        expect(subject).to receive(:ListHandler)
+        subject.AddHandler(options)
+      end
+
+      it "returns true" do
+        expect(Yast::Report).to_not receive(:Error)
+        expect(subject.AddHandler(options)).to eq true
+      end
+    end
+  end
 end

test/y2network/proposal_settings_test.rb

@@ -17,8 +17,8 @@
   def stub_features(features)
     Yast.import "ProductFeatures"
     Yast::ProductFeatures.Import(features)
-    # stub restore as during Stage normal it restores features
-    allow(Yast::ProductFeatures).to receive(:Restore)
+    # Prevent to Restore or Init
+    allow(Yast::ProductFeatures).to receive(:InitIfNeeded)
   end
 
   describe ".instance" do