Merge a887636 into 560fefb

Rakefile

@@ -3,4 +3,6 @@ require "yast/rake"
 Yast::Tasks.configuration do |conf|
   # lets ignore license check for now
   conf.skip_license_check << /.*/
+  # ensure we are not getting worse with documentation
+  conf.documentation_minimal = 62 if conf.respond_to?(:documentation_minimal=)
 end

doc/network-ng.md

@@ -3,7 +3,9 @@
 This document tries to describe the details of the network-ng data model and how all the pieces are
 combined.
 
-## Overall Approach
+## Data Model
+
+### Overall Approach
 
 {Y2Network::Config} and other associated classes represent the network configuration in a backend
 agnostic way. It should not matter whether you are using `sysconfig` files, NetworkManager or
@@ -25,7 +27,7 @@ filesystem by using a *configuration writer*.
 Obviously, we should implement configuration readers and writers for whathever backend we would like
 to support. At this point of time, only `Sysconfig` and `Autoinst` are supported.
 
-## The Configuration Classes
+### The Configuration Classes
 
 {Y2Network::Config} offers and API to deal with network configuration, but it collaborates with
 other classes.
@@ -38,31 +40,77 @@ other classes.
   Currently it is bound to an interface name, but we plan to provide more advanced matchers.
 * {Y2Network::Routing}: holds routing information, including IP forwarding settings, routing tables, etc.
 
