From abc4dc76be96c651dd0fa1d7ff8d2aa7ffe0d977 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 24 Aug 2018 12:47:58 +0100 Subject: [PATCH] WIP --- src/include/tftp-server/dialogs.rb | 308 ++++++++++++----------------- src/modules/TftpServer.rb | 43 ++-- test/tftpserver_test.rb | 156 +++++++++++++++ 3 files changed, 309 insertions(+), 198 deletions(-) create mode 100644 test/tftpserver_test.rb diff --git a/src/include/tftp-server/dialogs.rb b/src/include/tftp-server/dialogs.rb index 70efad8..68d2f19 100644 --- a/src/include/tftp-server/dialogs.rb +++ b/src/include/tftp-server/dialogs.rb @@ -27,25 +27,9 @@ def initialize_tftp_server_dialogs(include_target) Yast.import "Wizard" end - # Widget to define state and start mode of the service - # - # @return [Yast2::ServiceWidget] - def service_widget - @service_widget ||= Yast2::ServiceWidget.new(TftpServer.service) - end - - # Firewall widget using CWM - # - # @return [Hash] see CWMFirewallInterfaces.CreateOpenFirewallWidget - def firewall_widget - @firewall_widget ||= CWMFirewallInterfaces.CreateOpenFirewallWidget( - "services" => ["tftp"], - "display_details" => true - ) - end - # Check for required packaged to be installed - # @return `abort if aborted and `next otherwise + # + # @return [Symbol] :abort if aborted and :next otherwise def Packages return :abort if !Package.InstallAll(TftpServer.required_packages) @@ -53,7 +37,8 @@ def Packages end # Read settings dialog - # @return `abort if aborted and `next otherwise + # + # @return [Symbol] :abort if aborted and :next otherwise def ReadDialog ret = true @@ -67,14 +52,16 @@ def ReadDialog end # Write settings dialog - # @return `abort if aborted and `next otherwise + # + # @return [Symbol] :abort if aborted and :next otherwise def WriteDialog ret = TftpServer.Write ret ? :next : :abort end # Main dialog - # @return dialog result + # + # @return [Symbol] dialog result (:next, :cancel, :abort) def MainDialog Wizard.SetScreenShotName("tftp-server-1-main") @@ -85,144 +72,148 @@ def MainDialog # Tftp-server dialog caption caption = _("TFTP Server Configuration") - # dialog help text - help_text = _("

Configuring a TFTP Server

") - # dialog help text - help_text = Ops.add( - help_text, - _( - "

Use this to enable a server for TFTP (trivial file transfer protocol). The server will be started using xinetd.

" - ) - ) - # enlighten newbies, #102946 - # dialog help text - help_text = Ops.add( - help_text, - _("

Note that TFTP and FTP are not the same.

") - ) - # dialog help text - help_text = Ops.add( - help_text, - _( - "

Boot Image Directory:\n" + - "Specify the directory where served files are located. The usual value is\n" + - "/tftpboot. The directory will be created if it does not exist. \n" + - "The server uses this as its root directory (using the -s option).

