[CORE-8020] tls: Watch for IN_MOVED_TO#150
[CORE-8020] tls: Watch for IN_MOVED_TO#150BenPope wants to merge 1 commit intoredpanda-data:v24.3.xfrom
Conversation
Signed-off-by: Ben Pope <[email protected]>
BenPope
requested review from
a team,
IoannisRP and
oleiman
and removed request for
a team
| } | ||
| } | ||
| future<fsnotifier::watch_token> add_watch(const sstring& filename, fsnotifier::flags flags = fsnotifier::flags::close_write|fsnotifier::flags::delete_self) { | ||
| future<fsnotifier::watch_token> add_watch(const sstring& filename, fsnotifier::flags flags = fsnotifier::flags::close_write|fsnotifier::flags::delete_self|fsnotifier::flags::move_to) { |
There was a problem hiding this comment.
Interesting. The man page made me think that move_to is generated for the containing directory rather than the file and that move_self is more appropriate.
What happens if someone does mv existing.crt existing.crt.bkp as a way of deleting certificates? I'm wondering if that would trigger a delete_self or if we should be watching move_from as well.
There was a problem hiding this comment.
fwiw, I don't think mv existing.crt existing.crt.bkp will reliably invalidate the inode corresponding to existing.crt.
| } | ||
| } | ||
| future<fsnotifier::watch_token> add_watch(const sstring& filename, fsnotifier::flags flags = fsnotifier::flags::close_write|fsnotifier::flags::delete_self) { | ||
| future<fsnotifier::watch_token> add_watch(const sstring& filename, fsnotifier::flags flags = fsnotifier::flags::close_write|fsnotifier::flags::delete_self|fsnotifier::flags::move_to) { |
There was a problem hiding this comment.
When executing
mv existing.crt.bak existing.crt
existing.crt should trigger a delete_self, which is already watched for, and the parent dir should issue a move_to.
I do not understand why adding move_to to the file watcher fixes the issue. What am I missing here?
The add_dir_watch function seems to be there to cater for moving files around, but for some reason it is only applied on rebuild and not on init.
There was a problem hiding this comment.
Testing this naively at the command line, it doesn't seem like this type of mv generates a delete event, or at least not guaranteed right away.
which makes sense I think because inotify monitors the state of the inodes pointed to by the directory entry, rather than the name->inode mapping itself?
There was a problem hiding this comment.
Ah, never mind, this is a file watch. Now I'm confused too!
There was a problem hiding this comment.
Apologies, you are right. i should have been more careful in the description.
I was referring to the case where existing.crt already exists and we are trying to overwrite it with a move.
There was a problem hiding this comment.
well, that type of move does not immediately notify delete on the directory watch, but it does notify delete_self on the file watch. Anyway, your original question is apt I think.
oleiman
left a comment
oleiman
left a comment
There was a problem hiding this comment.
ok, i think i get it. from reading
Lines 445 to 464 in 7820b86
so we're sort of extending the parent-dir fallback watcher to hopefully pick this up?
I don't really understand why mv new existing doesn't generate a delete_self event sometimes (ever?). Seems like it should also trigger this watcher though.
4 participants
Fixes CORE-8020
A common pattern for updating certificates is to do:
So configure the watch to detect
IN_MOVED_TO