Skip to content

Commit

Permalink
Fix uri dirname parsing for swift restore
Browse files Browse the repository at this point in the history
The swift protocol is the only file storage used by `EvmDatabaseOps`
at this time that uses query params as part of setting it's options.

The previous implementation of `.with_file_storage` would use a simple
`File.dirname` to determine the mount point.  This causes the query
params to be completely stripped from the uri, and messes with the
configuration set by the user via the query params.

There is also a quirk with the implementation of `URI::FTP` (stdlib)
that overrides `.path` differently since it swaps it's `"/"` characters
for `"%2F"`, which we don't happening unannounced.

So for the fix, we use `URI::Generic.new` directly, and avoid the
`URI.parse` method since it will make use of `URI::FTP` in those cases.
Split is used as just a convenient way of getting the args for
`URI::Generic.new`.
  • Loading branch information
NickLaMuro committed Nov 5, 2018
1 parent 61d1edd commit 2d9e5d7
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 1 deletion.
5 changes: 4 additions & 1 deletion lib/evm_database_ops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,10 @@ def self.restore(db_opts, connect_opts = {})
if db_opts[:local_file].nil?
if action == :restore
uri = connect_opts[:uri]
connect_opts[:uri] = File.dirname(connect_opts[:uri])

connect_uri = URI::Generic.new(*URI.split(uri))
connect_uri.path = File.dirname(connect_uri.path)
connect_opts[:uri] = connect_uri.to_s
else
connect_opts[:remote_file_name] ||= File.basename(backup_file_name(action))
#
Expand Down
18 changes: 18 additions & 0 deletions spec/lib/evm_database_ops_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -291,10 +291,28 @@ def execute_with_file_storage(action = :backup)
execute_with_file_storage(:restore)
end

it "parses the dirname of the `uri` and passes that in `connect_opts`" do
expected_connect_opts = { :uri => "smb://tmp/" }
allow(file_storage).to receive(:download)
expect(MiqFileStorage).to receive(:with_interface_class).with(expected_connect_opts)
execute_with_file_storage(:restore)
end

it "returns calculated uri" do
allow(file_storage).to receive(:download).and_yield(input_path)
expect(execute_with_file_storage(:restore) { "block result" }).to eq("smb://tmp/foo")
end

context "with query_params in the URI" do
let(:connect_opts) { { :uri => "swift://container/foo.gz?2plus2=5" } }

it "retains query_params when parsing dirname" do
expected_connect_opts = { :uri => "swift://container/?2plus2=5" }
allow(file_storage).to receive(:download)
expect(MiqFileStorage).to receive(:with_interface_class).with(expected_connect_opts)
execute_with_file_storage(:restore)
end
end
end
end
end
Expand Down

0 comments on commit 2d9e5d7

Please sign in to comment.