-## Backend Support
+### Multi-Backend Support
+
+As mentioned above, Y2Network is designed to support different backends. It is expected to implement
+a reader and a writer for each backend (except for AutoYaST, which is an special case). The reader
+will be responsible for checking the system's configuration and building a {Y2Network::Config}
+object, containing interfaces, configurations, routes, etc. On the other hand, the writer will be
+responsible for updating the system using that configuration object.
+
+As a developer, you rarely will need to access to readers/writers because `Yast::Lan` already offers
+an API to read and write the configuration. See the [Accessing the
+Configuration](#accessing-the-configuration) section for further details.
 
-In order to support a new backend, we need to implement a configuration readers and writers. The
-{Y2Network::Sysconfig} module implements support to deal with `sysconfig` files.
+#### Sysconfig
+
+The sysconfig backend support is composed by these files:
 
     src/lib/y2network/sysconfig
     ├── config_reader.rb <- READER
     ├── config_writer.rb <- WRITER
-    ├── connection_config_reader_handlers
-    │   ├── eth.rb
-    │   ├── wlan.rb
-    │   └── ...
     ├── connection_config_reader.rb
+    ├── connection_config_readers
+    │   ├── ethernet.rb
+    │   ├── wireless.rb
+    │   └── ...
+    ├── connection_config_writer.rb
+    ├── connection_config_writers
+    │   ├── ethernet.rb
+    │   ├── wireless.rb
+    │   └── ...
     ├── dns_reader.rb
     ├── dns_writer.rb
     ├── interface_file.rb
     ├── interfaces_reader.rb
-    ├── interfaces_writer.rb
     └── routes_file.rb
 
-As you can see, there are many classes, but the relevant ones are just `ConfigReader` and `ConfigWriter`.
+{Y2Network::Sysconfig::ConfigReader} and {Y2Network::Sysconfig::ConfigWriter} are the reader and
+writer classes. Each of them cooperates with a set of ancillary classes in order to get the job
+done.
+
+{Y2Network::Sysconfig::DNSReader}, {Y2Network::Sysconfig::InterfacesReader} and
+{Y2Network::Sysconfig::ConnectionConfigReader} are involved in reading the configuration. The logic
+to read the configuration for a connection (e.g., `ifcfg-eth0`, `ifcfg-wlan0`, etc.) is implemented
+in a set of smaller classes (one for each time of connection) under
+{Y2Network::Sysconfig::ConnectionConfigReaders}.
+
+{Y2Network::Sysconfig::DNSWriter} and {Y2Network::Sysconfig::ConnectionConfigWriter}, including
+smaller classes under {Y2Network::Sysconfig::ConnectionConfigWriters}, are involved in writing the
+configuration. In this case, it does not exist a `InterfacesWriter` class, because when it comes to
+interfaces the configuration is handled through connection configuration files.
 
-## Accessing the Configuration
+Last but not least, there are additional classes like {Y2Network::Sysconfig::RoutesFile} and
+{Y2Network::Sysconfig::InterfaceFile} which abstract the details of reading/writing `ifcfg` files.
 
-The `Yast::Lan` module is still the entry point to read and write the network configuration. Basically, it keeps two configuration objects, one for the running system and another want for the wanted configuration.
+#### AutoYaST
+
+AutoYaST is a special case in the sense that it reads the information from a profile, instead of
+using the running system as reference. Additionally, it does not implement a writer because the
+configuration will be written using a different backend (like sysconfig).
+
+    src/lib/y2network/autoinst/
+    ├── config_reader.rb
+    ├── dns_reader.rb
+    └── routing_reader.rb
+
+For the time being, it only implements support to read DNS and routing information.
+
+### Accessing the Configuration
+
+The `Yast::Lan` module is still the entry point to read and write the network configuration.
+Basically, it keeps two configuration objects, one for the running system and another want for the
+wanted configuration.
 
     Yast.import "Lan"
     Yast::Lan.read(:cache)
@@ -78,20 +126,41 @@ The `Yast::Lan` module is still the entry point to read and write the network co
 Any change you want to apply to the running system should be performed by modifying the
 `yast_config` and writing the changes.
 
-## AutoYaST Support
+## New UI layer
+
+### Interface Configuration Builders
+
+In the old code, there was no clear separation between UI and business logic. In order to improve
+the situation, we introduced the concept of [interface configuration
+builders](https://github.com/yast/yast-network/blob/network-ng/src/lib/y2network/interface_config_builder.rb).
+
+We already have implemented support for several interface types. You can find them under the
+[Y2Network::InterfaceConfigBuilders
+namespace](https://github.com/yast/yast-network/tree/843f75bfdb71d4026b3f97facf18eece479b8a0e/src/lib/y2network/interface_config_builders).
 
-AutoYaST is somehow and special case, as the configuration is read from the profile instead of the
-running system. So in this scenario, YaST2 Network will read the configuration using the `Autoinst`
-reader and will write it to the final system using the one corresponding to the wanted backend.
+### Widgets
+
+The user interaction is driven by a set of sequences, which determine which dialogs are shown to the
+user. Each of those dialogs contain a set of widgets, usually grouped in tabs. The content of the
+dialog depends on the interface type.
+
+Below you can find some pointers to relevant sequences, dialogs and widgets:
+
+* Sequences:
+  *  [Sequences::Interface](https://github.com/yast/yast-network/blob/358bcd13b4e92e7c4e9c0e477c83196ca67b578e/src/lib/y2network/sequences/interface.rb)
+* Dialogs:
+  * [Dialogs::AddInterface](https://github.com/yast/yast-network/blob/358bcd13b4e92e7c4e9c0e477c83196ca67b578e/src/lib/y2network/dialogs/add_interface.rb)
+  * [Dialogs::EditInterface](https://github.com/yast/yast-network/blob/358bcd13b4e92e7c4e9c0e477c83196ca67b578e/src/lib/y2network/dialogs/edit_interface.rb)
+* [Y2Network::Widgets](https://github.com/yast/yast-network/tree/358bcd13b4e92e7c4e9c0e477c83196ca67b578e/src/lib/y2network/widgets)
 
 ## Current Status
 
 ### Reading/Writing Interfaces
 
 | Interface type  | read | write |
 |-----------------|------|-------|
-| Ethernet        |  ✓  |   ⌛   |
-| Wireless        |  ✓  |   ⌛   |
+| Ethernet        |  ✓  |   ✓   |
+| Wireless        |  ✓  |   ✓   |
 | InfiniBand      |  ⌛   |       |
 | Bridge          |  ⌛   |       |
 | Bonding         |      |       |
@@ -101,3 +170,9 @@ reader and will write it to the final system using the one corresponding to the
 | USB             |      |       |
 | Dummy           |      |       |
 | s390 types      |      |       |
+
+## (Short Term) Plan
+
+- [ ] Finish reading/writing interfaces
+- [ ] Move missing UI logic to interface configuration builders
+- [ ] Replace LanItems with the new data model

src/include/network/lan/address.rb

@@ -68,15 +68,15 @@ def AddressDialog(builder:)
       log.info "ShowAndRun: #{ret}"
 
       if ret != :back && ret != :abort
-        bootproto = builder["BOOTPROTO"]
-        ipaddr = builder["IPADDR"]
+        bootproto = builder.boot_protocol
+        ipaddr = builder.ip_address
 
         # IP is mandatory for static configuration. Makes no sense to write static
         # configuration without that.
         return ret if bootproto == "static" && ipaddr.empty?
 
         if bootproto == "static"
-          update_hostname(ipaddr, builder["HOSTNAME"] || "")
+          update_hostname(ipaddr, builder.hostname || "")
         elsif LanItems.isCurrentDHCP && !LanItems.isCurrentHotplug
           # fixed bug #73739 - if dhcp is used, dont set default gw statically
           # but also: reset default gw only if DHCP* is used, this branch covers
@@ -106,7 +106,7 @@ def AddressDialog(builder:)
       # rollback if changes are canceled, as still some widgets edit LanItems directly
       LanItems.Rollback if ret != :next
       # proceed with WLAN settings if appropriate, #42420
-      ret = :wire if ret == :next && builder.type == "wlan"
+      ret = :wire if ret == :next && builder.type.wireless?
 
       log.info "AddressDialog res: #{ret.inspect}"
       ret

src/include/network/lan/cmdline.rb

@@ -29,6 +29,7 @@
 
 require "shellwords"
 require "y2network/interface_config_builder"
+require "y2network/boot_protocol"
 
 module Yast
   module NetworkLanCmdlineInclude
@@ -212,7 +213,10 @@ def EditHandler(options)
       return false if validateId(options, config) == false
 
       LanItems.current = getItem(options, config)
-      builder = Y2Network::InterfaceConfigBuilder.for(LanItems.GetCurrentType())
+
+      config = Lan.yast_config.copy
+      connection_config = config.connections.find { |c| c.name == LanItems.GetCurrentName }
+      builder = Y2Network::InterfaceConfigBuilder.for(LanItems.GetCurrentType(), config: connection_config)
       builder.name = LanItems.GetCurrentName()
       LanItems.SetItem(builder: builder)
       update_builder_from_options!(builder, options)
@@ -271,19 +275,19 @@ def infered_type(options)
     # @param builder [Y2Network::InterfaceConfigBuilder]
     # @return [Boolean] true when the options are valid; false otherwise
     def validate_config(builder)
-      unless ["none", "static", "dhcp"].include? builder["BOOTPROTO"]
+      if builder.boot_protocol.nil?
         Report.Error(_("Impossible value for bootproto."))
         return false
       end
 
-      if builder["BOOTPROTO"] == "static" && builder["IPADDR"].empty?
+      if builder.boot_protocol.name == "static" && builder.ip_address.empty?
         Report.Error(
           _("For static configuration, the \"ip\" option is needed.")
         )
         return false
       end
 
-      unless ["auto", "ifplugd", "nfsroot"].include? builder["STARTMODE"]
+      unless builder.startmode
         Report.Error(_("Impossible value for startmode."))
         return false
       end
@@ -297,26 +301,25 @@ def validate_config(builder)
     # @param builder [Y2Network::InterfaceConfigBuilder]
     # @param options [Hash{String => String}] action options
     def update_builder_from_options!(builder, options)
-      case builder.type
+      case builder.type.short_name
       when "bond"
-        builder["BOND_SLAVES"] = options.fetch("slaves", "").split(" ")
+        builder.slaves = options.fetch("slaves", "").split(" ")
       when "vlan"
-        builder["ETHERDEVICE"] = options.fetch("ethdevice", "")
+        builder.etherdevice = options.fetch("ethdevice", "")
       when "br"
-        builder["BRIDGE_PORTS"] = options.fetch("bridge_ports", "")
+        builder.ports = options.fetch("bridge_ports", "")
       end
 
       default_bootproto = options.keys.include?("ip") ? "static" : "none"
-      builder["BOOTPROTO"] = options.fetch("bootproto", default_bootproto)
-      if builder["BOOTPROTO"] == "static"
-        builder["IPADDR"] = options.fetch("ip", "")
-        builder["PREFIX"] = options.fetch("prefix", "")
-        builder["NETMASK"] = options.fetch("netmask", "255.255.255.0") if builder["PREFIX"].empty?
+      builder.boot_protocol = options.fetch("bootproto", default_bootproto)
+      if builder.boot_protocol.name == "static"
+        builder.ip_address = options.fetch("ip", "")
+        builder.subnet_prefix = options.fetch("prefix", options.fetch("netmask", "255.255.255.0"))
       else
-        builder["IPADDR"] = ""
-        builder["NETMASK"] = ""
+        builder.ip_address = ""
+        builder.subnet_prefix = ""
       end
-      builder["STARTMODE"] = options.fetch("startmode", "auto")
+      builder.startmode = options.fetch("startmode", "auto")
     end
   end
 end

src/include/network/lan/complex.rb

@@ -305,13 +305,20 @@ def enableDisableButtons
 
     # Automatically configures slaves when user enslaves them into a bond or bridge device
     def UpdateSlaves
+      # TODO: sometimes it is called when no device is selected and then it crashes
+      return if LanItems.GetCurrentName.nil? || LanItems.GetCurrentName.empty?
+
       current = LanItems.current
-      master_builder = Y2Network::InterfaceConfigBuilder.for(LanItems.GetCurrentType())
+      config = Lan.yast_config.copy
+      connection_config = config.connections.find { |c| c.name == LanItems.GetCurrentName }
+      Yast.y2milestone("update slaves for #{current}:#{LanItems.GetCurrentName}:#{LanItems.GetCurrentType}")
+      master_builder = Y2Network::InterfaceConfigBuilder.for(LanItems.GetCurrentType(), config: connection_config)
       master_builder.name = LanItems.GetCurrentName()
 
       Lan.autoconf_slaves.each do |dev|
         if LanItems.FindAndSelect(dev)
-          builder = Y2Network::InterfaceConfigBuilder.for(LanItems.GetCurrentType())
+          connection_config = config.connections.find { |c| c.name == LanItems.GetCurrentName }
+          builder = Y2Network::InterfaceConfigBuilder.for(LanItems.GetCurrentType(), config: connection_config)
           builder.name = LanItems.GetCurrentName()
           LanItems.SetItem(builder: builder)
         else
@@ -328,17 +335,17 @@ def UpdateSlaves
         end
         # clear defaults, some defaults are invalid for slaves and can cause troubles
         # in related sysconfig scripts or makes no sence for slaves (e.g. ip configuration).
-        builder["NETMASK"] = ""
-        builder["BOOTPROTO"] = "none"
-        case master_builder.type
+        builder.subnet_prefix = ""
+        builder.boot_protocol = "none"
+        case master_builder.type.short_name
         when "bond"
           LanItems.startmode = "hotplug"
           # If there is already a rule based on the bus_id, do not update it.
           if LanItems.current_udev_rule.empty? || LanItems.GetItemUdev("KERNELS").empty?
             LanItems.update_item_udev_rule!(:bus_id)
           end
         when "br"
-          builder["IPADDR"] = ""
+          builder.ip_address = ""
         else
           raise "Adapting slaves for wrong type #{master_builder.type.inspect}"
         end
@@ -407,8 +414,9 @@ def handleOverview(_key, event)
           return :redraw
         when :edit
           if LanItems.IsCurrentConfigured
-
-            builder = Y2Network::InterfaceConfigBuilder.for(LanItems.GetCurrentType())
+            config = Lan.yast_config.copy
+            connection_config = config.connections.find { |c| c.name == LanItems.GetCurrentName }
+            builder = Y2Network::InterfaceConfigBuilder.for(LanItems.GetCurrentType(), config: connection_config)
             builder.name = LanItems.GetCurrentName()
             LanItems.SetItem(builder: builder)
 
@@ -535,9 +543,7 @@ def input_done?(ret)
       true
     end
 
-    def MainDialog(init_tab, builder:)
-      @builder = builder
-
+    def MainDialog(init_tab)
       caption = _("Network Settings")
       widget_descr = {
         "tab" => CWMTab.CreateWidget(
@@ -588,8 +594,6 @@ def MainDialog(init_tab, builder:)
         break if input_done?(ret)
       end
 
-      @builder = nil
-
       ret
     end