manager: Add permissions check to custom_url to avoid failed tasks

[DaveWysochanskiRH]

Tasks that are created with custom_url pointing at local files with incorrect permissions will almost immediately fail, leaving failed tasks and delayed feedback to the user. Move the permissions check into the custom URL parsing inside the manager.wsgi file, which is much sooner than where it is currently done inside download_remote(). Checking permissions early gives the user early feedback via the http interface, and we also avoid creating tasks that will immediately fail due to improper permissions on the file.

Signed-off-by: Dave Wysochanski dwysocha@redhat.com

[DaveWysochanskiRH]

For testing, I double checked that uid / gid was the same at the point of this check as in the failed check inside download_remote(), and ran through a couple sample files with various permissions problems.

[DaveWysochanskiRH]

One optimization could be to also check the "custom_vmem_url" path in the same manner. In practice, vmem files don't show up too often, and it would be a corner of a corner case for the "custom_url" path to be readable and yet the "custom_vmem_url" to be unreadable, so I didn't include it.

[DaveWysochanskiRH]

Still testing... found one problem with non-existent files.

[DaveWysochanskiRH]

Ok I think 8edcbe0 fixes this and does not introduce any new issues. Main tests were:

1. Check for file that exists but is not readable (permission)
2. Check for file that does not exist

In our retrace-server environment, both of the above scenarios happen, with the first scenario being fairly common, and the second is much less common.

[DaveWysochanskiRH]

In 40f1f620f7aeb43bb485c4d67557aac6eeaeda99 I also updated the commit message as well as the code, to hopefully make this all more clear.

[DaveWysochanskiRH]

Also, 40f1f62 also catches other problems seen in production, such as someone giving a directory instead of a file. In that case, the following response is given:

Cannot open URL for reading: '[Errno 21] Is a directory: '/foo/bar''

[DaveWysochanskiRH]

@mgrabovsky any further comments / suggestions or can this be merged? For testing, last week I put 40f1f62 into our system and asked for feedback but so far no feedback and it seems to fix the problems with failed tasks to to perms or non-existent files.

[mgrabovsky]

Sorry, I haven't got around to re-reviewing this yet. I'll get to it to tomorrow.