Use more native methods of SmartProxy for validation

lib/smart_proxy_remote_execution_ssh.rb

@@ -6,83 +6,6 @@
 module Proxy::RemoteExecution
   module Ssh
     class << self
-      def validate!
-        unless private_key_file
-          raise "settings for `ssh_identity_key` not set"
-        end
-
-        unless File.exist?(private_key_file)
-          raise "Ssh public key file #{private_key_file} doesn't exist.\n"\
-            "You can generate one with `ssh-keygen -t rsa -b 4096 -f #{private_key_file} -N ''`"
-        end
-
-        unless File.exist?(public_key_file)
-          raise "Ssh public key file #{public_key_file} doesn't exist"
-        end
-
-        validate_mode!
-        validate_ssh_log_level!
-        validate_mqtt_settings!
-      end
-
-      def private_key_file
-        File.expand_path(Plugin.settings.ssh_identity_key_file)
-      end
-
-      def public_key_file
-        File.expand_path("#{private_key_file}.pub")
-      end
-
-      def validate_mode!
-        Plugin.settings.mode = Plugin.settings.mode.to_sym
-
-        unless Plugin::MODES.include? Plugin.settings.mode
-          raise "Mode has to be one of #{Plugin::MODES.join(', ')}, given #{Plugin.settings.mode}"
-        end
-
-        if Plugin.settings.async_ssh
-          Plugin.logger.warn('Option async_ssh is deprecated, use ssh-async mode instead.')
-
-          case Plugin.settings.mode
-          when :ssh
-            Plugin.logger.warn('Deprecated option async_ssh used together with ssh mode, switching mode to ssh-async.')
-            Plugin.settings.mode = :'ssh-async'
-          when :'async-ssh'
-            # This is a noop
-          else
-            Plugin.logger.warn('Deprecated option async_ssh used together with incompatible mode, ignoring.')
-          end
-        end
-      end
-
-      def validate_mqtt_settings!
-        return unless Plugin.settings.mode == :'pull-mqtt'
-
-        raise 'mqtt_broker has to be set when pull-mqtt mode is used' if Plugin.settings.mqtt_broker.nil?
-        raise 'mqtt_port has to be set when pull-mqtt mode is used' if Plugin.settings.mqtt_port.nil?
-      end
-
-      def validate_ssh_log_level!
-        wanted_level = Plugin.settings.ssh_log_level.to_s
-        levels = Plugin::SSH_LOG_LEVELS
-        unless levels.include? wanted_level
-          raise "Wrong value '#{Plugin.settings.ssh_log_level}' for ssh_log_level, must be one of #{levels.join(', ')}"
-        end
-
-        current = ::Proxy::SETTINGS.log_level.to_s.downcase
-
-        # regular log levels correspond to upcased ssh logger levels
-        ssh, regular = [wanted_level, current].map do |wanted|
-          levels.each_with_index.find { |value, _index| value == wanted }.last
-        end
-
-        if ssh < regular
-          raise 'ssh_log_level cannot be more verbose than regular log level'
-        end
-
-        Plugin.settings.ssh_log_level = Plugin.settings.ssh_log_level.to_sym
-      end
-
       def job_storage
         @job_storage ||= Proxy::RemoteExecution::Ssh::JobStorage.new
       end

lib/smart_proxy_remote_execution_ssh/api.rb

@@ -10,7 +10,9 @@ class Api < ::Sinatra::Base
       include Proxy::Dynflow::Helpers
 
       get "/pubkey" do
-        File.read(Ssh.public_key_file)
+        File.read(Proxy::RemoteExecution::Plugin.settings.ssh_identity_public_key_file)
+      rescue Errno::ENOENT
+        halt 404
       end
 
       post "/session" do

lib/smart_proxy_remote_execution_ssh/cockpit.rb

@@ -242,7 +242,7 @@ def params
       end
 
       def key_file
-        @key_file ||= Proxy::RemoteExecution::Ssh.private_key_file
+        @key_file ||= Proxy::RemoteExecution::Plugin.settings.ssh_identity_key_file
       end
 
       def buf_socket

