Re-insert MiqFileStorage and MiqFTPLib

[NickLaMuro]

EDIT @Fryguy: To be merged with ManageIQ/manageiq-gems-pending#454

It's working! (I think...)

This puts back the code for MiqFileStorage (mounting, object storage, etc.) and the MiqFTPLib code from manageiq-gems-pending back into this repo. This code really has no business living in gems pending, and until we find a better place for it ("core" gem or what not), this will allow us to clean up the gems pending repo a bit more.

Note: The MiqFTPLib technically never was a part of this repo in any form, but moving it here in one swoop with the rest of the code that uses it.

How (seriously...)

The code to build these commits uses a [WIP] branch of the code-extactor project:

juliancheal/code-extractor#11

That I have been using to handle and reproduce the steps needed to perform what I have so far (as of opening this PR). As of right now, that code most likely won't be merged into the master branch of that project, but it is a good reference for the steps I took and the trial and error that was required to get the branch to it's current point.

Configuration

The configuration that I used was the following:

:name: object_storage_and_ftp_lib
:reinsert: true
:target_name: "ManageIQ/manageiq"
:target_remote: "[email protected]:NickLaMuro/manageiq.git"
:target_base_branch: "master"
:upstream: "[email protected]:NickLaMuro/manageiq-gems-pending.git"
:upstream_name: "ManageIQ/manageiq-gems-pending"
:extractions:
  - lib/gems/pending/util/mount/
  - lib/gems/pending/util/object_storage/
  - lib/gems/pending/util/miq_file_storage.rb
  - lib/gems/pending/util/miq_ftp_lib.rb
  - lib/gems/pending/util/miq_object_storage.rb
  - spec/util/mount/
  - spec/util/object_storage/
  - spec/util/miq_file_storage_spec.rb
  - spec/util/miq_ftp_lib_spec.rb
  - spec/util/miq_object_storage_spec.rb
