Skip to content

Commit

Permalink
Only try and set permissions if permissions have changed. (#295)
Browse files Browse the repository at this point in the history
* Only try and set permissions if permissions have changed.

This is a workaround for jruby/jruby#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.
  • Loading branch information
robbavey authored Jun 9, 2021
1 parent 3624548 commit 55aac0a
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 5 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)

Expand Down
7 changes: 5 additions & 2 deletions lib/filewatch/helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,18 @@ 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)

# 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
Expand Down
15 changes: 13 additions & 2 deletions lib/filewatch/sincedb_collection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion logstash-input-file.gemspec
Original file line number Diff line number Diff line change
@@ -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"
Expand Down

0 comments on commit 55aac0a

Please sign in to comment.