lib/smart_proxy_remote_execution_ssh/plugin.rb

@@ -1,10 +1,11 @@
+require_relative 'validators'
+
 module Proxy::RemoteExecution::Ssh
   class Plugin < Proxy::Plugin
     SSH_LOG_LEVELS = %w[debug info error fatal].freeze
     MODES = %i[ssh async-ssh pull pull-mqtt].freeze
 
-    http_rackup_path File.expand_path("http_config.ru", File.expand_path("../", __FILE__))
-    https_rackup_path File.expand_path("http_config.ru", File.expand_path("../", __FILE__))
+    rackup_path File.expand_path("http_config.ru", __dir__)
 
     settings_file "remote_execution_ssh.yml"
     default_settings :ssh_identity_key_file   => '~/.ssh/id_rsa_foreman_proxy',
@@ -20,9 +21,51 @@ class Plugin < Proxy::Plugin
                      # :mqtt_port               => nil,
                      :mode                    => :ssh
 
+    load_validators ssh_log_level: Proxy::RemoteExecution::Ssh::Validators::SshLogLevel,
+                    rex_ssh_mode: Proxy::RemoteExecution::Ssh::Validators::RexSshMode
+
+    load_programmable_settings do |settings|
+      if settings[:ssh_identity_key_file]
+        settings[:ssh_identity_key_file] = File.expand_path(settings[:ssh_identity_key_file])
+      end
+
+      if settings[:ssh_identity_public_key_file]
+        settings[:ssh_identity_public_key_file] = File.expand_path(settings[:ssh_identity_public_key_file])
+      elsif settings[:ssh_identity_key_file]
+        settings[:ssh_identity_public_key_file] = "#{settings[:ssh_identity_key_file]}.pub"
+      end
+
+      if settings[:async_ssh]
+        Plugin.logger.warn('Option async_ssh is deprecated, use ssh-async mode instead.')
+
+        case setting_value
+        when :ssh
+          Plugin.logger.warn('Deprecated option async_ssh used together with ssh mode, switching mode to ssh-async.')
+          settings[:mode] = :'ssh-async'
+        when :'async-ssh'
+          # This is a noop
+        else
+          Plugin.logger.warn('Deprecated option async_ssh used together with incompatible mode, ignoring.')
+        end
+      end
+
+      settings[:mode] = settings[:mode].to_sym
+      settings[:ssh_log_level] = settings[:ssh_log_level].to_sym
+    end
+
+    validate_readable :ssh_identity_key_file, :ssh_identity_public_key_file
+    validate :ssh_log_level, ssh_log_level: SSH_LOG_LEVELS
+    validate :mode, rex_ssh_mode: MODES
+    validate_presence :mqtt_broker, :mqtt_port, if: ->(settings) { settings[:mode] == :'pull-mqtt' }
+
     plugin :ssh, Proxy::RemoteExecution::Ssh::VERSION
-    after_activation do
+
+    requires :dynflow, '~> 0.5'
+
+    load_classes do
       require 'smart_proxy_dynflow'
+      require 'smart_proxy_dynflow/task_launcher'
+      require 'smart_proxy_dynflow/runner'
       require 'smart_proxy_remote_execution_ssh/version'
       require 'smart_proxy_remote_execution_ssh/cockpit'
       require 'smart_proxy_remote_execution_ssh/api'
@@ -32,9 +75,10 @@ class Plugin < Proxy::Plugin
       require 'smart_proxy_remote_execution_ssh/runners'
       require 'smart_proxy_remote_execution_ssh/utils'
       require 'smart_proxy_remote_execution_ssh/job_storage'
+    end
 
-      Proxy::RemoteExecution::Ssh.validate!
-
+    # Not really Smart Proxy dependency injection, but similar enough
+    load_dependency_injection_wirings do |container_instance, settings|
       Proxy::Dynflow::TaskLauncherRegistry.register('ssh', Proxy::Dynflow::TaskLauncher::Batch)
     end
 

lib/smart_proxy_remote_execution_ssh/validators.rb