\n" - ) + Wizard.SetContentsButtons( + caption, + contents, + help, + Label.BackButton, + Label.OKButton ) - help_text = Ops.add(help_text, Ops.get_string(fw_cwm_widget, "help", "")) + Wizard.HideBackButton + Wizard.SetAbortButton(:abort, Label.CancelButton) + + # Initialize the widget (set the current value) + CWMFirewallInterfaces.OpenFirewallInit(firewall_widget, "") + + UI.ChangeWidget(Id(:viewlog), :Enabled, !Mode.config) + + result = handle_events + + Wizard.RestoreScreenShotName + result + end + + private - contents = HVSquash( + # Dialog contents + # + # @return [Yast::Term] + def contents + HVSquash( VBox( - # RadioButtonGroup( - # Id(:rbg), - # VBox( - # Left( - # RadioButton( - # Id(:tftpno), - # Opt(:notify), - # # Radio button label, disable TFTP server - # _("&Disable"), - # !start - # ) - # ), - # Left( - # RadioButton( - # Id(:tftpyes), - # Opt(:notify), - # # Radio button label, disable TFTP server - # _("&Enable"), - # start - # ) - # ) - # ) - # ), service_widget.content, VSpacing(1), - TextAndButton( - # Text entry label + HBox( # Directory where served files (usually boot images) reside - TextEntry(Id(:directory), _("&Boot Image Directory"), directory), - # push button label - # select a directory from the filesystem - PushButton(Id(:browse), _("Bro&wse...")) + Bottom(TextEntry(Id(:directory), _("&Boot Image Directory"), TftpServer.directory)), + HSpacing(0.5), + # Select a directory from the filesystem + Bottom(PushButton(Id(:browse), _("Bro&wse..."))) ), VSpacing(1), firewall_widget["custom_widget"] || Empty(), - # Ops.get_term(fw_cwm_widget, "custom_widget", Empty()), VSpacing(2), - # push button label - # display a log file + # Display a log file PushButton(Id(:viewlog), _("&View Log")) ) ) + end - Wizard.SetContentsButtons( - caption, - contents, - help_text, - Label.BackButton, - Label.OKButton - ) - Wizard.HideBackButton - Wizard.SetAbortButton(:abort, Label.CancelButton) - - # initialize the widget (set the current value) - CWMFirewallInterfaces.OpenFirewallInit(firewall_widget, "") - - UI.ChangeWidget(Id(:viewlog), :Enabled, !Mode.config) - + # Handles dialog events + # + # @return [Symbol] :next, :cancel, :abort + def handle_events input = nil loop do event = UI.WaitForEvent input = event["ID"] - # handle the events, enable/disable the button, show the popup if button clicked + # Handle the events, enable/disable the button, show the popup if button clicked CWMFirewallInterfaces.OpenFirewallHandle(firewall_widget, "", event) case input when :browse ask_directory - update_directory_widget when :viewlog show_log when :next if check_directory - # grab current settings, store them to SuSEFirewall:: + # Grab current settings, store them to SuSEFirewall:: CWMFirewallInterfaces.OpenFirewallStore(firewall_widget, "", event) save_service break end when :cancel, :abort - break if Popup.ReallyAbort(true) + break if Popup.ReallyAbort(changes?) end end - Wizard.RestoreScreenShotName input end + # Help text + # + # @return [String] + def help + _("

Configuring a TFTP Server

") + + _("

Use this to enable a server for TFTP (trivial file transfer protocol). The server will be started using xinetd.

") + + _("

Note that TFTP and FTP are not the same.

") + + _( + "

Boot Image Directory:\n" + + "Specify the directory where served files are located. The usual value is\n" + + "/tftpboot. The directory will be created if it does not exist. \n" + + "The server uses this as its root directory (using the -s option).

