From 463c79b8d416a65b3576be64798487acf74e8c46 Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Thu, 20 Sep 2018 13:41:14 -0400 Subject: [PATCH 1/3] Move database admin sample_urls to database_admin The next move is to localize these. But first step, get them out of prompts and into database_admin.rb --- .../appliance_console/database_admin.rb | 13 ++- lib/manageiq/appliance_console/prompts.rb | 11 --- spec/database_admin_spec.rb | 79 +++++++++++-------- spec/prompts_spec.rb | 10 --- 4 files changed, 57 insertions(+), 56 deletions(-) diff --git a/lib/manageiq/appliance_console/database_admin.rb b/lib/manageiq/appliance_console/database_admin.rb index c2302e446..a195808ed 100644 --- a/lib/manageiq/appliance_console/database_admin.rb +++ b/lib/manageiq/appliance_console/database_admin.rb @@ -29,6 +29,13 @@ class DatabaseAdmin < HighLine WARN + SAMPLE_URLS = { + 'nfs' => 'nfs://host.mydomain.com/exported/my_exported_folder/db.backup', + 'smb' => 'smb://host.mydomain.com/my_share/daily_backup/db.backup', + 's3' => 's3://mybucket/my_subdirectory/daily_backup/db.backup', + 'ftp' => 'ftp://host.mydomain.com/path/to/daily_backup/db.backup' + } + attr_reader :action, :backup_type, :task, :task_params, :delete_agree, :uri, :filename def initialize(action = :restore, input = $stdin, output = $stdout) @@ -232,10 +239,14 @@ def remote_file_prompt_args_for(remote_type) else "location to save the remote #{action} file to" end - prompt += "\nExample: #{SAMPLE_URLS[remote_type]}" + prompt += "\nExample: #{sample_url(remote_type)}" [prompt, remote_type] end + def sample_url(scheme) + SAMPLE_URLS[scheme] + end + def processing_message msg = if action == :restore "\nRestoring the database..." diff --git a/lib/manageiq/appliance_console/prompts.rb b/lib/manageiq/appliance_console/prompts.rb index 581481f41..10a85aa25 100644 --- a/lib/manageiq/appliance_console/prompts.rb +++ b/lib/manageiq/appliance_console/prompts.rb @@ -15,17 +15,6 @@ module Prompts NONE_REGEXP = /^('?NONE'?)?$/i.freeze HOSTNAME_REGEXP = /^(([a-zA-Z]|[a-zA-Z][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z]|[A-Za-z][A-Za-z0-9\-]*[A-Za-z0-9])$/ - SAMPLE_URLS = { - 'nfs' => 'nfs://host.mydomain.com/exported/my_exported_folder/db.backup', - 'smb' => 'smb://host.mydomain.com/my_share/daily_backup/db.backup', - 's3' => 's3://mybucket/my_subdirectory/daily_backup/db.backup', - 'ftp' => 'ftp://host.mydomain.com/path/to/daily_backup/db.backup' - } - - def sample_url(scheme) - SAMPLE_URLS[scheme] - end - def ask_for_uri(prompt, expected_scheme, opts = {}) require 'uri' just_ask(prompt, nil, nil, 'a valid URI') do |q| diff --git a/spec/database_admin_spec.rb b/spec/database_admin_spec.rb index 5c0146703..976958438 100644 --- a/spec/database_admin_spec.rb +++ b/spec/database_admin_spec.rb @@ -171,9 +171,9 @@ end describe "#ask_nfs_file_options" do - let(:uri) { File.dirname(subject.sample_url('nfs')) } - let(:filename) { File.basename(subject.sample_url('nfs')) } - let(:example_uri) { File.join(uri, filename) } + let(:example_uri) { subject.send(:sample_url, 'nfs') } + let(:uri) { File.dirname(example_uri) } + let(:filename) { File.basename(example_uri) } let(:prmpt) { "location of the remote backup file\nExample: #{example_uri}" } let(:errmsg) { "a valid URI" } @@ -219,9 +219,9 @@ end describe "#ask_smb_file_options" do - let(:uri) { File.dirname(subject.sample_url('smb')) } - let(:filename) { File.basename(subject.sample_url('smb')) } - let(:example_uri) { File.join(uri, filename) } + let(:example_uri) { subject.send(:sample_url, 'smb') } + let(:uri) { File.dirname(example_uri) } + let(:filename) { File.basename(example_uri) } let(:user) { 'example.com/admin' } let(:pass) { 'supersecret' } let(:uri_prompt) { "Enter the location of the remote backup file\nExample: #{example_uri}" } @@ -292,9 +292,9 @@ end describe "#ask_s3_file_options" do - let(:uri) { File.dirname(subject.sample_url('s3')) } - let(:filename) { File.basename(subject.sample_url('s3')) } - let(:example_uri) { File.join(uri, filename) } + let(:example_uri) { subject.send(:sample_url, 's3') } + let(:uri) { File.dirname(example_uri) } + let(:filename) { File.basename(example_uri) } let(:access_key_id) { 'foobar' } let(:secret_access_key) { 'supersecret' } let(:region) { 'us-east-2' } @@ -394,9 +394,9 @@ end describe "#ask_ftp_file_options" do - let(:uri) { File.dirname(subject.sample_url('ftp')) } - let(:filename) { File.basename(subject.sample_url('ftp')) } - let(:example_uri) { File.join(uri, filename) } + let(:example_uri) { subject.send(:sample_url, 'ftp') } + let(:uri) { File.dirname(example_uri) } + let(:filename) { File.basename(example_uri) } let(:user) { 'admin' } let(:pass) { 'supersecret' } let(:uri_prompt) { "Enter the location of the remote backup file\nExample: #{example_uri}" } @@ -831,9 +831,9 @@ def confirm_and_execute end describe "#ask_nfs_file_options" do - let(:uri) { File.dirname(subject.sample_url('nfs')) } - let(:filename) { File.basename(subject.sample_url('nfs')) } - let(:example_uri) { File.join(uri, filename) } + let(:example_uri) { subject.send(:sample_url, 'nfs') } + let(:uri) { File.dirname(example_uri) } + let(:filename) { File.basename(example_uri) } let(:prmpt) { "location to save the remote backup file to\nExample: #{example_uri}" } let(:errmsg) { "a valid URI" } @@ -881,9 +881,9 @@ def confirm_and_execute end describe "#ask_smb_file_options" do - let(:uri) { File.dirname(subject.sample_url('smb')) } - let(:filename) { File.basename(subject.sample_url('smb')) } - let(:example_uri) { File.join(uri, filename) } + let(:example_uri) { subject.send(:sample_url, 'smb') } + let(:uri) { File.dirname(example_uri) } + let(:filename) { File.basename(example_uri) } let(:user) { 'example.com/admin' } let(:pass) { 'supersecret' } let(:file_prompt) { "location to save the backup file to" } @@ -957,9 +957,9 @@ def confirm_and_execute end describe "#ask_s3_file_options" do - let(:uri) { File.dirname(subject.sample_url('s3')) } - let(:filename) { File.basename(subject.sample_url('s3')) } - let(:example_uri) { File.join(uri, filename) } + let(:example_uri) { subject.send(:sample_url, 's3') } + let(:uri) { File.dirname(example_uri) } + let(:filename) { File.basename(example_uri) } let(:access_key_id) { 'foobar' } let(:secret_access_key) { 'supersecret' } let(:region) { 'us-east-2' } @@ -1033,7 +1033,7 @@ def confirm_and_execute context "with an empty path URI" do let(:uri) { 's3://mybucket' } let(:filename) { 'database_backup.tar.gz' } - let(:example_uri) { subject.sample_url('s3') } + let(:example_uri) { subject.send(:sample_url, 's3') } before do say [filename, uri, region, access_key_id, secret_access_key] @@ -1087,9 +1087,9 @@ def confirm_and_execute end describe "#ask_ftp_file_options" do - let(:uri) { File.dirname(subject.sample_url('ftp')) } - let(:filename) { File.basename(subject.sample_url('ftp')) } - let(:example_uri) { File.join(uri, filename) } + let(:example_uri) { subject.send(:sample_url, 'ftp') } + let(:uri) { File.dirname(example_uri) } + let(:filename) { File.basename(example_uri) } let(:user) { 'admin' } let(:pass) { 'supersecret' } let(:uri_prompt) { "Enter the location to save the remote backup file to\nExample: #{example_uri}" } @@ -1501,9 +1501,9 @@ def confirm_and_execute end describe "#ask_nfs_file_options" do - let(:uri) { File.dirname(subject.sample_url('nfs')) } - let(:filename) { File.basename(subject.sample_url('nfs')) } - let(:example_uri) { File.join(uri, filename) } + let(:example_uri) { subject.send(:sample_url, 'nfs') } + let(:uri) { File.dirname(example_uri) } + let(:filename) { File.basename(example_uri) } let(:prmpt) { "location to save the remote dump file to\nExample: #{example_uri}" } let(:errmsg) { "a valid URI" } @@ -1549,9 +1549,9 @@ def confirm_and_execute end describe "#ask_smb_file_options" do - let(:uri) { File.dirname(subject.sample_url('smb')) } - let(:filename) { File.basename(subject.sample_url('smb')) } - let(:example_uri) { File.join(uri, filename) } + let(:example_uri) { subject.send(:sample_url, 'smb') } + let(:uri) { File.dirname(example_uri) } + let(:filename) { File.basename(example_uri) } let(:user) { 'example.com/admin' } let(:pass) { 'supersecret' } let(:file_prompt) { "location to save the dump file to" } @@ -1625,9 +1625,9 @@ def confirm_and_execute end describe "#ask_ftp_file_options" do - let(:uri) { File.dirname(subject.sample_url('ftp')) } - let(:filename) { File.basename(subject.sample_url('ftp')) } - let(:example_uri) { File.join(uri, filename) } + let(:example_uri) { subject.send(:sample_url, 'ftp') } + let(:uri) { File.dirname(example_uri) } + let(:filename) { File.basename(example_uri) } let(:user) { 'admin' } let(:pass) { 'supersecret' } let(:uri_prompt) { "Enter the location to save the remote dump file to\nExample: #{example_uri}" } @@ -1954,5 +1954,16 @@ def confirm_and_execute end end end + + # private, but moved out of prompt and keeping tests around + describe "#sample_url" do + it "should show an example for nfs" do + expect(subject.send(:sample_url, 'nfs')).to match(%r{nfs://}) + end + + it "should show an example for smb" do + expect(subject.send(:sample_url, 'smb')).to match(%r{smb://}) + end + end end # rubocop:enable Layout/TrailingWhitespace diff --git a/spec/prompts_spec.rb b/spec/prompts_spec.rb index 65d8e15c1..bb5a8ff98 100644 --- a/spec/prompts_spec.rb +++ b/spec/prompts_spec.rb @@ -5,16 +5,6 @@ Class.new(HighLine) { include ManageIQ::ApplianceConsole::Prompts }.new(input, output) end - context "#sample_url" do - it "should show an example for nfs" do - expect(subject.sample_url('nfs')).to match(%r{nfs://}) - end - - it "should show an example for smb" do - expect(subject.sample_url('smb')).to match(%r{smb://}) - end - end - context "#ask_for_remote_backup_uri" do it "should ask for smb uri" do response = "smb://host.com/path/file.txt" From 688f94ae80a7b70bf18826b30f57bba2203d7531 Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Fri, 21 Sep 2018 14:18:25 -0400 Subject: [PATCH 2/3] localize upload target text and sample_urls This allows the file types to be customized via localization downside, the name of the file_option types are now using send --- .../appliance_console/database_admin.rb | 39 ++++++++----------- locales/appliance/en.yml | 17 ++++++++ 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/lib/manageiq/appliance_console/database_admin.rb b/lib/manageiq/appliance_console/database_admin.rb index a195808ed..9b4230c95 100644 --- a/lib/manageiq/appliance_console/database_admin.rb +++ b/lib/manageiq/appliance_console/database_admin.rb @@ -5,12 +5,11 @@ module ApplianceConsole class DatabaseAdmin < HighLine include ManageIQ::ApplianceConsole::Prompts - LOCAL_FILE = "Local file".freeze - NFS_FILE = "Network File System (NFS)".freeze - SMB_FILE = "Samba (SMB)".freeze - S3_FILE = "Amazon S3 (S3)".freeze - FTP_FILE = "File Transfer Protocol (FTP)".freeze - FILE_OPTIONS = [LOCAL_FILE, NFS_FILE, SMB_FILE, S3_FILE, FTP_FILE, CANCEL].freeze + LOCAL_FILE = "local".freeze + NFS_FILE = "nfs".freeze + SMB_FILE = "smb".freeze + S3_FILE = "s3".freeze + FTP_FILE = "ftp".freeze DB_RESTORE_FILE = "/tmp/evm_db.backup".freeze DB_DEFAULT_DUMP_FILE = "/tmp/evm_db.dump".freeze @@ -29,13 +28,6 @@ class DatabaseAdmin < HighLine WARN - SAMPLE_URLS = { - 'nfs' => 'nfs://host.mydomain.com/exported/my_exported_folder/db.backup', - 'smb' => 'smb://host.mydomain.com/my_share/daily_backup/db.backup', - 's3' => 's3://mybucket/my_subdirectory/daily_backup/db.backup', - 'ftp' => 'ftp://host.mydomain.com/path/to/daily_backup/db.backup' - } - attr_reader :action, :backup_type, :task, :task_params, :delete_agree, :uri, :filename def initialize(action = :restore, input = $stdin, output = $stdout) @@ -62,14 +54,11 @@ def activate end def ask_file_location - case @backup_type = ask_with_menu(*file_menu_args) - when LOCAL_FILE then ask_local_file_options - when NFS_FILE then ask_nfs_file_options - when SMB_FILE then ask_smb_file_options - when S3_FILE then ask_s3_file_options - when FTP_FILE then ask_ftp_file_options - when CANCEL then raise MiqSignalError + @backup_type = ask_with_menu(*file_menu_args) do |menu| + menu.choice(CANCEL) { |_| raise MiqSignalError } end + # calling methods like ask_ftp_file_options and ask_s3_file_options + send("ask_#{backup_type}_file_options") end def ask_local_file_options @@ -192,10 +181,16 @@ def allowed_to_execute? agree("Are you sure you would like to restore the database? (Y/N): ") end + def file_options + @file_options ||= I18n.t("database_admin.menu_order").each_with_object({}) do |file_option, h| + h[I18n.t("database_admin.#{file_option}")] = file_option + end + end + def file_menu_args [ action == :restore ? "Restore Database File" : "#{action.capitalize} Output File Name", - FILE_OPTIONS, + file_options, LOCAL_FILE, nil ] @@ -244,7 +239,7 @@ def remote_file_prompt_args_for(remote_type) end def sample_url(scheme) - SAMPLE_URLS[scheme] + I18n.t("database_admin.sample_url.#{scheme}") end def processing_message diff --git a/locales/appliance/en.yml b/locales/appliance/en.yml index 7d3b9dd1d..e7fc2a037 100644 --- a/locales/appliance/en.yml +++ b/locales/appliance/en.yml @@ -42,3 +42,20 @@ en: shutdown: Shut Down Appliance summary: Summary Information quit: Quit + database_admin: + menu_order: + - local + - nfs + - smb + - s3 + - ftp + local: Local file + nfs: Network File System (NFS) + smb: Samba (SMB) + s3: Amazon S3 (S3) + ftp: File Transfer Protocol (FTP) + sample_url: + nfs: nfs://host.mydomain.com/exported/my_exported_folder/db.backup + smb: smb://host.mydomain.com/my_share/daily_backup/db.backup + s3: s3://mybucket/my_subdirectory/daily_backup/db.backup + ftp: ftp://host.mydomain.com/path/to/daily_backup/db.backup From 51eb422d2f303226e14c261bd03798c63b47be10 Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Mon, 24 Sep 2018 12:43:59 -0400 Subject: [PATCH 3/3] anonymous ftp upload option https://bugzilla.redhat.com/show_bug.cgi?id=1535345 https://bugzilla.redhat.com/show_bug.cgi?id=1632433 This will allow customers to upload files to support sites To add this option, add to `en.yml`: ```yml database_admin: menu_order: - local - ftp://ftp.example.com/incoming/999999-db.backup local: Local file prompts: ftp.example.com: filename_text: "The case number dash (-) filename. (e.g.: 12345-db.backup)" filename_validator: "^[0-9]{4,}-..*" ``` --- .../appliance_console/database_admin.rb | 41 ++++++- spec/database_admin_spec.rb | 101 ++++++++++++++++++ 2 files changed, 139 insertions(+), 3 deletions(-) diff --git a/lib/manageiq/appliance_console/database_admin.rb b/lib/manageiq/appliance_console/database_admin.rb index 9b4230c95..949516531 100644 --- a/lib/manageiq/appliance_console/database_admin.rb +++ b/lib/manageiq/appliance_console/database_admin.rb @@ -1,4 +1,5 @@ require 'manageiq/appliance_console/errors' +require 'uri' module ManageIQ module ApplianceConsole @@ -57,8 +58,12 @@ def ask_file_location @backup_type = ask_with_menu(*file_menu_args) do |menu| menu.choice(CANCEL) { |_| raise MiqSignalError } end - # calling methods like ask_ftp_file_options and ask_s3_file_options - send("ask_#{backup_type}_file_options") + if URI(backup_type).scheme + ask_custom_file_options(backup_type) + else + # calling methods like ask_ftp_file_options and ask_s3_file_options + send("ask_#{backup_type}_file_options") + end end def ask_local_file_options @@ -134,6 +139,20 @@ def ask_ftp_file_options @task_params = ["--", params] end + def ask_custom_file_options(server_uri) + @filename = just_ask(*filename_prompt_args) unless action == :restore + sample_case = server_uri.split("/").last + hostname = URI(server_uri).host + uri_filename = ask_custom_prompt(hostname, 'filename', "Target filename (e.g.: #{sample_case})") + @uri = server_uri.gsub(sample_case, uri_filename) + + params = { :uri => uri } + params[:remote_file_name] = filename if filename + + @task = "evm:db:#{action}:remote" + @task_params = ["--", params] + end + def ask_to_delete_backup_after_restore if action == :restore && backup_type == LOCAL_FILE say("The local database restore file is located at: '#{uri}'.\n") @@ -177,13 +196,20 @@ def confirm_and_execute def allowed_to_execute? return true unless action == :restore + say("\nNote: A database restore cannot be undone. The restore will use the file: #{uri}.\n") agree("Are you sure you would like to restore the database? (Y/N): ") end def file_options @file_options ||= I18n.t("database_admin.menu_order").each_with_object({}) do |file_option, h| - h[I18n.t("database_admin.#{file_option}")] = file_option + # special anonymous ftp sites are defined by uri + uri = URI(file_option) + if uri.scheme + h["#{uri.scheme} to #{uri.host}"] = file_option + else + h[I18n.t("database_admin.#{file_option}")] = file_option + end end end @@ -202,6 +228,15 @@ def setting_header private + def ask_custom_prompt(type, prompt_name, default_prompt) + # type (domain name) has a period in it, so we need to look it up by [] instead of the traditional i18n method + prompts = I18n.t("database_admin.prompts", :default => nil).try(:[], type.to_sym) + prompt_text = prompts && prompts["#{prompt_name}_text".to_sym] || default_prompt + prompt_regex = prompts && prompts["#{prompt_name}_validator".to_sym] + validator = prompt_regex ? ->(x) { x.to_s =~ /#{prompt_regex}/ } : ->(x) { x.to_s.present? } + just_ask(prompt_text, nil, validator) + end + def should_exclude_tables? ask_yn?("Would you like to exclude tables in the dump") do |q| q.readline = true diff --git a/spec/database_admin_spec.rb b/spec/database_admin_spec.rb index 976958438..f863b725d 100644 --- a/spec/database_admin_spec.rb +++ b/spec/database_admin_spec.rb @@ -1464,6 +1464,25 @@ def confirm_and_execute say "6" expect { subject.ask_file_location }.to raise_error signal_error end + + context "with localized file upload" do + it "displays anonymous ftp option" do + expect(I18n).to receive(:t).with("database_admin.menu_order").and_return(%w(local ftp://example.com/inbox/filename.txt)) + expect(I18n).to receive(:t).with("database_admin.local").and_return("The Local file") + expect(subject).to receive(:ask_local_file_options).once + say "" + subject.ask_file_location + expect_output <<-PROMPT.strip_heredoc.chomp + " " + Dump Output File Name + + 1) The Local file + 2) ftp to example.com + 3) Cancel + + Choose the dump output file name: |1| + PROMPT + end + end end describe "#ask_local_file_options" do @@ -1716,6 +1735,88 @@ def confirm_and_execute end end + describe "#ask_custom_file_options" do + let(:example_uri) { "ftp://example.com/inbox/sample.txt" } + let(:uri) { "ftp://example.com/inbox/sample.txt".gsub("sample.txt", target) } + let(:host) { URI(example_uri).host } + let(:filename) { "/tmp/localfile.txt" } + let(:target) { "123456-filename.txt" } + let(:uri_prompt) { "Enter the location to save the remote backup file to\nExample: #{example_uri}" } + let(:user_prompt) { "Enter the username with access to this file.\nExample: 'mydomain.com/user'" } + let(:pass_prompt) { "Enter the password for #{user}" } + let(:errmsg) { "a valid URI" } + + let(:expected_task_params) do + [ + "--", + { + :uri => uri, + :remote_file_name => filename + } + ] + end + + context "with a valid target" do + before do + say [filename, target] + expect(subject.ask_custom_file_options(example_uri)).to be_truthy + end + + it "sets @uri to point to the ftp share url" do + expect(subject.uri).to eq(uri) + end + + it "sets @filename to nil" do + expect(subject.filename).to eq(filename) + end + + it "sets @task to point to 'evm:db:dump:remote'" do + expect(subject.task).to eq("evm:db:dump:remote") + end + + it "sets @task_params to point to the ftp file" do + expect(subject.task_params).to eq(expected_task_params) + end + end + + context "with invalid target (then valid)" do + before do + say [filename, "", target] + expect(subject.ask_custom_file_options(example_uri)).to be_truthy + end + + it "sets @task_params to point to the ftp file" do + expect(subject.task_params).to eq(expected_task_params) + end + end + + context "with custom prompts" do + before do + expect(I18n).to receive(:t).with("database_admin.prompts", :default => nil).and_return( + host.to_sym => { + :filename_text => "Target please", + :filename_validator => "^[0-9]+-.+$" + } + ) + + # if it doesn't ask again, it won't get the right task_params + say [filename, "", "bad-2", target] + expect(subject.ask_custom_file_options(example_uri)).to be_truthy + expect_readline_question_asked "Enter the location to save the dump file to: |/tmp/evm_db.dump|" + expect_readline_question_asked "Target please: " + expect_output [ + "Please provide in the specified format", + "? Please provide in the specified format", + "? ", + ].join("\n") + end + + it "uses custom validation" do + expect(subject.task_params).to eq(expected_task_params) + end + end + end + describe "#ask_to_delete_backup_after_restore" do context "when @backup_type is LOCAL_FILE" do let(:uri) { described_class::DB_RESTORE_FILE }