@@ -0,0 +1,42 @@
+module Proxy
+  module RemoteExecution
+    module Ssh
+      module Validators
+        class SshLogLevel < ::Proxy::PluginValidators::Base
+          def validate!(settings)
+            setting_value = settings[@setting_name].to_s
+
+            unless @params.include?(setting_value)
+              raise ::Proxy::Error::ConfigurationError, "Parameter '#{@setting_name}' must be one of #{@params.join(', ')}"
+            end
+
+            current = ::Proxy::SETTINGS.log_level.to_s.downcase
+
+            # regular log levels correspond to upcased ssh logger levels
+            ssh, regular = [setting_value, current].map do |wanted|
+              @params.each_with_index.find { |value, _index| value == wanted }.last
+            end
+
+            if ssh < regular
+              raise ::Proxy::Error::ConfigurationError, "Parameter '#{@setting_name}' cannot be more verbose than regular log level (#{current})"
+            end
+
+            true
+          end
+        end
+
+        class RexSshMode < ::Proxy::PluginValidators::Base
+          def validate!(settings)
+            setting_value = settings[@setting_name]
+
+            unless @params.include?(setting_value)
+              raise ::Proxy::Error::ConfigurationError, "Parameter '#{@setting_name}' must be one of #{@params.join(', ')}"
+            end
+
+            true
+          end
+        end
+      end
+    end
+  end
+end

test/api_test.rb

@@ -25,11 +25,17 @@ module Proxy::RemoteExecution::Ssh
   class ApiTest < MiniTest::Spec
     include Rack::Test::Methods
 
+    def setup
+      super
+      Proxy::RemoteExecution::Ssh::Plugin.load_test_settings(ssh_identity_key_file: FAKE_PRIVATE_KEY_FILE, ssh_identity_public_key_file: FAKE_PUBLIC_KEY_FILE)
+    end
+
     let(:app) { Proxy::RemoteExecution::Ssh::Api.new }
 
     describe '/pubkey' do
       it 'returns the content of the public key' do
         get '/pubkey'
+        _(last_response.status).must_equal 200, last_response.body
         _(last_response.body).must_equal '===public-key==='
       end
     end

test/integration_test.rb

