Skip to content

Commit 55aac0a

Browse files
authored
Only try and set permissions if permissions have changed. (#295)
* 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.
1 parent 3624548 commit 55aac0a

File tree

4 files changed

+26
-5
lines changed

4 files changed

+26
-5
lines changed

CHANGELOG.md

+7
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
## 4.3.1
2+
- Add extra safety to `chown` call in `atomic_write`, avoiding plugin crashes and falling back to a
3+
`non_atomic_write` in the event of failure [#295](https://github.com/logstash-plugins/logstash-input-file/pull/295)
4+
- Refactor: unify event updates to happen in one place [#297](https://github.com/logstash-plugins/logstash-input-file/pull/297)
5+
- Test: Actually retry tests on `RSpec::Expectations::ExpectationNotMetError` and retry instead of relying on timeout
6+
[#297](https://github.com/logstash-plugins/logstash-input-file/pull/297)
7+
18
## 4.3.0
29
- Add ECS Compatibility Mode [#291](https://github.com/logstash-plugins/logstash-input-file/pull/291)
310

lib/filewatch/helper.rb

+5-2
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,18 @@ def write_atomically(file_name)
3030
temp_file.binmode
3131
return_val = yield temp_file
3232
temp_file.close
33+
new_stat = File.stat(temp_file)
3334

3435
# Overwrite original file with temp file
3536
File.rename(temp_file.path, file_name)
3637

3738
# Unable to get permissions of the original file => return
3839
return return_val if old_stat.nil?
3940

40-
# Set correct uid/gid on new file
41-
File.chown(old_stat.uid, old_stat.gid, file_name) if old_stat
41+
# Set correct uid/gid on new file if ownership is different.
42+
if old_stat && (old_stat.gid != new_stat.gid || old_stat.uid != new_stat.uid)
43+
File.chown(old_stat.uid, old_stat.gid, file_name) if old_stat
44+
end
4245

4346
return_val
4447
end

lib/filewatch/sincedb_collection.rb

+13-2
Original file line numberDiff line numberDiff line change
@@ -225,13 +225,24 @@ def sincedb_write(time = Time.now)
225225

226226
# @return expired keys
227227
def atomic_write(time)
228-
FileHelper.write_atomically(@full_path) do |io|
229-
@serializer.serialize(@sincedb, io, time.to_f)
228+
logger.trace? && logger.trace("non_atomic_write: ", :time => time)
229+
begin
230+
FileHelper.write_atomically(@full_path) do |io|
231+
@serializer.serialize(@sincedb, io, time.to_f)
232+
end
233+
rescue Errno::EPERM, Errno::EACCES => e
234+
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)
235+
@write_method = method(:non_atomic_write)
236+
non_atomic_write(time)
237+
rescue => e
238+
logger.warn("sincedb_write: unable to write atomically, attempting non-atomic write: #{path} error:", :exception => e.class, :message => e.message)
239+
non_atomic_write(time)
230240
end
231241
end
232242

233243
# @return expired keys
234244
def non_atomic_write(time)
245+
logger.trace? && logger.trace("non_atomic_write: ", :time => time)
235246
File.open(@full_path, "w+") do |io|
236247
@serializer.serialize(@sincedb, io, time.to_f)
237248
end

logstash-input-file.gemspec

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
Gem::Specification.new do |s|
22

33
s.name = 'logstash-input-file'
4-
s.version = '4.3.0'
4+
s.version = '4.3.1'
55
s.licenses = ['Apache-2.0']
66
s.summary = "Streams events from files"
77
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"

0 commit comments

Comments
 (0)