\n" + ) + + firewall_widget["help"] || "" + end + + # Widget to define state and start mode of the service + # + # @return [Yast2::ServiceWidget] + def service_widget + @service_widget ||= Yast2::ServiceWidget.new(TftpServer.service) + end + + # Firewall widget using CWM + # + # @return [Hash] see CWMFirewallInterfaces.CreateOpenFirewallWidget + def firewall_widget + @firewall_widget ||= CWMFirewallInterfaces.CreateOpenFirewallWidget( + "services" => ["tftp"], + "display_details" => true + ) + end + + # Value of the input field to indicate the Boot Image Directory + # + # @return [String] def directory - @directory ||= UI.QueryWidget(Id(:directory), :Value) + UI.QueryWidget(Id(:directory), :Value) end + # Opens a dialog to ask for the directory + # + # @note The input field is updated with the selected directory. def ask_directory search_path = directory.empty? ? "/" : directory - @directory = UI.AskForExistingDirectory(search_path, "") - end - - def update_directory_widget + directory = UI.AskForExistingDirectory(search_path, "") UI.ChangeWidget(Id(:directory), :Value, directory) end - def show_log - # show both service and socket logs since current boot - query = Y2Journal::Query.new(interval: "0", filters: { "unit" => ["tftp.service", "tftp.socket"] }) - Y2Journal::EntriesDialog.new(query: query).run + # Asks whether to create the directory (usefull when the directory does not exist) + # + # @return [Boolean] + def ask_create_directory + Popup.YesNo(Message.DirectoryDoesNotExistCreate(directory)) end + # Checks whether the given path is valid, and if so, it asks for creating the directory + # when it does not exist yet + # + # @return [Boolean] true when the given path is valid and exists (or should be created); + # false otherwise. def check_directory if !valid_directory? show_directory_error @@ -234,92 +225,49 @@ def check_directory end end + # Checks whether the given directory path is valid + # + # @return [Boolean] def valid_directory? directory.start_with?("/") && !directory.match?(/[ \t]/) end + # Checks whether the given directory path already exists + # + # @return [Boolean] + def exist_directory? + return true if Mode.config + + SCR.Read(path(".target.size"), directory) >= 0 + end + + # Opens a popup to indicate the error when the given directory path is not valid def show_directory_error message = _("The directory must start with a slash (/)\nand must not contain spaces.") Popup.Error(message) end - def exist_directory? - return true if Mode.config - - SCR.Read(path(".target.size"), directory) >= 0 + # Shows both service and socket logs since current boot + def show_log + query = Y2Journal::Query.new(interval: "0", filters: { "unit" => ["tftp.service", "tftp.socket"] }) + Y2Journal::EntriesDialog.new(query: query).run end - def ask_create_directory - Popup.YesNo(Message.DirectoryDoesNotExistCreate(directory)) + # Whether something has been edited + # + # @note Changes in the Service Widget are not taken into account. + # + # @return [Boolean] + def changes? + CWMFirewallInterfaces.OpenFirewallModified("") || directory != TftpServer.directory end - def save - # TftpServer.start = start - TftpServer.directory = directory if start - end - # # start = UI.QueryWidget(Id(:rbg), :CurrentButton) == :tftpyes - # directory = Convert.to_string(UI.QueryWidget(Id(:directory), :Value)) - - # # discard the difference in disabled fields: - # # directory is only considered if start is on - # changed = CWMFirewallInterfaces.OpenFirewallModified("") || - # start != TftpServer.start || # "" because method doesn't use parameter at all, nice :( - # start && directory != TftpServer.directory - - # if ret == :browse - # directory = UI.AskForExistingDirectory( - # directory != "" ? directory : "/", - # "" - # ) - # UI.ChangeWidget(Id(:directory), :Value, directory) if directory != nil - # elsif ret == :viewlog - # # show both service and socket logs for current boot - # query = Y2Journal::Query.new(interval: "0", filters: { "unit" => ["tftp.service", "tftp.socket"] }) - # Y2Journal::EntriesDialog.new(query: query).run - # end - - # # validity checks - # if ret == :next && start - # if CheckDirectorySyntax(directory) - # #ok, say that it will be created - # if !Mode.config && - # Ops.less_than(SCR.Read(path(".target.size"), directory), 0) - # # the dir does not exist - # ret = Popup.YesNo(Message.DirectoryDoesNotExistCreate(directory)) ? ret : nil - # end - # else - # UI.SetFocus(Id(:directory)) - # # error popup - # Popup.Error( - # _( - # "The directory must start with a slash (/)\nand must not contain spaces." - # ) - # ) - # ret = nil - # end - # end - # end - - # end until ret == :next || - # (ret == :back || ret == :abort) && (!changed || Popup.ReallyAbort(true)) - - # if ret == :next - # # grab current settings, store them to SuSEFirewall:: - # CWMFirewallInterfaces.OpenFirewallStore(fw_cwm_widget, "", event) - - # TftpServer.start = start - # TftpServer.directory = directory if start - # end - - # Wizard.RestoreScreenShotName - # Convert.to_symbol(ret) - # end - - def TextAndButton(text, button) - text = deep_copy(text) - button = deep_copy(button) - HBox(Bottom(text), HSpacing(0.5), Bottom(button)) + # Saves the service changes + def save_service + service_widget.store + TftpServer.start = TftpServer.service.active? + TftpServer.directory = directory end end end diff --git a/src/modules/TftpServer.rb b/src/modules/TftpServer.rb index d0472e7..92068bb 100644 --- a/src/modules/TftpServer.rb +++ b/src/modules/TftpServer.rb @@ -22,11 +22,16 @@ module Yast class TftpServerClass < Module + include Yast::Logger SOCKET_NAME = "tftp" PACKAGE_NAME = "tftp" - include Yast::Logger + # @!method start + # Whether the socket should be enabled and started + # + # @return [Boolean] + attr_reader :start def main textdomain "tftp-server" @@ -35,6 +40,7 @@ def main Yast.import "Progress" Yast.import "Report" Yast.import "Summary" + Yast.import "Mode" # Any settings modified? # As we have only a single dialog which handles it by itself, @@ -164,13 +170,15 @@ def WriteOnly # and then switch to user which is used for tftp service SCR.Execute(path(".target.bash_output"), "/usr/bin/chown #{@sysconfig.user}: #{Shellwords.escape(@directory)}") - # enable and (re)start systemd socket - if @start - socket.enable - socket.start - else - socket.disable - socket.stop + if Mode.auto || Mode.commandline + # enable and (re)start systemd socket + if start + socket.enable + socket.start + else + socket.disable + socket.stop + end end # TODO only when we have our own Progress @@ -186,9 +194,13 @@ def WriteOnly def Write return false if !WriteOnly() - # in.tftpd will linger around for 15 minutes waiting for a new connection - # so we must kill it otherwise it will be using the old parameters - Yast2::Systemd::Service.find!("tftp").stop + if Mode.auto || Mode.commandline + # in.tftpd will linger around for 15 minutes waiting for a new connection + # so we must kill it otherwise it will be using the old parameters + Yast2::Systemd::Service.find!("tftp").stop + else + service.save + end # TODO only when we have our own Progress #boolean progress_orig = Progress::set (false); @@ -209,7 +221,6 @@ def Set(settings) nil end - # Get all tftp-server settings from the first parameter # (For use by autoinstallation.) # @param [Hash] settings The YCP structure to be imported. @@ -241,7 +252,6 @@ def Export deep_copy(settings) end - # Mergeing config to existing system configuration. It is useful for delayed write. # So if package will be installed later this method re-apply changes on top of newly parsed # file. @@ -268,12 +278,9 @@ def Summary summary end - # Return needed packages and packages to be removed - # during autoinstallation. - # @return [Hash] of lists. + # Return needed packages and packages to be removed during autoinstallation # - # - + # @return [Hash] of lists def AutoPackages install_pkgs = deep_copy(@required_packages) remove_pkgs = [] diff --git a/test/tftpserver_test.rb b/test/tftpserver_test.rb new file mode 100644 index 0000000..2ae2157 --- /dev/null +++ b/test/tftpserver_test.rb @@ -0,0 +1,156 @@ +#!/usr/bin/env rspec +# encoding: utf-8 + +# Copyright (c) [2018] 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 "TftpServer" + +describe "Yast::TftpServer" do + describe "#Write" do + subject(:tftp_server) { Yast::TftpServerClass.new } + + before do + allow(Yast2::SystemService).to receive(:find).with("tftp").and_return(service) + + allow(Yast2::Systemd::Service).to receive(:find!).with("tftp").and_return(systemd_service) + + allow(Yast2::Systemd::Socket).to receive(:find!).with("tftp").and_return(socket) + + allow(Y2Firewall::Firewalld).to receive(:instance).and_return(firewalld) + + allow(CFA::TftpSysconfig).to receive(:new).and_return(sysconfig) + + allow(Yast::SCR).to receive(:Execute) + + allow(Yast::Mode).to receive(:auto) { auto } + allow(Yast::Mode).to receive(:commandline) { commandline } + + tftp_server.main + end + + let(:service) { instance_double(Yast2::SystemService, save: true) } + + let(:systemd_service) { instance_double(Yast2::Systemd::Service, stop: true) } + + let(:socket) { instance_double(Yast2::Systemd::Socket, enable: true, disable: true, start: true, stop: true) } + + let(:sysconfig) do + instance_double( + CFA::TftpSysconfig, + directory: "/path/to/boot_image_directory", + :directory= => nil, + save: true, + user: nil + ) + end + + let(:firewalld) { instance_double(Y2Firewall::Firewalld, write_only: true, reload: true) } + + let(:auto) { false } + let(:commandline) { false } + + shared_examples "old behavior" do + it "does not save the system service" do + expect(service).to_not receive(:save) + + tftp_server.Write + end + + it "stops the systemd service" do + expect(systemd_service).to receive(:stop) + + tftp_server.Write + end + + context "when the socket should not be started" do + before do + allow(tftp_server).to receive(:start).and_return(false) + end + + it "disables the socket" do + expect(socket).to receive(:disable) + + tftp_server.Write + end + + it "stops the socket" do + expect(socket).to receive(:stop) + + tftp_server.Write + end + end + + context "when the socket should be started" do + before do + allow(tftp_server).to receive(:start).and_return(true) + end + + it "enables the socket" do + expect(socket).to receive(:enable) + + tftp_server.Write + end + + it "starts the socket" do + expect(socket).to receive(:start) + + tftp_server.Write + end + end + end + + context "when running in command line" do + let(:commandline) { true } + + include_examples "old behavior" + end + + context "when running in AutoYaST mode" do + let(:auto) { true } + + include_examples "old behavior" + end + + context "when running in normal mode" do + it "does not stop the systemd service directly" do + expect(systemd_service).to_not receive(:stop) + + tftp_server.Write + end + + it "does not modify the systemd socket directly" do + expect(socket).to_not receive(:enable) + expect(socket).to_not receive(:disable) + expect(socket).to_not receive(:start) + expect(socket).to_not receive(:stop) + + tftp_server.Write + end + + it "saves the system service" do + expect(service).to receive(:save) + + tftp_server.Write + end + end + end +end