Bug #6304
closed
[API] Fix config file race condition and improve test cases for DNS update hooks
Added by Tom Clegg almost 10 years ago.
Updated almost 8 years ago.
Description
Node.dns_server_update() in
source:services/api/app/models/node.rb is currently vulnerable to the following race (fixable by using a tempfile library):
- Request handler A writes hostname.conf.tmp.
- Request handler B opens hostname.conf.tmp for writing.
- Request handler A issues the move command.
Catch only the expected exceptions on the various write and update operations (IOError and SystemCallError?), not unexpected ones.
- Description updated (diff)
- Description updated (diff)
- Target version changed from Arvados Future Sprints to 2017-04-12 sprint
- Assigned To set to Lucas Di Pentima
- Status changed from New to In Progress
An ensure block should delete the stray temp file if we got as far as creating one (e.g., if f.puts fails). Something like this?
tmpfile = nil
begin
...
Tempfile.open(...) do |f|
tmpfile = f.path
f.puts ...
end
File.rename tmpfile, hostfile
tmpfile = nil
rescue ...
...
ensure
if tmpfile
File.unlink tmpfile
end
end
It seems that the new "dns update with dir configured but no command configured" test actually tests "dns_server_update is a no-op when dns_server_conf_template is not set".
- Description updated (diff)
Updates at 2f46fb576
Ensure dangling temp files are cleaned up.
Pending: test case for this is not working properly. Tom will give it a look.
nit: prefer "assert_empty foo" to "assert foo.empty?" so the failure message says what unexpected stuff was found instead of just "false is not true".
LGTM, thanks!
- Status changed from In Progress to Resolved
- % Done changed from 0 to 100
Applied in changeset arvados|commit:be2a34c57eab7cc184c08ec1059554cb97911276.
Also available in: Atom
PDF
Updated by 
Updated by Tom Morris 
Updated by