FileDepotFtp: FTP.nlst cannot distinguish empty from non-existent dir

[isimluk]

Create directory structure by descending down

https://bugzilla.redhat.com/show_bug.cgi?id=1341502

The problem is that we cannot rely on fpt.nlst to give exception when
file is not available. With vsftpd 3.0.2-14.fc23 and Ruby-2.2.0 Net::FTP
I get behaviour like the following

    ftp.nlst('/')
    => ["/incoming", "/pub"]
    ftp.nlst('/incomming')
    => []
    ftp.nlst('/non-existent')
    => []

@miq-bot add_label darga/no, bug, core

[isimluk]

/cc @jrafanie You may want to take a look. As you have touched this code in past (#1603, #1677, ...)

I came to conclusion that we need to build statefull mock objects (vsftpd, iis, and perhaps more) to avoid further breakages.

[isimluk]

My theory, why we haven't seen this before is that we tested the previous bugs with either non existent directories or with non-empty. The new bug is about seeing the empty directory.

[jrafanie]

@isimluk I had explicit reasons for using descend.

See #1603

If this works on iis, or any of the many varieties of ftp servers, I'm ok with it.

[isimluk]

I see. The new code drills down the directory structure by mkdir A; chdir A the same way. So the IIS's inability to create directories like mkdir -p should not be a problem. However, I will try to get some IIS for testing.

[jrafanie]

@isimluk Please ask around and let me know if you can't get access for testing. Also, if you have ideas on how to capture this behavior so we can automate tests to avoid regression, I'd love to hear any ideas.

I'm fine with the change but would want some sanity checks on various ftps. Maybe @jcarter12 can DM you some information to test against the ftp they use for anonymous access.

[isimluk]

Also, if you have ideas on how to capture this behavior so we can automate tests
to avoid regression, I'd love to hear any ideas.

I think the approach I had taken in the second commit of this PR can really help us in that regard.

[jrafanie]

Looks good to me. I'd like an examples for iis and anonymous ftp that @jcarter12 before merge though. We've changed things for other ftps in the past and I feel like this might cause a regression without confirmation.

[gtanzillo]

Looks good to me too. Will advise QE on testing in the BZ.

[isimluk]

Meh, I referenced wrong bz first time. I have amended pr with the right bz now (https://bugzilla.redhat.com/show_bug.cgi?id=1341502) 😢

[miq-bot]

Checked commits isimluk/manageiq@f40f36a~...f87471a with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
2 files checked, 12 offenses detected

spec/models/file_depot_ftp_spec.rb

• ❗ - Line 33, Col 31 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• ❗ - Line 34, Col 32 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• ❗ - Line 38, Col 36 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• ❗ - Line 39, Col 37 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• ❗ - Line 40, Col 37 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• ❗ - Line 41, Col 37 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• ❗ - Line 49, Col 36 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• ❗ - Line 50, Col 37 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• ❗ - Line 51, Col 37 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• ❗ - Line 52, Col 37 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• ❕ - Line 41, Col 121 - Metrics/LineLength - Line is too long. [147/120]
• ❕ - Line 52, Col 121 - Metrics/LineLength - Line is too long. [147/120]

[gtanzillo]

👍

[isimluk]

Thanks!