:extra_cmds:
  - mv lib/gems/pending/util/* lib/
  - git mv spec/util/* spec/lib/
  - rm -rf lib/gems
  - git add lib
  - git commit -m "Move lib/gems/pending/util/ to lib/"
:destination: "mount_extractions"

(UPDATE: The above is out of date. See #19547 (comment) )

and simply used the executable that was added as part of that branch to run the "re-insert".

Once the whole process was completed, the following extra commands were run on this PR:

$ git branch -m reinject_mount_and_ftp_libs
$ git push -u code_extractor_target_for_object_storage_and_ftp_lib reinject_mount_and_ftp_libs

(just gave it a shorter branch name)

Links

• Partially addresses Move MiqFileStorage into core and provider specific gems manageiq-gems-pending#403

Steps for Testing/QA (TODO)

I highly suspect that the code in it's current form will break the test suite. I will be adding some follow up commits to address that.

[NickLaMuro]

Butts...

Still looks like I have to re-work this a bit... but I think it is a bit closer than it has been. Mostly the first commit isn't bringing everything over that it should, and the links to the commits are wrong (which I know why, and probably can figure out a fix).

[NickLaMuro]

A bit better now... the links are at least correct.

The first commit is still a little weird, since it has nothing to do with the code that I am injecting, so I am going to be revisiting the pruning code tomorrow.

But tomorrow... my head hurts from all of this git-foo...

[NickLaMuro]

Okay, quick update:

This is mostly working as expected thanks to this change:

juliancheal/code-extractor@0bbc2c1

However, I did have to update the extraction.yml to the following:

:name: object_storage_and_ftp_lib
:reinsert: true
:target_name: "ManageIQ/manageiq"
:target_remote: "[email protected]:NickLaMuro/manageiq.git"
:target_base_branch: "master"
:upstream: "[email protected]:NickLaMuro/manageiq-gems-pending.git"
:upstream_name: "ManageIQ/manageiq-gems-pending"
:extractions:
  - lib/gems/pending/util/mount/
  - lib/gems/pending/util/object_storage/
  - lib/gems/pending/util/miq_file_storage.rb
  - lib/gems/pending/util/miq_ftp_lib.rb
  - lib/gems/pending/util/miq_object_storage.rb
  - spec/support/contexts/generated_tmp_files.rb
  - spec/support/contexts/with_ftp_server.rb
  - spec/support/custom_matchers/
  - spec/util/mount/
  - spec/util/object_storage/
  - spec/util/miq_file_storage_spec.rb
  - spec/util/miq_ftp_lib_spec.rb
  - spec/util/miq_object_storage_spec.rb
:extra_cmds:
  - mv lib/gems/pending/util/* lib/
  - git mv spec/util/* spec/lib/
  - mkdir -p spec/support/examples_group
  - git mv spec/support/contexts/generated_tmp_files.rb spec/support/examples_group/
  - git mv spec/support/contexts/with_ftp_server.rb spec/support/examples_group/
  - rm -rf lib/gems spec/support/contexts
  - git add lib
  - git commit -m "Move lib/gems/pending/util/ to lib/"
:destination: "mount_extractions"

Leaving the original in the PR description for historical reasons, but I ran through all of the PRs this had touched, so I "think" this should be working now.

Hopefully there isn't any files that I missed... again...

[NickLaMuro]

Okay, tests were passing on the last build, and only brakeman was having issues.

I think it makes sense to possibly correct those in the future, but I would rather solve that in a different PR, so ignoring for the time being. For what it is worth, it isn't "adding" new vulnerabilities, since this code already existed previously, just in a different location.

@miq-bot assign @Fryguy

Let me know what you think. I am guessing there are a few things that are confusing, and possible some items that you don't like, so let me know if you want me to go over anything.

cc @carbonin @jerryk55

[Fryguy]

@NickLaMuro A few questions

• Is this a straight reinsert, or are there changes above the code that was transferred from manageiq-gems-pending?
• Is this ready to go? I see it's unWIP, but then there is a [FIXUP] commit
• Why do we remove dependencies from manageiq-gems-pending, but they are not added here (specifically fog-openstack...the others are more or less already here)

[Fryguy]

These still say "util/". but that's not where these live now.

[NickLaMuro]

I had thought about fixing this, but forgot when I saw the tests were passing, at first, I was confused at why this was still working, but I think the answer is simple:

The corresponding PR for this in manageiq-gems-pending is not merged, so without running this with manageiq-cross_repo_tests, this still loads just fine and the specs pass.

Will look into fixing these next week. Thanks!

[Fryguy]

Can you do a cross_repo test with the removal PR in the manageiq-gems-pending? Then this would prove that out.

[NickLaMuro]

Yup, that was the plan. And now is done (sorry for the delay... other things...):

ManageIQ/manageiq-cross_repo-tests#39

Will use that PR going forward... though sure would be nice to have used ManageIQ/miq_bot#454 for this as a test case...

( ͡° ᴥ ͡°)

[NickLaMuro]

@Fryguy

• Is this a straight reinsert, or are there changes above the code that was transferred from manageiq-gems-pending?

The only things I had to do was move some files around (and I think as you pointed out), make a few edits. Though, this question is answered more when added to the next...

• Is this ready to go? I see it's unWIP, but then there is a [FIXUP] commit

The main question ("Is this ready to go?"), I think mostly falls on you (and/or the powers that be) that this is a okay route to go with over all, which is mostly what I was waiting on.

The stuff that I did in juliancheal/code-extractor#11 was kind of a big "hey, here is how I got here" for this PR, since there was a lot of trial and error that I wanted documented in some fashion, as well as be reusable in some capacity with other items for gems pending that could use the same treatment.

I think the commits have been moved over, and the process that I used is #goodEnough™, but I wouldn't mind a thorough look over of that if someone has time. Also, I can walk someone through the steps next week since I realize the process to get here is pretty confusing (even for me...).

• Why do we remove dependencies from manageiq-gems-pending, but they are not added here (specifically fog-openstack...the others are more or less already here)

This is a easy question to answer. The reason those dependencies were raw like that was because we couldn't include manageiq-providers-openstack to them directly, because it was a git gem, and that isn't possible to add in the *.gemspec. So I opted with these to write the connectors directly with the raw API gems for the given providers (manageiq-providers-openstack and manageiq-providers-aws in this case).

But those gems are already included indirectly via the provider gems, so now we just don't need to include them here since they already exist via that indirect dependency.

That said, as we have discussed in the past, it would be ideal if the code for this would live with the providers, but since there isn't a plug-ability hook for database backup providers, this was the first step to getting there.

[NickLaMuro]

Is this a straight reinsert, or are there changes above the code that was transferred from manageiq-gems-pending?

So I answered this partially incorrectly.

There were a decent amount of changes, and even new files, done in manageiq-gems-pending, so this is not putting back he code that once was here.

In fact, all the commits that exist here are commits that only exist in manageiq-gems-pending, and not any that had existed prior in this repo. This is by design, as re-adding those previously existing commits would be confusing, and you even mentioned it being so in my past example of attempting this:

#15358

(Note: Those commits in that PR were techically "rewritten commits", but the SHAs ended up being different and the content changes were scoped to only the files we were moving back)

The one "slight" exception of commits that were pulled over was the first commit:

Re-insert extractions from ManageIQ/manageiq-gems-pending

*** Original Commit message shown below ***

Author:    Oleg Barenboim [email protected] Tue May 3 23:53:27 2016 -0500
Committer: Oleg Barenboim [email protected] Tue May 3 23:53:27 2016 -0500

Merge pull request #8404 from Ladas/fix_event_monitor_example_script

...

Which is taking the last commit that existed in both ManageIQ/manageiq, and using that as the base commit for this PR. That said, the content of that commit differs, as instead of the original content of the commit, it completely re-injects the state of the files that were meant to be moved over as they were at the state of the commit it is based off of.

---

This is one of the things that I really think needs to be critiqued heavily since I think this process would be reproducible for other parts of the "Keep in core" section in:

ManageIQ/manageiq-gems-pending#231

Happy to take our time on this to get it right, just re-aligning expectations since I realize this is a very confusing subject matter.

[NickLaMuro]

Okay, had to rebase this because some of the recent Gemfile changes causing the tests to fail in cross repo:

ManageIQ/manageiq-cross_repo-tests#39

I also re-ran the extract in such a way so as to remove the fixup commit that used to exist (previous version of the branch can be found here), and that was fixed by a modification to the extractions.yml:

--- a/extractions.yml
+++ b/extractions.yml
@@ -21,8 +21,8 @@
   - spec/util/miq_object_storage_spec.rb
 :extra_cmds:
   - mv lib/gems/pending/util/* lib/
+  - mkdir -p spec/support/examples_group spec/lib
   - git mv spec/util/* spec/lib/
-  - mkdir -p spec/support/examples_group
   - git mv spec/support/contexts/generated_tmp_files.rb spec/support/examples_group/
   - git mv spec/support/contexts/with_ftp_server.rb spec/support/examples_group/
   - rm -rf lib/gems spec/support/contexts

This allows the "Move lib/gems/pending/util/ to lib/" commit to do everything properly that was previously needing a [FIXUP] commit to move the offending files.

Current last two commits:

• "[Gemfile] Add ftpd to test dependencies"
• "Update brakeman.ignore for MiqObjectStorage"

Were git cherry-pick'd back into this branch. I pulled the from the reference branch:

https://github.com/NickLaMuro/manageiq/tree/reinject_mount_and_ftp_libs_old

Which there, they are the reinject_mount_and_ftp_libs_old^^[ref] and reinject_mount_and_ftp_libs_old^ [ref] commits respectively.

[Fryguy]

util still here?

[miq-bot]

Some comments on commits NickLaMuro/manageiq@9bc80e4~...982b901

spec/support/examples_group/with_ftp_server.rb

• ⚠️ - 15 - Detected puts. Remove all debugging statements.
• ⚠️ - 53 - Detected puts. Remove all debugging statements.

[miq-bot]

Checked commits NickLaMuro/manageiq@9bc80e4~...982b901 with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
25 files checked, 114 offenses detected

lib/miq_file_storage.rb

• ❗ - Line 120, Col 7 - Style/SafeNavigation - Use safe navigation (&.) instead of checking if an object exists before calling the method.
• ❗ - Line 187, Col 15 - Layout/ExtraSpacing - Unnecessary spacing detected.
• ❗ - Line 187, Col 19 - Layout/SpaceAroundOperators - Operator = should be surrounded by a single space.
• ❗ - Line 191, Col 9 - Style/SafeNavigation - Use safe navigation (&.) instead of checking if an object exists before calling the method.
• ❗ - Line 276, Col 11 - Style/RedundantBegin - Redundant begin block detected.
• ❗ - Line 54, Col 23 - Style/MutableConstant - Freeze mutable objects assigned to constants.

lib/miq_ftp_lib.rb

• ❗ - Line 44, Col 5 - Layout/EmptyLineAfterGuardClause - Add empty line after guard clause.
• ❗ - Line 60, Col 5 - Layout/EmptyLineAfterGuardClause - Add empty line after guard clause.
• ❗ - Line 67, Col 5 - Layout/EmptyLineAfterGuardClause - Add empty line after guard clause.

lib/miq_object_storage.rb

• ❗ - Line 15, Col 5 - Layout/EmptyLineAfterGuardClause - Add empty line after guard clause.
• ❗ - Line 28, Col 5 - Layout/EmptyLineAfterGuardClause - Add empty line after guard clause.
• ❗ - Line 42, Col 7 - Layout/EmptyLineAfterGuardClause - Add empty line after guard clause.

lib/mount/miq_generic_mount_session.rb

• ⚠️ - Line 25, Col 3 - Lint/DuplicateMethods - Method MiqGenericMountSession#logger is defined at both lib/mount/miq_generic_mount_session.rb:17 and lib/mount/miq_generic_mount_session.rb:25.
• ⚠️ - Line 322, Col 13 - Lint/UselessAssignment - Useless assignment to variable - userinfo. Use _ or _userinfo as a variable name to indicate that it won't be used.
• ⚠️ - Line 322, Col 29 - Lint/UselessAssignment - Useless assignment to variable - port. Use _ or _port as a variable name to indicate that it won't be used.
• ⚠️ - Line 322, Col 35 - Lint/UselessAssignment - Useless assignment to variable - registry. Use _ or _registry as a variable name to indicate that it won't be used.
• ⚠️ - Line 322, Col 52 - Lint/UselessAssignment - Useless assignment to variable - opaque. Use _ or _opaque as a variable name to indicate that it won't be used.
• ⚠️ - Line 322, Col 60 - Lint/UselessAssignment - Useless assignment to variable - query. Use _ or _query as a variable name to indicate that it won't be used.
• ⚠️ - Line 322, Col 67 - Lint/UselessAssignment - Useless assignment to variable - fragment. Use _ or _fragment as a variable name to indicate that it won't be used.
• ⚠️ - Line 322, Col 88 - Lint/UriEscapeUnescape - URI.encode method is obsolete and should not be used. Instead, use CGI.escape, URI.encode_www_form or URI.encode_www_form_component depending on your specific use case.
• ⚠️ - Line 323, Col 124 - Lint/UriEscapeUnescape - URI.encode method is obsolete and should not be used. Instead, use CGI.escape, URI.encode_www_form or URI.encode_www_form_component depending on your specific use case.
• ⚠️ - Line 323, Col 17 - Lint/UselessAssignment - Useless assignment to variable - userinfo_log. Use _ or _userinfo_log as a variable name to indicate that it won't be used.
• ⚠️ - Line 323, Col 41 - Lint/UselessAssignment - Useless assignment to variable - port_log. Use _ or _port_log as a variable name to indicate that it won't be used.
• ⚠️ - Line 323, Col 51 - Lint/UselessAssignment - Useless assignment to variable - registry_log. Use _ or _registry_log as a variable name to indicate that it won't be used.
• ⚠️ - Line 323, Col 76 - Lint/UselessAssignment - Useless assignment to variable - opaque_log. Use _ or _opaque_log as a variable name to indicate that it won't be used.
• ⚠️ - Line 323, Col 88 - Lint/UselessAssignment - Useless assignment to variable - query_log. Use _ or _query_log as a variable name to indicate that it won't be used.
• ⚠️ - Line 323, Col 99 - Lint/UselessAssignment - Useless assignment to variable - fragment_log. Use _ or _fragment_log as a variable name to indicate that it won't be used.
• ⚠️ - Line 362, Col 61 - Lint/UselessAssignment - Useless assignment to variable - err.
• ⚠️ - Line 452, Col 13 - Lint/UselessAssignment - Useless assignment to variable - userinfo. Use _ or _userinfo as a variable name to indicate that it won't be used.
• ⚠️ - Line 452, Col 23 - Lint/UselessAssignment - Useless assignment to variable - host. Use _ or _host as a variable name to indicate that it won't be used.
• ⚠️ - Line 452, Col 29 - Lint/UselessAssignment - Useless assignment to variable - port. Use _ or _port as a variable name to indicate that it won't be used.
• ⚠️ - Line 452, Col 35 - Lint/UselessAssignment - Useless assignment to variable - registry. Use _ or _registry as a variable name to indicate that it won't be used.
• ⚠️ - Line 452, Col 5 - Lint/UselessAssignment - Useless assignment to variable - scheme. Use _ or _scheme as a variable name to indicate that it won't be used.
• ⚠️ - Line 452, Col 51 - Lint/UselessAssignment - Useless assignment to variable - opaque. Use _ or _opaque as a variable name to indicate that it won't be used.
• ⚠️ - Line 452, Col 59 - Lint/UselessAssignment - Useless assignment to variable - query. Use _ or _query as a variable name to indicate that it won't be used.
• ⚠️ - Line 452, Col 66 - Lint/UselessAssignment - Useless assignment to variable - fragment. Use _ or _fragment as a variable name to indicate that it won't be used.
• ⚠️ - Line 452, Col 87 - Lint/UriEscapeUnescape - URI.encode method is obsolete and should not be used. Instead, use CGI.escape, URI.encode_www_form or URI.encode_www_form_component depending on your specific use case.
• ⚠️ - Line 470, Col 3 - Lint/IneffectiveAccessModifier - protected (on line 443) does not make singleton methods protected. Use protected inside a class << self block instead.
• ⚠️ - Line 69, Col 13 - Lint/UselessAssignment - Useless assignment to variable - userinfo. Use _ or _userinfo as a variable name to indicate that it won't be used.
• ⚠️ - Line 69, Col 23 - Lint/UselessAssignment - Useless assignment to variable - host. Use _ or _host as a variable name to indicate that it won't be used.
• ⚠️ - Line 69, Col 29 - Lint/UselessAssignment - Useless assignment to variable - port. Use _ or _port as a variable name to indicate that it won't be used.
• ⚠️ - Line 69, Col 35 - Lint/UselessAssignment - Useless assignment to variable - registry. Use _ or _registry as a variable name to indicate that it won't be used.
• ⚠️ - Line 69, Col 45 - Lint/UselessAssignment - Useless assignment to variable - share. Use _ or _share as a variable name to indicate that it won't be used.
• ⚠️ - Line 69, Col 52 - Lint/UselessAssignment - Useless assignment to variable - opaque. Use _ or _opaque as a variable name to indicate that it won't be used.
• ⚠️ - Line 69, Col 60 - Lint/UselessAssignment - Useless assignment to variable - query. Use _ or _query as a variable name to indicate that it won't be used.
• ⚠️ - Line 69, Col 67 - Lint/UselessAssignment - Useless assignment to variable - fragment. Use _ or _fragment as a variable name to indicate that it won't be used.
• ⚠️ - Line 69, Col 88 - Lint/UriEscapeUnescape - URI.encode method is obsolete and should not be used. Instead, use CGI.escape, URI.encode_www_form or URI.encode_www_form_component depending on your specific use case.
• ⚠️ - Line 89, Col 7 - Lint/UselessAssignment - Useless assignment to variable - opts.
• ❗ - Line 102, Col 5 - Style/ClassVars - Replace class var @@do_ping with a class instance var.
• ❗ - Line 107, Col 24 - Style/RedundantSelf - Redundant self detected.
• ❗ - Line 141, Col 115 - Style/RedundantSelf - Redundant self detected.
• ❗ - Line 148, Col 5 - Style/RescueStandardError - Avoid rescuing without specifying an error class.
• ❗ - Line 166, Col 5 - Layout/EmptyLineAfterGuardClause - Add empty line after guard clause.
• ❗ - Line 168, Col 5 - Style/SafeNavigation - Use safe navigation (&.) instead of checking if an object exists before calling the method.
• ❗ - Line 171, Col 5 - Style/RescueStandardError - Avoid rescuing without specifying an error class.
• ❗ - Line 173, Col 14 - Performance/RegexpMatch - Use match? instead of =~ when MatchData is not used.
• ❗ - Line 175, Col 9 - Style/SafeNavigation - Use safe navigation (&.) instead of checking if an object exists before calling the method.
• ❗ - Line 181, Col 5 - Style/SafeNavigation - Use safe navigation (&.) instead of checking if an object exists before calling the method.
• ❗ - Line 194, Col 5 - Layout/EmptyLineAfterGuardClause - Add empty line after guard clause.
• ❗ - Line 20, Col 5 - Layout/EmptyLineAfterGuardClause - Add empty line after guard clause.
• ❗ - Line 239, Col 1 - Layout/EmptyLinesAroundExceptionHandlingKeywords - Extra empty line detected before the rescue.
• ❗ - Line 240, Col 5 - Style/RescueStandardError - Avoid rescuing without specifying an error class.
• ❗ - Line 264, Col 5 - Style/RescueStandardError - Avoid rescuing without specifying an error class.
• ❗ - Line 306, Col 5 - Style/RescueStandardError - Avoid rescuing without specifying an error class.
• ❗ - Line 329, Col 5 - Layout/EmptyLineAfterGuardClause - Add empty line after guard clause.
• ❗ - Line 336, Col 12 - Style/RedundantSelf - Redundant self detected.
• ❗ - Line 344, Col 7 - Performance/RedundantMerge - Use @settings[:uri] = File.dirname(File.dirname(log_uri)) instead of @settings.merge!(:uri => File.dirname(File.dirname(log_uri))).
• ❗ - Line 364, Col 5 - Style/RescueStandardError - Avoid rescuing without specifying an error class.
• ❗ - Line 420, Col 7 - Security/Open - The use of Kernel#open is a serious security risk.
• ❗ - Line 43, Col 8 - Style/SpecialGlobalVars - Prefer $CHILD_STATUS from the stdlib 'English' module (don't forget to require it) over $?.
• ❗ - Line 457, Col 5 - Layout/EmptyLineAfterGuardClause - Add empty line after guard clause.
• ❗ - Line 465, Col 3 - Style/RescueStandardError - Avoid rescuing without specifying an error class.
• ❗ - Line 488, Col 5 - Layout/EmptyLineAfterGuardClause - Add empty line after guard clause.
• ❗ - Line 49, Col 5 - Layout/EmptyLineAfterGuardClause - Add empty line after guard clause.
• ❗ - Line 53, Col 5 - Style/SafeNavigation - Use safe navigation (&.) instead of checking if an object exists before calling the method.
• ❗ - Line 87, Col 7 - Naming/AccessorMethodName - Do not prefix reader method names with get_.
• ❗ - Line 88, Col 5 - Style/ClassVars - Replace class var @@ping_depot_options with a class instance var.
• ❗ - Line 97, Col 5 - Style/ClassVars - Replace class var @@ping_timeout with a class instance var.

lib/mount/miq_glusterfs_session.rb

• ⚠️ - Line 16, Col 17 - Lint/UriEscapeUnescape - URI.encode method is obsolete and should not be used. Instead, use CGI.escape, URI.encode_www_form or URI.encode_www_form_component depending on your specific use case.
• ❗ - Line 25, Col 116 - Layout/MultilineMethodCallBraceLayout - Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.

lib/mount/miq_nfs_session.rb

• ⚠️ - Line 15, Col 13 - Lint/UselessAssignment - Useless assignment to variable - userinfo. Use _ or _userinfo as a variable name to indicate that it won't be used.
• ⚠️ - Line 15, Col 30 - Lint/UselessAssignment - Useless assignment to variable - port. Use _ or _port as a variable name to indicate that it won't be used.
• ⚠️ - Line 15, Col 36 - Lint/UselessAssignment - Useless assignment to variable - registry. Use _ or _registry as a variable name to indicate that it won't be used.
• ⚠️ - Line 15, Col 5 - Lint/UselessAssignment - Useless assignment to variable - scheme. Use _ or _scheme as a variable name to indicate that it won't be used.
• ⚠️ - Line 15, Col 59 - Lint/UselessAssignment - Useless assignment to variable - opaque. Use _ or _opaque as a variable name to indicate that it won't be used.
• ⚠️ - Line 15, Col 67 - Lint/UselessAssignment - Useless assignment to variable - query. Use _ or _query as a variable name to indicate that it won't be used.
• ⚠️ - Line 15, Col 74 - Lint/UselessAssignment - Useless assignment to variable - fragment. Use _ or _fragment as a variable name to indicate that it won't be used.
• ⚠️ - Line 15, Col 95 - Lint/UriEscapeUnescape - URI.encode method is obsolete and should not be used. Instead, use CGI.escape, URI.encode_www_form or URI.encode_www_form_component depending on your specific use case.
• ❗ - Line 4, Col 11 - Style/MutableConstant - Freeze mutable objects assigned to constants.

lib/mount/miq_smb_session.rb

• ⚠️ - Line 17, Col 13 - Lint/UselessAssignment - Useless assignment to variable - userinfo. Use _ or _userinfo as a variable name to indicate that it won't be used.
• ⚠️ - Line 17, Col 30 - Lint/UselessAssignment - Useless assignment to variable - port. Use _ or _port as a variable name to indicate that it won't be used.
• ⚠️ - Line 17, Col 36 - Lint/UselessAssignment - Useless assignment to variable - registry. Use _ or _registry as a variable name to indicate that it won't be used.
• ⚠️ - Line 17, Col 5 - Lint/UselessAssignment - Useless assignment to variable - scheme. Use _ or _scheme as a variable name to indicate that it won't be used.
• ⚠️ - Line 17, Col 59 - Lint/UselessAssignment - Useless assignment to variable - opaque. Use _ or _opaque as a variable name to indicate that it won't be used.
• ⚠️ - Line 17, Col 67 - Lint/UselessAssignment - Useless assignment to variable - query. Use _ or _query as a variable name to indicate that it won't be used.
• ⚠️ - Line 17, Col 74 - Lint/UselessAssignment - Useless assignment to variable - fragment. Use _ or _fragment as a variable name to indicate that it won't be used.
• ⚠️ - Line 17, Col 95 - Lint/UriEscapeUnescape - URI.encode method is obsolete and should not be used. Instead, use CGI.escape, URI.encode_www_form or URI.encode_www_form_component depending on your specific use case.
• ❗ - Line 4, Col 11 - Style/MutableConstant - Freeze mutable objects assigned to constants.

lib/object_storage/miq_ftp_storage.rb

• ❗ - Line 64, Col 9 - Style/RedundantBegin - Redundant begin block detected.
• ❗ - Line 70, Col 11 - Style/SafeNavigation - Use safe navigation (&.) instead of checking if an object exists before calling the method.

lib/object_storage/miq_s3_storage.rb

• ❗ - Line 100, Col 3 - Style/RescueStandardError - Avoid rescuing without specifying an error class.

lib/object_storage/miq_swift_storage.rb

• ⚠️ - Line 19, Col 94 - Lint/UriEscapeUnescape - URI.encode method is obsolete and should not be used. Instead, use CGI.escape, URI.encode_www_form or URI.encode_www_form_component depending on your specific use case.
• ⚠️ - Line 33, Col 41 - Lint/UriEscapeUnescape - URI.encode method is obsolete and should not be used. Instead, use CGI.escape, URI.encode_www_form or URI.encode_www_form_component depending on your specific use case.
• ❗ - Line 112, Col 22 - Layout/EmptyLineAfterGuardClause - Add empty line after guard clause.
• ❗ - Line 123, Col 20 - Style/RescueStandardError - Avoid rescuing without specifying an error class.
• ❗ - Line 142, Col 5 - Layout/EmptyLineAfterGuardClause - Add empty line after guard clause.
• ❗ - Line 162, Col 3 - Style/RescueStandardError - Avoid rescuing without specifying an error class.
• ❗ - Line 179, Col 3 - Style/RescueStandardError - Avoid rescuing without specifying an error class.
• ❗ - Line 18, Col 5 - Layout/EmptyLineAfterGuardClause - Add empty line after guard clause.

spec/lib/miq_ftp_lib_spec.rb

• ❗ - Line 36, Col 5 - Style/PercentLiteralDelimiters - %w-literals should be delimited by [ and ].

spec/support/custom_matchers/exist_on_ftp_server.rb

• ❗ - Line 29, Col 7 - Style/RedundantBegin - Redundant begin block detected.

spec/support/custom_matchers/have_size_on_ftp_server_of.rb

• ❗ - Line 23, Col 7 - Style/RedundantBegin - Redundant begin block detected.

spec/support/examples_group/with_ftp_server.rb

• ❗ - Line 53, Col 5 - Naming/MemoizedInstanceVariableName - Memoized variable @existing_file does not match method name create_existing_file. Use @create_existing_file instead.
• ❗ - Line 58, Col 5 - Naming/MemoizedInstanceVariableName - Memoized variable @existing_dir does not match method name create_existing_dir. Use @create_existing_dir instead.

[miq-bot]

This pull request is not mergeable. Please rebase and repush.

[NickLaMuro]

Since #19742 is the replacement for this PR, and there is a high probability of that effort never being merged in favor of a different approach (see #19742 (comment) ), I am going to close this since it has just became unmergeable.