From 55aac0a4d5827e047e19580397ad4c09fc907b72 Mon Sep 17 00:00:00 2001 From: Rob Bavey Date: Wed, 9 Jun 2021 11:45:22 -0400 Subject: [PATCH] Only try and set permissions if permissions have changed. (#295) * Only try and set permissions if permissions have changed. This is a workaround for https://github.com/jruby/jruby/issues/6693 - stat is currently reporting incorrect values of `uid` and `gid` on aarch64 linux nodes, and appears to be setting `uid` and `gid` to 0, ie `root` It is highly unlikely that `chown` needs to be called on the file - and even more unlikely that `chown` would succeed anyway - `chmod` typically requires superuser privileges, unless the change of ownerhip is a change of group to another group the user is already a member of. This workaround updates the code to only attempt to call `chown` in the unlikely event that file ownership of the temp file is different from the original file, which should avoid the scenario that is currently occurring due to the above jruby bug. This commit also falls back to a non-atomic write in the event of a failure to write the file atomically. --- CHANGELOG.md | 7 +++++++ lib/filewatch/helper.rb | 7 +++++-- lib/filewatch/sincedb_collection.rb | 15 +++++++++++++-- logstash-input-file.gemspec | 2 +- 4 files changed, 26 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ee2d78b..302f30b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## 4.3.1 + - Add extra safety to `chown` call in `atomic_write`, avoiding plugin crashes and falling back to a + `non_atomic_write` in the event of failure [#295](https://github.com/logstash-plugins/logstash-input-file/pull/295) + - Refactor: unify event updates to happen in one place [#297](https://github.com/logstash-plugins/logstash-input-file/pull/297) + - Test: Actually retry tests on `RSpec::Expectations::ExpectationNotMetError` and retry instead of relying on timeout + [#297](https://github.com/logstash-plugins/logstash-input-file/pull/297) + ## 4.3.0 - Add ECS Compatibility Mode [#291](https://github.com/logstash-plugins/logstash-input-file/pull/291) diff --git a/lib/filewatch/helper.rb b/lib/filewatch/helper.rb index 5da8c0fd..2fbe1a27 100644 --- a/lib/filewatch/helper.rb +++ b/lib/filewatch/helper.rb @@ -30,6 +30,7 @@ def write_atomically(file_name) temp_file.binmode return_val = yield temp_file temp_file.close + new_stat = File.stat(temp_file) # Overwrite original file with temp file File.rename(temp_file.path, file_name) @@ -37,8 +38,10 @@ def write_atomically(file_name) # Unable to get permissions of the original file => return return return_val if old_stat.nil? - # Set correct uid/gid on new file - File.chown(old_stat.uid, old_stat.gid, file_name) if old_stat + # Set correct uid/gid on new file if ownership is different. + if old_stat && (old_stat.gid != new_stat.gid || old_stat.uid != new_stat.uid) + File.chown(old_stat.uid, old_stat.gid, file_name) if old_stat + end return_val end diff --git a/lib/filewatch/sincedb_collection.rb b/lib/filewatch/sincedb_collection.rb index d053b270..3694cef0 100644 --- a/lib/filewatch/sincedb_collection.rb +++ b/lib/filewatch/sincedb_collection.rb @@ -225,13 +225,24 @@ def sincedb_write(time = Time.now) # @return expired keys def atomic_write(time) - FileHelper.write_atomically(@full_path) do |io| - @serializer.serialize(@sincedb, io, time.to_f) + logger.trace? && logger.trace("non_atomic_write: ", :time => time) + begin + FileHelper.write_atomically(@full_path) do |io| + @serializer.serialize(@sincedb, io, time.to_f) + end + rescue Errno::EPERM, Errno::EACCES => e + logger.warn("sincedb_write: unable to write atomically due to permissions error, falling back to non-atomic write: #{path} error:", :exception => e.class, :message => e.message) + @write_method = method(:non_atomic_write) + non_atomic_write(time) + rescue => e + logger.warn("sincedb_write: unable to write atomically, attempting non-atomic write: #{path} error:", :exception => e.class, :message => e.message) + non_atomic_write(time) end end # @return expired keys def non_atomic_write(time) + logger.trace? && logger.trace("non_atomic_write: ", :time => time) File.open(@full_path, "w+") do |io| @serializer.serialize(@sincedb, io, time.to_f) end diff --git a/logstash-input-file.gemspec b/logstash-input-file.gemspec index 94d6e7b7..b5cf6c39 100644 --- a/logstash-input-file.gemspec +++ b/logstash-input-file.gemspec @@ -1,7 +1,7 @@ Gem::Specification.new do |s| s.name = 'logstash-input-file' - s.version = '4.3.0' + s.version = '4.3.1' s.licenses = ['Apache-2.0'] s.summary = "Streams events from files" s.description = "This gem is a Logstash plugin required to be installed on top of the Logstash core pipeline using $LS_HOME/bin/logstash-plugin install gemname. This gem is not a stand-alone program"