From 75ef1b6745f7546015d8a434a1e8d84c93855158 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Tue, 3 May 2016 23:53:27 -0500 Subject: [PATCH 01/55] Re-insert extractions from ManageIQ/manageiq-gems-pending *** Original Commit message shown below *** Author: Oleg Barenboim chessbyte@gmail.com Tue May 3 23:53:27 2016 -0500 Committer: Oleg Barenboim chessbyte@gmail.com Tue May 3 23:53:27 2016 -0500 Merge pull request #8404 from Ladas/fix_event_monitor_example_script Fix event_monitor_example.rb script (transferred from ManageIQ/manageiq-gems-pending@c0e7a765396f874997204aac52b38536996b95bb) --- .../mount/miq_generic_mount_session_spec.rb | 27 + .../util/mount/miq_generic_mount_session.rb | 475 ++++++++++++++++++ .../util/mount/miq_glusterfs_session.rb | 37 ++ .../pending/util/mount/miq_nfs_session.rb | 37 ++ .../pending/util/mount/miq_smb_session.rb | 53 ++ 5 files changed, 629 insertions(+) create mode 100644 lib/gems/pending/spec/util/mount/miq_generic_mount_session_spec.rb create mode 100644 lib/gems/pending/util/mount/miq_generic_mount_session.rb create mode 100644 lib/gems/pending/util/mount/miq_glusterfs_session.rb create mode 100644 lib/gems/pending/util/mount/miq_nfs_session.rb create mode 100644 lib/gems/pending/util/mount/miq_smb_session.rb diff --git a/lib/gems/pending/spec/util/mount/miq_generic_mount_session_spec.rb b/lib/gems/pending/spec/util/mount/miq_generic_mount_session_spec.rb new file mode 100644 index 00000000000..773bfa9de6c --- /dev/null +++ b/lib/gems/pending/spec/util/mount/miq_generic_mount_session_spec.rb @@ -0,0 +1,27 @@ +require "util/mount/miq_generic_mount_session" + +describe MiqGenericMountSession do + it "#connect returns a string pointing to the mount point" do + allow(described_class).to receive(:raw_disconnect) + s = described_class.new(:uri => '/tmp/abc') + s.logger = Logger.new("/dev/null") + + result = s.connect + expect(result).to be_kind_of(String) + expect(result).to_not be_blank + + s.disconnect + end + + it "#mount_share is unique" do + expect(described_class.new(:uri => '/tmp/abc').mount_share).to_not eq(described_class.new(:uri => '/tmp/abc').mount_share) + end + + it ".runcmd will retry with sudo if needed" do + cmd = "abc" + expect(described_class).to receive(:`).once.with("#{cmd} 2>&1").and_return("mount: only root can do that\n") + expect(described_class).to receive(:`).with("sudo #{cmd} 2>&1").and_return("works with sudo") + + described_class.runcmd(cmd) + end +end diff --git a/lib/gems/pending/util/mount/miq_generic_mount_session.rb b/lib/gems/pending/util/mount/miq_generic_mount_session.rb new file mode 100644 index 00000000000..f707ae7f3ae --- /dev/null +++ b/lib/gems/pending/util/mount/miq_generic_mount_session.rb @@ -0,0 +1,475 @@ +require 'active_support/core_ext/object/blank' +require 'fileutils' +require 'logger' +require 'sys-uname' +require 'uri' + +require 'util/miq-exception' +require 'util/miq-uuid' + +class MiqGenericMountSession + require 'util/mount/miq_nfs_session' + require 'util/mount/miq_smb_session' + require 'util/mount/miq_glusterfs_session' + + attr_accessor :settings, :mnt_point, :logger + + def initialize(log_settings) + raise "URI missing" unless log_settings.key?(:uri) + @settings = log_settings.dup + @mnt_point = nil + end + + def logger + @logger ||= $log.nil? ? :: Logger.new(STDOUT) : $log + end + + def runcmd(cmd_str) + self.class.runcmd(cmd_str) + end + + def self.runcmd(cmd_str) + rv = `#{cmd_str} 2>&1` + + # If sudo is required, ensure you have /etc/sudoers.d/miq + # Cmnd_Alias MOUNTALL = /bin/mount, /bin/umount + # %wheel ALL = NOPASSWD: MOUNTALL + rv = `sudo #{cmd_str} 2>&1` if rv.include?("mount: only root can do that") + + if $? != 0 + raise rv + end + end + + def self.in_depot_session(opts, &_block) + raise "No block provided!" unless block_given? + session = new_session(opts) + yield session + ensure + session.disconnect if session + end + + def self.new_session(opts) + klass = uri_scheme_to_class(opts[:uri]) + session = klass.new(:uri => opts[:uri], :username => opts[:username], :password => opts[:password]) + session.connect + session + end + + def self.uri_scheme_to_class(uri) + require 'uri' + scheme, userinfo, host, port, registry, share, opaque, query, fragment = URI.split(URI.encode(uri)) + case scheme + when 'smb' + MiqSmbSession + when 'nfs' + MiqNfsSession + when 'glusterfs' + MiqGlusterfsSession + else + raise "unsupported scheme #{scheme} from uri: #{uri}" + end + end + + def mount_share + require 'tmpdir' + @mnt_point = settings_mount_point || Dir.mktmpdir("miq_") + end + + def get_ping_depot_options + @@ping_depot_options ||= begin + opts = ::VMDB::Config.new("vmdb").config[:log][:collection] if defined?(::VMDB) && defined?(::VMDB::CONFIG) + opts = {:ping_depot => false} + opts + end + end + + def ping_timeout + get_ping_depot_options + @@ping_timeout ||= (@@ping_depot_options[:ping_depot_timeout] || 20) + end + + def do_ping? + get_ping_depot_options + @@do_ping ||= @@ping_depot_options[:ping_depot] == true + end + + def pingable? + log_header = "MIQ(#{self.class.name}-pingable?)" + return true unless self.do_ping? + return true unless @settings[:ports].kind_of?(Array) + + res = false + require 'net/ping' + begin + # To prevent "no route to host" type issues, assume refused connection indicates the host is reachable + before = Net::Ping::TCP.econnrefused + Net::Ping::TCP.econnrefused = true + + @settings[:ports].each do |port| + logger.info("#{log_header} pinging: #{@host} on #{port} with timeout: #{ping_timeout}") + tcp1 = Net::Ping::TCP.new(@host, port, ping_timeout) + res = tcp1.ping + logger.info("#{log_header} pinging: #{@host} on #{port} with timeout: #{ping_timeout}...result: #{res}") + break if res == true + end + ensure + Net::Ping::TCP.econnrefused = before + end + + res == true + end + + def connect + log_header = "MIQ(#{self.class.name}-connect)" + + # Replace any encoded spaces back into spaces since the mount commands accepts quoted spaces + @mount_path = @mount_path.to_s.gsub('%20', ' ') + + # # Grab only the share part of a path such as: /temp/default_1/evm_1/current_default_1_evm_1_20091120_192429_20091120_225653.zip + # @mount_path = @mount_path.split("/")[0..1].join("/") + + begin + raise "Connect: Cannot communicate with: #{@host} - verify the URI host value and your DNS settings" unless self.pingable? + + mount_share + rescue MiqException::MiqLogFileMountPointMissing => err + logger.warn("#{log_header} Connecting to host: [#{@host}], share: [#{@mount_path}] encountered error: [#{err.class.name}] [#{err.message}]...retrying after disconnect") + disconnect + retry + rescue => err + if err.kind_of?(RuntimeError) && err.message =~ /No such file or directory/ + msg = "No such file or directory when connecting to host: [#{@host}] share: [#{@mount_path}]" + raise MiqException::MiqLogFileNoSuchFileOrDirectory, msg + end + msg = "Connecting to host: [#{@host}], share: [#{@mount_path}] encountered error: [#{err.class.name}] [#{err.message}]" + logger.error("#{log_header} #{msg}...#{err.backtrace.join("\n")}") + disconnect + raise + end + end + + def disconnect + self.class.disconnect(@mnt_point, logger) + @mnt_point = nil + end + + def self.disconnect(mnt_point, logger = $log) + return if mnt_point.nil? + log_header = "MIQ(#{self.class.name}-disconnect)" + logger.info("#{log_header} Disconnecting mount point: #{mnt_point}") if logger + begin + raw_disconnect(mnt_point) + rescue => err + # Ignore mount point not found/mounted messages + unless err.message =~ /not found|mounted/ + msg = "[#{err.class.name}] [#{err.message}], disconnecting mount point: #{@mnt_point}" + logger.error("#{log_header} #{msg}") if logger + raise + end + end + FileUtils.rmdir(mnt_point) if File.exist?(mnt_point) + + logger.info("#{log_header} Disconnecting mount point: #{mnt_point}...Complete") if logger + end + + def active? + !@mnt_point.nil? + end + + def reconnect! + disconnect + connect + end + + def with_test_file(&_block) + raise "requires a block" unless block_given? + file = '/tmp/miq_verify_test_file' + begin + `echo "testing" > #{file}` + yield file + ensure + FileUtils.rm(file, :force => true) + end + end + + def verify + log_header = "MIQ(#{self.class.name}-verify)" + logger.info("#{log_header} [#{@settings[:uri]}]...") + res = true + + begin + connect + relpath = File.join(@mnt_point, relative_to_mount(@settings[:uri])) + + test_path = 'miqverify/test' + to = File.join(test_path, 'test_file') + fq_file_path = File.join(relpath, to) + + current_test = "create nested directories" + logger.info("#{log_header} [#{@settings[:uri]}] Testing #{current_test}...") + FileUtils.mkdir_p(File.dirname(fq_file_path)) + logger.info("#{log_header} [#{@settings[:uri]}] Testing #{current_test}...complete") + + with_test_file do |from| + current_test = "copy file" + logger.info("#{log_header} [#{@settings[:uri]}] Testing #{current_test}...") + FileUtils.cp(from, fq_file_path) + logger.info("#{log_header} [#{@settings[:uri]}] Testing #{current_test}...complete") + end + + current_test = "delete file" + logger.info("#{log_header} [#{@settings[:uri]}] Testing #{current_test}...") + FileUtils.rm(fq_file_path, :force => true) + logger.info("#{log_header} [#{@settings[:uri]}] Testing #{current_test}...complete") + + current_test = "remove nested directories" + logger.info("#{log_header} [#{@settings[:uri]}] Testing #{current_test}...") + FileUtils.rmdir(File.dirname(fq_file_path)) + FileUtils.rmdir(File.dirname(File.dirname(fq_file_path))) + logger.info("#{log_header} [#{@settings[:uri]}] Testing #{current_test}...complete") + + rescue => err + logger.error("#{log_header} Verify [#{current_test}] failed with error [#{err.class.name}] [#{err}], [#{err.backtrace[0]}]") + res = false, err.to_s + else + res = true, "" + ensure + disconnect + end + logger.info("#{log_header} [#{@settings[:uri]}]...result: [#{res.first}]") + res + end + + def add(source, dest_uri) + log_header = "MIQ(#{self.class.name}-add)" + + logger.info("#{log_header} Source: [#{source}], Destination: [#{dest_uri}]...") + + begin + reconnect! + relpath = File.join(@mnt_point, relative_to_mount(dest_uri)) + if File.exist?(relpath) + logger.info("#{log_header} Skipping add since URI: [#{dest_uri}] already exists") + return dest_uri + end + + logger.info("#{log_header} Building relative path: [#{relpath}]...") + FileUtils.mkdir_p(File.dirname(relpath)) + logger.info("#{log_header} Building relative path: [#{relpath}]...complete") + + logger.info("#{log_header} Copying file [#{source}] to [#{relpath}]...") + FileUtils.cp(source, relpath) + logger.info("#{log_header} Copying file [#{source}] to [#{relpath}] complete") + rescue => err + msg = "Adding [#{source}] to [#{dest_uri}], failed due to error: '#{err.message}'" + logger.error("#{log_header} #{msg}") + raise + ensure + disconnect + end + + logger.info("#{log_header} File URI added: [#{dest_uri}] complete") + dest_uri + end + + alias_method :upload, :add + + def download(local_file, remote_file) + log_header = "MIQ(#{self.class.name}-download)" + + logger.info("#{log_header} Target: [#{local_file}], Remote file: [#{remote_file}]...") + + begin + reconnect! + relpath = File.join(@mnt_point, relative_to_mount(remote_file)) + unless File.exist?(relpath) + logger.warn("#{log_header} Remote file: [#{remote_file}] does not exist!") + return + end + + logger.info("#{log_header} Copying file [#{relpath}] to [#{local_file}]...") + FileUtils.cp(relpath, local_file) + logger.info("#{log_header} Copying file [#{relpath}] to [#{local_file}] complete") + rescue => err + msg = "Downloading [#{remote_file}] to [#{local_file}], failed due to error: '#{err.message}'" + logger.error("#{log_header} #{msg}") + raise + ensure + disconnect + end + + logger.info("#{log_header} Download File: [#{remote_file}] complete") + local_file + end + + def log_uri_still_configured?(log_uri) + # Only remove the log file if the current depot @settings are based on the same base URI as the log_uri to be removed + return false if log_uri.nil? || @settings[:uri].nil? + + scheme, userinfo, host, port, registry, share, opaque, query, fragment = URI.split(URI.encode(@settings[:uri])) + scheme_log, userinfo_log, host_log, port_log, registry_log, share_log, opaque_log, query_log, fragment_log = URI.split(URI.encode(log_uri)) + + return false if scheme != scheme_log + return false if host != host_log + + # Since the depot URI is a base URI, remove all the directories in the log_uri from the base URI and check for empty? + return false unless (share.split("/") - share_log.split("/")).empty? + true + end + + def remove(log_uri) + log_header = "MIQ(#{self.class.name}-remove)" + + unless self.log_uri_still_configured?(log_uri) + logger.info("#{log_header} Skipping remove because log URI: [#{log_uri}] does not originate from the currently configured base URI: [#{@settings[:uri]}]") + return + end + + relpath = nil + begin + # Samba has issues mount directly in the directory of the file so mount on the parent directory + @settings.merge!(:uri => File.dirname(File.dirname(log_uri))) + reconnect! + + relpath = File.join(@mnt_point, relative_to_mount(log_uri)) + # path is now /temp/default_1/EVM_1/Archive_default_1_EVM_1_20091016_193633_20091016_204855.zip, trim the share and join with the mount point + # /mnt/miq_1258754934/default_1/EVM_1/Archive_default_1_EVM_1_20091016_193633_20091016_204855.zip + # relpath = File.join(@mnt_point, path.split('/')[2..-1] ) + + logger.info("#{log_header} URI: [#{log_uri}] using relative path: [#{relpath}] and mount path: [#{@mount_path}]...") + + unless File.exist?(relpath) + logger.info("#{log_header} Skipping since URI: [#{log_uri}] with relative path: [#{relpath}] does not exist") + return log_uri + end + + logger.info("#{log_header} Deleting [#{relpath}] on [#{log_uri}]...") + FileUtils.rm_rf(relpath) + logger.info("#{log_header} Deleting [#{relpath}] on [#{log_uri}]...complete") + rescue MiqException::MiqLogFileNoSuchFileOrDirectory => err + logger.warn("#{log_header} No such file or directory to delete: [#{log_uri}]") + rescue => err + msg = "Deleting [#{relpath}] on [#{log_uri}], failed due to err '#{err.message}'" + logger.error("#{log_header} #{msg}") + raise + ensure + disconnect + end + + logger.info("#{log_header} URI: [#{log_uri}]...complete") + log_uri + end + + def uri_to_local_path(remote_file) + File.join(@mnt_point, relative_to_mount(remote_file)) + end + + def local_path_to_uri(local_path) + relative_path = Pathname.new(local_path).relative_path_from(Pathname.new(@mnt_point)).to_s + File.join(@settings[:uri], relative_path) + end + + # + # These methods require an existing connection + # + + def glob(pattern) + with_mounted_exception_handling do + Dir.glob("#{mount_root}/#{pattern}").collect { |path| (path.split("/") - mount_root.split("/")).join("/") } + end + end + + def mkdir(path) + with_mounted_exception_handling do + FileUtils.mkdir_p("#{mount_root}/#{path}") + end + end + + def stat(file) + with_mounted_exception_handling do + File.stat("#{mount_root}/#{file}") + end + end + + def read(file) + with_mounted_exception_handling do + File.read("#{mount_root}/#{file}") + end + end + + def write(file, contents) + with_mounted_exception_handling do + mkdir(File.dirname(file)) + open(file, "w") { |fd| fd.write(contents) } + end + end + + def delete(file_or_directory) + with_mounted_exception_handling do + FileUtils.rm_rf("#{mount_root}/#{file_or_directory}") + end + end + + def open(*args, &block) + with_mounted_exception_handling do + args[0] = "#{mount_root}/#{args[0]}" + File.open(*args, &block) + end + end + + def file?(file) + with_mounted_exception_handling do + File.file?("#{mount_root}/#{file}") + end + end + + protected + + def mount_root + @mnt_point + end + + def relative_to_mount(uri) + log_header = "MIQ(#{self.class.name}-relative_to_mount)" + logger.info("#{log_header} mount point [#{@mount_path}], uri: [#{uri}]...") + scheme, userinfo, host, port, registry, path, opaque, query, fragment = URI.split(URI.encode(uri)) + + # Replace any encoded spaces back into spaces since the mount commands accepts quoted spaces + path.gsub!('%20', ' ') + + raise "path: #{path} or mount_path #{@mount_path} is blank" if path.nil? || @mount_path.nil? || path.empty? || @mount_path.empty? + res = (path.split("/") - @mount_path.split("/")).join("/") + logger.info("#{log_header} mount point [#{@mount_path}], uri: [#{uri}]...relative: [#{res}]") + res + end + + def with_mounted_exception_handling + yield + rescue => err + err.message.gsub!(@mnt_point, @settings[:uri]) + raise + end + + def self.raw_disconnect(mnt_point) + case Sys::Platform::IMPL + when :macosx + runcmd("sudo umount #{mnt_point}") + when :linux + runcmd("umount #{mnt_point}") + else + raise "platform not supported" + end + end + + private + + def settings_read_only? + @settings[:read_only] == true + end + + def settings_mount_point + return nil if @settings[:mount_point].blank? # Check if settings contains the mount_point to use + FileUtils.mkdir_p(@settings[:mount_point]).first + end +end diff --git a/lib/gems/pending/util/mount/miq_glusterfs_session.rb b/lib/gems/pending/util/mount/miq_glusterfs_session.rb new file mode 100644 index 00000000000..6c188253501 --- /dev/null +++ b/lib/gems/pending/util/mount/miq_glusterfs_session.rb @@ -0,0 +1,37 @@ +require 'util/mount/miq_generic_mount_session' + +class MiqGlusterfsSession < MiqGenericMountSession + PORTS = [2049, 111].freeze + + def initialize(log_settings) + super(log_settings.merge(:ports => PORTS)) + end + + def connect + _scheme, _userinfo, @host, _port, _registry, @mount_path, _opaque, _query, _fragment = + URI.split(URI.encode(@settings[:uri])) + super + end + + def mount_share + super + + log_header = "MIQ(#{self.class.name}-mount_share)" + logger.info( + "#{log_header} Connecting to host: [#{@host}], share: [#{@mount_path}] using mount point: [#{@mnt_point}]...") + + mount = "mount" + mount << " -r" if settings_read_only? + + # Quote the host:exported directory since the directory can have spaces in it + case Sys::Platform::IMPL + when :macosx + runcmd("sudo #{mount} -t glusterfs -o resvport '#{@host}:#{@mount_path}' #{@mnt_point}") + when :linux + runcmd("#{mount} -t glusterfs '#{@host}:#{@mount_path}' #{@mnt_point}") + else + raise "platform not supported" + end + logger.info("#{log_header} Connecting to host: [#{@host}], share: [#{@mount_path}]...Complete") + end +end diff --git a/lib/gems/pending/util/mount/miq_nfs_session.rb b/lib/gems/pending/util/mount/miq_nfs_session.rb new file mode 100644 index 00000000000..eb52a4c3f79 --- /dev/null +++ b/lib/gems/pending/util/mount/miq_nfs_session.rb @@ -0,0 +1,37 @@ +require 'util/mount/miq_generic_mount_session' + +class MiqNfsSession < MiqGenericMountSession + PORTS = [2049, 111] + + def initialize(log_settings) + super(log_settings.merge(:ports => PORTS)) + end + + def connect + scheme, userinfo, @host, port, registry, @mount_path, opaque, query, fragment = URI.split(URI.encode(@settings[:uri])) + super + end + + def mount_share + super + + log_header = "MIQ(#{self.class.name}-mount_share)" + logger.info("#{log_header} Connecting to host: [#{@host}], share: [#{@mount_path}] using mount point: [#{@mnt_point}]...") + # URI: nfs://192.168.252.139/exported/miq + # mount 192.168.252.139:/exported/miq /mnt/miq + + mount = "mount" + mount << " -r" if settings_read_only? + + # Quote the host:exported directory since the directory can have spaces in it + case Sys::Platform::IMPL + when :macosx + runcmd("sudo #{mount} -t nfs -o resvport '#{@host}:#{@mount_path}' #{@mnt_point}") + when :linux + runcmd("#{mount} '#{@host}:#{@mount_path}' #{@mnt_point}") + else + raise "platform not supported" + end + logger.info("#{log_header} Connecting to host: [#{@host}], share: [#{@mount_path}]...Complete") + end +end diff --git a/lib/gems/pending/util/mount/miq_smb_session.rb b/lib/gems/pending/util/mount/miq_smb_session.rb new file mode 100644 index 00000000000..e6c42124ca6 --- /dev/null +++ b/lib/gems/pending/util/mount/miq_smb_session.rb @@ -0,0 +1,53 @@ +require 'util/mount/miq_generic_mount_session' + +class MiqSmbSession < MiqGenericMountSession + PORTS = [445, 139] + + def initialize(log_settings) + super(log_settings.merge(:ports => PORTS)) + raise "username is a required value!" if @settings[:username].nil? + raise "password is a required value!" if @settings[:password].nil? + end + + def connect + scheme, userinfo, @host, port, registry, @mount_root, opaque, query, fragment = URI.split(URI.encode(@settings[:uri])) + @mount_path = @mount_root.split("/")[0..1].join("/") + super + end + + def mount_root + File.join(@mnt_point, (@mount_root.split("/") - @mount_path.split("/"))) + end + + def mount_share + super + + log_header = "MIQ(#{self.class.name}-mount_share)" + # Convert backslashes to slashes in case the username is in domain\username format + @settings[:username] = @settings[:username].tr('\\', '/') + + # To work around 2.6.18 kernel issue where a domain could be passed along incorrectly if not specified, explicitly provide both the username and domain (set the domain 'null' if not provided) + # https://bugzilla.samba.org/show_bug.cgi?id=4176 + split_username = @settings[:username].split('/') + case split_username.length + when 1 + # No domain provided + user = split_username.first + domain = 'null' + when 2 + domain, user = split_username + else + raise "Expected 'domain/username' or 'domain\\username' format, received: '#{@settings[:username]}'" + end + + mount = "mount" + mount << " -r" if settings_read_only? + + logger.info("#{log_header} Connecting to host: [#{@host}], share: [#{@mount_path}], domain: [#{domain}], user: [#{user}], using mount point: [#{@mnt_point}]...") + # mount -t cifs //192.168.252.140/temp /media/windows_share/ -o rw,username=jrafaniello,password=blah,domain=manageiq.com + + # Quote the hostname and share and username since they have spaces in it + runcmd("#{mount} -t cifs '//#{File.join(@host, @mount_path)}' #{@mnt_point} -o rw,username='#{user}',password='#{@settings[:password]}',domain='#{domain}'") + logger.info("#{log_header} Connecting to host: [#{@host}], share: [#{@mount_path}]...Complete") + end +end From d91a67c35081212a1402613423170d8aa8f51a74 Mon Sep 17 00:00:00 2001 From: Brandon Dunne Date: Tue, 18 Oct 2016 16:45:46 -0400 Subject: [PATCH 02/55] Move test directories to the root (transferred from ManageIQ/manageiq-gems-pending@2331b9385e645e42cad21a6359451d903a39bfe5) --- .../spec => spec}/util/mount/miq_generic_mount_session_spec.rb | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename {lib/gems/pending/spec => spec}/util/mount/miq_generic_mount_session_spec.rb (100%) diff --git a/lib/gems/pending/spec/util/mount/miq_generic_mount_session_spec.rb b/spec/util/mount/miq_generic_mount_session_spec.rb similarity index 100% rename from lib/gems/pending/spec/util/mount/miq_generic_mount_session_spec.rb rename to spec/util/mount/miq_generic_mount_session_spec.rb From 3e3550bab5ff175dbd403d7276a7f1368b050c24 Mon Sep 17 00:00:00 2001 From: Jerry Keselman Date: Tue, 26 Jun 2018 10:18:02 -0400 Subject: [PATCH 03/55] Support S3 for DB Backups Initial commit to support DB backups to S3. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1513520 (transferred from ManageIQ/manageiq-gems-pending@65ee18925b687af0289f9d3f588b4b05184577ce) --- lib/gems/pending/util/mount/miq_generic_mount_session.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/gems/pending/util/mount/miq_generic_mount_session.rb b/lib/gems/pending/util/mount/miq_generic_mount_session.rb index 4a20b06e174..0bb333e7553 100644 --- a/lib/gems/pending/util/mount/miq_generic_mount_session.rb +++ b/lib/gems/pending/util/mount/miq_generic_mount_session.rb @@ -9,6 +9,7 @@ class MiqGenericMountSession require 'util/mount/miq_nfs_session' + require 'util/mount/miq_s3_session' require 'util/mount/miq_smb_session' require 'util/mount/miq_glusterfs_session' @@ -53,7 +54,7 @@ def self.in_depot_session(opts, &_block) def self.new_session(opts) klass = uri_scheme_to_class(opts[:uri]) - session = klass.new(:uri => opts[:uri], :username => opts[:username], :password => opts[:password]) + session = klass.new(:uri => opts[:uri], :username => opts[:username], :password => opts[:password], :region => opts[:region]) session.connect session end @@ -62,6 +63,8 @@ def self.uri_scheme_to_class(uri) require 'uri' scheme, userinfo, host, port, registry, share, opaque, query, fragment = URI.split(URI.encode(uri)) case scheme + when 's3' + MiqS3Session when 'smb' MiqSmbSession when 'nfs' @@ -464,6 +467,10 @@ def self.raw_disconnect(mnt_point) end end + def copy_dump_to_store + # nothing to do + end + private def settings_read_only? From cba8356f90ae92c81c9a0ed44261eedcc9d23dc7 Mon Sep 17 00:00:00 2001 From: Jerry Keselman Date: Mon, 9 Jul 2018 15:25:57 -0400 Subject: [PATCH 04/55] Rework Step to Upload DB Dump to S3 Based on PR review comments, use the existing MountSession "add" operation to upload the database file to S3. Correctly handle the situation where the bucket name requested exists but access is denied, indicating (probably) that is owned by another user. (The S3 namespace is Global - each user does not get their own a namespace). (transferred from ManageIQ/manageiq-gems-pending@67fd6ee20ba91ffc6a35e12076c7bd65ed444542) --- lib/gems/pending/util/mount/miq_generic_mount_session.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gems/pending/util/mount/miq_generic_mount_session.rb b/lib/gems/pending/util/mount/miq_generic_mount_session.rb index 0bb333e7553..19f0b854c9d 100644 --- a/lib/gems/pending/util/mount/miq_generic_mount_session.rb +++ b/lib/gems/pending/util/mount/miq_generic_mount_session.rb @@ -246,7 +246,7 @@ def verify res end - def add(source, dest_uri) + def add(source, dest_uri, _object_file = nil) log_header = "MIQ(#{self.class.name}-add)" logger.info("#{log_header} Source: [#{source}], Destination: [#{dest_uri}]...") From 6075f796bd6e451fda406707a644f1a595f38057 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Luka=C5=A1=C3=ADk?= Date: Thu, 23 Feb 2017 13:18:00 +0100 Subject: [PATCH 05/55] If we run sudo for mount, we need to do the same for umount The comment already mentioned that. (transferred from ManageIQ/manageiq-gems-pending@99668909be271c2791dbd6dd669f8c7d7ae31316) --- lib/gems/pending/util/mount/miq_generic_mount_session.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/gems/pending/util/mount/miq_generic_mount_session.rb b/lib/gems/pending/util/mount/miq_generic_mount_session.rb index f707ae7f3ae..e899eb14689 100644 --- a/lib/gems/pending/util/mount/miq_generic_mount_session.rb +++ b/lib/gems/pending/util/mount/miq_generic_mount_session.rb @@ -34,7 +34,9 @@ def self.runcmd(cmd_str) # If sudo is required, ensure you have /etc/sudoers.d/miq # Cmnd_Alias MOUNTALL = /bin/mount, /bin/umount # %wheel ALL = NOPASSWD: MOUNTALL - rv = `sudo #{cmd_str} 2>&1` if rv.include?("mount: only root can do that") + if rv.include?("mount: only root can do that") || rv.include?('umount failed: Operation not permitted') + rv = `sudo #{cmd_str} 2>&1` + end if $? != 0 raise rv From bf6bc11f43ca2dd3675d6c1d76c16276b685f60c Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Fri, 3 Aug 2018 18:55:02 -0500 Subject: [PATCH 06/55] Add MiqFileStorage and MiqFileStorage::Interface `MiqFileStorage` is meant to be an top level class for fetching classes that interface with `MiqFileStorage::Interface`. `MiqFileStorage::Interface` is meant to be the super class for all of the class to inherit from, to make sure that they conform to a spec that `MiqFileStorage` can work with. (transferred from ManageIQ/manageiq-gems-pending@1c5c8c575cd8f13e25548d3ed44fc8675efb4094) --- lib/gems/pending/util/miq_file_storage.rb | 254 +++++++++++ .../util/mount/miq_generic_mount_session.rb | 9 +- .../util/mount/miq_glusterfs_session.rb | 4 + .../pending/util/mount/miq_nfs_session.rb | 4 + .../pending/util/mount/miq_smb_session.rb | 4 + spec/util/miq_file_storage_spec.rb | 399 ++++++++++++++++++ 6 files changed, 672 insertions(+), 2 deletions(-) create mode 100644 lib/gems/pending/util/miq_file_storage.rb create mode 100644 spec/util/miq_file_storage_spec.rb diff --git a/lib/gems/pending/util/miq_file_storage.rb b/lib/gems/pending/util/miq_file_storage.rb new file mode 100644 index 00000000000..4b5a7b4e8eb --- /dev/null +++ b/lib/gems/pending/util/miq_file_storage.rb @@ -0,0 +1,254 @@ +# This class is meant to be a abstract interface for defining a file_storage +# class. +# +# The storage class can either be of a type of "object storage", which includes: +# * protocols like FTP +# * document storage like s3 and OpenStack's Swift +# +# And mountable filesystems like: +# * NFS +# * SMB +# +# The class is meant to allow a shared interface for working with these +# different forms of file storage, while maintaining their differences in +# implementation where necessary. Connection will be handled separately by the +# subclasses, but they must conform to the top level interface. +# +class MiqFileStorage + class InvalidSchemeError < ArgumentError + def initialize(bad_scheme = nil) + super(error_message(bad_scheme)) + end + + def error_message(bad_scheme) + valid_schemes = ::MiqFileStorage.storage_interface_classes.keys.inspect + "#{bad_scheme} is not a valid MiqFileStorage uri scheme. Accepted schemes are #{valid_schemes}" + end + end + + def self.with_interface_class(opts) + klass = fetch_interface_class(opts) + block_given? ? yield(klass) : klass + end + + def self.fetch_interface_class(opts) + return nil unless opts[:uri] + + require 'uri' + scheme, _ = URI.split(URI::DEFAULT_PARSER.escape(opts[:uri])) + klass = storage_interface_classes[scheme] + + raise InvalidSchemeError, scheme if klass.nil? + + klass.new_with_opts(opts) + end + private_class_method :fetch_interface_class + + def self.storage_interface_classes + @storage_interface_classes ||= Interface.descendants.each_with_object({}) do |klass, memo| + memo[klass.uri_scheme] = klass if klass.uri_scheme + end + end + + class Interface + BYTE_HASH_MATCH = /^(?\d+(\.\d+)?)\s*(?K|M|G)?$/i + BYTE_HASH = { + "k" => 1.kilobyte, + "m" => 1.megabyte, + "g" => 1.gigabyte + }.freeze + + attr_reader :remote_file_path, :byte_count, :source_input, :input_writer + + def self.new_with_opts(opts) # rubocop:disable Lint/UnusedMethodArgument + raise NotImplementedError, "#{name}.new_with_opts is not defined" + end + + def self.uri_scheme + nil + end + + # :call-seq: + # add( remote_uri ) { |input_writer| ... } + # add( remote_uri, byte_count ) { |input_writer| ... } + # + # add( local_io, remote_uri ) + # add( local_io, remote_uri, byte_count ) + # + # Add a file to the destination URI. + # + # In the block form of the method, only the remote_uri is required, and it + # is assumed the input will be a generated in the executed block (most + # likely an external process) to a unix pipe that can be written to. The + # pipe generated by this method and passed in to the block as a file + # location to the `input_stream`). + # + # In the non-block form, a source must be provided as the first argument + # either as an IO object that can be read from, or a file path, and the + # second argument is the remote_uri as in the block form. + # + # An additional argument in both forms as the last argument is `byte_count` + # can also be included. If passed, it will be assumed that the resulting + # input will be split, and the naming for the splits will be: + # + # - filename.00001 + # - filename.00002 + # ... + # + # Block form: + # + # nfs_session.add("path/to/file", "200M") do |input_stream| + # `pg_dump -f #{input_stream} vmdb_production` + # end + # + # Non-block form: + # + # nfs_session.add("path/to/local_file", "path/to/remote_file") + # nfs_session.add("path/to/local_file", "path/to/remote_file", "200M") + # + def add(*upload_args, &block) + initialize_upload_vars(*upload_args) + mkdir(File.dirname(@remote_file_path)) + thread = handle_io_block(&block) + result = if byte_count + upload_splits + else + upload_single(@remote_file_path) + end + # `.join` will raise any errors from the thread, so we want to do that + # here (if a thread exists of course). + thread.join if thread + result + ensure + reset_vars + end + alias upload add + + def mkdir(dir) # rubocop:disable Lint/UnusedMethodArgument + raise NotImplementedError, "#{self.class}##{__callee__} is not defined" + end + + # :call-seq: + # download( local_io, remote_uri ) + # download( nil, remote_uri ) { |input_writer| ... } + # + # Download a file from a remote uri. + # + # In non-block form, the remote_uri is saved to the local_io. + # + # In block form, the local_io is omitted, and it is set to a PTY writer + # path that will assumed to be read by the block provided. + def download(local_file, remote_file_uri, &block) + @remote_file_path = remote_file_uri + if block_given? + thread = handle_io_block(&block) + download_single(remote_file_uri, input_writer) + input_writer.close + thread.join + else + download_single(remote_file_uri, local_file) + end + ensure + reset_vars + end + + private + + # NOTE: Needs to be overwritten in the subclass! + # + # Classes that inherit from `MiqFileStorage` need to make sure to create a + # method that overwrites this one to handle the specifics of uploading for + # their particular ObjectStore protocol or MountSession. + # + # `dest_uri` is the current file that will be uploaded. If file splitting + # is occurring, this will update the filename passed into `.add` to include + # a `.0000X` suffix, where the suffix is padded up to 5 digits in total. + # + # `#upload_single` doesn't need to worry about determining the file name + # itself for splitting, but if any relative path munging is necessary, that + # should be done here (see `MiqGenericMountSession#upload_single` for an + # example) + # + # `source_input` available as an attr_reader in this method, and will + # always be a local IO object that is available for reading. + # + # `byte_count` is also an attr_reader that is available, and will either be + # `nil` if no file splitting is occurring, or a integer representing the + # maximum number of bytes to uploaded for this particular `dest_uri`. + # + # + # Ideally, making use of `IO.copy_stream` will simplify this process + # significantly, as you can pass it `source_input`, `dest_uri`, and + # `byte_count` respectively, and it will automatically handle streaming the + # data from one IO object to the other. In mount based situations, where + # `dest_uri` is a file path (in `MiqGenericMountSession#upload_single`, + # this is converted to `relpath`), this does not need to be converted to a + # `File` IO object as `IO.copy_stream` will do that for you. + def upload_single(dest_uri) # rubocop:disable Lint/UnusedMethodArgument + raise NotImplementedError, "#{self.class}#upload_single is not defined" + end + + def upload_splits + @position = 0 + until source_input.eof? + upload_single(next_split_filename) + @position += byte_count + end + end + + def initialize_upload_vars(*upload_args) + upload_args.pop if (@byte_count = parse_byte_value(upload_args.last)) + @remote_file_path = upload_args.pop + + unless upload_args.empty? + source = upload_args.pop + @source_input = source.kind_of?(IO) ? source : File.open(source, "r") + end + end + + def parse_byte_value(bytes) + match = bytes.to_s.match(BYTE_HASH_MATCH) || return + + bytes = match[:BYTE_NUM].to_f + if match[:BYTE_QUALIFIER] + bytes *= BYTE_HASH[match[:BYTE_QUALIFIER].downcase] + end + bytes.to_i + end + + def handle_io_block + if block_given? + require "tmpdir" + + # create pathname, but don't create the file for it (next line) + fifo_path = Pathname.new(Dir::Tmpname.create("") {}) + File.mkfifo(fifo_path) + + # For #Reasons(TM), the reader must be opened first + @source_input = File.open(fifo_path.to_s, IO::RDONLY | IO::NONBLOCK) + @input_writer = File.open(fifo_path.to_s, IO::WRONLY | IO::NONBLOCK) + + Thread.new do + begin + yield fifo_path # send the path to the block to get executed + ensure + @input_writer.close # close the file so we know we hit EOF (for #add) + end + end + end + end + + def reset_vars + File.delete(@input_writer.path) if @input_writer + @position, @byte_count, @remote_file_path, @source_input, @input_writer = nil + end + + def next_split_filename + "#{remote_file_path}.#{'%05d' % (@position / byte_count + 1)}" + end + + def download_single(source, destination) # rubocop:disable Lint/UnusedMethodArgument + raise NotImplementedError, "#{self.class}#download_single is not defined" + end + end +end diff --git a/lib/gems/pending/util/mount/miq_generic_mount_session.rb b/lib/gems/pending/util/mount/miq_generic_mount_session.rb index bb36a3bb4fb..6151a754f7e 100644 --- a/lib/gems/pending/util/mount/miq_generic_mount_session.rb +++ b/lib/gems/pending/util/mount/miq_generic_mount_session.rb @@ -6,8 +6,9 @@ require 'util/miq-exception' require 'util/miq-uuid' +require 'util/miq_file_storage' -class MiqGenericMountSession +class MiqGenericMountSession < MiqFileStorage::Interface require 'util/mount/miq_nfs_session' require 'util/mount/miq_s3_session' require 'util/mount/miq_smb_session' @@ -54,11 +55,15 @@ def self.in_depot_session(opts, &_block) def self.new_session(opts) klass = uri_scheme_to_class(opts[:uri]) - session = klass.new(opts.slice(:uri, :username, :password, :region)) + session = klass.new_with_opts(opts) session.connect session end + def self.new_with_opts(opts) + new(opts.slice(:uri, :username, :password, :region)) + end + def self.uri_scheme_to_class(uri) require 'uri' scheme, userinfo, host, port, registry, share, opaque, query, fragment = URI.split(URI.encode(uri)) diff --git a/lib/gems/pending/util/mount/miq_glusterfs_session.rb b/lib/gems/pending/util/mount/miq_glusterfs_session.rb index 6c188253501..d550963a8cf 100644 --- a/lib/gems/pending/util/mount/miq_glusterfs_session.rb +++ b/lib/gems/pending/util/mount/miq_glusterfs_session.rb @@ -3,6 +3,10 @@ class MiqGlusterfsSession < MiqGenericMountSession PORTS = [2049, 111].freeze + def self.uri_scheme + "glusterfs".freeze + end + def initialize(log_settings) super(log_settings.merge(:ports => PORTS)) end diff --git a/lib/gems/pending/util/mount/miq_nfs_session.rb b/lib/gems/pending/util/mount/miq_nfs_session.rb index eb52a4c3f79..92036657536 100644 --- a/lib/gems/pending/util/mount/miq_nfs_session.rb +++ b/lib/gems/pending/util/mount/miq_nfs_session.rb @@ -3,6 +3,10 @@ class MiqNfsSession < MiqGenericMountSession PORTS = [2049, 111] + def self.uri_scheme + "nfs".freeze + end + def initialize(log_settings) super(log_settings.merge(:ports => PORTS)) end diff --git a/lib/gems/pending/util/mount/miq_smb_session.rb b/lib/gems/pending/util/mount/miq_smb_session.rb index e6c42124ca6..535c94adcd8 100644 --- a/lib/gems/pending/util/mount/miq_smb_session.rb +++ b/lib/gems/pending/util/mount/miq_smb_session.rb @@ -3,6 +3,10 @@ class MiqSmbSession < MiqGenericMountSession PORTS = [445, 139] + def self.uri_scheme + "smb".freeze + end + def initialize(log_settings) super(log_settings.merge(:ports => PORTS)) raise "username is a required value!" if @settings[:username].nil? diff --git a/spec/util/miq_file_storage_spec.rb b/spec/util/miq_file_storage_spec.rb new file mode 100644 index 00000000000..add48b824d9 --- /dev/null +++ b/spec/util/miq_file_storage_spec.rb @@ -0,0 +1,399 @@ +require "util/mount/miq_generic_mount_session" + +describe MiqFileStorage do + def opts_for_nfs + opts[:uri] = "nfs://example.com/share/path/to/file.txt" + end + + def opts_for_smb + opts[:uri] = "smb://example.com/share/path/to/file.txt" + opts[:username] = "user" + opts[:password] = "pass" + end + + def opts_for_glusterfs + opts[:uri] = "glusterfs://example.com/share/path/to/file.txt" + end + + def opts_for_fakefs + opts[:uri] = "foo://example.com/share/path/to/file.txt" + end + + describe ".with_interface_class" do + let(:opts) { {} } + + shared_examples ".with_interface_class implementation" do |class_name| + let(:klass) { Object.const_get(class_name) } + + it "instanciates as #{class_name}" do + interface_instance = described_class.with_interface_class(opts) + expect(interface_instance.class).to eq(klass) + end + + it "with a block, passes the instance, and returns the result" do + instance_double = double(class_name.to_s) + interface_block = ->(instance) { instance.add } + + expect(klass).to receive(:new).and_return(instance_double) + expect(instance_double).to receive(:add).and_return(:foo) + + expect(described_class.with_interface_class(opts, &interface_block)).to eq(:foo) + end + end + + context "with a nil uri" do + it "returns nil" do + expect(described_class.with_interface_class(opts)).to eq(nil) + end + end + + context "with an nfs:// uri" do + before { opts_for_nfs } + + include_examples ".with_interface_class implementation", "MiqNfsSession" + end + + context "with an smb:// uri" do + before { opts_for_smb } + + include_examples ".with_interface_class implementation", "MiqSmbSession" + end + + context "with an glusterfs:// uri" do + before { opts_for_glusterfs } + + include_examples ".with_interface_class implementation", "MiqGlusterfsSession" + end + + context "with an unknown uri scheme" do + before { opts_for_fakefs } + + it "raises an MiqFileStorage::InvalidSchemeError" do + valid_schemes = MiqFileStorage.storage_interface_classes.keys + error_class = MiqFileStorage::InvalidSchemeError + error_message = "foo is not a valid MiqFileStorage uri scheme. Accepted schemes are #{valid_schemes}" + + expect { described_class.with_interface_class(opts) }.to raise_error(error_class).with_message(error_message) + end + end + end + + ##### Interface Methods ##### + + describe MiqFileStorage::Interface do + shared_examples "an interface method" do |method_str, *args| + subject { method_str[0] == "#" ? described_class.new : described_class } + let(:method) { method_str[1..-1] } + + it "raises NotImplementedError" do + expected_error_message = "MiqFileStorage::Interface#{method_str} is not defined" + expect { subject.send(method, *args) }.to raise_error(NotImplementedError, expected_error_message) + end + end + + shared_examples "upload functionality" do |method| + let(:local_io) { IO.pipe.first } + let(:remote_file_path) { "baz/bar/foo" } + let(:byte_count) { 1234 } + let(:args) { [local_io, remote_file_path] } + + before do + subject.instance_variable_set(:@position, 0) + expect(subject).to receive(:initialize_upload_vars).with(*args).and_call_original + expect(subject).to receive(:handle_io_block).with(no_args) + expect(subject).to receive(:mkdir).with("baz/bar") + end + + it "resets all vars" do + subject.instance_variable_set(:@position, 10) + subject.instance_variable_set(:@byte_count, 10) + allow(subject).to receive(:upload_single) + allow(subject).to receive(:upload_splits) + + subject.send(method, *args) + + expect(subject.instance_variable_get(:@position)).to be nil + expect(subject.byte_count).to be nil + expect(subject.remote_file_path).to be nil + expect(subject.source_input).to be nil + expect(subject.input_writer).to be nil + end + + context "without a byte_count" do + it "calls #upload_single" do + expect(subject).to receive(:upload_single).with(remote_file_path).once + expect(subject).to receive(:upload_splits).never + subject.send(method, *args) + end + end + + context "with a byte_count" do + let(:args) { [local_io, remote_file_path, byte_count] } + + it "calls #upload_splits" do + expect(subject).to receive(:upload_splits).once + expect(subject).to receive(:upload_single).never + subject.send(method, *args) + end + end + end + + describe "#add" do + include_examples "upload functionality", :add + end + + describe "#upload" do + include_examples "upload functionality", :upload + end + + describe "#mkdir" do + it_behaves_like "an interface method", "#mkdir", "foo/bar/baz" + end + + describe "#upload_single" do + it_behaves_like "an interface method", "#upload_single", "path/to/file" + end + + describe "#download_single" do + it_behaves_like "an interface method", "#download_single", "nfs://1.2.3.4/foo", "foo" + end + + describe ".new_with_opts" do + it_behaves_like "an interface method", ".new_with_opts", {} + end + + describe ".uri_scheme" do + it "returns nil by default" do + expect(described_class.uri_scheme).to eq(nil) + end + end + + describe "#upload_splits" do + let(:file_name) { "path/to/file" } + + it "uploads multiple files of the byte count size" do + subject.instance_variable_set(:@position, 0) + subject.instance_variable_set(:@byte_count, 10) + subject.instance_variable_set(:@remote_file_path, file_name) + + source_input_stub = double('@source_input') + allow(subject).to receive(:source_input).and_return(source_input_stub) + allow(source_input_stub).to receive(:eof?).and_return(false, false, true) + + expect(subject).to receive(:upload_single).with("#{file_name}.00001") + expect(subject).to receive(:upload_single).with("#{file_name}.00002") + + subject.send(:upload_splits) + end + end + + describe "#initialize_upload_vars (private)" do + let(:local_io) { File.open(local_io_str) } + let(:local_io_str) { Tempfile.new.path } + let(:remote_path) { "/path/to/remote_file" } + let(:byte_count_int) { 1024 } + let(:byte_count_str) { "5M" } + let(:upload_args) { [] } + let(:pty_master) { double("pty_master") } + let(:pty_slave) { double("pty_slave") } + + before do + subject.send(:initialize_upload_vars, *upload_args) + end + after { FileUtils.rm_rf local_io_str } + + context "with byte_count passed" do + let(:upload_args) { [remote_path, byte_count_int] } + + it "assigns @byte_count to the parse value" do + expect(subject.byte_count).to eq(1024) + end + + it "assigns @remote_file_path" do + expect(subject.remote_file_path).to eq("/path/to/remote_file") + end + + it "assigns @source_input nil (set in #handle_io_block)" do + expect(subject.source_input).to eq(nil) + end + + it "assigns @input_writer nil (set in #handle_io_block)" do + expect(subject.input_writer).to eq(nil) + end + + context "with local_io as an IO object passed" do + let(:upload_args) { [local_io, remote_path, byte_count_str] } + + it "assigns @byte_count to the parse value" do + expect(subject.byte_count).to eq(5.megabytes) + end + + it "assigns @source_input to the passed value" do + expect(subject.source_input).to eq(local_io) + end + + it "@input_writer is nil" do + expect(subject.input_writer).to eq(nil) + end + end + + context "with local_io passed" do + let(:upload_args) { [local_io_str, remote_path, byte_count_str] } + + it "assigns @byte_count to the parse value" do + expect(subject.byte_count).to eq(5.megabytes) + end + + it "assigns @source_input to the passed value" do + expect(File.identical?(subject.source_input, local_io_str)).to be true + end + + it "@input_writer is nil" do + expect(subject.input_writer).to eq(nil) + end + end + end + + context "without byte_count passed" do + let(:upload_args) { [remote_path] } + + it "@byte_count is nil" do + expect(subject.byte_count).to eq(nil) + end + + it "assigns @remote_file_path" do + expect(subject.remote_file_path).to eq("/path/to/remote_file") + end + + it "assigns @source_input nil (set in #handle_io_block)" do + expect(subject.source_input).to eq(nil) + end + + it "assigns @input_writer nil (set in #handle_io_block)" do + expect(subject.input_writer).to eq(nil) + end + + context "with local_io passed" do + let(:upload_args) { [local_io, remote_path] } + + it "assigns @byte_count to the parse value" do + expect(subject.byte_count).to eq(nil) + end + + it "assigns @source_input to the passed value" do + expect(subject.source_input).to eq(local_io) + end + + it "@input_writer is nil" do + expect(subject.input_writer).to eq(nil) + end + end + + context "with local_io passed" do + let(:upload_args) { [local_io_str, remote_path] } + + it "assigns @byte_count to the parse value" do + expect(subject.byte_count).to eq(nil) + end + + it "assigns @source_input to the passed value" do + expect(File.identical?(subject.source_input, local_io_str)).to be true + end + + it "@input_writer is nil" do + expect(subject.input_writer).to eq(nil) + end + end + end + end + + describe "#parse_byte_value (private)" do + it "returns 2 for '2'" do + expect(subject.send(:parse_byte_value, "2")).to eq(2) + end + + it "returns 2048 for '2k'" do + expect(subject.send(:parse_byte_value, "2k")).to eq(2048) + end + + it "returns 1536 for '1.5K'" do + expect(subject.send(:parse_byte_value, "1.5K")).to eq(1536) + end + + it "returns 3145728 for '3M'" do + expect(subject.send(:parse_byte_value, "3M")).to eq(3.megabytes) + end + + it "returns 1073741824 for '1g'" do + expect(subject.send(:parse_byte_value, "1g")).to eq(1.gigabyte) + end + + it "returns nil for nil" do + expect(subject.send(:parse_byte_value, nil)).to eq(nil) + end + + it "returns 100 for 100 (integer)" do + expect(subject.send(:parse_byte_value, 100)).to eq(100) + end + end + + describe "#handle_io_block" do + let(:input_writer) { Tempfile.new } + let(:source_input) { Tempfile.new } + + after do + input_writer.unlink + source_input.unlink + end + + context "with a block" do + let(:block) { ->(_input_writer) { sleep 0.1 } } + + before do + expect(File).to receive(:mkfifo) + expect(File).to receive(:open).and_return(source_input, input_writer) + end + + it "creates a thread for handling the input IO" do + thread_count = Thread.list.count + thread = subject.send(:handle_io_block, &block) + expect(Thread.list.count).to eq(thread_count + 1) + thread.join + end + + it "closes input_writer" do + expect(input_writer.closed?).to eq(false) + thread = subject.send(:handle_io_block, &block) + thread.join + expect(input_writer.closed?).to eq(true) + end + end + + context "without a block" do + it "doesn't create a new thread for IO generation" do + thread_count = Thread.list.count + nil_result = subject.send(:handle_io_block) + + expect(nil_result).to be(nil) + expect(Thread.list.count).to eq(thread_count) + end + end + + context "with a block that causes an error" do + let(:err_block) { ->(_input_writer) { raise "err-mah-gerd" } } + + before do + expect(File).to receive(:mkfifo) + expect(File).to receive(:open).and_return(source_input, input_writer) + end + + it "does not hang the process and closes the writer" do + expect(input_writer.closed?).to eq(false) + thread = subject.send(:handle_io_block, &err_block) + expect { thread.join }.to raise_error StandardError + expect(input_writer.closed?).to eq(true) + end + end + end + end +end From 1c865957a0bc3b7329261f5339a0550923692295 Mon Sep 17 00:00:00 2001 From: Jerry Keselman Date: Fri, 20 Jul 2018 14:07:24 -0400 Subject: [PATCH 07/55] Review Comments (transferred from ManageIQ/manageiq-gems-pending@b716e29b58699419e8a5951128495034af12820e) --- lib/gems/pending/util/mount/miq_generic_mount_session.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/gems/pending/util/mount/miq_generic_mount_session.rb b/lib/gems/pending/util/mount/miq_generic_mount_session.rb index 19f0b854c9d..559edfc9fcc 100644 --- a/lib/gems/pending/util/mount/miq_generic_mount_session.rb +++ b/lib/gems/pending/util/mount/miq_generic_mount_session.rb @@ -54,7 +54,7 @@ def self.in_depot_session(opts, &_block) def self.new_session(opts) klass = uri_scheme_to_class(opts[:uri]) - session = klass.new(:uri => opts[:uri], :username => opts[:username], :password => opts[:password], :region => opts[:region]) + session = klass.new(opts.slice(:uri, :username, :password, :region)) session.connect session end @@ -467,10 +467,6 @@ def self.raw_disconnect(mnt_point) end end - def copy_dump_to_store - # nothing to do - end - private def settings_read_only? From c34d38943f61deb06f698d8af43616d5fd903d17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Luka=C5=A1=C3=ADk?= Date: Mon, 6 Mar 2017 13:25:25 +0100 Subject: [PATCH 08/55] Do not decide upon failure message but command that was run (transferred from ManageIQ/manageiq-gems-pending@e2e64940b756a2b914d63ca6f4fc325f8f587003) --- lib/gems/pending/util/mount/miq_generic_mount_session.rb | 2 +- spec/util/mount/miq_generic_mount_session_spec.rb | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/gems/pending/util/mount/miq_generic_mount_session.rb b/lib/gems/pending/util/mount/miq_generic_mount_session.rb index e899eb14689..4a20b06e174 100644 --- a/lib/gems/pending/util/mount/miq_generic_mount_session.rb +++ b/lib/gems/pending/util/mount/miq_generic_mount_session.rb @@ -34,7 +34,7 @@ def self.runcmd(cmd_str) # If sudo is required, ensure you have /etc/sudoers.d/miq # Cmnd_Alias MOUNTALL = /bin/mount, /bin/umount # %wheel ALL = NOPASSWD: MOUNTALL - if rv.include?("mount: only root can do that") || rv.include?('umount failed: Operation not permitted') + if $CHILD_STATUS.exitstatus == 1 && cmd_str =~ /^(mount|umount) / rv = `sudo #{cmd_str} 2>&1` end diff --git a/spec/util/mount/miq_generic_mount_session_spec.rb b/spec/util/mount/miq_generic_mount_session_spec.rb index 773bfa9de6c..c637235528b 100644 --- a/spec/util/mount/miq_generic_mount_session_spec.rb +++ b/spec/util/mount/miq_generic_mount_session_spec.rb @@ -18,9 +18,10 @@ end it ".runcmd will retry with sudo if needed" do - cmd = "abc" - expect(described_class).to receive(:`).once.with("#{cmd} 2>&1").and_return("mount: only root can do that\n") - expect(described_class).to receive(:`).with("sudo #{cmd} 2>&1").and_return("works with sudo") + cmd = "mount X Y" + expect(described_class).to receive(:`).once.with("#{cmd} 2>&1") + expect(described_class).to receive(:`).with("sudo #{cmd} 2>&1") + expect($CHILD_STATUS).to receive(:exitstatus).once.and_return(1) described_class.runcmd(cmd) end From d32a9d4377ae1a3d37e2e19edf3690f739666699 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Fri, 10 Aug 2018 19:28:19 -0500 Subject: [PATCH 09/55] Update MiqGenericMountSession to MiqFileStorage::Interface (transferred from ManageIQ/manageiq-gems-pending@e1b467becb8504a9989f1f24db61b55120dff043) --- .../util/mount/miq_generic_mount_session.rb | 54 +++++++++++-------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/lib/gems/pending/util/mount/miq_generic_mount_session.rb b/lib/gems/pending/util/mount/miq_generic_mount_session.rb index 6151a754f7e..73bbf7f3779 100644 --- a/lib/gems/pending/util/mount/miq_generic_mount_session.rb +++ b/lib/gems/pending/util/mount/miq_generic_mount_session.rb @@ -251,55 +251,59 @@ def verify res end - def add(source, dest_uri) + def add(*upload_args) + dest_uri = nil log_header = "MIQ(#{self.class.name}-add)" - logger.info("#{log_header} Source: [#{source}], Destination: [#{dest_uri}]...") + # Don't think this log line is possible when using MiqFileStorage::Interface + # + # logger.info("#{log_header} Source: [#{source}], Destination: [#{dest_uri}]...") begin reconnect! - relpath = File.join(@mnt_point, relative_to_mount(dest_uri)) - if File.exist?(relpath) - logger.info("#{log_header} Skipping add since URI: [#{dest_uri}] already exists") - return dest_uri - end - - logger.info("#{log_header} Building relative path: [#{relpath}]...") - FileUtils.mkdir_p(File.dirname(relpath)) - logger.info("#{log_header} Building relative path: [#{relpath}]...complete") - logger.info("#{log_header} Copying file [#{source}] to [#{relpath}]...") - FileUtils.cp(source, relpath) - logger.info("#{log_header} Copying file [#{source}] to [#{relpath}] complete") + dest_uri = super rescue => err - msg = "Adding [#{source}] to [#{dest_uri}], failed due to error: '#{err.message}'" + msg = "Adding [#{source_for_log}] to [#{remote_file_path}], failed due to error: '#{err.message}'" logger.error("#{log_header} #{msg}") raise ensure disconnect end - logger.info("#{log_header} File URI added: [#{dest_uri}] complete") + logger.info("#{log_header} File URI added: [#{remote_file_path}] complete") dest_uri end - alias_method :upload, :add + def upload_single(dest_uri) + log_header = "MIQ(#{self.class.name}-upload_single)" + relpath = uri_to_local_path(dest_uri) + if File.exist?(relpath) + logger.info("#{log_header} Skipping add since URI: [#{dest_uri}] already exists") + return dest_uri + end + + logger.info("#{log_header} Copying file [#{source_for_log}] to [#{dest_uri}]...") + IO.copy_stream(source_input, relpath, byte_count) + logger.info("#{log_header} Copying file [#{source_for_log}] to [#{dest_uri}] complete") + dest_uri + end - def download(local_file, remote_file) + def download_single(remote_file, local_file) log_header = "MIQ(#{self.class.name}-download)" logger.info("#{log_header} Target: [#{local_file}], Remote file: [#{remote_file}]...") begin reconnect! - relpath = File.join(@mnt_point, relative_to_mount(remote_file)) + relpath = File.join(mnt_point, relative_to_mount(remote_file)) unless File.exist?(relpath) logger.warn("#{log_header} Remote file: [#{remote_file}] does not exist!") return end logger.info("#{log_header} Copying file [#{relpath}] to [#{local_file}]...") - FileUtils.cp(relpath, local_file) + IO.copy_stream(relpath, local_file) logger.info("#{log_header} Copying file [#{relpath}] to [#{local_file}] complete") rescue => err msg = "Downloading [#{remote_file}] to [#{local_file}], failed due to error: '#{err.message}'" @@ -392,7 +396,11 @@ def glob(pattern) def mkdir(path) with_mounted_exception_handling do - FileUtils.mkdir_p("#{mount_root}/#{path}") + log_header = "MIQ(#{self.class.name}-mkdir)" + new_path = uri_to_local_path(path) + logger.info("#{log_header} Building relative path: [#{new_path}]...") + FileUtils.mkdir_p(new_path) + logger.info("#{log_header} Building relative path: [#{new_path}]...complete") end end @@ -482,4 +490,8 @@ def settings_mount_point return nil if @settings[:mount_point].blank? # Check if settings contains the mount_point to use FileUtils.mkdir_p(@settings[:mount_point]).first end + + def source_for_log + @input_writer ? "" : @source_input.path + end end From 33b5560cf9503ebb2cc1357b0b06f7b6c4a76526 Mon Sep 17 00:00:00 2001 From: Jerry Keselman Date: Thu, 26 Jul 2018 10:32:09 -0400 Subject: [PATCH 10/55] S3 Restore Support Changes to support restore of database from S3 objects. (transferred from ManageIQ/manageiq-gems-pending@7485a64ee913cd7dbecf296870db87e3aaa34833) --- lib/gems/pending/util/mount/miq_generic_mount_session.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/gems/pending/util/mount/miq_generic_mount_session.rb b/lib/gems/pending/util/mount/miq_generic_mount_session.rb index bb36a3bb4fb..f3517466f35 100644 --- a/lib/gems/pending/util/mount/miq_generic_mount_session.rb +++ b/lib/gems/pending/util/mount/miq_generic_mount_session.rb @@ -187,6 +187,14 @@ def reconnect! connect end + def self.supports_objects? + false + end + + def supports_objects? + false + end + def with_test_file(&_block) raise "requires a block" unless block_given? file = '/tmp/miq_verify_test_file' From aa430cc476ee06e46fe2ec13db9fe98afebbb6b7 Mon Sep 17 00:00:00 2001 From: Jerry Keselman Date: Fri, 27 Jul 2018 16:45:27 -0400 Subject: [PATCH 11/55] PR Review Comments Based on review comments change MiqS3Session.disconnect to self.raw_disconnect. Additionally remove the previously added 3rd argument to MiqGenericMountSession.add which is no longer needed. (transferred from ManageIQ/manageiq-gems-pending@60b37d0e8703c1a7bd7a4f99470bfbcb27203e5b) --- lib/gems/pending/util/mount/miq_generic_mount_session.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gems/pending/util/mount/miq_generic_mount_session.rb b/lib/gems/pending/util/mount/miq_generic_mount_session.rb index 559edfc9fcc..bb36a3bb4fb 100644 --- a/lib/gems/pending/util/mount/miq_generic_mount_session.rb +++ b/lib/gems/pending/util/mount/miq_generic_mount_session.rb @@ -246,7 +246,7 @@ def verify res end - def add(source, dest_uri, _object_file = nil) + def add(source, dest_uri) log_header = "MIQ(#{self.class.name}-add)" logger.info("#{log_header} Source: [#{source}], Destination: [#{dest_uri}]...") From a9a905f89c12377633c84656ff68193a4b13deb9 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Fri, 10 Aug 2018 21:14:20 -0500 Subject: [PATCH 12/55] Add MiqLocalMountSession This mount session is intended as being a pass through for when local files are used. The same API's for `#add` from MiqGenericMountSession are still usable for this class, and it will just write to the local file system instead. This also allows us to test the file splitting code without needing a mount session in place to do so. (transferred from ManageIQ/manageiq-gems-pending@1bd3433c5227f00e60111cfe125ff9b8b4808761) --- .../util/mount/miq_generic_mount_session.rb | 3 +- .../util/mount/miq_local_mount_session.rb | 28 +++++ .../mount/miq_local_mount_session_spec.rb | 111 ++++++++++++++++++ 3 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 lib/gems/pending/util/mount/miq_local_mount_session.rb create mode 100644 spec/util/mount/miq_local_mount_session_spec.rb diff --git a/lib/gems/pending/util/mount/miq_generic_mount_session.rb b/lib/gems/pending/util/mount/miq_generic_mount_session.rb index 73bbf7f3779..e840242d079 100644 --- a/lib/gems/pending/util/mount/miq_generic_mount_session.rb +++ b/lib/gems/pending/util/mount/miq_generic_mount_session.rb @@ -9,6 +9,7 @@ require 'util/miq_file_storage' class MiqGenericMountSession < MiqFileStorage::Interface + require 'util/mount/miq_local_mount_session' require 'util/mount/miq_nfs_session' require 'util/mount/miq_s3_session' require 'util/mount/miq_smb_session' @@ -296,7 +297,7 @@ def download_single(remote_file, local_file) begin reconnect! - relpath = File.join(mnt_point, relative_to_mount(remote_file)) + relpath = uri_to_local_path(remote_file) unless File.exist?(relpath) logger.warn("#{log_header} Remote file: [#{remote_file}] does not exist!") return diff --git a/lib/gems/pending/util/mount/miq_local_mount_session.rb b/lib/gems/pending/util/mount/miq_local_mount_session.rb new file mode 100644 index 00000000000..0395e7c2093 --- /dev/null +++ b/lib/gems/pending/util/mount/miq_local_mount_session.rb @@ -0,0 +1,28 @@ +require 'util/mount/miq_generic_mount_session' + +# MiqLocalMountSession is meant to be a representation of the local file system +# that conforms to the same interface as MiqLocalMountSession (and by proxy, +# MiqFileSystem::Interface). +# +# See MiqGenericMountSession for info on methods available. +class MiqLocalMountSession < MiqGenericMountSession + def self.uri_scheme + "file".freeze + end + + # no-op these since they are not relavent to the local file system + # + # rubocop:disable Style/SingleLineMethods, Layout/EmptyLineBetweenDefs + def connect; end # :nodoc: + def disconnect; end # :nodoc: + def mount_share; end # :nodoc: + # rubocop:enable Style/SingleLineMethods, Layout/EmptyLineBetweenDefs + + def relative_to_mount(remote_file) # :nodoc: + remote_file + end + + def uri_to_local_path(remote_file) # :nodoc: + File.expand_path(remote_file) + end +end diff --git a/spec/util/mount/miq_local_mount_session_spec.rb b/spec/util/mount/miq_local_mount_session_spec.rb new file mode 100644 index 00000000000..ed9892f432e --- /dev/null +++ b/spec/util/mount/miq_local_mount_session_spec.rb @@ -0,0 +1,111 @@ +require 'util/mount/miq_local_mount_session' +require 'tempfile' + +describe MiqLocalMountSession do + shared_context "generated tmp files" do + let!(:tmpfile_size) { 10.megabytes } + let!(:source_path) { Pathname.new(source_file.path) } + let!(:source_file) do + Tempfile.new("source_file").tap do |file| + file.write("0" * tmpfile_size) + file.close + end + end + let!(:dest_path) do + Pathname.new(Dir::Tmpname.create("") {}) + end + + after do + source_file.unlink + Dir["#{source_path.expand_path}.*"].each do |file| + File.delete(file) + end + end + end + + subject { described_class.new(:uri => "file://") } + + describe "#add" do + include_context "generated tmp files" + + it "copies single files" do + expect(subject.add(source_path.to_s, dest_path.to_s)).to eq(dest_path.to_s) + expect(File.exist?(dest_path)).to be true + expect(Pathname.new(dest_path).lstat.size).to eq(10.megabytes) + end + + it "copies file to splits" do + expected_splitfiles = (1..5).map do |suffix| + source_path.dirname.join("#{dest_path.basename}.0000#{suffix}") + end + + File.open(source_path) do |f| # with an IO object this time + subject.add(f, dest_path.to_s, "2M") + end + + expected_splitfiles.each do |filename| + expect(File.exist?(filename)).to be true + expect(Pathname.new(filename).lstat.size).to eq(2.megabytes) + end + end + + it "can take input from a command" do + expected_splitfiles = (1..5).map do |suffix| + source_path.dirname.join("#{dest_path.basename}.0000#{suffix}") + end + + subject.add(dest_path.to_s, "2M") do |input_writer| + `#{Gem.ruby} -e "File.write('#{input_writer}', '0' * #{tmpfile_size})"` + end + + expected_splitfiles.each do |filename| + expect(File.exist?(filename)).to be true + expect(Pathname.new(filename).lstat.size).to eq(2.megabytes) + end + end + + context "with a slightly smaller input file than 10MB" do + let(:tmpfile_size) { 10.megabytes - 1.kilobyte } + + it "properly chunks the file" do + expected_splitfiles = (1..10).map do |suffix| + name = "#{dest_path.basename}.%05d" % {:suffix => suffix} + source_path.dirname.join(name) + end + + # using pathnames this time + subject.add(source_path, dest_path.to_s, 1.megabyte) + + expected_splitfiles[0, 9].each do |filename| + expect(File.exist?(filename)).to be true + expect(Pathname.new(filename).lstat.size).to eq(1.megabyte) + end + + last_split = expected_splitfiles.last + expect(File.exist?(last_split)).to be true + expect(Pathname.new(last_split).lstat.size).to eq(1.megabyte - 1.kilobyte) + end + end + end + + describe "#download" do + include_context "generated tmp files" + + it "downloads the file" do + subject.download(dest_path.to_s, source_path.to_s) + expect(File.exist?(dest_path)).to be true + expect(Pathname.new(dest_path).lstat.size).to eq(10.megabytes) + end + + it "can take input from a command" do + source_data = nil + subject.download(nil, source_path) do |input_writer| + source_data = `#{Gem.ruby} -e "print File.read('#{input_writer}')"` + end + + expect(File.exist?(dest_path)).to be false + expect(source_data.size).to eq(10.megabytes) + expect(source_data).to eq(File.read(source_path)) + end + end +end From 2d794b2766f7753f8c84c698fdf5c61f57d21624 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Mon, 6 Aug 2018 19:41:36 -0500 Subject: [PATCH 13/55] Add with_ftp_server rspec shared_context Context that make sure to spin up an FTP server when included, and will make some helper methods available. Uses the `ftpd` gem to handle the ftp server. (transferred from ManageIQ/manageiq-gems-pending@04552b09e685db1619f0a6161e0e0b8795267486) --- spec/support/contexts/with_ftp_server.rb | 90 ++++++++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 spec/support/contexts/with_ftp_server.rb diff --git a/spec/support/contexts/with_ftp_server.rb b/spec/support/contexts/with_ftp_server.rb new file mode 100644 index 00000000000..0add97244ae --- /dev/null +++ b/spec/support/contexts/with_ftp_server.rb @@ -0,0 +1,90 @@ +require "ftpd" +require "tmpdir" +require "tempfile" +require "fileutils" + +class FtpSingletonServer + class << self + attr_reader :driver + end + + def self.run_ftp_server + @driver = FTPServerDriver.new + @ftp_server = Ftpd::FtpServer.new(@driver) + @ftp_server.on_exception do |e| + STDOUT.puts e.inspect + end + @ftp_server.start + end + + def self.bound_port + @ftp_server.bound_port + end + + def self.stop_ftp_server + @ftp_server.stop + @ftp_server = nil + + @driver.cleanup + @driver = nil + end +end + +class FTPServerDriver + attr_reader :existing_file, :existing_dir + + def initialize + create_tmp_dir + end + + def authenticate(username, password) + username == "ftpuser" && password == "ftppass" + end + + def file_system(_user) + Ftpd::DiskFileSystem.new(@ftp_dir) + end + + def cleanup + FileUtils.remove_entry(@ftp_dir) + end + + def create_existing_file(size = 0) + @existing_file ||= Tempfile.new("", @ftp_dir).tap { |tmp| tmp.puts "0" * size } + end + + # Create a dir under the @ftp_dir, but only return the created directory name + def create_existing_dir + @existing_dir ||= Dir.mktmpdir(nil, @ftp_dir).sub("#{@ftp_dir}/", "") + end + + private + + def create_tmp_dir + @ftp_dir = Dir.mktmpdir + end +end + +shared_context "with ftp server", :with_ftp_server do + before(:all) { FtpSingletonServer.run_ftp_server } + after(:all) { FtpSingletonServer.stop_ftp_server } + + # HACK: Avoid permission denied errors with `ftpd` starting on port 21, but + # our FTP lib always assuming that we are using the default port + # + # The hack basically sets the default port for `Net::FTP` to the bound port + # of the running server + before(:each) do + stub_const("Net::FTP::FTP_PORT", FtpSingletonServer.bound_port) + end + + let(:valid_ftp_creds) { { :username => "ftpuser", :password => "ftppass" } } + + def existing_ftp_file(size = 0) + FtpSingletonServer.driver.create_existing_file(size) + end + + def existing_ftp_dir + FtpSingletonServer.driver.create_existing_dir + end +end From e968263149a842ac401daf8f6c44d7346b367a2e Mon Sep 17 00:00:00 2001 From: Jerry Keselman Date: Wed, 1 Aug 2018 14:19:29 -0400 Subject: [PATCH 14/55] Remove supports_objects? based on Review Comments (transferred from ManageIQ/manageiq-gems-pending@aa9d9fbb37e7f1b481f37b6b82a5f054378f3b3b) --- lib/gems/pending/util/mount/miq_generic_mount_session.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/gems/pending/util/mount/miq_generic_mount_session.rb b/lib/gems/pending/util/mount/miq_generic_mount_session.rb index f3517466f35..bb36a3bb4fb 100644 --- a/lib/gems/pending/util/mount/miq_generic_mount_session.rb +++ b/lib/gems/pending/util/mount/miq_generic_mount_session.rb @@ -187,14 +187,6 @@ def reconnect! connect end - def self.supports_objects? - false - end - - def supports_objects? - false - end - def with_test_file(&_block) raise "requires a block" unless block_given? file = '/tmp/miq_verify_test_file' From 13ff99310cb05f6a5e13d85b0ab38f0fb10c2508 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Wed, 12 Sep 2018 15:43:36 -0500 Subject: [PATCH 15/55] Remove MiqObjectStorage.new_with_opts Since this is going to be overwritten in each class (most likely), and already is for S3, it makes sense that we don't include this since it doesn't do anything by being here. (transferred from ManageIQ/manageiq-gems-pending@9f91d868c4c0ba6d1fb4ac8d7a51bfe7bd851144) --- lib/gems/pending/util/miq_object_storage.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/gems/pending/util/miq_object_storage.rb b/lib/gems/pending/util/miq_object_storage.rb index b7ea0f2eb19..923a43ea3f6 100644 --- a/lib/gems/pending/util/miq_object_storage.rb +++ b/lib/gems/pending/util/miq_object_storage.rb @@ -6,10 +6,6 @@ class MiqObjectStorage < MiqFileStorage::Interface attr_accessor :settings attr_writer :logger - def self.new_with_opts(opts) - new(opts.slice(:uri, :username, :password)) - end - def initialize(settings) raise "URI missing" unless settings.key?(:uri) @settings = settings.dup From 7b831a06cc36d065a90ec94c023065b5e388cc5f Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Mon, 20 Aug 2018 16:42:21 -0500 Subject: [PATCH 16/55] Add MiqObjectStore This is the top level class for Object Storage based file storages, which will include types like s3 and FTP. Doesn't do much, but shared methods between all of the Object Storage classes will be put here. (transferred from ManageIQ/manageiq-gems-pending@d79deabbfb80a6b5c3f0bec614aa223427d266d2) --- lib/gems/pending/util/miq_object_storage.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 lib/gems/pending/util/miq_object_storage.rb diff --git a/lib/gems/pending/util/miq_object_storage.rb b/lib/gems/pending/util/miq_object_storage.rb new file mode 100644 index 00000000000..845a0bbfd4e --- /dev/null +++ b/lib/gems/pending/util/miq_object_storage.rb @@ -0,0 +1,19 @@ +require 'util/miq_file_storage' + +class MiqObjectStorage < MiqFileStorage::Interface + attr_accessor :settings + attr_writer :logger + + def self.new_with_opts(opts) + new(opts.slice(:uri, :username, :password)) + end + + def initialize(settings) + raise "URI missing" unless settings.key?(:uri) + @settings = settings.dup + end + + def logger + @logger ||= $log.nil? ? :: Logger.new(STDOUT) : $log + end +end From 1b22c0e3fdafde694c736a3357ec8357166eb65d Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Mon, 6 Aug 2018 19:43:25 -0500 Subject: [PATCH 17/55] Adds MiqFtpLib This is lifted from the FileDepotFtp code in the manageiq repo, and is just some shared methods around connecting to an FTP endpoint. Adds some tests around it as well. (transferred from ManageIQ/manageiq-gems-pending@25baa0bf5e058e9e110d41a14df8f903f85ca36a) --- lib/gems/pending/util/miq_ftp_lib.rb | 72 ++++++++++ spec/util/miq_ftp_lib_spec.rb | 193 +++++++++++++++++++++++++++ 2 files changed, 265 insertions(+) create mode 100644 lib/gems/pending/util/miq_ftp_lib.rb create mode 100644 spec/util/miq_ftp_lib_spec.rb diff --git a/lib/gems/pending/util/miq_ftp_lib.rb b/lib/gems/pending/util/miq_ftp_lib.rb new file mode 100644 index 00000000000..ddada42d068 --- /dev/null +++ b/lib/gems/pending/util/miq_ftp_lib.rb @@ -0,0 +1,72 @@ +require 'net/ftp' + +# Helper methods for net/ftp based classes and files. +# +# Will setup a `@ftp` attr_accessor to be used as the return value for +# `.connect`, the main method being provided in this class. +module MiqFtpLib + def self.included(klass) + klass.send(:attr_accessor, :ftp) + end + + def connect(cred_hash = nil) + host = URI(uri).hostname + + begin + _log.info("Connecting to FTP host #{host_ref}...") + @ftp = Net::FTP.new(host) + # Use passive mode to avoid firewall issues see http://slacksite.com/other/ftp.html#passive + @ftp.passive = true + # @ftp.debug_mode = true if settings[:debug] # TODO: add debug option + creds = cred_hash ? [cred_hash[:username], cred_hash[:password]] : login_credentials + @ftp.login(*creds) + _log.info("Successfully connected FTP host #{host_ref}...") + rescue SocketError => err + _log.error("Failed to connect. #{err.message}") + raise + rescue Net::FTPPermError => err + _log.error("Failed to login. #{err.message}") + raise + else + @ftp + end + end + + def file_exists?(file_or_directory) + !ftp.nlst(file_or_directory.to_s).empty? + rescue Net::FTPPermError + false + end + + private + + def host_ref + return @host_ref if @host_ref + @host_ref = URI(uri).hostname + @host_ref << " (#{name})" if respond_to?(:name) + @host_ref + end + + def create_directory_structure(directory_path) + pwd = ftp.pwd + directory_path.to_s.split('/').each do |directory| + unless ftp.nlst.include?(directory) + _log.info("creating #{directory}") + ftp.mkdir(directory) + end + ftp.chdir(directory) + end + ftp.chdir(pwd) + end + + def with_connection(cred_hash = nil) + raise _("no block given") unless block_given? + _log.info("Connecting through #{self.class.name}: [#{host_ref}]") + begin + connect(cred_hash) + yield @ftp + ensure + @ftp.try(:close) && @ftp = nil + end + end +end diff --git a/spec/util/miq_ftp_lib_spec.rb b/spec/util/miq_ftp_lib_spec.rb new file mode 100644 index 00000000000..21792c5d5bb --- /dev/null +++ b/spec/util/miq_ftp_lib_spec.rb @@ -0,0 +1,193 @@ +require 'util/miq_ftp_lib' +require 'logger' # probably loaded elsewhere, but for the below classes + +class FTPKlass + include MiqFtpLib + + attr_accessor :uri + + def self.instance_logger + Logger.new(File::NULL) # null logger (for testing) + end + + private + + def _log + self.class.instance_logger + end +end + +class OtherFTPKlass + include MiqFtpLib + + attr_accessor :uri + + def _log + private_log_method + end + + private + + def private_log_method + Logger.new(File::NULL) # null logger (for testing) + end + + def login_credentials + %w(ftpuser ftppass) + end +end + +shared_examples "connecting" do |valid_cred_hash| + let(:cred_hash) { valid_cred_hash } + + before { subject.uri = "ftp://localhost" } + + it "logs in with valid credentials" do + expect { subject.connect(cred_hash) }.not_to raise_error + end + + it "sets the connection to passive" do + subject.connect(cred_hash) + expect(subject.ftp.passive).to eq(true) + end + + context "with an invalid ftp credentials" do + let(:cred_hash) { { :username => "invalid", :password => "alsoinvalid" } } + + it "raises a Net::FTPPermError" do + expect { subject.connect(cred_hash) }.to raise_error(Net::FTPPermError) + end + end +end + +shared_examples "with a connection" do |valid_cred_hash| + let(:cred_hash) { valid_cred_hash } + let(:error_msg) { "no block given" } + + before do + subject.uri = "ftp://localhost" + allow(subject).to receive(:_).with(error_msg).and_return(error_msg) + end + + def with_connection(&block) + subject.send(:with_connection, cred_hash, &block) + end + + def get_socket(ftp) + ftp.instance_variable_get(:@sock).instance_variable_get(:@io) + end + + it "passes the ftp object to the block" do + with_connection do |ftp| + expect(ftp).to be_a(Net::FTP) + expect(subject.ftp).to be(ftp) + end + end + + it "closes the ftp connection after the block is finished" do + ftp_instance = subject.connect(cred_hash) + # stub further calls to `#connect` + expect(subject).to receive(:connect).and_return(ftp_instance) + + with_connection { |ftp| } + expect(subject.ftp).to eq(nil) + expect(ftp_instance.closed?).to eq(true) + end + + it "raises an error if no block is given" do + expect { with_connection }.to raise_error(RuntimeError, error_msg) + end +end + +describe MiqFtpLib do + subject { FTPKlass.new } + + describe "when included" do + it "has a `ftp` accessor" do + ftp_instance = Net::FTP.new + subject.ftp = ftp_instance + + expect(subject.ftp).to eq ftp_instance + end + end + + describe "#connect", :with_ftp_server do + context "with credentials hash" do + subject { FTPKlass.new } + + include_examples "connecting", :username => "ftpuser", :password => "ftppass" + end + + context "with login_credentials method" do + subject { OtherFTPKlass.new } + + include_examples "connecting" + end + end + + describe "#with_connection", :with_ftp_server do + context "with credentials hash" do + subject { FTPKlass.new } + + include_examples "with a connection", :username => "ftpuser", :password => "ftppass" + end + + context "with login_credentials method" do + subject { OtherFTPKlass.new } + + include_examples "with a connection" + end + end + + describe "#file_exists?", :with_ftp_server do + let(:existing_file) { File.basename(existing_ftp_file) } + + subject { FTPKlass.new.tap { |ftp| ftp.uri = "ftp://localhost" } } + before { subject.connect(valid_ftp_creds) } + + it "returns true if the file exists" do + expect(subject.file_exists?(existing_file)).to eq(true) + end + + it "returns false if the file does not exist" do + expect(subject.file_exists?("#{existing_file}.fake")).to eq(false) + end + end + + # Note: Don't use `file_exists?` to try and test the directory existance. + # Most FTP implementations will send the results of `nlst` as the contents of + # a directory if a directory is given. + # + # In our current implementation, this will return a empty list if the + # directory is empty, thus causing the check to fail. Testing against the + # `ftp.nlst(parent_dir)` will make sure the directory in question is included + # in it's parent. + describe "#create_directory_structure", :with_ftp_server do + subject { OtherFTPKlass.new.tap { |ftp| ftp.uri = "ftp://localhost" } } + before { subject.connect(valid_ftp_creds) } + + it "creates a new nested directory" do + new_dir = "foo/bar/baz" + parent_dir = File.dirname(new_dir) + + expect(subject.ftp.nlst(parent_dir).include?("baz")).to eq(false) + subject.send(:create_directory_structure, new_dir) + expect(subject.ftp.nlst(parent_dir).include?("baz")).to eq(true) + end + + context "to an existing directory" do + it "creates the nested directory without messing with the existing" do + existing_dir = existing_ftp_dir + new_dir = File.join(existing_ftp_dir, "foo/bar/baz") + parent_dir = File.dirname(new_dir) + + expect(subject.ftp.nlst.include?(existing_dir)).to eq(true) + expect(subject.ftp.nlst(parent_dir).include?("baz")).to eq(false) + + subject.send(:create_directory_structure, new_dir) + expect(subject.ftp.nlst.include?(existing_dir)).to eq(true) + expect(subject.ftp.nlst(parent_dir).include?("baz")).to eq(true) + end + end + end +end From e750f9d393e5f4ce451627960c0e5cad1631df26 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Tue, 28 Aug 2018 18:23:33 -0500 Subject: [PATCH 18/55] Add "generated tmp files" context This was originally part of the MiqLocalMountSession spec, and could be useful for other specs as well, specifically FTP based object stores in the future. (transferred from ManageIQ/manageiq-gems-pending@f272e2e2e2d713e0901cb1f896759604df37052f) --- spec/support/contexts/generated_tmp_files.rb | 17 +++++++++++++++ .../mount/miq_local_mount_session_spec.rb | 21 +------------------ 2 files changed, 18 insertions(+), 20 deletions(-) create mode 100644 spec/support/contexts/generated_tmp_files.rb diff --git a/spec/support/contexts/generated_tmp_files.rb b/spec/support/contexts/generated_tmp_files.rb new file mode 100644 index 00000000000..26426ba3fa8 --- /dev/null +++ b/spec/support/contexts/generated_tmp_files.rb @@ -0,0 +1,17 @@ +shared_context "generated tmp files" do + let!(:tmpfile_size) { 10.megabytes } + let!(:source_path) { Pathname.new(source_file.path) } + let!(:source_file) do + Tempfile.new("source_file").tap do |file| + file.write("0" * tmpfile_size) + file.close + end + end + + after do + source_file.unlink + Dir["#{source_path.expand_path}.*"].each do |file| + File.delete(file) + end + end +end diff --git a/spec/util/mount/miq_local_mount_session_spec.rb b/spec/util/mount/miq_local_mount_session_spec.rb index ed9892f432e..50ba10c4171 100644 --- a/spec/util/mount/miq_local_mount_session_spec.rb +++ b/spec/util/mount/miq_local_mount_session_spec.rb @@ -2,26 +2,7 @@ require 'tempfile' describe MiqLocalMountSession do - shared_context "generated tmp files" do - let!(:tmpfile_size) { 10.megabytes } - let!(:source_path) { Pathname.new(source_file.path) } - let!(:source_file) do - Tempfile.new("source_file").tap do |file| - file.write("0" * tmpfile_size) - file.close - end - end - let!(:dest_path) do - Pathname.new(Dir::Tmpname.create("") {}) - end - - after do - source_file.unlink - Dir["#{source_path.expand_path}.*"].each do |file| - File.delete(file) - end - end - end + let!(:dest_path) { Pathname.new(Dir::Tmpname.create("") {}) } subject { described_class.new(:uri => "file://") } From e3f39c64b9a0d8ec5e3034facca06b0892a6eebb Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Wed, 22 Aug 2018 00:50:54 -0500 Subject: [PATCH 19/55] Convert MiqS3Session to MiqS3Storage Converts MiqS3Session to an MiqObjectStorage subclass, MiqS3Storage. A decent amount here didn't change, but it updates the `#add` and `#download` methods to use the MiqFileStorage::Interface and instead override `#upload_single` and `#download_single`. Many of the tests didn't make it over since a lot of it was in regards to "mounting", which ObjectStorage classes don't do and is now obsolete, as well as cleaned up MiqGenericMountSession from any references that were s3 specific. Also dried up the code a bit from what was implemented in MiqS3Session. Regarding the elephant in the room... ------------------------------------- Yes, I backported Aws::S3::MultipartStreamUploader from the v3 portion of the aws-sdk as part of this commit. Some form of "upload streaming" was necessary for file splitting to work. I could have implemented something myself with probably a little less code, but what was already part of the v3 API, is probably tested extensively, and works with the v2 Seahorse client and lib made more sense to implement via this way instead of trying to role my own. It is a lot of extra code, but I have a flag in place for when we do upgrade to aws-sdk v3 in the future, an error should be raised in the test suite to have this patch removed. (transferred from ManageIQ/manageiq-gems-pending@f3e3e9c54f1e33f9ff046ab5cdc668c0012210d2) --- lib/gems/pending/util/miq_object_storage.rb | 2 + .../util/mount/miq_generic_mount_session.rb | 5 +- .../util/object_storage/miq_s3_storage.rb | 101 ++++++++++++++++++ .../object_storage/miq_s3_storage_spec.rb | 13 +++ 4 files changed, 117 insertions(+), 4 deletions(-) create mode 100644 lib/gems/pending/util/object_storage/miq_s3_storage.rb create mode 100644 spec/util/object_storage/miq_s3_storage_spec.rb diff --git a/lib/gems/pending/util/miq_object_storage.rb b/lib/gems/pending/util/miq_object_storage.rb index 845a0bbfd4e..b7ea0f2eb19 100644 --- a/lib/gems/pending/util/miq_object_storage.rb +++ b/lib/gems/pending/util/miq_object_storage.rb @@ -1,6 +1,8 @@ require 'util/miq_file_storage' class MiqObjectStorage < MiqFileStorage::Interface + require 'util/object_storage/miq_s3_storage' + attr_accessor :settings attr_writer :logger diff --git a/lib/gems/pending/util/mount/miq_generic_mount_session.rb b/lib/gems/pending/util/mount/miq_generic_mount_session.rb index e840242d079..b5bb8f81bef 100644 --- a/lib/gems/pending/util/mount/miq_generic_mount_session.rb +++ b/lib/gems/pending/util/mount/miq_generic_mount_session.rb @@ -11,7 +11,6 @@ class MiqGenericMountSession < MiqFileStorage::Interface require 'util/mount/miq_local_mount_session' require 'util/mount/miq_nfs_session' - require 'util/mount/miq_s3_session' require 'util/mount/miq_smb_session' require 'util/mount/miq_glusterfs_session' @@ -62,15 +61,13 @@ def self.new_session(opts) end def self.new_with_opts(opts) - new(opts.slice(:uri, :username, :password, :region)) + new(opts.slice(:uri, :username, :password)) end def self.uri_scheme_to_class(uri) require 'uri' scheme, userinfo, host, port, registry, share, opaque, query, fragment = URI.split(URI.encode(uri)) case scheme - when 's3' - MiqS3Session when 'smb' MiqSmbSession when 'nfs' diff --git a/lib/gems/pending/util/object_storage/miq_s3_storage.rb b/lib/gems/pending/util/object_storage/miq_s3_storage.rb new file mode 100644 index 00000000000..06cb665114a --- /dev/null +++ b/lib/gems/pending/util/object_storage/miq_s3_storage.rb @@ -0,0 +1,101 @@ +require 'util/miq_object_storage' + +class MiqS3Storage < MiqObjectStorage + attr_reader :bucket_name + + def self.uri_scheme + "s3".freeze + end + + def self.new_with_opts(opts) + new(opts.slice(:uri, :username, :password, :region)) + end + + def initialize(settings) + super(settings) + + # NOTE: This line to be removed once manageiq-ui-class region change implemented. + @settings[:region] ||= "us-east-1" + @bucket_name = URI(@settings[:uri]).host + + raise "username, password, and region are required values!" if @settings[:username].nil? || @settings[:password].nil? || @settings[:region].nil? + end + + # Extract the path from the URI, so strip off the "s3://" scheme, the bucket + # hostname, leaving only the path minus the leading '/' + def uri_to_object_key(remote_file) + # `path` is `[5]` in the returned result of URI.split + URI.split(remote_file)[5][1..-1] + end + + def upload_single(dest_uri) + object_key = uri_to_object_key(dest_uri) + logger.debug("Writing [#{source_input}] to => Bucket [#{bucket_name}] Key [#{dest_uri}]") + + with_standard_s3_error_handling("uploading", source_input) do + bucket.object(object_key).upload_stream do |write_stream| + IO.copy_stream(source_input, write_stream, byte_count) + end + end + end + + def download_single(source, destination) + object_key = uri_to_object_key(remote_file) + logger.debug("Downloading [#{source}] from bucket [#{bucket_name}] to local file [#{destination}]") + + with_standard_s3_error_handling("downloading", source) do + if destination.kind_of?(IO) + s3.client.get_object(:bucket => bucket_name, :key => object_key) do |chunk| + destination.write(chunk) + end + else # assume file path + bucket.object(source).download_file(destination) + end + end + local_file + end + + # no-op mostly + # + # dirs don't need to be created ahead of time in s3, unlike mounted file + # systems. + # + # For convenience though, calling bucket, which will initialize and create + # (if needed) the s3 bucket to be used for this instance. + def mkdir(_dir) + bucket + end + + def bucket + @bucket ||= s3.bucket(bucket_name).tap do |bucket| + if bucket.exists? + logger.debug("Found bucket #{bucket_name}") + else + logger.debug("Bucket #{bucket_name} does not exist, creating.") + bucket.create + end + end + end + + private + + def s3 + require 'aws-sdk' + require 'util/extensions/aws-sdk/s3_upload_stream_patch' + @s3 ||= Aws::S3::Resource.new(:region => @settings[:region], + :access_key_id => @settings[:username], + :secret_access_key => @settings[:password]) + end + + def with_standard_s3_error_handling(action, object) + yield + rescue Aws::S3::Errors::AccessDenied, Aws::S3::Errors::Forbidden => err + logger.error("Access to S3 bucket #{bucket_name} restricted. Try a different name. #{err}") + msg = "Access to S3 bucket #{bucket_name} restricted. Try a different name. #{err}" + raise err, msg, err.backtrace + rescue => err + logger.error("Error #{action} #{object} from S3. #{err}") + msg = "Error #{action} #{object} from S3. #{err}" + raise err, msg, err.backtrace + end +end diff --git a/spec/util/object_storage/miq_s3_storage_spec.rb b/spec/util/object_storage/miq_s3_storage_spec.rb new file mode 100644 index 00000000000..1d62c998aae --- /dev/null +++ b/spec/util/object_storage/miq_s3_storage_spec.rb @@ -0,0 +1,13 @@ +require "util/object_storage/miq_s3_storage" + +describe MiqS3Storage do + before(:each) do + @uri = "s3://tmp/abc/def" + @session = described_class.new(:uri => @uri, :username => 'user', :password => 'pass', :region => 'region') + end + + it "#uri_to_object_path returns a new object path" do + result = @session.uri_to_object_key(@uri) + expect(result).to eq("abc/def") + end +end From 6a5066e1c0d983edfcc51cd2c9bd542a44f1be11 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Tue, 28 Aug 2018 21:18:10 -0500 Subject: [PATCH 20/55] Add MiqFtpStorage This adds FTP as a ObjectStorage target, which allows the FTP protocol to be used in conjunction with the MiqFileStorage::Interface. Most of this is pretty straight forward and uses the vanilla Net::FTP mechanics, but upload_single got a bit involved. Because we want to stream as much of this content as possible, and file splitting is also a requirement for uploads, breaking into the internals of Net::FTP was necessary to achieve uploads where we were only partially reading the input. `Net::FTP#storbinary` only allows sending the entire file, and being able to specify the size of the upload chunks and offset, but not how much of the file intotal to send. IO.copy_stream thankfully does most of the heavy lifting and logic for us, and since we are streaming the content through that, should send the `.write` calls in a small enough size that doesn't go over the `Net::FTP::DEFAULT_BLOCKSIZE`... I hope... (transferred from ManageIQ/manageiq-gems-pending@11346b9bedcd294fedc10bd0355f0600d6b8a6da) --- lib/gems/pending/util/miq_object_storage.rb | 1 + .../util/object_storage/miq_ftp_storage.rb | 86 +++++++++++++ .../custom_matchers/exist_on_ftp_server.rb | 52 ++++++++ .../have_size_on_ftp_server_of.rb | 30 +++++ spec/util/miq_file_storage_spec.rb | 11 ++ .../object_storage/miq_ftp_storage_spec.rb | 119 ++++++++++++++++++ 6 files changed, 299 insertions(+) create mode 100644 lib/gems/pending/util/object_storage/miq_ftp_storage.rb create mode 100644 spec/support/custom_matchers/exist_on_ftp_server.rb create mode 100644 spec/support/custom_matchers/have_size_on_ftp_server_of.rb create mode 100644 spec/util/object_storage/miq_ftp_storage_spec.rb diff --git a/lib/gems/pending/util/miq_object_storage.rb b/lib/gems/pending/util/miq_object_storage.rb index 923a43ea3f6..6cb15873fa7 100644 --- a/lib/gems/pending/util/miq_object_storage.rb +++ b/lib/gems/pending/util/miq_object_storage.rb @@ -2,6 +2,7 @@ class MiqObjectStorage < MiqFileStorage::Interface require 'util/object_storage/miq_s3_storage' + require 'util/object_storage/miq_ftp_storage' attr_accessor :settings attr_writer :logger diff --git a/lib/gems/pending/util/object_storage/miq_ftp_storage.rb b/lib/gems/pending/util/object_storage/miq_ftp_storage.rb new file mode 100644 index 00000000000..d2a97163038 --- /dev/null +++ b/lib/gems/pending/util/object_storage/miq_ftp_storage.rb @@ -0,0 +1,86 @@ +require 'util/miq_ftp_lib' +require 'util/miq_object_storage' +require 'logger' + +class MiqFtpStorage < MiqObjectStorage + include MiqFtpLib + + attr_reader :uri, :username, :password + + def self.uri_scheme + "ftp".freeze + end + + def self.new_with_opts(opts) + new(opts.slice(:uri, :username, :password)) + end + + def initialize(settings) + super + @uri = @settings[:uri] + @username = @settings[:username] + @password = @settings[:password] + end + + # Override for connection handling + def add(*upload_args) + with_connection { super } + end + + # Override for connection handling + def download(*download_args) + with_connection { super } + end + + # Specific version of Net::FTP#storbinary that doesn't use an existing local + # file, and only uploads a specific size (byte_count) from the input_file + def upload_single(dest_uri) + ftp.synchronize do + ftp.send(:with_binary, true) do + conn = ftp.send(:transfercmd, "STOR #{uri_to_relative(dest_uri)}") + IO.copy_stream(source_input, conn, byte_count) + conn.close + ftp.send(:voidresp) + end + end + dest_uri + rescue Errno::EPIPE + # EPIPE, in this case, means that the data connection was unexpectedly + # terminated. Rather than just raising EPIPE to the caller, check the + # response on the control connection. If getresp doesn't raise a more + # appropriate exception, re-raise the original exception. + ftp.send(:getresp) + raise + end + + def download_single(source, destination) + ftp.getbinaryfile(uri_to_relative(source), destination) + end + + def mkdir(dir) + create_directory_structure(uri_to_relative(dir)) + end + + private + + def login_credentials + [username, password].compact + end + + # Currently assumes you have just connected and are at the root logged in + # dir. Net::FTP (or ftp in general) doesn't seem to have a concept of a + # "root dir" based on your login, so this should be used right after + # `.connect`, or shortly there after. + # + # Or, you should be returning to the directory you came from prior to using + # this method again, or not using `ftp.chdir` at all. + def uri_to_relative(filepath) + result = URI.split(filepath)[5] + result = result[1..-1] if result[0] == "/".freeze + result + end + + def _log + logger + end +end diff --git a/spec/support/custom_matchers/exist_on_ftp_server.rb b/spec/support/custom_matchers/exist_on_ftp_server.rb new file mode 100644 index 00000000000..980f6c01940 --- /dev/null +++ b/spec/support/custom_matchers/exist_on_ftp_server.rb @@ -0,0 +1,52 @@ +# Assumes this in run in a :with_ftp_server context so an FTP server on +# localhost is available. +# +# See spec/support/with_ftp_server.rb for more info. +RSpec::Matchers.define :exist_on_ftp_server do + match do |actual| + !list_in_ftp(actual).empty? + end + + failure_message do |actual| + fail_msg(actual) + end + + failure_message_when_negated do |actual| + fail_msg(actual, :negated => true) + end + + def with_connection + Net::FTP.open("localhost") do |ftp| + ftp.login("ftpuser", "ftppass") + yield ftp + end + end + + # Do searches with Net::FTP instead of normal directory scan (even though we + # could) just so we are exercising the FTP interface as expected. + def list_in_ftp(file_or_dir) + with_connection do |ftp| + begin + ftp.nlst(to_path_string(file_or_dir)) + rescue Net::FTPPermError + [] + end + end + end + + def fail_msg(actual, negated: false) + dir = File.dirname(actual) + entries = list_in_ftp(dir) + exist = negated ? "not exist" : "exist" + <<~MSG + expected: #{to_path_string(actual)} to #{exist} in ftp directory" + + Entries for #{dir}: + #{entries.empty? ? " []" : entries.map { |e| " #{e}" }.join("\n")} + MSG + end + + def to_path_string(path) + path.try(:path) || URI.split(path.to_s)[5] + end +end diff --git a/spec/support/custom_matchers/have_size_on_ftp_server_of.rb b/spec/support/custom_matchers/have_size_on_ftp_server_of.rb new file mode 100644 index 00000000000..277aa6f5dde --- /dev/null +++ b/spec/support/custom_matchers/have_size_on_ftp_server_of.rb @@ -0,0 +1,30 @@ +# Assumes this in run in a :with_ftp_server context so an FTP server on +# localhost is available. +# +# See spec/support/with_ftp_server.rb for more info. +RSpec::Matchers.define :have_size_on_ftp_server_of do |expected| + match do |filepath| + size = size_on_ftp(filepath) + size == expected + end + + def with_connection + Net::FTP.open("localhost") do |ftp| + ftp.login("ftpuser", "ftppass") + yield ftp + end + end + + # Do searches with Net::FTP instead of normal directory scan (even though we + # could) just so we are exercising the FTP interface as expected. + def size_on_ftp(file_or_dir) + path = file_or_dir.try(:path) || URI.split(file_or_dir.to_s)[5] + with_connection do |ftp| + begin + ftp.size(path) + rescue Net::FTPPermError + 0 + end + end + end +end diff --git a/spec/util/miq_file_storage_spec.rb b/spec/util/miq_file_storage_spec.rb index add48b824d9..95852044918 100644 --- a/spec/util/miq_file_storage_spec.rb +++ b/spec/util/miq_file_storage_spec.rb @@ -1,4 +1,5 @@ require "util/mount/miq_generic_mount_session" +require "util/miq_object_storage" describe MiqFileStorage do def opts_for_nfs @@ -15,6 +16,10 @@ def opts_for_glusterfs opts[:uri] = "glusterfs://example.com/share/path/to/file.txt" end + def opts_for_ftp + opts[:uri] = "ftp://example.com/share/path/to/file.txt" + end + def opts_for_fakefs opts[:uri] = "foo://example.com/share/path/to/file.txt" end @@ -65,6 +70,12 @@ def opts_for_fakefs include_examples ".with_interface_class implementation", "MiqGlusterfsSession" end + context "with an ftp:// uri" do + before { opts_for_ftp } + + include_examples ".with_interface_class implementation", "MiqFtpStorage" + end + context "with an unknown uri scheme" do before { opts_for_fakefs } diff --git a/spec/util/object_storage/miq_ftp_storage_spec.rb b/spec/util/object_storage/miq_ftp_storage_spec.rb new file mode 100644 index 00000000000..188665d74e6 --- /dev/null +++ b/spec/util/object_storage/miq_ftp_storage_spec.rb @@ -0,0 +1,119 @@ +require 'util/object_storage/miq_ftp_storage.rb' + +describe MiqFtpStorage, :with_ftp_server do + subject { described_class.new(ftp_creds.merge(:uri => "ftp://localhost")) } + let(:ftp_creds) { { :username => "ftpuser", :password => "ftppass" } } + + describe "#add" do + include_context "generated tmp files" + + shared_examples "adding files" do |dest_path| + let(:dest_path) { dest_path } + + it "copies single files" do + expect(subject.add(source_path.to_s, dest_path.to_s)).to eq(dest_path.to_s) + expect(dest_path).to exist_on_ftp_server + expect(dest_path).to have_size_on_ftp_server_of(10.megabytes) + end + + it "copies file to splits" do + expected_splitfiles = (1..5).map do |suffix| + "#{dest_path}.0000#{suffix}" + end + + File.open(source_path) do |f| # with an IO object this time + subject.add(f, dest_path.to_s, "2M") + end + + expected_splitfiles.each do |filename| + expect(filename).to exist_on_ftp_server + expect(filename).to have_size_on_ftp_server_of(2.megabytes) + end + end + + it "can take input from a command" do + expected_splitfiles = (1..5).map do |suffix| + "#{dest_path}.0000#{suffix}" + end + + subject.add(dest_path.to_s, "2M") do |input_writer| + `#{Gem.ruby} -e "File.write('#{input_writer}', '0' * #{tmpfile_size})"` + end + + expected_splitfiles.each do |filename| + expect(filename).to exist_on_ftp_server + expect(filename).to have_size_on_ftp_server_of(2.megabytes) + end + end + + context "with slightly a slightly smaller input file than 10MB" do + let(:tmpfile_size) { 10.megabytes - 1.kilobyte } + + it "properly chunks the file" do + expected_splitfiles = (1..10).map do |suffix| + "#{dest_path}.%05d" % {:suffix => suffix} + end + + # using pathnames this time + subject.add(source_path, dest_path.to_s, 1.megabyte) + + expected_splitfiles[0, 9].each do |filename| + expect(filename).to exist_on_ftp_server + expect(filename).to have_size_on_ftp_server_of(1.megabytes) + end + + last_split = expected_splitfiles.last + expect(last_split).to exist_on_ftp_server + expect(last_split).to have_size_on_ftp_server_of(1.megabyte - 1.kilobyte) + end + end + end + + context "using a 'relative path'" do + include_examples "adding files", "path/to/file" + end + + context "using a 'absolute path'" do + include_examples "adding files", "/path/to/my_file" + end + + context "using a uri" do + include_examples "adding files", "ftp://localhost/foo/bar/baz" + end + end + + describe "#download" do + let(:dest_path) { Dir::Tmpname.create("") { |name| name } } + let(:source_file) { existing_ftp_file(10.megabytes) } + let(:source_path) { File.basename(source_file.path) } + + after { File.delete(dest_path) if File.exist?(dest_path) } + + it "downloads the file" do + subject.download(dest_path, source_path) + + # Sanity check that what we are downloading is the size we expect + expect(source_path).to exist_on_ftp_server + expect(source_path).to have_size_on_ftp_server_of(10.megabytes) + + expect(File.exist?(dest_path)).to be true + expect(File.stat(dest_path).size).to eq(10.megabytes) + end + + it "can take input from a command" do + source_data = nil + subject.download(nil, source_path) do |input_writer| + source_data = `#{Gem.ruby} -e "print File.read('#{input_writer}')"` + end + + # Sanity check that what we are downloading is the size we expect + # (and we didn't actually download the file to disk) + expect(File.exist?(dest_path)).to be false + expect(source_path).to exist_on_ftp_server + expect(source_path).to have_size_on_ftp_server_of(10.megabytes) + + # Nothing written, just printed the streamed file in the above command + expect(source_data.size).to eq(10.megabytes) + end + end +end From 5639e2006d638e4d2533fa1f973b176f701cf5f7 Mon Sep 17 00:00:00 2001 From: Jerry Keselman Date: Mon, 10 Sep 2018 10:02:49 -0400 Subject: [PATCH 21/55] DB Backups to Openstack Swift Add an Openstack Swift session object to allow backups to Swift. (transferred from ManageIQ/manageiq-gems-pending@ecf715f5075655fb7f0ae453f1360c67f27f644d) --- lib/gems/pending/util/mount/miq_generic_mount_session.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/gems/pending/util/mount/miq_generic_mount_session.rb b/lib/gems/pending/util/mount/miq_generic_mount_session.rb index b5bb8f81bef..c98a749360b 100644 --- a/lib/gems/pending/util/mount/miq_generic_mount_session.rb +++ b/lib/gems/pending/util/mount/miq_generic_mount_session.rb @@ -11,6 +11,8 @@ class MiqGenericMountSession < MiqFileStorage::Interface require 'util/mount/miq_local_mount_session' require 'util/mount/miq_nfs_session' + require 'util/mount/miq_s3_session' + require 'util/mount/miq_swift_session' require 'util/mount/miq_smb_session' require 'util/mount/miq_glusterfs_session' @@ -68,6 +70,10 @@ def self.uri_scheme_to_class(uri) require 'uri' scheme, userinfo, host, port, registry, share, opaque, query, fragment = URI.split(URI.encode(uri)) case scheme + when 's3' + MiqS3Session + when 'swift' + MiqSwiftSession when 'smb' MiqSmbSession when 'nfs' From e37e0bd3ea532549d9d8263804b062ec23205a40 Mon Sep 17 00:00:00 2001 From: Jerry Keselman Date: Thu, 20 Sep 2018 10:20:39 -0400 Subject: [PATCH 22/55] Fix Travis Failure S3 was moved over to the object file session rather than this mount session subclass by a separate PR, so adding it back in via this PR is incorrect and causes Travis build failures. (transferred from ManageIQ/manageiq-gems-pending@5c3ebdc9401123a275d62f4e5d836b7356e82c22) --- lib/gems/pending/util/mount/miq_generic_mount_session.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/gems/pending/util/mount/miq_generic_mount_session.rb b/lib/gems/pending/util/mount/miq_generic_mount_session.rb index c98a749360b..df7ff339169 100644 --- a/lib/gems/pending/util/mount/miq_generic_mount_session.rb +++ b/lib/gems/pending/util/mount/miq_generic_mount_session.rb @@ -11,7 +11,6 @@ class MiqGenericMountSession < MiqFileStorage::Interface require 'util/mount/miq_local_mount_session' require 'util/mount/miq_nfs_session' - require 'util/mount/miq_s3_session' require 'util/mount/miq_swift_session' require 'util/mount/miq_smb_session' require 'util/mount/miq_glusterfs_session' @@ -70,8 +69,6 @@ def self.uri_scheme_to_class(uri) require 'uri' scheme, userinfo, host, port, registry, share, opaque, query, fragment = URI.split(URI.encode(uri)) case scheme - when 's3' - MiqS3Session when 'swift' MiqSwiftSession when 'smb' From e6798763d4c0db8758018a6219f69206e6189c92 Mon Sep 17 00:00:00 2001 From: Jerry Keselman Date: Tue, 9 Oct 2018 16:48:09 -0400 Subject: [PATCH 23/55] Swift DB Backup Support with MiqObjectStorage Methodology Removing the swift class for the Mount Session, and adding a new swift class for Object Storage instead. This works with the streaming db backups/dumps now implemented. (transferred from ManageIQ/manageiq-gems-pending@f3a3f05b737e70fe9511026cf233032889b9c311) --- lib/gems/pending/util/miq_object_storage.rb | 33 ++++ .../util/mount/miq_generic_mount_session.rb | 1 - .../util/object_storage/miq_swift_storage.rb | 148 ++++++++++++++++++ 3 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 lib/gems/pending/util/object_storage/miq_swift_storage.rb diff --git a/lib/gems/pending/util/miq_object_storage.rb b/lib/gems/pending/util/miq_object_storage.rb index 6cb15873fa7..b9d2ad96c53 100644 --- a/lib/gems/pending/util/miq_object_storage.rb +++ b/lib/gems/pending/util/miq_object_storage.rb @@ -1,12 +1,16 @@ +require 'net/protocol' require 'util/miq_file_storage' class MiqObjectStorage < MiqFileStorage::Interface require 'util/object_storage/miq_s3_storage' require 'util/object_storage/miq_ftp_storage' + require 'util/object_storage/miq_swift_storage' attr_accessor :settings attr_writer :logger + DEFAULT_CHUNKSIZE = Net::BufferedIO::BUFSIZE + def initialize(settings) raise "URI missing" unless settings.key?(:uri) @settings = settings.dup @@ -15,4 +19,33 @@ def initialize(settings) def logger @logger ||= $log.nil? ? :: Logger.new(STDOUT) : $log end + + private + + DONE_READING = "" + def read_single_chunk(chunksize = DEFAULT_CHUNKSIZE) + @buf_left ||= byte_count + return DONE_READING unless @buf_left.nil? || @buf_left.positive? + cur_readsize = if (@buf_left.nil? || @buf_left - chunksize >= 0) + chunksize + else + @buf_left + end + buf = source_input.read(cur_readsize) + @buf_left -= chunksize if @buf_left + buf.to_s + end + + def write_single_split_file_for(file_io) + loop do + input_data = read_single_chunk + break if input_data.empty? + file_io.write(input_data) + end + clear_split_vars + end + + def clear_split_vars + @buf_left = nil + end end diff --git a/lib/gems/pending/util/mount/miq_generic_mount_session.rb b/lib/gems/pending/util/mount/miq_generic_mount_session.rb index df7ff339169..986bb1585a5 100644 --- a/lib/gems/pending/util/mount/miq_generic_mount_session.rb +++ b/lib/gems/pending/util/mount/miq_generic_mount_session.rb @@ -11,7 +11,6 @@ class MiqGenericMountSession < MiqFileStorage::Interface require 'util/mount/miq_local_mount_session' require 'util/mount/miq_nfs_session' - require 'util/mount/miq_swift_session' require 'util/mount/miq_smb_session' require 'util/mount/miq_glusterfs_session' diff --git a/lib/gems/pending/util/object_storage/miq_swift_storage.rb b/lib/gems/pending/util/object_storage/miq_swift_storage.rb new file mode 100644 index 00000000000..3ec6933fc15 --- /dev/null +++ b/lib/gems/pending/util/object_storage/miq_swift_storage.rb @@ -0,0 +1,148 @@ +require 'pp' +# require 'util/miq_object_storage' +require 'util/mount/miq_generic_mount_session' + +class MiqSwiftStorage < MiqObjectStorage + attr_reader :container_name + + def self.uri_scheme + "swift".freeze + end + + def self.new_with_opts(opts) + new(opts.slice(:uri, :username, :password)) + end + + def initialize(settings) + super(settings) + + # NOTE: This line to be removed once manageiq-ui-class region change implemented. + @bucket_name = URI(@settings[:uri]).host + + raise "username and password are required values!" if @settings[:username].nil? || @settings[:password].nil? + _scheme, _userinfo, @host, @port, _registry, @mount_path, _opaque, query, _fragment = URI.split(URI.encode(@settings[:uri])) + query_params(query) + @swift = nil + @username = @settings[:username] + @password = @settings[:password] + @container_name = @mount_path[0] == File::Separator ? @mount_path[1..-1] : @mount_path + end + + def uri_to_local_path(remote_file) + # Strip off the leading "swift:/" from the URI" + File.join(@mnt_point, URI(remote_file).host, URI(remote_file).path) + end + + def uri_to_object_path(remote_file) + # Strip off the leading "swift://" and the container name from the URI" + # Also remove teh leading delimiter. + object_file_with_bucket = URI.split(URI.encode(remote_file))[5] + object_file_with_bucket.split(File::Separator)[2..-1].join(File::Separator) + end + + def upload_single(dest_uri) + # + # Get the remote path, and parse out the bucket name. + # + object_file = uri_to_object_path(dest_uri) + # + # write dump file to swift + # + logger.debug("Writing [#{source_input}] to => Bucket [#{container_name}] using object file name [#{object_file}]") + begin + swift_file = container.files.new(:key => object_file) + params = { + :expects => [201, 202], + :headers => {}, + :request_block => -> { read_single_chunk }, + :idempotent => false, + :method => "PUT", + :path => "#{Fog::OpenStack.escape(swift_file.directory.key)}/#{Fog::OpenStack.escape(swift_file.key)}" + } + swift_file.service.send(:request, params) + clear_split_vars + rescue Excon::Errors::Unauthorized => err + logger.error("Access to Swift container #{@container_name} failed due to a bad username or password. #{err}") + msg = "Access to Swift container #{@container_name} failed due to a bad username or password. #{err}" + raise err, msg, err.backtrace + rescue => err + logger.error("Error uploading #{source_input} to Swift container #{@container_name}. #{err}") + msg = "Error uploading #{source_input} to Swift container #{@container_name}. #{err}" + raise err, msg, err.backtrace + end + end + + def mkdir(_dir) + container + end + + def container + @container ||= begin + container = swift.directories.get(container_name) + logger.debug("Swift container [#{container}] found") if container + container ||= create_container + container + rescue Fog::Storage::OpenStack::NotFound + logger.debug("Swift container #{container_name} does not exist. Creating.") + create_container + rescue => err + logger.error("Error getting Swift container #{container_name}. #{err}") + msg = "Error getting Swift container #{container_name}. #{err}" + raise err, msg, err.backtrace + end + end + + private + + def swift + return @swift if @swift + require 'manageiq/providers/openstack/legacy/openstack_handle' + extra_options = {} + extra_options[:domain_id] = @domain_id + extra_options[:service] = "Compute" + @osh ||= OpenstackHandle::Handle.new(@username, @password, @host, @port, @api_version, @security_protocol, extra_options) + @osh.connection_options = {:instrumentor => $fog_log} + begin + @swift ||= @osh.swift_service + rescue Excon::Errors::Unauthorized => err + logger.error("Access to Swift host #{@host} failed due to a bad username or password. #{err}") + msg = "Access to Swift host #{@host} failed due to a bad username or password. #{err}" + raise err, msg, err.backtrace + rescue => err + logger.error("Error connecting to Swift host #{@host}. #{err}") + msg = "Error connecting to Swift host #{@host}. #{err}" + raise err, msg, err.backtrace + end + end + + def create_container + container = swift.directories.create(:key => container_name) + logger.debug("Swift container [#{container}] created") + container + rescue => err + logger.error("Error creating Swift container #{container_name}. #{err}") + msg = "Error creating Swift container #{container_name}. #{err}" + raise err, msg, err.backtrace + end + + def download_single(source, destination) + object_key = @container_name + logger.debug("Downloading [#{source}] from bucket [#{bucket_name}] to local file [#{destination}]") + + with_standard_s3_error_handling("downloading", source) do + if destination.kind_of?(IO) + s3.client.get_object(:bucket => bucket_name, :key => object_key) do |chunk| + destination.write(chunk) + end + else # assume file path + bucket.object(source).download_file(destination) + end + end + local_file + end + + def query_params(query_string) + parts = URI.decode_www_form(query_string).to_h + @region, @api_version, @domain_id, @security_protocol = parts.values_at("region", "api_version", "domain_id", "security_protocol") + end +end From 7943183bb7a15413f4b466f6ac25dab8551975dd Mon Sep 17 00:00:00 2001 From: Jerry Keselman Date: Tue, 9 Oct 2018 17:02:53 -0400 Subject: [PATCH 24/55] Remove ref to $fog_log As per review comment don't use $fog_log here. (transferred from ManageIQ/manageiq-gems-pending@aa8f8394015a963a6f6778727e7c73800865bb63) --- lib/gems/pending/util/object_storage/miq_swift_storage.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/gems/pending/util/object_storage/miq_swift_storage.rb b/lib/gems/pending/util/object_storage/miq_swift_storage.rb index 3ec6933fc15..ee2387a48e6 100644 --- a/lib/gems/pending/util/object_storage/miq_swift_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_swift_storage.rb @@ -101,7 +101,6 @@ def swift extra_options[:domain_id] = @domain_id extra_options[:service] = "Compute" @osh ||= OpenstackHandle::Handle.new(@username, @password, @host, @port, @api_version, @security_protocol, extra_options) - @osh.connection_options = {:instrumentor => $fog_log} begin @swift ||= @osh.swift_service rescue Excon::Errors::Unauthorized => err From 64c4f0741e5e5d61bf7afef97523211b9545d39a Mon Sep 17 00:00:00 2001 From: Jerry Keselman Date: Wed, 10 Oct 2018 16:54:12 -0400 Subject: [PATCH 25/55] Review Comments Review comments by various reviews. Some nitpicks but the major change is to not use Openstack Provider handle code but instead use Fog::Openstack. (transferred from ManageIQ/manageiq-gems-pending@49ccf0f07b48dc7aef0b8254716ad6dc9391990f) --- lib/gems/pending/util/miq_object_storage.rb | 16 ++--- .../util/mount/miq_generic_mount_session.rb | 2 - .../util/object_storage/miq_swift_storage.rb | 58 ++++++++++++------- 3 files changed, 44 insertions(+), 32 deletions(-) diff --git a/lib/gems/pending/util/miq_object_storage.rb b/lib/gems/pending/util/miq_object_storage.rb index b9d2ad96c53..94f6d1b2b67 100644 --- a/lib/gems/pending/util/miq_object_storage.rb +++ b/lib/gems/pending/util/miq_object_storage.rb @@ -24,15 +24,15 @@ def logger DONE_READING = "" def read_single_chunk(chunksize = DEFAULT_CHUNKSIZE) - @buf_left ||= byte_count + @buf_left ||= byte_count return DONE_READING unless @buf_left.nil? || @buf_left.positive? - cur_readsize = if (@buf_left.nil? || @buf_left - chunksize >= 0) - chunksize - else - @buf_left - end - buf = source_input.read(cur_readsize) - @buf_left -= chunksize if @buf_left + cur_readsize = if (@buf_left.nil? || @buf_left - chunksize >= 0) + chunksize + else + @buf_left + end + buf = source_input.read(cur_readsize) + @buf_left -= chunksize if @buf_left buf.to_s end diff --git a/lib/gems/pending/util/mount/miq_generic_mount_session.rb b/lib/gems/pending/util/mount/miq_generic_mount_session.rb index 986bb1585a5..b5bb8f81bef 100644 --- a/lib/gems/pending/util/mount/miq_generic_mount_session.rb +++ b/lib/gems/pending/util/mount/miq_generic_mount_session.rb @@ -68,8 +68,6 @@ def self.uri_scheme_to_class(uri) require 'uri' scheme, userinfo, host, port, registry, share, opaque, query, fragment = URI.split(URI.encode(uri)) case scheme - when 'swift' - MiqSwiftSession when 'smb' MiqSmbSession when 'nfs' diff --git a/lib/gems/pending/util/object_storage/miq_swift_storage.rb b/lib/gems/pending/util/object_storage/miq_swift_storage.rb index ee2387a48e6..7982ae9e191 100644 --- a/lib/gems/pending/util/object_storage/miq_swift_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_swift_storage.rb @@ -35,7 +35,7 @@ def uri_to_local_path(remote_file) def uri_to_object_path(remote_file) # Strip off the leading "swift://" and the container name from the URI" - # Also remove teh leading delimiter. + # Also remove the leading delimiter. object_file_with_bucket = URI.split(URI.encode(remote_file))[5] object_file_with_bucket.split(File::Separator)[2..-1].join(File::Separator) end @@ -62,12 +62,12 @@ def upload_single(dest_uri) swift_file.service.send(:request, params) clear_split_vars rescue Excon::Errors::Unauthorized => err - logger.error("Access to Swift container #{@container_name} failed due to a bad username or password. #{err}") msg = "Access to Swift container #{@container_name} failed due to a bad username or password. #{err}" + logger.error(msg) raise err, msg, err.backtrace rescue => err - logger.error("Error uploading #{source_input} to Swift container #{@container_name}. #{err}") msg = "Error uploading #{source_input} to Swift container #{@container_name}. #{err}" + logger.error(msg) raise err, msg, err.backtrace end end @@ -76,6 +76,16 @@ def mkdir(_dir) container end + + # + # Some calls to Fog::Storage::OpenStack::Directories#get will + # return 'nil', and not return an error. This would cause errors down the + # line. + # + # Instead of investigating further, we created a new method that is in charge of + # OpenStack container creation, and that is called from '#container' if 'nil' is + # returned, or the error for 'NotFound' is called to cover both cases. + # def container @container ||= begin container = swift.directories.get(container_name) @@ -86,8 +96,8 @@ def container logger.debug("Swift container #{container_name} does not exist. Creating.") create_container rescue => err - logger.error("Error getting Swift container #{container_name}. #{err}") msg = "Error getting Swift container #{container_name}. #{err}" + logger.error(msg) raise err, msg, err.backtrace end end @@ -96,31 +106,35 @@ def container def swift return @swift if @swift - require 'manageiq/providers/openstack/legacy/openstack_handle' - extra_options = {} - extra_options[:domain_id] = @domain_id - extra_options[:service] = "Compute" - @osh ||= OpenstackHandle::Handle.new(@username, @password, @host, @port, @api_version, @security_protocol, extra_options) - begin - @swift ||= @osh.swift_service - rescue Excon::Errors::Unauthorized => err - logger.error("Access to Swift host #{@host} failed due to a bad username or password. #{err}") - msg = "Access to Swift host #{@host} failed due to a bad username or password. #{err}" - raise err, msg, err.backtrace - rescue => err - logger.error("Error connecting to Swift host #{@host}. #{err}") - msg = "Error connecting to Swift host #{@host}. #{err}" - raise err, msg, err.backtrace - end + require 'fog/openstack' + + auth_url = URI::Generic.build( + :scheme => @security_protocol == 'non-ssl' ? "http" : "https", + :host => @host, + :port => @port.to_i, + :path => "/#{@api_version}#{@api_version == "v3" ? "/auth" : ".0"}/tokens" + ).to_s + + connection_params = { + :openstack_auth_url => auth_url, + :openstack_username => @username, + :openstack_api_key => @password, + :openstack_project_domain_id => @domain_id, + :openstack_user_domain_id => @domain_id, + :openstack_region => @region, + :connection_options => { :debug_request => true } + } + + @swift = Fog::Storage::OpenStack.new(connection_params) end def create_container container = swift.directories.create(:key => container_name) - logger.debug("Swift container [#{container}] created") + logger.debug("Swift container [#{container_name}] created") container rescue => err - logger.error("Error creating Swift container #{container_name}. #{err}") msg = "Error creating Swift container #{container_name}. #{err}" + logger.error(msg) raise err, msg, err.backtrace end From a4dad52c1fa903d9278b3c21742cf02eecd83a10 Mon Sep 17 00:00:00 2001 From: Jerry Keselman Date: Thu, 11 Oct 2018 15:44:37 -0400 Subject: [PATCH 26/55] Initial Set of MiqSwiftStorage Tests and Stub #download_single. Major and minor changes - some indentation, some new tests (thanks Nick L) Stubbed out download_single method since it isn't fully working yet. (transferred from ManageIQ/manageiq-gems-pending@11cb304dd8fca77979bbdd3592f999f384ae0f6b) --- .../util/object_storage/miq_swift_storage.rb | 38 +++--- spec/support/contexts/generated_tmp_files.rb | 13 +- spec/util/miq_file_storage_spec.rb | 21 +++ spec/util/miq_object_storage_spec.rb | 121 ++++++++++++++++++ 4 files changed, 170 insertions(+), 23 deletions(-) create mode 100644 spec/util/miq_object_storage_spec.rb diff --git a/lib/gems/pending/util/object_storage/miq_swift_storage.rb b/lib/gems/pending/util/object_storage/miq_swift_storage.rb index 7982ae9e191..b128656503b 100644 --- a/lib/gems/pending/util/object_storage/miq_swift_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_swift_storage.rb @@ -1,6 +1,5 @@ require 'pp' -# require 'util/miq_object_storage' -require 'util/mount/miq_generic_mount_session' +require 'util/miq_object_storage' class MiqSwiftStorage < MiqObjectStorage attr_reader :container_name @@ -21,7 +20,7 @@ def initialize(settings) raise "username and password are required values!" if @settings[:username].nil? || @settings[:password].nil? _scheme, _userinfo, @host, @port, _registry, @mount_path, _opaque, query, _fragment = URI.split(URI.encode(@settings[:uri])) - query_params(query) + query_params(query) if query @swift = nil @username = @settings[:username] @password = @settings[:password] @@ -80,21 +79,28 @@ def mkdir(_dir) # # Some calls to Fog::Storage::OpenStack::Directories#get will # return 'nil', and not return an error. This would cause errors down the - # line. + # line in '#upload' or '#download'. # # Instead of investigating further, we created a new method that is in charge of # OpenStack container creation, and that is called from '#container' if 'nil' is - # returned, or the error for 'NotFound' is called to cover both cases. + # returned from 'swift.directories.get(container_name)', or call '#create_container' + # in the rescue case for 'NotFound' to cover that scenario as well # - def container + def container(create_if_missing = true) @container ||= begin container = swift.directories.get(container_name) logger.debug("Swift container [#{container}] found") if container container ||= create_container container rescue Fog::Storage::OpenStack::NotFound - logger.debug("Swift container #{container_name} does not exist. Creating.") - create_container + if create_if_missing + logger.debug("Swift container #{container_name} does not exist. Creating.") + create_container + else + msg = ("Swift container #{container_name} does not exist. #{err}") + logger.error(msg) + raise err, msg, err.backtrace + end rescue => err msg = "Error getting Swift container #{container_name}. #{err}" logger.error(msg) @@ -138,20 +144,8 @@ def create_container raise err, msg, err.backtrace end - def download_single(source, destination) - object_key = @container_name - logger.debug("Downloading [#{source}] from bucket [#{bucket_name}] to local file [#{destination}]") - - with_standard_s3_error_handling("downloading", source) do - if destination.kind_of?(IO) - s3.client.get_object(:bucket => bucket_name, :key => object_key) do |chunk| - destination.write(chunk) - end - else # assume file path - bucket.object(source).download_file(destination) - end - end - local_file + def download_single(_source, _destination) + raise NotImplementedError, "MiqSwiftStorage.download_single Not Yet Implemented" end def query_params(query_string) diff --git a/spec/support/contexts/generated_tmp_files.rb b/spec/support/contexts/generated_tmp_files.rb index 26426ba3fa8..8a94e313caa 100644 --- a/spec/support/contexts/generated_tmp_files.rb +++ b/spec/support/contexts/generated_tmp_files.rb @@ -9,9 +9,20 @@ end after do + # When source_file.unlink is called, it will make it so `source_file.path` + # returns `nil`. Cache it's value incase it hasn't been accessed in the + # tests so we can clear out the generated files properly. + _source_path = source_path + source_file.unlink - Dir["#{source_path.expand_path}.*"].each do |file| + Dir["#{_source_path.expand_path}.*"].each do |file| File.delete(file) end + + if defined?(dest_path) && dest_path.to_s.include?(Dir.tmpdir) + Dir["#{dest_path}*"].each do |file| + File.delete(file) + end + end end end diff --git a/spec/util/miq_file_storage_spec.rb b/spec/util/miq_file_storage_spec.rb index 95852044918..564007bc59e 100644 --- a/spec/util/miq_file_storage_spec.rb +++ b/spec/util/miq_file_storage_spec.rb @@ -20,6 +20,18 @@ def opts_for_ftp opts[:uri] = "ftp://example.com/share/path/to/file.txt" end + def opts_for_swift_without_params + opts[:uri] = "swift://example.com/share/path/to/file.txt" + opts[:username] = "user" + opts[:password] = "pass" + end + + def opts_for_swift_with_params + opts[:uri] = "swift://example.com/share/path/to/file.txt?region=foo" + opts[:username] = "user" + opts[:password] = "pass" + end + def opts_for_fakefs opts[:uri] = "foo://example.com/share/path/to/file.txt" end @@ -76,6 +88,15 @@ def opts_for_fakefs include_examples ".with_interface_class implementation", "MiqFtpStorage" end + context "with an swift:// uri" do + before { opts_for_swift_with_params } + include_examples ".with_interface_class implementation", "MiqSwiftStorage" + end + context "with an swift:// uri and no query params" do + before { opts_for_swift_without_params } + include_examples ".with_interface_class implementation", "MiqSwiftStorage" + end + context "with an unknown uri scheme" do before { opts_for_fakefs } diff --git a/spec/util/miq_object_storage_spec.rb b/spec/util/miq_object_storage_spec.rb new file mode 100644 index 00000000000..94830b63041 --- /dev/null +++ b/spec/util/miq_object_storage_spec.rb @@ -0,0 +1,121 @@ +require "fileutils" +require "util/mount/miq_generic_mount_session" +require "util/miq_object_storage" + +class MockLocalFileStorage < MiqObjectStorage + def initialize source_path = nil, byte_count = 2.megabytes + @byte_count = byte_count + @source_input = File.open(source_path, "rb") if source_path + @root_dir = Dir.tmpdir + end + + def mkdir(dir) + FileUtils.mkdir_p(File.join(@root_dir, dir)) + end +end + +describe MiqObjectStorage do + describe "#write_single_split_file_for (private)" do + include_context "generated tmp files" + + subject { MockLocalFileStorage.new source_path } + let(:dest_path) { Dir::Tmpname.create("") { |name| name } } + + it "copies file to splits" do + expected_splitfiles = (1..5).map do |suffix| + "#{dest_path}.0000#{suffix}" + end + + expected_splitfiles.each do |file| + subject.send(:write_single_split_file_for, File.open(file, "wb")) + end + + expected_splitfiles.each do |filename| + expect(File.exist?(filename)).to be true + expect(Pathname.new(filename).lstat.size).to eq(2.megabytes) + end + end + + context "with slightly a slightly smaller input file than 10MB" do + let(:tmpfile_size) { 10.megabytes - 1.kilobyte } + subject { MockLocalFileStorage.new source_path, 1.megabyte } + + it "properly chunks the file" do + expected_splitfiles = (1..10).map do |suffix| + "#{dest_path}.%05d" % {:suffix => suffix} + end + + expected_splitfiles.each do |file| + subject.send(:write_single_split_file_for, File.open(file, "wb")) + end + + expected_splitfiles[0, 9].each do |filename| + expect(File.exist?(filename)).to be true + expect(File.size(filename)).to eq(1.megabytes) + end + + last_split = expected_splitfiles.last + expect(File.exist?(last_split)).to be true + expect(File.size(last_split)).to eq(1.megabyte - 1.kilobyte) + end + end + + context "non-split files (byte_count == nil)" do + subject { MockLocalFileStorage.new source_path, byte_count } + + let(:byte_count) { nil } + + it "streams the whole file over" do + subject.send(:write_single_split_file_for, File.open(dest_path, "wb")) + expect(File.exist?(dest_path)).to be true + expect(Pathname.new(dest_path).lstat.size).to eq(tmpfile_size) + end + end + end + + describe "#read_single_chunk (private)" do + include_context "generated tmp files" + + subject { MockLocalFileStorage.new source_path } + let(:dest_path) { Dir::Tmpname.create("") { |name| name } } + let(:chunksize) { MockLocalFileStorage::DEFAULT_CHUNKSIZE } + + it "reads 16384 by default" do + chunk_of_data = subject.send(:read_single_chunk) + expect(chunk_of_data).to eq("0" * chunksize) + end + + it "reads the amount of data equal to chunksize when that is passed" do + chunk_of_data = subject.send(:read_single_chunk, 1.kilobyte) + expect(chunk_of_data).to eq("0" * 1.kilobyte) + end + + context "near the end of the split file" do + let(:data_left) { 123 } + let(:penultimate_chunkize) { chunksize - data_left } + + before do + # read an odd amount of data + read_times = 2.megabytes / chunksize + (read_times - 1).times { subject.send(:read_single_chunk) } + subject.send(:read_single_chunk, penultimate_chunkize) + end + + it "reads only what is necessary to finish the split file" do + chunk_of_data = subject.send(:read_single_chunk) + expect(chunk_of_data).to eq("0" * data_left) + end + + it "stops reading until `#clear_split_vars` is called" do + expect(subject.send(:read_single_chunk)).to eq("0" * data_left) + expect(subject.send(:read_single_chunk)).to eq("") + expect(subject.send(:read_single_chunk)).to eq("") + expect(subject.send(:read_single_chunk)).to eq("") + + subject.send(:clear_split_vars) + + expect(subject.send(:read_single_chunk)).to eq("0" * chunksize) + end + end + end +end From 7d06a80f3008d8326bbc7a4713da261953401015 Mon Sep 17 00:00:00 2001 From: Jerry Keselman Date: Fri, 12 Oct 2018 15:04:13 -0400 Subject: [PATCH 27/55] Lots of spacing and comment changes brought up in review. Fixed spacing, parentheses, and reworded and added some comments. (transferred from ManageIQ/manageiq-gems-pending@c8730ec274ec14a7e9443c7b06c16e06afb84c10) --- lib/gems/pending/util/miq_object_storage.rb | 4 +-- .../util/object_storage/miq_swift_storage.rb | 27 ++++++++++++------- spec/util/miq_file_storage_spec.rb | 9 ++++--- spec/util/miq_object_storage_spec.rb | 3 +-- 4 files changed, 27 insertions(+), 16 deletions(-) diff --git a/lib/gems/pending/util/miq_object_storage.rb b/lib/gems/pending/util/miq_object_storage.rb index 94f6d1b2b67..d859cec95a3 100644 --- a/lib/gems/pending/util/miq_object_storage.rb +++ b/lib/gems/pending/util/miq_object_storage.rb @@ -22,11 +22,11 @@ def logger private - DONE_READING = "" + DONE_READING = "".freeze def read_single_chunk(chunksize = DEFAULT_CHUNKSIZE) @buf_left ||= byte_count return DONE_READING unless @buf_left.nil? || @buf_left.positive? - cur_readsize = if (@buf_left.nil? || @buf_left - chunksize >= 0) + cur_readsize = if @buf_left.nil? || @buf_left - chunksize >= 0 chunksize else @buf_left diff --git a/lib/gems/pending/util/object_storage/miq_swift_storage.rb b/lib/gems/pending/util/object_storage/miq_swift_storage.rb index b128656503b..f3083f06980 100644 --- a/lib/gems/pending/util/object_storage/miq_swift_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_swift_storage.rb @@ -1,4 +1,3 @@ -require 'pp' require 'util/miq_object_storage' class MiqSwiftStorage < MiqObjectStorage @@ -16,7 +15,7 @@ def initialize(settings) super(settings) # NOTE: This line to be removed once manageiq-ui-class region change implemented. - @bucket_name = URI(@settings[:uri]).host + @bucket_name = URI(@settings[:uri]).host raise "username and password are required values!" if @settings[:username].nil? || @settings[:password].nil? _scheme, _userinfo, @host, @port, _registry, @mount_path, _opaque, query, _fragment = URI.split(URI.encode(@settings[:uri])) @@ -58,6 +57,16 @@ def upload_single(dest_uri) :method => "PUT", :path => "#{Fog::OpenStack.escape(swift_file.directory.key)}/#{Fog::OpenStack.escape(swift_file.key)}" } + # + # Because of how `Fog::OpenStack` (and probably `Fog::Core`) is designed, + # it has hidden the functionality to provide a block for streaming uploads + # that is available out of the box with Excon. + # + # we use .send here because #request is private + # we can't use #put_object (public) directly because it doesn't allow a 202 response code, + # which is what swift responds with when we pass it the :request_block + # (This allows us to stream the response in chunks) + # swift_file.service.send(:request, params) clear_split_vars rescue Excon::Errors::Unauthorized => err @@ -75,29 +84,29 @@ def mkdir(_dir) container end - # # Some calls to Fog::Storage::OpenStack::Directories#get will # return 'nil', and not return an error. This would cause errors down the # line in '#upload' or '#download'. # # Instead of investigating further, we created a new method that is in charge of - # OpenStack container creation, and that is called from '#container' if 'nil' is - # returned from 'swift.directories.get(container_name)', or call '#create_container' - # in the rescue case for 'NotFound' to cover that scenario as well + # OpenStack container creation, '#create_container', and that is called from '#container' + # if 'nil' is returned from 'swift.directories.get(container_name)', or in the rescue case + # for 'NotFound' to cover that scenario as well # + def container(create_if_missing = true) @container ||= begin - container = swift.directories.get(container_name) + container = swift.directories.get(container_name) logger.debug("Swift container [#{container}] found") if container - container ||= create_container + unless container raise Fog::Storage::OpenStack::NotFound container rescue Fog::Storage::OpenStack::NotFound if create_if_missing logger.debug("Swift container #{container_name} does not exist. Creating.") create_container else - msg = ("Swift container #{container_name} does not exist. #{err}") + msg = "Swift container #{container_name} does not exist. #{err}" logger.error(msg) raise err, msg, err.backtrace end diff --git a/spec/util/miq_file_storage_spec.rb b/spec/util/miq_file_storage_spec.rb index 564007bc59e..420ea90e9ed 100644 --- a/spec/util/miq_file_storage_spec.rb +++ b/spec/util/miq_file_storage_spec.rb @@ -90,11 +90,14 @@ def opts_for_fakefs context "with an swift:// uri" do before { opts_for_swift_with_params } - include_examples ".with_interface_class implementation", "MiqSwiftStorage" + + include_examples ".with_interface_class implementation", "MiqSwiftStorage" end - context "with an swift:// uri and no query params" do + + context "with an swift:// uri and no query params" do before { opts_for_swift_without_params } - include_examples ".with_interface_class implementation", "MiqSwiftStorage" + + include_examples ".with_interface_class implementation", "MiqSwiftStorage" end context "with an unknown uri scheme" do diff --git a/spec/util/miq_object_storage_spec.rb b/spec/util/miq_object_storage_spec.rb index 94830b63041..901390549bc 100644 --- a/spec/util/miq_object_storage_spec.rb +++ b/spec/util/miq_object_storage_spec.rb @@ -3,7 +3,7 @@ require "util/miq_object_storage" class MockLocalFileStorage < MiqObjectStorage - def initialize source_path = nil, byte_count = 2.megabytes + def initialize(source_path = nil, byte_count = 2.megabytes) @byte_count = byte_count @source_input = File.open(source_path, "rb") if source_path @root_dir = Dir.tmpdir @@ -62,7 +62,6 @@ def mkdir(dir) context "non-split files (byte_count == nil)" do subject { MockLocalFileStorage.new source_path, byte_count } - let(:byte_count) { nil } it "streams the whole file over" do From cf6331e05dc3bcd91bf3a092a7841fe6eca8918b Mon Sep 17 00:00:00 2001 From: Jerry Keselman Date: Mon, 15 Oct 2018 14:57:16 -0400 Subject: [PATCH 28/55] Remove #uri_to_local_path and add initial tests for MiqSwiftStorage Review comments, adding some tests, and removing unused uri_to_local_path method. (transferred from ManageIQ/manageiq-gems-pending@2e8c9e8f293e96fb6dc62bac529292a621e387b3) --- lib/gems/pending/util/miq_object_storage.rb | 2 +- .../util/object_storage/miq_swift_storage.rb | 7 +------ spec/support/contexts/generated_tmp_files.rb | 4 ++-- .../object_storage/miq_swift_storage_spec.rb | 18 ++++++++++++++++++ 4 files changed, 22 insertions(+), 9 deletions(-) create mode 100644 spec/util/object_storage/miq_swift_storage_spec.rb diff --git a/lib/gems/pending/util/miq_object_storage.rb b/lib/gems/pending/util/miq_object_storage.rb index d859cec95a3..1f62998c684 100644 --- a/lib/gems/pending/util/miq_object_storage.rb +++ b/lib/gems/pending/util/miq_object_storage.rb @@ -25,7 +25,7 @@ def logger DONE_READING = "".freeze def read_single_chunk(chunksize = DEFAULT_CHUNKSIZE) @buf_left ||= byte_count - return DONE_READING unless @buf_left.nil? || @buf_left.positive? + return DONE_READING.dup unless @buf_left.nil? || @buf_left.positive? cur_readsize = if @buf_left.nil? || @buf_left - chunksize >= 0 chunksize else diff --git a/lib/gems/pending/util/object_storage/miq_swift_storage.rb b/lib/gems/pending/util/object_storage/miq_swift_storage.rb index f3083f06980..a4d4a19c319 100644 --- a/lib/gems/pending/util/object_storage/miq_swift_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_swift_storage.rb @@ -26,11 +26,6 @@ def initialize(settings) @container_name = @mount_path[0] == File::Separator ? @mount_path[1..-1] : @mount_path end - def uri_to_local_path(remote_file) - # Strip off the leading "swift:/" from the URI" - File.join(@mnt_point, URI(remote_file).host, URI(remote_file).path) - end - def uri_to_object_path(remote_file) # Strip off the leading "swift://" and the container name from the URI" # Also remove the leading delimiter. @@ -99,7 +94,7 @@ def container(create_if_missing = true) @container ||= begin container = swift.directories.get(container_name) logger.debug("Swift container [#{container}] found") if container - unless container raise Fog::Storage::OpenStack::NotFound + raise Fog::Storage::OpenStack::NotFound unless container container rescue Fog::Storage::OpenStack::NotFound if create_if_missing diff --git a/spec/support/contexts/generated_tmp_files.rb b/spec/support/contexts/generated_tmp_files.rb index 8a94e313caa..2300e5e69cf 100644 --- a/spec/support/contexts/generated_tmp_files.rb +++ b/spec/support/contexts/generated_tmp_files.rb @@ -12,10 +12,10 @@ # When source_file.unlink is called, it will make it so `source_file.path` # returns `nil`. Cache it's value incase it hasn't been accessed in the # tests so we can clear out the generated files properly. - _source_path = source_path + tmp_source_path = source_path source_file.unlink - Dir["#{_source_path.expand_path}.*"].each do |file| + Dir["#{tmp_source_path.expand_path}.*"].each do |file| File.delete(file) end diff --git a/spec/util/object_storage/miq_swift_storage_spec.rb b/spec/util/object_storage/miq_swift_storage_spec.rb new file mode 100644 index 00000000000..74f302687ce --- /dev/null +++ b/spec/util/object_storage/miq_swift_storage_spec.rb @@ -0,0 +1,18 @@ +require "util/object_storage/miq_swift_storage" + +describe MiqSwiftStorage do + before(:each) do + @uri = "swift://foo.com/abc/def" + @object_storage = described_class.new(:uri => @uri, :username => 'user', :password => 'pass', :region => 'region') + end + + it "#initialize sets the container_name" do + container_name = @object_storage.container_name + expect(container_name).to eq("abc/def") + end + + it "#uri_to_object_path returns a new object path" do + result = @object_storage.uri_to_object_path(@uri) + expect(result).to eq("def") + end +end From 18e9eb3982d0266f720c35bccfd42cceb5731734 Mon Sep 17 00:00:00 2001 From: Jerry Keselman Date: Tue, 16 Oct 2018 16:40:16 -0400 Subject: [PATCH 29/55] Make #auth_url a private Method separate from #swift. In order to test the private mechanisms within more easily, pull the auth_url construction out of the private method #swift. Add tests for it as well as for #uri_to_object_path, and to make sure #initialize sets the container name correctly. Add fog-openstack and mime-types to the gemspec. (transferred from ManageIQ/manageiq-gems-pending@47dd52c69b2ec5693e35064202316327146b210f) --- .../util/object_storage/miq_swift_storage.rb | 14 ++--- .../object_storage/miq_swift_storage_spec.rb | 63 ++++++++++++++++--- 2 files changed, 60 insertions(+), 17 deletions(-) diff --git a/lib/gems/pending/util/object_storage/miq_swift_storage.rb b/lib/gems/pending/util/object_storage/miq_swift_storage.rb index a4d4a19c319..13263a20aac 100644 --- a/lib/gems/pending/util/object_storage/miq_swift_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_swift_storage.rb @@ -13,8 +13,6 @@ def self.new_with_opts(opts) def initialize(settings) super(settings) - - # NOTE: This line to be removed once manageiq-ui-class region change implemented. @bucket_name = URI(@settings[:uri]).host raise "username and password are required values!" if @settings[:username].nil? || @settings[:password].nil? @@ -114,16 +112,18 @@ def container(create_if_missing = true) private - def swift - return @swift if @swift - require 'fog/openstack' - - auth_url = URI::Generic.build( + def auth_url + URI::Generic.build( :scheme => @security_protocol == 'non-ssl' ? "http" : "https", :host => @host, :port => @port.to_i, :path => "/#{@api_version}#{@api_version == "v3" ? "/auth" : ".0"}/tokens" ).to_s + end + + def swift + return @swift if @swift + require 'fog/openstack' connection_params = { :openstack_auth_url => auth_url, diff --git a/spec/util/object_storage/miq_swift_storage_spec.rb b/spec/util/object_storage/miq_swift_storage_spec.rb index 74f302687ce..ac16d7e2b71 100644 --- a/spec/util/object_storage/miq_swift_storage_spec.rb +++ b/spec/util/object_storage/miq_swift_storage_spec.rb @@ -1,18 +1,61 @@ require "util/object_storage/miq_swift_storage" describe MiqSwiftStorage do - before(:each) do - @uri = "swift://foo.com/abc/def" - @object_storage = described_class.new(:uri => @uri, :username => 'user', :password => 'pass', :region => 'region') - end + let(:object_storage) { described_class.new(:uri => uri, :username => 'user', :password => 'pass') } + + context "using a uri without query parameters" do + let(:uri) { "swift://foo.com/abc/def" } + + it "#initialize sets the container_name" do + container_name = object_storage.container_name + expect(container_name).to eq("abc/def") + end - it "#initialize sets the container_name" do - container_name = @object_storage.container_name - expect(container_name).to eq("abc/def") + it "#uri_to_object_path returns a new object path" do + result = object_storage.uri_to_object_path(uri) + expect(result).to eq("def") + end end - it "#uri_to_object_path returns a new object path" do - result = @object_storage.uri_to_object_path(@uri) - expect(result).to eq("def") + describe "#auth_url (private)" do + context "with non-ssl security protocol" do + let(:uri) { "swift://foo.com:5678/abc/def?region=region&api_version=v3&security_protocol=non-ssl" } + + it "sets the scheme to http" do + expect(URI(object_storage.send(:auth_url)).scheme).to eq("http") + end + + it "sets the host to foo.com" do + expect(URI(object_storage.send(:auth_url)).host).to eq("foo.com") + end + end + + context "with ssl security protocol" do + let(:uri) { "swift://foo.com:5678/abc/def?region=region&api_version=v3&security_protocol=ssl" } + + it "sets the scheme to https" do + expect(URI(object_storage.send(:auth_url)).scheme).to eq("https") + end + + it "sets the host to foo.com" do + expect(URI(object_storage.send(:auth_url)).host).to eq("foo.com") + end + end + + context "with v3 api version" do + let(:uri) { "swift://foo.com:5678/abc/def?region=region&api_version=v3&security_protocol=ssl" } + + it "sets the path to a v3 path" do + expect(URI(object_storage.send(:auth_url)).path).to eq("/v3/auth/tokens") + end + end + + context "with v2 api version" do + let(:uri) { "swift://foo.com:5678/abc/def?region=region&api_version=v2&security_protocol=ssl" } + + it "sets the path to a v2 path" do + expect(URI(object_storage.send(:auth_url)).path).to eq("/v2.0/tokens") + end + end end end From b11688a45eb51f796edd7b970a207f5751484d1f Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Wed, 31 Oct 2018 17:54:31 -0500 Subject: [PATCH 30/55] [MiqS3Storage] Bug: remote_file -> source Wrong variable name. (transferred from ManageIQ/manageiq-gems-pending@0c1198f96d5f4f1e6d918645aedb6cf56f8e9d91) --- lib/gems/pending/util/object_storage/miq_s3_storage.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gems/pending/util/object_storage/miq_s3_storage.rb b/lib/gems/pending/util/object_storage/miq_s3_storage.rb index 06cb665114a..adf85ebad1c 100644 --- a/lib/gems/pending/util/object_storage/miq_s3_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_s3_storage.rb @@ -40,7 +40,7 @@ def upload_single(dest_uri) end def download_single(source, destination) - object_key = uri_to_object_key(remote_file) + object_key = uri_to_object_key(source) logger.debug("Downloading [#{source}] from bucket [#{bucket_name}] to local file [#{destination}]") with_standard_s3_error_handling("downloading", source) do From 5ad8013b9bc0f1f17b902c82ea00b8f30103c2ed Mon Sep 17 00:00:00 2001 From: Jerry Keselman Date: Wed, 17 Oct 2018 11:26:58 -0400 Subject: [PATCH 31/55] Add tests to make sure Query String is empty in auth_url (transferred from ManageIQ/manageiq-gems-pending@518f48fe467e6283112dd4a713067b30982290a5) --- spec/util/object_storage/miq_swift_storage_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/util/object_storage/miq_swift_storage_spec.rb b/spec/util/object_storage/miq_swift_storage_spec.rb index ac16d7e2b71..248a61cf5c9 100644 --- a/spec/util/object_storage/miq_swift_storage_spec.rb +++ b/spec/util/object_storage/miq_swift_storage_spec.rb @@ -28,6 +28,10 @@ it "sets the host to foo.com" do expect(URI(object_storage.send(:auth_url)).host).to eq("foo.com") end + + it "unsets the query string" do + expect(URI(object_storage.send(:auth_url)).query).to eq(nil) + end end context "with ssl security protocol" do @@ -40,6 +44,10 @@ it "sets the host to foo.com" do expect(URI(object_storage.send(:auth_url)).host).to eq("foo.com") end + + it "unsets the query string" do + expect(URI(object_storage.send(:auth_url)).query).to eq(nil) + end end context "with v3 api version" do From 122bd1da9317e00d8ef2b64fb914034867d85f48 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Wed, 31 Oct 2018 17:55:12 -0500 Subject: [PATCH 32/55] [MiqS3Storage] Bug: Remove local_file return val For `MiqS3Storage#download_single`, the `local_file` local variable doesn't exist, and most likely was copied over from the `#add` method. A return value for this method is currently not needed or used, so this simply removes it. (transferred from ManageIQ/manageiq-gems-pending@86cb97a8e59e9c94a0e3531c4332170cdc875c76) --- lib/gems/pending/util/object_storage/miq_s3_storage.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/gems/pending/util/object_storage/miq_s3_storage.rb b/lib/gems/pending/util/object_storage/miq_s3_storage.rb index adf85ebad1c..faba0d54336 100644 --- a/lib/gems/pending/util/object_storage/miq_s3_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_s3_storage.rb @@ -52,7 +52,6 @@ def download_single(source, destination) bucket.object(source).download_file(destination) end end - local_file end # no-op mostly From f515545c3311b734c5eb57830bab75d4c97e4540 Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Tue, 16 Oct 2018 19:04:42 -0400 Subject: [PATCH 33/55] Be more lenient for locked down ftp servers When uploading to an anonymous ftp server, sometimes ls (nlst) is disabled This logs that case and continues on this is part of anonymous ftp feature https://bugzilla.redhat.com/show_bug.cgi?id=1632433 (transferred from ManageIQ/manageiq-gems-pending@7231f3bfbc4d12daf158be2b8200422e5418791d) --- lib/gems/pending/util/miq_ftp_lib.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/gems/pending/util/miq_ftp_lib.rb b/lib/gems/pending/util/miq_ftp_lib.rb index ddada42d068..16b934d20c9 100644 --- a/lib/gems/pending/util/miq_ftp_lib.rb +++ b/lib/gems/pending/util/miq_ftp_lib.rb @@ -56,7 +56,11 @@ def create_directory_structure(directory_path) end ftp.chdir(directory) end - ftp.chdir(pwd) + rescue Net::FTPPermError + raise unless @username.nil? && @password.nil? + _log.info("introspection of directories disabled. skipping create_directory_structure") + ensure + ftp.chdir(pwd) if pwd end def with_connection(cred_hash = nil) From fcc344e9787e5563831b22bf91500c1020181ee7 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Wed, 31 Oct 2018 17:57:59 -0500 Subject: [PATCH 34/55] [MiqS3Storage] Force encoding on download The chunks that come in from the `aws-sdk` seem to not enforce a binary encoding, so for our purposes, match the chunk string to the encoding of the destination. Should cover most cases. (transferred from ManageIQ/manageiq-gems-pending@12ea13284a42a17c0bd0d6ad292602caca32a37d) --- lib/gems/pending/util/object_storage/miq_s3_storage.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gems/pending/util/object_storage/miq_s3_storage.rb b/lib/gems/pending/util/object_storage/miq_s3_storage.rb index faba0d54336..b1d03f3c76a 100644 --- a/lib/gems/pending/util/object_storage/miq_s3_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_s3_storage.rb @@ -46,7 +46,7 @@ def download_single(source, destination) with_standard_s3_error_handling("downloading", source) do if destination.kind_of?(IO) s3.client.get_object(:bucket => bucket_name, :key => object_key) do |chunk| - destination.write(chunk) + destination.write(chunk.force_encoding(destination.external_encoding)) end else # assume file path bucket.object(source).download_file(destination) From 8f2b1659004e226c02ab7554ccbdd6c20e8b1aaa Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Sun, 28 Oct 2018 10:23:45 -0500 Subject: [PATCH 35/55] Disable MiqFileStorage#handle_io_block error spec Is failing at a pretty consistent rate on Travis, and this avoids confusion for now by disabling it. (transferred from ManageIQ/manageiq-gems-pending@4e87834ab6f7356129561f63330974b41d569db3) --- spec/util/miq_file_storage_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/util/miq_file_storage_spec.rb b/spec/util/miq_file_storage_spec.rb index 420ea90e9ed..53050f3a904 100644 --- a/spec/util/miq_file_storage_spec.rb +++ b/spec/util/miq_file_storage_spec.rb @@ -418,6 +418,7 @@ def opts_for_fakefs let(:err_block) { ->(_input_writer) { raise "err-mah-gerd" } } before do + skip "currently fails consistenly on Travis" expect(File).to receive(:mkfifo) expect(File).to receive(:open).and_return(source_input, input_writer) end From 701d10d00d35c3d935a88d6c8ad81610ce31e423 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Fri, 2 Nov 2018 14:04:21 -0500 Subject: [PATCH 36/55] [MiqSwiftStorage] Assign @container_name correctly There was a bug when passing in a `:uri` to `MiqSwiftStorage` where if the path for uri contained a filename as well: swift://example.com/container/path/to/file?query=params It would assign the `@container_name` to the full path (minus the leading slash), and not just the top level "directory". This fixes that so it is correct. (transferred from ManageIQ/manageiq-gems-pending@aa9436d406614ad1d8c5892570d5b46ccc1da94d) --- .../util/object_storage/miq_swift_storage.rb | 7 ++-- .../object_storage/miq_swift_storage_spec.rb | 32 ++++++++++++++----- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/lib/gems/pending/util/object_storage/miq_swift_storage.rb b/lib/gems/pending/util/object_storage/miq_swift_storage.rb index 13263a20aac..9a4b30b2704 100644 --- a/lib/gems/pending/util/object_storage/miq_swift_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_swift_storage.rb @@ -16,12 +16,15 @@ def initialize(settings) @bucket_name = URI(@settings[:uri]).host raise "username and password are required values!" if @settings[:username].nil? || @settings[:password].nil? - _scheme, _userinfo, @host, @port, _registry, @mount_path, _opaque, query, _fragment = URI.split(URI.encode(@settings[:uri])) + _scheme, _userinfo, @host, @port, _registry, path, _opaque, query, _fragment = URI.split(URI.encode(@settings[:uri])) query_params(query) if query @swift = nil @username = @settings[:username] @password = @settings[:password] - @container_name = @mount_path[0] == File::Separator ? @mount_path[1..-1] : @mount_path + + # Omit leading slash (if it exists), and grab the rest of the characters + # before the next file separator + @container_name = path.gsub(/^\/?([^\/]+)\/.*/, '\1') end def uri_to_object_path(remote_file) diff --git a/spec/util/object_storage/miq_swift_storage_spec.rb b/spec/util/object_storage/miq_swift_storage_spec.rb index 248a61cf5c9..8f3a8fa7e96 100644 --- a/spec/util/object_storage/miq_swift_storage_spec.rb +++ b/spec/util/object_storage/miq_swift_storage_spec.rb @@ -3,17 +3,33 @@ describe MiqSwiftStorage do let(:object_storage) { described_class.new(:uri => uri, :username => 'user', :password => 'pass') } - context "using a uri without query parameters" do - let(:uri) { "swift://foo.com/abc/def" } + describe "#initialize" do + context "using a uri with query parameters" do + let(:uri) { "swift://foo.com:5678/abc/def?region=region&api_version=v3&security_protocol=non-ssl" } + + it "#initialize sets the container_name" do + container_name = object_storage.container_name + expect(container_name).to eq("abc") + end - it "#initialize sets the container_name" do - container_name = object_storage.container_name - expect(container_name).to eq("abc/def") + it "#uri_to_object_path returns a new object path" do + result = object_storage.uri_to_object_path(uri) + expect(result).to eq("def") + end end - it "#uri_to_object_path returns a new object path" do - result = object_storage.uri_to_object_path(uri) - expect(result).to eq("def") + context "using a uri without query parameters" do + let(:uri) { "swift://foo.com/abc/def/my_file.tar.gz" } + + it "#initialize sets the container_name" do + container_name = object_storage.container_name + expect(container_name).to eq("abc") + end + + it "#uri_to_object_path returns a new object path" do + result = object_storage.uri_to_object_path(uri) + expect(result).to eq("def/my_file.tar.gz") + end end end From 0bbfc74587afb49672d46491a2d43bc766dd97a9 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Wed, 31 Oct 2018 18:22:25 -0500 Subject: [PATCH 37/55] [MiqGenericMountSession] Support @byte_count in #download_single (transferred from ManageIQ/manageiq-gems-pending@85e1a973e24a7eae123dbd75f356a0a2e3bc315f) --- .../pending/util/mount/miq_generic_mount_session.rb | 2 +- spec/util/mount/miq_local_mount_session_spec.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/gems/pending/util/mount/miq_generic_mount_session.rb b/lib/gems/pending/util/mount/miq_generic_mount_session.rb index b5bb8f81bef..70bc614ae50 100644 --- a/lib/gems/pending/util/mount/miq_generic_mount_session.rb +++ b/lib/gems/pending/util/mount/miq_generic_mount_session.rb @@ -301,7 +301,7 @@ def download_single(remote_file, local_file) end logger.info("#{log_header} Copying file [#{relpath}] to [#{local_file}]...") - IO.copy_stream(relpath, local_file) + IO.copy_stream(relpath, local_file, byte_count) logger.info("#{log_header} Copying file [#{relpath}] to [#{local_file}] complete") rescue => err msg = "Downloading [#{remote_file}] to [#{local_file}], failed due to error: '#{err.message}'" diff --git a/spec/util/mount/miq_local_mount_session_spec.rb b/spec/util/mount/miq_local_mount_session_spec.rb index 50ba10c4171..968196684f1 100644 --- a/spec/util/mount/miq_local_mount_session_spec.rb +++ b/spec/util/mount/miq_local_mount_session_spec.rb @@ -88,5 +88,18 @@ expect(source_data.size).to eq(10.megabytes) expect(source_data).to eq(File.read(source_path)) end + + # Note, we are using `#download` in the spec, but really, this is a feature + # for `#download_single`. `#download` really doesn't make use of this + # directly. + it "respects the `byte_count` attribute" do + downloaded_data = StringIO.new + + subject.instance_variable_set(:@byte_count, 42) + subject.download(downloaded_data, source_path) + + expect(downloaded_data.string.size).to eq(42) + expect(downloaded_data.string).to eq("0" * 42) + end end end From 1452779b6ce6f3b5799869ee7c0e8b8f44ac1b10 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Fri, 2 Nov 2018 14:08:40 -0500 Subject: [PATCH 38/55] [MiqSwiftStorage] Align comments to 80 char width Also adds some spacing for clarity. (transferred from ManageIQ/manageiq-gems-pending@10ac180979cd998ed680ab1737fcd6cfec2efc08) --- .../pending/util/object_storage/miq_swift_storage.rb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/gems/pending/util/object_storage/miq_swift_storage.rb b/lib/gems/pending/util/object_storage/miq_swift_storage.rb index 9a4b30b2704..58dde84e05c 100644 --- a/lib/gems/pending/util/object_storage/miq_swift_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_swift_storage.rb @@ -55,15 +55,17 @@ def upload_single(dest_uri) } # # Because of how `Fog::OpenStack` (and probably `Fog::Core`) is designed, - # it has hidden the functionality to provide a block for streaming uploads - # that is available out of the box with Excon. + # it has hidden the functionality to provide a block for streaming + # uploads that is available out of the box with Excon. # # we use .send here because #request is private - # we can't use #put_object (public) directly because it doesn't allow a 202 response code, - # which is what swift responds with when we pass it the :request_block - # (This allows us to stream the response in chunks) + # + # we can't use #put_object (public) directly because it doesn't allow a + # 202 response code, which is what swift responds with when we pass it + # the :request_block (This allows us to stream the response in chunks) # swift_file.service.send(:request, params) + clear_split_vars rescue Excon::Errors::Unauthorized => err msg = "Access to Swift container #{@container_name} failed due to a bad username or password. #{err}" From 5de134a1eaa6c520e34bde5c45ed5b91d4e2de1b Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Wed, 31 Oct 2018 18:14:12 -0500 Subject: [PATCH 39/55] [MiqS3Storage] Support StringIO in #download_single (transferred from ManageIQ/manageiq-gems-pending@918a2dc5ac7fc0da8db26d485fd7a7dae88cccb9) --- lib/gems/pending/util/object_storage/miq_s3_storage.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gems/pending/util/object_storage/miq_s3_storage.rb b/lib/gems/pending/util/object_storage/miq_s3_storage.rb index b1d03f3c76a..e62e77c0892 100644 --- a/lib/gems/pending/util/object_storage/miq_s3_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_s3_storage.rb @@ -44,7 +44,7 @@ def download_single(source, destination) logger.debug("Downloading [#{source}] from bucket [#{bucket_name}] to local file [#{destination}]") with_standard_s3_error_handling("downloading", source) do - if destination.kind_of?(IO) + if destination.kind_of?(IO) || destination.kind_of?(StringIO) s3.client.get_object(:bucket => bucket_name, :key => object_key) do |chunk| destination.write(chunk.force_encoding(destination.external_encoding)) end From b2f4688f306d6e42d54dd9a2c7e9264291211809 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Wed, 31 Oct 2018 18:23:07 -0500 Subject: [PATCH 40/55] [MiqFtpStorage] Support @byte_count in #download_single Since there is no way in the spec of `Net::FTP#getbinaryfile` to only download a specified number of bytes, it was required to rework a form of `Net::FTP#retrbinary` (a private method of that class) to allow only reading from the socket for the specified number of bytes, then moving on. The goal was to remain consistent with `#add` and continue to use `IO.copy_stream`, a slight tweak that was necessary to get that working. The `conn` variable is a `BufferedSocket`, which basically has IO like, but wraps an existing `@io` with some methods and tries to maintain a IO-like class interface. Unfortunately, it's `#read` implementation doesn't have the same number of args as `IO#read`: (method definitions) IO read( [ length, [ output ] ] ) BufferedSocket read( [ len ] ) When `IO.copy_stream` calls `.read` under the covers, it does pass in the second argument of `output` to the result, which in turn causes an error if we just pass `conn` as the first argument to `copy_stream`. The underlying `@io` attribute for `conn` does actually adhere to the argument interface of `copy_stream`, and since we don't miss anything by working around it, we just used that instead for our use here. This should, however, probably be fixed in ruby's stdlib for `Net::FTP`. (transferred from ManageIQ/manageiq-gems-pending@1a4e2cb7993846b4f8c7557ca7fdac74869e839b) --- .../util/object_storage/miq_ftp_storage.rb | 13 ++++++++++++- spec/util/object_storage/miq_ftp_storage_spec.rb | 16 ++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lib/gems/pending/util/object_storage/miq_ftp_storage.rb b/lib/gems/pending/util/object_storage/miq_ftp_storage.rb index d2a97163038..c6429c006fb 100644 --- a/lib/gems/pending/util/object_storage/miq_ftp_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_ftp_storage.rb @@ -54,7 +54,18 @@ def upload_single(dest_uri) end def download_single(source, destination) - ftp.getbinaryfile(uri_to_relative(source), destination) + ftp.synchronize do + ftp.send(:with_binary, true) do + begin + conn = ftp.send(:transfercmd, "RETR #{uri_to_relative(source)}") + IO.copy_stream(conn.io, destination, byte_count) + conn.shutdown(Socket::SHUT_WR) + conn.read_timeout = 1 + ensure + conn.close if conn + end + end + end end def mkdir(dir) diff --git a/spec/util/object_storage/miq_ftp_storage_spec.rb b/spec/util/object_storage/miq_ftp_storage_spec.rb index 188665d74e6..27824b12b07 100644 --- a/spec/util/object_storage/miq_ftp_storage_spec.rb +++ b/spec/util/object_storage/miq_ftp_storage_spec.rb @@ -115,5 +115,21 @@ # Nothing written, just printed the streamed file in the above command expect(source_data.size).to eq(10.megabytes) end + + # Note, we are using `#download` in the spec, but really, this is a feature + # for `#download_single`. `#download` really doesn't make use of this + # directly. + it "respects the `byte_count` attribute" do + downloaded_data = StringIO.new + subject.instance_variable_set(:@byte_count, 42) + subject.download(downloaded_data, source_path) + + # Sanity check that what we are downloading is the size we expect + expect(source_path).to exist_on_ftp_server + expect(source_path).to have_size_on_ftp_server_of(10.megabytes) + + expect(downloaded_data.string.size).to eq(42) + expect(downloaded_data.string).to eq("0" * 42) + end end end From 33d94ec88b3c1fbcebf13b14eebaf4a639e03a40 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Wed, 31 Oct 2018 18:14:58 -0500 Subject: [PATCH 41/55] [MiqS3Storage] Support @byte_count in #download_single Makes use of `Aws::S3::Client`'s `:range` option for `#get_object` to only download the specified number of bytes. The spec for this is meant to be compatible with the `Content-Range` header for HTTP: https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.16 This the value passed to `:range` will be translated to a valid `Content-Range` header, as shown here: https://docs.aws.amazon.com/sdk-for-ruby/v3/api/Aws/S3/Client.html#get_object-instance_method See: "Example: To retrieve a byte range of an object" (transferred from ManageIQ/manageiq-gems-pending@89a3e3b0fc9368d1e705d2118f0e17db4d26035a) --- lib/gems/pending/util/object_storage/miq_s3_storage.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/gems/pending/util/object_storage/miq_s3_storage.rb b/lib/gems/pending/util/object_storage/miq_s3_storage.rb index e62e77c0892..3a0443588aa 100644 --- a/lib/gems/pending/util/object_storage/miq_s3_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_s3_storage.rb @@ -45,7 +45,14 @@ def download_single(source, destination) with_standard_s3_error_handling("downloading", source) do if destination.kind_of?(IO) || destination.kind_of?(StringIO) - s3.client.get_object(:bucket => bucket_name, :key => object_key) do |chunk| + get_object_opts = { + :bucket => bucket_name, + :key => object_key + } + # :range is indexed starting at zero + get_object_opts[:range] = "bytes=0-#{byte_count - 1}" if byte_count + + s3.client.get_object(get_object_opts) do |chunk| destination.write(chunk.force_encoding(destination.external_encoding)) end else # assume file path From 1d7ea9d18b6b1bb76b74939cceb88bb578c16945 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Fri, 2 Nov 2018 14:10:24 -0500 Subject: [PATCH 42/55] [MiqObjectStorage] Add #write_chunk_proc This method is meant to be used as a shared method for handling encoding issues across all object stores as needed. This is extracted from the s3 version since it can eventually be applied to Swift as well. (transferred from ManageIQ/manageiq-gems-pending@db1acce6bc2a3faaf3bd6630fb614d4f537b7ccf) --- lib/gems/pending/util/miq_object_storage.rb | 11 +++++++++++ .../pending/util/object_storage/miq_s3_storage.rb | 4 +--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/gems/pending/util/miq_object_storage.rb b/lib/gems/pending/util/miq_object_storage.rb index 1f62998c684..19b499e6c91 100644 --- a/lib/gems/pending/util/miq_object_storage.rb +++ b/lib/gems/pending/util/miq_object_storage.rb @@ -45,6 +45,17 @@ def write_single_split_file_for(file_io) clear_split_vars end + def write_chunk_proc(destination) + # We use a `proc` here instead of `lambda` because we are only concerned + # about the first argument, and other arguments (additional ones added by + # Excon's response_call signature, for example) are unneeded. + # + # `lambda do` will do argument checking, while `proc do` won't. + proc do |chunk| + destination.write(chunk.force_encoding(destination.external_encoding)) + end + end + def clear_split_vars @buf_left = nil end diff --git a/lib/gems/pending/util/object_storage/miq_s3_storage.rb b/lib/gems/pending/util/object_storage/miq_s3_storage.rb index 3a0443588aa..be43f966294 100644 --- a/lib/gems/pending/util/object_storage/miq_s3_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_s3_storage.rb @@ -52,9 +52,7 @@ def download_single(source, destination) # :range is indexed starting at zero get_object_opts[:range] = "bytes=0-#{byte_count - 1}" if byte_count - s3.client.get_object(get_object_opts) do |chunk| - destination.write(chunk.force_encoding(destination.external_encoding)) - end + s3.client.get_object(get_object_opts, &write_chunk_proc(destination)) else # assume file path bucket.object(source).download_file(destination) end From 40741c1d2c340440cb66eafc41bdec5f833baf7f Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Fri, 2 Nov 2018 14:17:50 -0500 Subject: [PATCH 43/55] [MiqSwiftStorage] Add #download_single Adds support for streaming downloads in `MiqSwiftStorage`. Of Note: - `#write_chunk_proc` is being used to stream the contents. - Technically `Excon` will pass back 3 arguments to the `:response_block`, but since we only need the first one `chunk`, this is fine. - The `container_key =` line also makes sure we are loading `fog/openstack` prior to calling any objects from there. - Using `swift` directly for the `.send`, since that is what is used when calling `swift.container.directory.service` anyway. - Still have to use `.send` for this unfortunately. (transferred from ManageIQ/manageiq-gems-pending@d175b753275b37e1db487f283414de850e684c91) --- .../util/object_storage/miq_swift_storage.rb | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/lib/gems/pending/util/object_storage/miq_swift_storage.rb b/lib/gems/pending/util/object_storage/miq_swift_storage.rb index 58dde84e05c..64e69739bf3 100644 --- a/lib/gems/pending/util/object_storage/miq_swift_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_swift_storage.rb @@ -78,6 +78,23 @@ def upload_single(dest_uri) end end + def download_single(source, destination) + object_file = uri_to_object_path(source) + logger.debug("Downloading [#{source}] from Container [#{container_name}] to local file [#{destination}]") + + with_standard_swift_error_handling("downloading") do + container_key = container.key # also makes sure 'fog/openstack' is loaded + params = { + :expects => 200, + :headers => {}, + :response_block => write_chunk_proc(destination), + :method => "GET", + :path => "#{Fog::OpenStack.escape(container_key)}/#{Fog::OpenStack.escape(object_file)}" + } + swift.send(:request, params) + end + end + def mkdir(_dir) container end @@ -153,10 +170,6 @@ def create_container raise err, msg, err.backtrace end - def download_single(_source, _destination) - raise NotImplementedError, "MiqSwiftStorage.download_single Not Yet Implemented" - end - def query_params(query_string) parts = URI.decode_www_form(query_string).to_h @region, @api_version, @domain_id, @security_protocol = parts.values_at("region", "api_version", "domain_id", "security_protocol") From fea921dc03192b1a75e8a680b563deb36e2cde9b Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Thu, 1 Nov 2018 11:50:08 -0500 Subject: [PATCH 44/55] [MiqSwiftStorage] Add #with_standard_swift_error_handling Meant to be used as a shared set of error handling code for swift errors. Will be used in #download_single when that is implemented in the future as well. (transferred from ManageIQ/manageiq-gems-pending@eb533f49db10a1fe43ee654049af34eeabc516b7) --- .../util/object_storage/miq_swift_storage.rb | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/lib/gems/pending/util/object_storage/miq_swift_storage.rb b/lib/gems/pending/util/object_storage/miq_swift_storage.rb index 3b2216c5378..7712a6a74f7 100644 --- a/lib/gems/pending/util/object_storage/miq_swift_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_swift_storage.rb @@ -43,7 +43,8 @@ def upload_single(dest_uri) # write dump file to swift # logger.debug("Writing [#{source_input}] to => Bucket [#{container_name}] using object file name [#{object_file}]") - begin + + with_standard_swift_error_handling("uploading") do swift_file = container.files.new(:key => object_file) params = { :expects => [201, 202], @@ -67,14 +68,6 @@ def upload_single(dest_uri) swift_file.service.send(:request, params) clear_split_vars - rescue Excon::Errors::Unauthorized => err - msg = "Access to Swift container #{@container_name} failed due to a bad username or password. #{err}" - logger.error(msg) - raise err, msg, err.backtrace - rescue => err - msg = "Error uploading #{source_input} to Swift container #{@container_name}. #{err}" - logger.error(msg) - raise err, msg, err.backtrace end end @@ -176,4 +169,16 @@ def query_params(query_string) parts = URI.decode_www_form(query_string).to_h @region, @api_version, @domain_id, @security_protocol = parts.values_at("region", "api_version", "domain_id", "security_protocol") end + + def with_standard_swift_error_handling(action) + yield + rescue Excon::Errors::Unauthorized => err + msg = "Access to Swift container #{@container_name} failed due to a bad username or password. #{err}" + logger.error(msg) + raise err, msg, err.backtrace + rescue => err + msg = "Error #{action} #{source_input} to Swift container #{@container_name}. #{err}" + logger.error(msg) + raise err, msg, err.backtrace + end end From 2610d2470335dea9e488a62e57dd6fdd345bf553 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Wed, 31 Oct 2018 18:11:08 -0500 Subject: [PATCH 45/55] [MiqFileStorage] Add #magic_number_for Used to determine the magic number for a remote uri. Relies on `#download_single` being able to support a specific `@byte_count` to download only what is necessary and writing to a StringIO. (transferred from ManageIQ/manageiq-gems-pending@a24d7036e53e481a80ec4bcb5626183be9cebb00) --- lib/gems/pending/util/miq_file_storage.rb | 44 +++++++++++++++++++ .../mount/miq_local_mount_session_spec.rb | 25 +++++++---- 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/lib/gems/pending/util/miq_file_storage.rb b/lib/gems/pending/util/miq_file_storage.rb index 4b5a7b4e8eb..d756abf92b2 100644 --- a/lib/gems/pending/util/miq_file_storage.rb +++ b/lib/gems/pending/util/miq_file_storage.rb @@ -152,6 +152,50 @@ def download(local_file, remote_file_uri, &block) reset_vars end + # :call-seq: + # magic_number_for( remote_uri ) + # magic_number_for( remote_uri, {:accepted => {:key => "magic_str", ...} } ) + # + # Determine a magic number for a remote file. + # + # If no options[:accepted] is passed, then only the first 256 bytes of the + # file are downloaded, and just that data is returned. + # + # If a hash of magic number keys and values for those magic numbers is + # passed, then it will download the largest byte size for the magic number + # values, and compare against the list, returning the first match. + # + # Example: + # + # magics = { :pgdump => PostgresAdmin::PG_DUMP_MAGIC } + # + # magic_number_for("example.org/my_dump.gz", :accepted => magics) + # #=> :pgdump + # magic_number_for("example.org/my_file.rb", :accepted => magics) + # #=> nil + # + # NOTE: This is an extremely niave implementation for remote magic number + # checking, and is only really meant for working with the known magics + # PostgresAdmin. Many other use cases would need to be considered, since + # magic numbers can also checked against the tail of the file, and are not + # limited to the first 256 bytes as has been arbitrarily decided on here. + def magic_number_for(uri, options = {}) + # Amount of bytes to download for checking magic + @byte_count = options.fetch(:accepted, {}).values.map(&:length).max || 256 + uri_data_io = StringIO.new + download_single(uri, uri_data_io) + uri_data = uri_data_io.string + + if (magics = options[:accepted]) + result = magics.detect { |_, magic| uri_data.force_encoding(magic.encoding).start_with?(magic) } + result && result.first + else + uri_data + end + ensure + reset_vars + end + private # NOTE: Needs to be overwritten in the subclass! diff --git a/spec/util/mount/miq_local_mount_session_spec.rb b/spec/util/mount/miq_local_mount_session_spec.rb index 968196684f1..47df8c9ea57 100644 --- a/spec/util/mount/miq_local_mount_session_spec.rb +++ b/spec/util/mount/miq_local_mount_session_spec.rb @@ -88,18 +88,25 @@ expect(source_data.size).to eq(10.megabytes) expect(source_data).to eq(File.read(source_path)) end + end + + describe "#magic_number_for" do + include_context "generated tmp files" - # Note, we are using `#download` in the spec, but really, this is a feature - # for `#download_single`. `#download` really doesn't make use of this - # directly. - it "respects the `byte_count` attribute" do - downloaded_data = StringIO.new + it "returns 256 bytes by default" do + result = subject.magic_number_for(source_path) - subject.instance_variable_set(:@byte_count, 42) - subject.download(downloaded_data, source_path) + expect(result.size).to eq(256) + expect(result).to eq("0" * 256) + end + + describe "with a hash of accepted magics" do + it "returns key for the passed in magic number value" do + magics = { :zero => "000", :one => "1", :foo => "bar" } + result = subject.magic_number_for(source_path, :accepted => magics) - expect(downloaded_data.string.size).to eq(42) - expect(downloaded_data.string).to eq("0" * 42) + expect(result).to eq(:zero) + end end end end From 9b550a6a780c135dcacf548be1bd5b577cb216ba Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Fri, 2 Nov 2018 14:25:13 -0500 Subject: [PATCH 46/55] [MiqSwiftStorage] Support @byte_count in #download_single Very similar to what is done in `s3`, but we need to update the `expects` to allow for the `206: Partial Content` exit code as well when getting as specific range. (transferred from ManageIQ/manageiq-gems-pending@d1971b8aa674699d29f96e0a2c1dd3b8cb2ee89f) --- lib/gems/pending/util/object_storage/miq_swift_storage.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/gems/pending/util/object_storage/miq_swift_storage.rb b/lib/gems/pending/util/object_storage/miq_swift_storage.rb index 64e69739bf3..3b2216c5378 100644 --- a/lib/gems/pending/util/object_storage/miq_swift_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_swift_storage.rb @@ -85,12 +85,14 @@ def download_single(source, destination) with_standard_swift_error_handling("downloading") do container_key = container.key # also makes sure 'fog/openstack' is loaded params = { - :expects => 200, + :expects => [200, 206], :headers => {}, :response_block => write_chunk_proc(destination), :method => "GET", :path => "#{Fog::OpenStack.escape(container_key)}/#{Fog::OpenStack.escape(object_file)}" } + # Range is indexed starting at zero + params[:headers]['Range'] = "bytes=0-#{byte_count - 1}" if byte_count swift.send(:request, params) end end From 8a59edfee3fe7812ec3ac1edd38cf4309225111c Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Fri, 9 Nov 2018 18:04:23 -0600 Subject: [PATCH 47/55] [MiqSwiftStorage] Spec cleanup Removes some unneeded lines, and unnecessary words in `it` lines. (transferred from ManageIQ/manageiq-gems-pending@354d08f1b35d2deb7d403a5cf0e34ce7d198c6cb) --- spec/util/object_storage/miq_swift_storage_spec.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/spec/util/object_storage/miq_swift_storage_spec.rb b/spec/util/object_storage/miq_swift_storage_spec.rb index 8f3a8fa7e96..4d6c831fb14 100644 --- a/spec/util/object_storage/miq_swift_storage_spec.rb +++ b/spec/util/object_storage/miq_swift_storage_spec.rb @@ -7,9 +7,8 @@ context "using a uri with query parameters" do let(:uri) { "swift://foo.com:5678/abc/def?region=region&api_version=v3&security_protocol=non-ssl" } - it "#initialize sets the container_name" do - container_name = object_storage.container_name - expect(container_name).to eq("abc") + it "sets the container_name" do + expect(object_storage.container_name).to eq("abc") end it "#uri_to_object_path returns a new object path" do @@ -21,9 +20,8 @@ context "using a uri without query parameters" do let(:uri) { "swift://foo.com/abc/def/my_file.tar.gz" } - it "#initialize sets the container_name" do - container_name = object_storage.container_name - expect(container_name).to eq("abc") + it "sets the container_name" do + expect(object_storage.container_name).to eq("abc") end it "#uri_to_object_path returns a new object path" do From 67eb4d01b5eba35bd60fadb3661f099cabf2a364 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Wed, 31 Oct 2018 18:36:27 -0500 Subject: [PATCH 48/55] [MiqFtpStorage] Add #magic_number_for support Since we need a connection for any download/upload to work (sets @ftp on the instance), we need a slight override of `#magic_number_for` to wrap it with a connection. (transferred from ManageIQ/manageiq-gems-pending@f70c48df745b9d869b698862c2f9659d718530fd) --- .../util/object_storage/miq_ftp_storage.rb | 5 ++++ .../object_storage/miq_ftp_storage_spec.rb | 29 +++++++++++-------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/lib/gems/pending/util/object_storage/miq_ftp_storage.rb b/lib/gems/pending/util/object_storage/miq_ftp_storage.rb index c6429c006fb..da81a0d16af 100644 --- a/lib/gems/pending/util/object_storage/miq_ftp_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_ftp_storage.rb @@ -32,6 +32,11 @@ def download(*download_args) with_connection { super } end + # Override for connection handling + def magic_number_for(*magic_number_for_args) + with_connection { super } + end + # Specific version of Net::FTP#storbinary that doesn't use an existing local # file, and only uploads a specific size (byte_count) from the input_file def upload_single(dest_uri) diff --git a/spec/util/object_storage/miq_ftp_storage_spec.rb b/spec/util/object_storage/miq_ftp_storage_spec.rb index 27824b12b07..09d3e2e822f 100644 --- a/spec/util/object_storage/miq_ftp_storage_spec.rb +++ b/spec/util/object_storage/miq_ftp_storage_spec.rb @@ -115,21 +115,26 @@ # Nothing written, just printed the streamed file in the above command expect(source_data.size).to eq(10.megabytes) end + end - # Note, we are using `#download` in the spec, but really, this is a feature - # for `#download_single`. `#download` really doesn't make use of this - # directly. - it "respects the `byte_count` attribute" do - downloaded_data = StringIO.new - subject.instance_variable_set(:@byte_count, 42) - subject.download(downloaded_data, source_path) + describe "#magic_number_for" do + let(:source_file) { existing_ftp_file(10.megabytes) } + let(:source_path) { File.basename(source_file.path) } - # Sanity check that what we are downloading is the size we expect - expect(source_path).to exist_on_ftp_server - expect(source_path).to have_size_on_ftp_server_of(10.megabytes) + it "returns 256 bytes by default" do + result = subject.magic_number_for(source_path) + + expect(result.size).to eq(256) + expect(result).to eq("0" * 256) + end - expect(downloaded_data.string.size).to eq(42) - expect(downloaded_data.string).to eq("0" * 42) + describe "with a hash of accepted magics" do + it "returns key for the passed in magic number value" do + magics = { :zero => "000", :one => "1", :foo => "bar" } + result = subject.magic_number_for(source_path, :accepted => magics) + + expect(result).to eq(:zero) + end end end end From ad6eee188e46c5ad45470e08dd1af3ddd1a37d10 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Fri, 9 Nov 2018 18:10:26 -0600 Subject: [PATCH 49/55] [MiqSwiftStorage] Fix @container_name parsing If there is no trailing slash in the parsed out `path` from `URI.parse`, then setting of `@container_name` didn't match the regexp, and so it caused the path generated for the URL in the `#container` method to generate as: "/v1/AUTH_asdfglkjwearimsdf/%2Fcontainer_name" Instead of: "/v1/AUTH_asdfglkjwearimsdf/container_name" Where `"%2f"` is due to `fog-core`'s URI escaping code of the string passed into the `swift.directories.get(container_name)` when it includes a slash (so `"/container_name" => "%2Fcontainer_name"`). The fix makes sure the regexp is correct, and the removal of the extra `\/` check at the end was removed. Some extra tests were put in place to make sure everything still works as expected. (transferred from ManageIQ/manageiq-gems-pending@242bf6ff9484de7efc3af2c71923aa0ee1771ae8) --- .../util/object_storage/miq_swift_storage.rb | 2 +- .../object_storage/miq_swift_storage_spec.rb | 28 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/lib/gems/pending/util/object_storage/miq_swift_storage.rb b/lib/gems/pending/util/object_storage/miq_swift_storage.rb index 7712a6a74f7..ebefd5ddd2f 100644 --- a/lib/gems/pending/util/object_storage/miq_swift_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_swift_storage.rb @@ -24,7 +24,7 @@ def initialize(settings) # Omit leading slash (if it exists), and grab the rest of the characters # before the next file separator - @container_name = path.gsub(/^\/?([^\/]+)\/.*/, '\1') + @container_name = path.gsub(/^\/?([^\/]+).*/, '\1') end def uri_to_object_path(remote_file) diff --git a/spec/util/object_storage/miq_swift_storage_spec.rb b/spec/util/object_storage/miq_swift_storage_spec.rb index 4d6c831fb14..5d54edd3d54 100644 --- a/spec/util/object_storage/miq_swift_storage_spec.rb +++ b/spec/util/object_storage/miq_swift_storage_spec.rb @@ -29,6 +29,34 @@ expect(result).to eq("def/my_file.tar.gz") end end + + context "using a uri with only a container_name" do + let(:uri) { "swift://foo.com/container_name" } + + it "sets the container_name" do + expect(object_storage.container_name).to eq("container_name") + end + end + + context "using a uri with only a container_name and params" do + let(:uri) { "swift://foo.com/container_name?region=region&api_version=v3&security_protocol=non-ssl" } + + it "sets the container_name" do + expect(object_storage.container_name).to eq("container_name") + end + + it "sets the region" do + expect(object_storage.instance_variable_get(:@region)).to eq("region") + end + + it "sets the api_version" do + expect(object_storage.instance_variable_get(:@api_version)).to eq("v3") + end + + it "sets the security_protocol" do + expect(object_storage.instance_variable_get(:@security_protocol)).to eq("non-ssl") + end + end end describe "#auth_url (private)" do From edcdc794a45a232e8ef17127b46e6dd6280f73fb Mon Sep 17 00:00:00 2001 From: Alexander Zagaynov Date: Thu, 17 Oct 2019 14:49:04 +0200 Subject: [PATCH 50/55] fix require statements (transferred from ManageIQ/manageiq-gems-pending@f0a5d5a8dbfe889ad4d3a0cec56d8180cf7c61cf) --- lib/gems/pending/util/object_storage/miq_s3_storage.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/gems/pending/util/object_storage/miq_s3_storage.rb b/lib/gems/pending/util/object_storage/miq_s3_storage.rb index be43f966294..63696d4c692 100644 --- a/lib/gems/pending/util/object_storage/miq_s3_storage.rb +++ b/lib/gems/pending/util/object_storage/miq_s3_storage.rb @@ -84,8 +84,8 @@ def bucket private def s3 - require 'aws-sdk' - require 'util/extensions/aws-sdk/s3_upload_stream_patch' + require 'aws-sdk-s3' + @s3 ||= Aws::S3::Resource.new(:region => @settings[:region], :access_key_id => @settings[:username], :secret_access_key => @settings[:password]) From 1d202f9f0ddea09a7f7d83a669e36c53db2b1889 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Fri, 6 Sep 2019 11:50:58 -0500 Subject: [PATCH 51/55] [MiqGenericMountSession] Fix .source_for_log Does better fallback handling and nil checking for this method. This can fail as part of `MiqGenericMountSession#add`'s `rescue` statement when the `@source_input` is not defined, and that will mask the actual error that was raised. This fixes that bug so the proper error is shown in the logs. (transferred from ManageIQ/manageiq-gems-pending@7f8b69a681cc789726a834da75d879adf0a1e1a5) --- lib/gems/pending/util/mount/miq_generic_mount_session.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/gems/pending/util/mount/miq_generic_mount_session.rb b/lib/gems/pending/util/mount/miq_generic_mount_session.rb index 70bc614ae50..a25fc0ef60b 100644 --- a/lib/gems/pending/util/mount/miq_generic_mount_session.rb +++ b/lib/gems/pending/util/mount/miq_generic_mount_session.rb @@ -490,6 +490,12 @@ def settings_mount_point end def source_for_log - @input_writer ? "" : @source_input.path + if @input_writer + "" + elsif @source_input + @source_input.path + else + "" + end end end From d76ed1680edbf32c40efcdade00ec26d39c868a2 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Mon, 20 Jan 2020 19:42:29 -0600 Subject: [PATCH 52/55] Move lib/gems/pending/util/ to lib/ --- lib/{gems/pending/util => }/miq_file_storage.rb | 0 lib/{gems/pending/util => }/miq_ftp_lib.rb | 0 lib/{gems/pending/util => }/miq_object_storage.rb | 0 lib/{gems/pending/util => }/mount/miq_generic_mount_session.rb | 0 lib/{gems/pending/util => }/mount/miq_glusterfs_session.rb | 0 lib/{gems/pending/util => }/mount/miq_local_mount_session.rb | 0 lib/{gems/pending/util => }/mount/miq_nfs_session.rb | 0 lib/{gems/pending/util => }/mount/miq_smb_session.rb | 0 lib/{gems/pending/util => }/object_storage/miq_ftp_storage.rb | 0 lib/{gems/pending/util => }/object_storage/miq_s3_storage.rb | 0 lib/{gems/pending/util => }/object_storage/miq_swift_storage.rb | 0 spec/{util => lib}/miq_file_storage_spec.rb | 0 spec/{util => lib}/miq_ftp_lib_spec.rb | 0 spec/{util => lib}/miq_object_storage_spec.rb | 0 spec/{util => lib}/mount/miq_generic_mount_session_spec.rb | 0 spec/{util => lib}/mount/miq_local_mount_session_spec.rb | 0 spec/{util => lib}/object_storage/miq_ftp_storage_spec.rb | 0 spec/{util => lib}/object_storage/miq_s3_storage_spec.rb | 0 spec/{util => lib}/object_storage/miq_swift_storage_spec.rb | 0 spec/support/{contexts => examples_group}/generated_tmp_files.rb | 0 spec/support/{contexts => examples_group}/with_ftp_server.rb | 0 21 files changed, 0 insertions(+), 0 deletions(-) rename lib/{gems/pending/util => }/miq_file_storage.rb (100%) rename lib/{gems/pending/util => }/miq_ftp_lib.rb (100%) rename lib/{gems/pending/util => }/miq_object_storage.rb (100%) rename lib/{gems/pending/util => }/mount/miq_generic_mount_session.rb (100%) rename lib/{gems/pending/util => }/mount/miq_glusterfs_session.rb (100%) rename lib/{gems/pending/util => }/mount/miq_local_mount_session.rb (100%) rename lib/{gems/pending/util => }/mount/miq_nfs_session.rb (100%) rename lib/{gems/pending/util => }/mount/miq_smb_session.rb (100%) rename lib/{gems/pending/util => }/object_storage/miq_ftp_storage.rb (100%) rename lib/{gems/pending/util => }/object_storage/miq_s3_storage.rb (100%) rename lib/{gems/pending/util => }/object_storage/miq_swift_storage.rb (100%) rename spec/{util => lib}/miq_file_storage_spec.rb (100%) rename spec/{util => lib}/miq_ftp_lib_spec.rb (100%) rename spec/{util => lib}/miq_object_storage_spec.rb (100%) rename spec/{util => lib}/mount/miq_generic_mount_session_spec.rb (100%) rename spec/{util => lib}/mount/miq_local_mount_session_spec.rb (100%) rename spec/{util => lib}/object_storage/miq_ftp_storage_spec.rb (100%) rename spec/{util => lib}/object_storage/miq_s3_storage_spec.rb (100%) rename spec/{util => lib}/object_storage/miq_swift_storage_spec.rb (100%) rename spec/support/{contexts => examples_group}/generated_tmp_files.rb (100%) rename spec/support/{contexts => examples_group}/with_ftp_server.rb (100%) diff --git a/lib/gems/pending/util/miq_file_storage.rb b/lib/miq_file_storage.rb similarity index 100% rename from lib/gems/pending/util/miq_file_storage.rb rename to lib/miq_file_storage.rb diff --git a/lib/gems/pending/util/miq_ftp_lib.rb b/lib/miq_ftp_lib.rb similarity index 100% rename from lib/gems/pending/util/miq_ftp_lib.rb rename to lib/miq_ftp_lib.rb diff --git a/lib/gems/pending/util/miq_object_storage.rb b/lib/miq_object_storage.rb similarity index 100% rename from lib/gems/pending/util/miq_object_storage.rb rename to lib/miq_object_storage.rb diff --git a/lib/gems/pending/util/mount/miq_generic_mount_session.rb b/lib/mount/miq_generic_mount_session.rb similarity index 100% rename from lib/gems/pending/util/mount/miq_generic_mount_session.rb rename to lib/mount/miq_generic_mount_session.rb diff --git a/lib/gems/pending/util/mount/miq_glusterfs_session.rb b/lib/mount/miq_glusterfs_session.rb similarity index 100% rename from lib/gems/pending/util/mount/miq_glusterfs_session.rb rename to lib/mount/miq_glusterfs_session.rb diff --git a/lib/gems/pending/util/mount/miq_local_mount_session.rb b/lib/mount/miq_local_mount_session.rb similarity index 100% rename from lib/gems/pending/util/mount/miq_local_mount_session.rb rename to lib/mount/miq_local_mount_session.rb diff --git a/lib/gems/pending/util/mount/miq_nfs_session.rb b/lib/mount/miq_nfs_session.rb similarity index 100% rename from lib/gems/pending/util/mount/miq_nfs_session.rb rename to lib/mount/miq_nfs_session.rb diff --git a/lib/gems/pending/util/mount/miq_smb_session.rb b/lib/mount/miq_smb_session.rb similarity index 100% rename from lib/gems/pending/util/mount/miq_smb_session.rb rename to lib/mount/miq_smb_session.rb diff --git a/lib/gems/pending/util/object_storage/miq_ftp_storage.rb b/lib/object_storage/miq_ftp_storage.rb similarity index 100% rename from lib/gems/pending/util/object_storage/miq_ftp_storage.rb rename to lib/object_storage/miq_ftp_storage.rb diff --git a/lib/gems/pending/util/object_storage/miq_s3_storage.rb b/lib/object_storage/miq_s3_storage.rb similarity index 100% rename from lib/gems/pending/util/object_storage/miq_s3_storage.rb rename to lib/object_storage/miq_s3_storage.rb diff --git a/lib/gems/pending/util/object_storage/miq_swift_storage.rb b/lib/object_storage/miq_swift_storage.rb similarity index 100% rename from lib/gems/pending/util/object_storage/miq_swift_storage.rb rename to lib/object_storage/miq_swift_storage.rb diff --git a/spec/util/miq_file_storage_spec.rb b/spec/lib/miq_file_storage_spec.rb similarity index 100% rename from spec/util/miq_file_storage_spec.rb rename to spec/lib/miq_file_storage_spec.rb diff --git a/spec/util/miq_ftp_lib_spec.rb b/spec/lib/miq_ftp_lib_spec.rb similarity index 100% rename from spec/util/miq_ftp_lib_spec.rb rename to spec/lib/miq_ftp_lib_spec.rb diff --git a/spec/util/miq_object_storage_spec.rb b/spec/lib/miq_object_storage_spec.rb similarity index 100% rename from spec/util/miq_object_storage_spec.rb rename to spec/lib/miq_object_storage_spec.rb diff --git a/spec/util/mount/miq_generic_mount_session_spec.rb b/spec/lib/mount/miq_generic_mount_session_spec.rb similarity index 100% rename from spec/util/mount/miq_generic_mount_session_spec.rb rename to spec/lib/mount/miq_generic_mount_session_spec.rb diff --git a/spec/util/mount/miq_local_mount_session_spec.rb b/spec/lib/mount/miq_local_mount_session_spec.rb similarity index 100% rename from spec/util/mount/miq_local_mount_session_spec.rb rename to spec/lib/mount/miq_local_mount_session_spec.rb diff --git a/spec/util/object_storage/miq_ftp_storage_spec.rb b/spec/lib/object_storage/miq_ftp_storage_spec.rb similarity index 100% rename from spec/util/object_storage/miq_ftp_storage_spec.rb rename to spec/lib/object_storage/miq_ftp_storage_spec.rb diff --git a/spec/util/object_storage/miq_s3_storage_spec.rb b/spec/lib/object_storage/miq_s3_storage_spec.rb similarity index 100% rename from spec/util/object_storage/miq_s3_storage_spec.rb rename to spec/lib/object_storage/miq_s3_storage_spec.rb diff --git a/spec/util/object_storage/miq_swift_storage_spec.rb b/spec/lib/object_storage/miq_swift_storage_spec.rb similarity index 100% rename from spec/util/object_storage/miq_swift_storage_spec.rb rename to spec/lib/object_storage/miq_swift_storage_spec.rb diff --git a/spec/support/contexts/generated_tmp_files.rb b/spec/support/examples_group/generated_tmp_files.rb similarity index 100% rename from spec/support/contexts/generated_tmp_files.rb rename to spec/support/examples_group/generated_tmp_files.rb diff --git a/spec/support/contexts/with_ftp_server.rb b/spec/support/examples_group/with_ftp_server.rb similarity index 100% rename from spec/support/contexts/with_ftp_server.rb rename to spec/support/examples_group/with_ftp_server.rb From e472d01f0b4b5dd83a9f9712fef9d30f9b871583 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Fri, 3 Jan 2020 10:47:30 -0600 Subject: [PATCH 53/55] [MiqFileStorage] Remove 'util/*' from requires Since this has been moved from `manageiq-gems-pending`, this is no longer needed, and keeping it in causes the build to fail. --- lib/evm_database_ops.rb | 2 +- lib/miq_object_storage.rb | 8 ++++---- lib/mount/miq_generic_mount_session.rb | 10 +++++----- lib/mount/miq_glusterfs_session.rb | 2 +- lib/mount/miq_local_mount_session.rb | 2 +- lib/mount/miq_nfs_session.rb | 2 +- lib/mount/miq_smb_session.rb | 2 +- lib/object_storage/miq_ftp_storage.rb | 4 ++-- lib/object_storage/miq_s3_storage.rb | 2 +- lib/object_storage/miq_swift_storage.rb | 2 +- spec/lib/miq_file_storage_spec.rb | 4 ++-- spec/lib/miq_ftp_lib_spec.rb | 2 +- spec/lib/miq_object_storage_spec.rb | 4 ++-- spec/lib/mount/miq_generic_mount_session_spec.rb | 2 +- spec/lib/mount/miq_local_mount_session_spec.rb | 2 +- spec/lib/object_storage/miq_ftp_storage_spec.rb | 2 +- spec/lib/object_storage/miq_s3_storage_spec.rb | 2 +- spec/lib/object_storage/miq_swift_storage_spec.rb | 2 +- 18 files changed, 28 insertions(+), 28 deletions(-) diff --git a/lib/evm_database_ops.rb b/lib/evm_database_ops.rb index 3b51fca3eb9..def9da12b66 100644 --- a/lib/evm_database_ops.rb +++ b/lib/evm_database_ops.rb @@ -2,7 +2,7 @@ require 'util/postgres_admin' require 'mount/miq_generic_mount_session' -require 'util/miq_object_storage' +require 'miq_object_storage' class EvmDatabaseOps include Vmdb::Logging diff --git a/lib/miq_object_storage.rb b/lib/miq_object_storage.rb index 19b499e6c91..f04bd6f7552 100644 --- a/lib/miq_object_storage.rb +++ b/lib/miq_object_storage.rb @@ -1,10 +1,10 @@ require 'net/protocol' -require 'util/miq_file_storage' +require 'miq_file_storage' class MiqObjectStorage < MiqFileStorage::Interface - require 'util/object_storage/miq_s3_storage' - require 'util/object_storage/miq_ftp_storage' - require 'util/object_storage/miq_swift_storage' + require 'object_storage/miq_s3_storage' + require 'object_storage/miq_ftp_storage' + require 'object_storage/miq_swift_storage' attr_accessor :settings attr_writer :logger diff --git a/lib/mount/miq_generic_mount_session.rb b/lib/mount/miq_generic_mount_session.rb index a25fc0ef60b..299ea20dfe3 100644 --- a/lib/mount/miq_generic_mount_session.rb +++ b/lib/mount/miq_generic_mount_session.rb @@ -6,13 +6,13 @@ require 'util/miq-exception' require 'util/miq-uuid' -require 'util/miq_file_storage' +require 'miq_file_storage' class MiqGenericMountSession < MiqFileStorage::Interface - require 'util/mount/miq_local_mount_session' - require 'util/mount/miq_nfs_session' - require 'util/mount/miq_smb_session' - require 'util/mount/miq_glusterfs_session' + require 'mount/miq_local_mount_session' + require 'mount/miq_nfs_session' + require 'mount/miq_smb_session' + require 'mount/miq_glusterfs_session' attr_accessor :settings, :mnt_point, :logger diff --git a/lib/mount/miq_glusterfs_session.rb b/lib/mount/miq_glusterfs_session.rb index d550963a8cf..03d45ca38eb 100644 --- a/lib/mount/miq_glusterfs_session.rb +++ b/lib/mount/miq_glusterfs_session.rb @@ -1,4 +1,4 @@ -require 'util/mount/miq_generic_mount_session' +require 'mount/miq_generic_mount_session' class MiqGlusterfsSession < MiqGenericMountSession PORTS = [2049, 111].freeze diff --git a/lib/mount/miq_local_mount_session.rb b/lib/mount/miq_local_mount_session.rb index 0395e7c2093..fae29bb4f46 100644 --- a/lib/mount/miq_local_mount_session.rb +++ b/lib/mount/miq_local_mount_session.rb @@ -1,4 +1,4 @@ -require 'util/mount/miq_generic_mount_session' +require 'mount/miq_generic_mount_session' # MiqLocalMountSession is meant to be a representation of the local file system # that conforms to the same interface as MiqLocalMountSession (and by proxy, diff --git a/lib/mount/miq_nfs_session.rb b/lib/mount/miq_nfs_session.rb index 92036657536..80b51acd0a8 100644 --- a/lib/mount/miq_nfs_session.rb +++ b/lib/mount/miq_nfs_session.rb @@ -1,4 +1,4 @@ -require 'util/mount/miq_generic_mount_session' +require 'mount/miq_generic_mount_session' class MiqNfsSession < MiqGenericMountSession PORTS = [2049, 111] diff --git a/lib/mount/miq_smb_session.rb b/lib/mount/miq_smb_session.rb index 535c94adcd8..59a6b41b2e5 100644 --- a/lib/mount/miq_smb_session.rb +++ b/lib/mount/miq_smb_session.rb @@ -1,4 +1,4 @@ -require 'util/mount/miq_generic_mount_session' +require 'mount/miq_generic_mount_session' class MiqSmbSession < MiqGenericMountSession PORTS = [445, 139] diff --git a/lib/object_storage/miq_ftp_storage.rb b/lib/object_storage/miq_ftp_storage.rb index da81a0d16af..5ce61ad0c8f 100644 --- a/lib/object_storage/miq_ftp_storage.rb +++ b/lib/object_storage/miq_ftp_storage.rb @@ -1,5 +1,5 @@ -require 'util/miq_ftp_lib' -require 'util/miq_object_storage' +require 'miq_ftp_lib' +require 'miq_object_storage' require 'logger' class MiqFtpStorage < MiqObjectStorage diff --git a/lib/object_storage/miq_s3_storage.rb b/lib/object_storage/miq_s3_storage.rb index 63696d4c692..64541a91160 100644 --- a/lib/object_storage/miq_s3_storage.rb +++ b/lib/object_storage/miq_s3_storage.rb @@ -1,4 +1,4 @@ -require 'util/miq_object_storage' +require 'miq_object_storage' class MiqS3Storage < MiqObjectStorage attr_reader :bucket_name diff --git a/lib/object_storage/miq_swift_storage.rb b/lib/object_storage/miq_swift_storage.rb index ebefd5ddd2f..82ae907b276 100644 --- a/lib/object_storage/miq_swift_storage.rb +++ b/lib/object_storage/miq_swift_storage.rb @@ -1,4 +1,4 @@ -require 'util/miq_object_storage' +require 'miq_object_storage' class MiqSwiftStorage < MiqObjectStorage attr_reader :container_name diff --git a/spec/lib/miq_file_storage_spec.rb b/spec/lib/miq_file_storage_spec.rb index 53050f3a904..a6ccb394382 100644 --- a/spec/lib/miq_file_storage_spec.rb +++ b/spec/lib/miq_file_storage_spec.rb @@ -1,5 +1,5 @@ -require "util/mount/miq_generic_mount_session" -require "util/miq_object_storage" +require "mount/miq_generic_mount_session" +require "miq_object_storage" describe MiqFileStorage do def opts_for_nfs diff --git a/spec/lib/miq_ftp_lib_spec.rb b/spec/lib/miq_ftp_lib_spec.rb index 21792c5d5bb..fb462657894 100644 --- a/spec/lib/miq_ftp_lib_spec.rb +++ b/spec/lib/miq_ftp_lib_spec.rb @@ -1,4 +1,4 @@ -require 'util/miq_ftp_lib' +require 'miq_ftp_lib' require 'logger' # probably loaded elsewhere, but for the below classes class FTPKlass diff --git a/spec/lib/miq_object_storage_spec.rb b/spec/lib/miq_object_storage_spec.rb index 901390549bc..86dd9945eda 100644 --- a/spec/lib/miq_object_storage_spec.rb +++ b/spec/lib/miq_object_storage_spec.rb @@ -1,6 +1,6 @@ require "fileutils" -require "util/mount/miq_generic_mount_session" -require "util/miq_object_storage" +require "mount/miq_generic_mount_session" +require "miq_object_storage" class MockLocalFileStorage < MiqObjectStorage def initialize(source_path = nil, byte_count = 2.megabytes) diff --git a/spec/lib/mount/miq_generic_mount_session_spec.rb b/spec/lib/mount/miq_generic_mount_session_spec.rb index c637235528b..0ab6226ccda 100644 --- a/spec/lib/mount/miq_generic_mount_session_spec.rb +++ b/spec/lib/mount/miq_generic_mount_session_spec.rb @@ -1,4 +1,4 @@ -require "util/mount/miq_generic_mount_session" +require "mount/miq_generic_mount_session" describe MiqGenericMountSession do it "#connect returns a string pointing to the mount point" do diff --git a/spec/lib/mount/miq_local_mount_session_spec.rb b/spec/lib/mount/miq_local_mount_session_spec.rb index 47df8c9ea57..1999f14783f 100644 --- a/spec/lib/mount/miq_local_mount_session_spec.rb +++ b/spec/lib/mount/miq_local_mount_session_spec.rb @@ -1,4 +1,4 @@ -require 'util/mount/miq_local_mount_session' +require 'mount/miq_local_mount_session' require 'tempfile' describe MiqLocalMountSession do diff --git a/spec/lib/object_storage/miq_ftp_storage_spec.rb b/spec/lib/object_storage/miq_ftp_storage_spec.rb index 09d3e2e822f..a1097fe4a89 100644 --- a/spec/lib/object_storage/miq_ftp_storage_spec.rb +++ b/spec/lib/object_storage/miq_ftp_storage_spec.rb @@ -1,4 +1,4 @@ -require 'util/object_storage/miq_ftp_storage.rb' +require 'object_storage/miq_ftp_storage.rb' describe MiqFtpStorage, :with_ftp_server do subject { described_class.new(ftp_creds.merge(:uri => "ftp://localhost")) } diff --git a/spec/lib/object_storage/miq_s3_storage_spec.rb b/spec/lib/object_storage/miq_s3_storage_spec.rb index 1d62c998aae..50b6328cbf5 100644 --- a/spec/lib/object_storage/miq_s3_storage_spec.rb +++ b/spec/lib/object_storage/miq_s3_storage_spec.rb @@ -1,4 +1,4 @@ -require "util/object_storage/miq_s3_storage" +require "object_storage/miq_s3_storage" describe MiqS3Storage do before(:each) do diff --git a/spec/lib/object_storage/miq_swift_storage_spec.rb b/spec/lib/object_storage/miq_swift_storage_spec.rb index 5d54edd3d54..6874c44a497 100644 --- a/spec/lib/object_storage/miq_swift_storage_spec.rb +++ b/spec/lib/object_storage/miq_swift_storage_spec.rb @@ -1,4 +1,4 @@ -require "util/object_storage/miq_swift_storage" +require "object_storage/miq_swift_storage" describe MiqSwiftStorage do let(:object_storage) { described_class.new(:uri => uri, :username => 'user', :password => 'pass') } From 6f4d9a6cdad64b0bb7ed2521b498994ba472db47 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Fri, 22 Nov 2019 17:41:39 -0600 Subject: [PATCH 54/55] Update brakeman.ignore for MiqObjectStorage - Adds ignore cases for two items that have been there for a bit - Removes an outdated warning. --- config/brakeman.ignore | 48 ++++++++++++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/config/brakeman.ignore b/config/brakeman.ignore index de2755302c0..219ae2fee6c 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -21,24 +21,44 @@ "note": "The chomp.to_i ensures we get a number and we protect against 0 with a conditional. The only other possible avenue for attack is if the attacker could replace pgrep, but then they already have root access, so it's a moot point." }, { - "warning_type": "File Access", - "warning_code": 16, - "fingerprint": "4e1918c2d5ff2beacc21db09f696af724d62f1a2a6a101e8e3cb564d0e8a94cd", - "check_name": "FileAccess", - "message": "Model attribute used in file name", - "file": "app/models/miq_report/import_export.rb", - "line": 85, - "link": "http://brakemanscanner.org/docs/warning_types/file_access/", - "code": "YAML.load_file(MiqReport.view_yaml_filename(db, current_user, options))", + "warning_type": "Command Injection", + "warning_code": 14, + "fingerprint": "6a9ec4613af89e29c750be8db27e7b64118ebef6a458357995c51614f26e4f4a", + "check_name": "Execute", + "message": "Possible command injection", + "file": "lib/mount/miq_generic_mount_session.rb", + "line": 34, + "link": "http://brakemanscanner.org/docs/warning_types/command_injection/", + "code": "`#{cmd_str} 2>&1`", "render_path": null, "location": { "type": "method", - "class": "MiqReport::ImportExport::ClassMethods", - "method": "load_from_view_options" + "class": "MiqGenericMountSession", + "method": "s(:self).runcmd" }, - "user_input": "MiqReport.view_yaml_filename(db, current_user, options)", + "user_input": "cmd_str", "confidence": "Medium", - "note": "Temporarily skipped, found in new brakeman version" + "note": "" + }, + { + "warning_type": "Command Injection", + "warning_code": 14, + "fingerprint": "84d4a4e5555b6b750216afadc01f9e385a8a1d56c97b1a8aa3f10925f446932b", + "check_name": "Execute", + "message": "Possible command injection", + "file": "lib/mount/miq_generic_mount_session.rb", + "line": 40, + "link": "http://brakemanscanner.org/docs/warning_types/command_injection/", + "code": "`sudo #{cmd_str} 2>&1`", + "render_path": null, + "location": { + "type": "method", + "class": "MiqGenericMountSession", + "method": "s(:self).runcmd" + }, + "user_input": "cmd_str", + "confidence": "Medium", + "note": "" }, { "warning_type": "Command Injection", @@ -81,6 +101,6 @@ "note": "Temporarily skipped, found in new brakeman version" } ], - "updated": "2017-11-01 11:16:49 -0400", + "updated": "2019-11-22 17:39:13 -0600", "brakeman_version": "3.7.2" } From 8820a90638a237ccc34de5a51f54dad4cf215ea6 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Fri, 22 Nov 2019 17:15:51 -0600 Subject: [PATCH 55/55] [Gemfile] Add ftpd to test dependencies This is used by the `MiqFtpLib` specs --- Gemfile | 1 + 1 file changed, 1 insertion(+) diff --git a/Gemfile b/Gemfile index 1488d263357..8e3c412b43c 100644 --- a/Gemfile +++ b/Gemfile @@ -253,6 +253,7 @@ unless ENV["APPLIANCE"] gem "capybara", "~>2.5.0", :require => false gem "coveralls", :require => false gem "factory_bot", "~>5.1", :require => false + gem "ftpd", "~> 2.1.0", :require => false # TODO: faker is used for url generation in git repository factory and the lenovo # provider, via a xclarity_client dependency