@@ -0,0 +1,104 @@
+require 'test_helper'
+require 'json'
+require 'root/root'
+require 'root/root_v2_api'
+require 'smart_proxy_remote_execution_ssh/plugin'
+
+class SmartProxyRemoteExecutionSshApiFeaturesTest < MiniTest::Test
+  include Rack::Test::Methods
+
+  def app
+    Proxy::PluginInitializer.new(Proxy::Plugins.instance).initialize_plugins
+    Proxy::RootV2Api.new
+  end
+
+  def test_features_for_default_mode_without_dynflow
+    Proxy::LegacyModuleLoader.any_instance.expects(:load_configuration_file).with('dynflow.yml').returns(enabled: false)
+    Proxy::DefaultModuleLoader.any_instance.expects(:load_configuration_file).with('remote_execution_ssh.yml').returns(
+      enabled: true,
+      ssh_identity_key_file: FAKE_PRIVATE_KEY_FILE,
+      mqtt_broker: 'broker.example.com',
+      mqtt_port: 1883,
+    )
+
+    get '/features'
+
+    response = JSON.parse(last_response.body)
+
+    mod = response['ssh']
+    refute_nil(mod)
+    assert_equal('failed', mod['state'], Proxy::LogBuffer::Buffer.instance.info[:failed_modules][:ssh])
+    assert_equal("Disabling all modules in the group ['ssh'] due to a failure in one of them: 'dynflow' required by 'ssh' could not be found.",
+                 Proxy::LogBuffer::Buffer.instance.info[:failed_modules][:ssh])
+  end
+
+  def test_features_for_default_mode_with_dynflow
+    Proxy::LegacyModuleLoader.any_instance.expects(:load_configuration_file).with('dynflow.yml').returns(enabled: true)
+    Proxy::DefaultModuleLoader.any_instance.expects(:load_configuration_file).with('remote_execution_ssh.yml').returns(
+      enabled: true,
+      ssh_identity_key_file: FAKE_PRIVATE_KEY_FILE,
+    )
+
+    get '/features'
+
+    response = JSON.parse(last_response.body)
+
+    mod = response['dynflow']
+    refute_nil(mod)
+    assert_equal('running', mod['state'], Proxy::LogBuffer::Buffer.instance.info[:failed_modules][:dynflow])
+
+    mod = response['ssh']
+    refute_nil(mod)
+    assert_equal('running', mod['state'], Proxy::LogBuffer::Buffer.instance.info[:failed_modules][:ssh])
+    assert_equal([], mod['capabilities'], 'Has no capabilities')
+    assert_equal({}, mod['settings'], 'Has no settings')
+  end
+
+  def test_features_for_pull_mqtt_mode_without_required_options
+    Proxy::LegacyModuleLoader.any_instance.expects(:load_configuration_file).with('dynflow.yml').returns(enabled: true)
+    Proxy::DefaultModuleLoader.any_instance.expects(:load_configuration_file).with('remote_execution_ssh.yml').returns(
+      enabled: true,
+      ssh_identity_key_file: FAKE_PRIVATE_KEY_FILE,
+      mode: 'pull-mqtt',
+    )
+
+    get '/features'
+
+    response = JSON.parse(last_response.body)
+
+    mod = response['dynflow']
+    refute_nil(mod)
+    assert_equal('running', mod['state'], Proxy::LogBuffer::Buffer.instance.info[:failed_modules][:dynflow])
+
+    mod = response['ssh']
+    refute_nil(mod)
+    assert_equal('failed', mod['state'], Proxy::LogBuffer::Buffer.instance.info[:failed_modules][:ssh])
+    assert_equal("Disabling all modules in the group ['ssh'] due to a failure in one of them: Parameter 'mqtt_broker' is expected to have a non-empty value",
+                 Proxy::LogBuffer::Buffer.instance.info[:failed_modules][:ssh])
+  end
+
+  def test_features_with_dynflow_and_required_options
+    Proxy::LegacyModuleLoader.any_instance.expects(:load_configuration_file).with('dynflow.yml').returns(enabled: true)
+    Proxy::DefaultModuleLoader.any_instance.expects(:load_configuration_file).with('remote_execution_ssh.yml').returns(
+      enabled: true,
+      ssh_identity_key_file: FAKE_PRIVATE_KEY_FILE,
+      mode: 'pull-mqtt',
+      mqtt_broker: 'broker.example.com',
+      mqtt_port: 1883,
+    )
+
+    get '/features'
+
+    response = JSON.parse(last_response.body)
+
+    mod = response['dynflow']
+    refute_nil(mod)
+    assert_equal('running', mod['state'], Proxy::LogBuffer::Buffer.instance.info[:failed_modules][:dynflow])
+
+    mod = response['ssh']
+    refute_nil(mod)
+    assert_equal('running', mod['state'], Proxy::LogBuffer::Buffer.instance.info[:failed_modules][:ssh])
+    assert_equal([], mod['capabilities'], 'Has no capabilities')
+    assert_equal({}, mod['settings'], 'Has no settings')
+  end
+end

test/test_helper.rb

@@ -21,9 +21,6 @@
 FileUtils.mkdir_p(logdir) unless File.exist?(logdir)
 
 def prepare_fake_keys
-  Proxy::RemoteExecution::Ssh::Plugin.settings.ssh_identity_key_file = FAKE_PRIVATE_KEY_FILE
-  # Workaround for Proxy::RemoteExecution::Ssh::Plugin.settings.ssh_identity_key_file returning nil
-  Proxy::RemoteExecution::Ssh::Plugin.settings.stubs(:ssh_identity_key_file).returns(FAKE_PRIVATE_KEY_FILE)
   FileUtils.mkdir_p(DATA_DIR) unless File.exist?(DATA_DIR)
   File.write(FAKE_PRIVATE_KEY_FILE, '===private-key===')
   File.write(FAKE_PUBLIC_KEY_FILE, '===public-key===')