From 68edd0dd26fbbaa8f058ea17164b09c9cbf29b2c Mon Sep 17 00:00:00 2001 From: kushynoda Date: Sun, 9 Feb 2025 07:53:36 +0300 Subject: [PATCH 1/3] Added umask support to system configuration - Implemented 'umask' option in system config to address #4810. - Users can now define 'umask' in Fluentd configuration instead of CLI args. - Improves usability for services and container images by removing reliance on '--umask' argument. Signed-off-by: kushynoda --- fluent.conf | 6 ++- lib/fluent/root_agent.rb | 27 ++++++++++ test/test_config.rb | 111 +++++++++++++++++++++++++++++++++++---- test/test_root_agent.rb | 46 ++++++++++++++++ 4 files changed, 180 insertions(+), 10 deletions(-) diff --git a/fluent.conf b/fluent.conf index 98d7a38856..aa6b02541d 100644 --- a/fluent.conf +++ b/fluent.conf @@ -1,6 +1,11 @@ # In v1 configuration, type and id are @ prefix parameters. # @type and @id are recommended. type and id are still available for backward compatibility +## System-wide configurations + + umask 0027 + + ## built-in TCP input ## $ echo | fluent-cat @@ -12,7 +17,6 @@ # # @type unix # - # HTTP input # http://localhost:8888/?json= diff --git a/lib/fluent/root_agent.rb b/lib/fluent/root_agent.rb index 552819e4bf..8185166977 100644 --- a/lib/fluent/root_agent.rb +++ b/lib/fluent/root_agent.rb @@ -180,6 +180,33 @@ def configure(conf) label_configs.each { |name, e| @labels[name].configure(e) } setup_error_label(error_label_config) if error_label_config + # Get umask value from system config + system_conf = conf.elements.find { |e| e.name == "system" } + if system_conf + if system_conf.has_key?("umask") + umask_str = system_conf.to_h["umask"] + begin + umask_value = Integer(umask_str, 8) # ArgumentError for invalid octal + if umask_value > 0 + File.umask(umask_value) + log.info "Applied umask: #{sprintf("%04o", File.umask)}" + end + rescue ArgumentError + raise Fluent::ConfigError, "Invalid umask value: #{umask_str} (must be valid octal)" + end + else + # No umask found in system config, set default + default_umask = 0022 + File.umask(default_umask) + log.info "No umask specified in config, using default: #{sprintf("%04o", File.umask)}" + end + else + # No system section found, set default + default_umask = 0022 + File.umask(default_umask) + log.info "No system section found in config, using default umask: #{sprintf("%04o", File.umask)}" + end + super setup_source_only_buffer_agent if @source_only_mode.enabled? diff --git a/test/test_config.rb b/test/test_config.rb index 7533e6e16b..7922afb734 100644 --- a/test/test_config.rb +++ b/test/test_config.rb @@ -161,6 +161,8 @@ def test_check_not_fetchd sub_test_case "yaml config" do def test_included write_config "#{TMP_DIR}/config_test_not_fetched.yaml", <<-EOS + system: + umask: "0022" config: - source: $type: dummy @@ -195,12 +197,13 @@ def test_included flush_mode: interval flush_interval: 1s EOS - root_conf = read_config("#{TMP_DIR}/config_test_not_fetched.yaml", use_yaml: true) - dummy_source_conf = root_conf.elements.first - tcp_source_conf = root_conf.elements[1] + root_conf = read_config("#{TMP_DIR}/config_test_not_fetched.yaml", use_yaml: true) + system_conf = root_conf.elements.find { |e| e.name == "system" } + dummy_source_conf = root_conf.elements.find { |e| e.name == "source" && e['@type'] == "dummy" } + tcp_source_conf = root_conf.elements.find { |e| e.name == "source" && e['@type'] == "tcp" } parse_tcp_conf = tcp_source_conf.elements.first - match_conf = root_conf.elements[2] - label_conf = root_conf.elements[3] + match_conf = root_conf.elements.find { |e| e.name == "match" && e.arg == "tag.*" } + label_conf = root_conf.elements.find { |e| e.name == "label" } fluent_log_conf = label_conf.elements.first fluent_log_buffer_conf = fluent_log_conf.elements.first @@ -222,6 +225,7 @@ def test_included 'memory', 'interval', '1s', + '0022', ], [ dummy_source_conf['@type'], @@ -240,11 +244,14 @@ def test_included fluent_log_buffer_conf['@type'], fluent_log_buffer_conf['flush_mode'], fluent_log_buffer_conf['flush_interval'], + system_conf['umask'], ]) end def test_included_glob write_config "#{TMP_DIR}/config.yaml", <<-EOS + system: + umask: "0022" config: - !include "include/*.yaml" EOS @@ -272,10 +279,11 @@ def test_included_glob flush_interval: 1s EOS root_conf = read_config("#{TMP_DIR}/config.yaml", use_yaml: true) - tcp_source_conf = root_conf.elements.first - dummy_source_conf = root_conf.elements[1] + system_conf = root_conf.elements.find { |e| e.name == "system" } + tcp_source_conf = root_conf.elements.find { |e| e.name == "source" && e['@type'] == "tcp" } + dummy_source_conf = root_conf.elements.find { |e| e.name == "source" && e['@type'] == "dummy" } parse_tcp_conf = tcp_source_conf.elements.first - match_conf = root_conf.elements[2] + match_conf = root_conf.elements.find { |e| e.name == "match" && e.arg == "tag.*" } assert_equal( [ @@ -287,6 +295,7 @@ def test_included_glob 'tag.dummy', 'stdout', 'tag.*', + '0022', ], [ tcp_source_conf['@type'], @@ -297,6 +306,7 @@ def test_included_glob dummy_source_conf['tag'], match_conf['@type'], match_conf.arg, + system_conf['umask'], ]) end @@ -343,6 +353,42 @@ def test_check_not_fetchd 10.times { match_conf['type'] } assert_equal before_size, match_conf.unused.size end + + test 'yaml system config with umask' do + write_config "#{TMP_DIR}/config_test_umask.yaml", <<-EOS + system: + umask: "0022" + config: + - source: + $type: dummy + tag: tag.dummy + - match: + $tag: tag.* + $type: stdout + EOS + conf = read_config("#{TMP_DIR}/config_test_umask.yaml", use_yaml: true) + assert_equal "0022", conf.elements.find { |e| e.name == "system" }["umask"] + end + + test 'system config with invalid umask' do + conf = <<-EOC + + umask 0999 # invalid octal + + EOC + conf = Fluent::Config.parse(conf, "(test)", "(test_dir)", true) + assert_equal "0999", conf.elements.find { |e| e.name == "system" }["umask"] + end + + test 'system config without umask' do + conf = <<-EOC + + # no umask setting + + EOC + conf = Fluent::Config.parse(conf, "(test)", "(test_dir)", true) + assert_nil conf.elements.find { |e| e.name == "system" }["umask"] + end end def write_config(path, data, encoding: 'utf-8') @@ -371,5 +417,52 @@ def write_config(path, data, encoding: 'utf-8') assert_equal('value', c['key']) assert_equal('value2', c['key2']) end + + test 'system config with umask' do + conf = <<-EOC + + umask 0022 + + EOC + conf = Fluent::Config.parse(conf, "(test)", "(test_dir)", true) + ra = Fluent::RootAgent.new(log: $log) + old_umask = File.umask + begin + ra.configure(conf) + assert_equal 0022, File.umask + ensure + File.umask(old_umask) + end + end + + test 'system config with invalid umask' do + conf = <<-EOC + + umask 0999 # invalid octal + + EOC + conf = Fluent::Config.parse(conf, "(test)", "(test_dir)", true) + ra = Fluent::RootAgent.new(log: $log) + assert_raise(Fluent::ConfigError) do + ra.configure(conf) + end + end + + test 'system config without umask' do + conf = <<-EOC + + # no umask setting + + EOC + conf = Fluent::Config.parse(conf, "(test)", "(test_dir)", true) + ra = Fluent::RootAgent.new(log: $log) + old_umask = File.umask + begin + ra.configure(conf) + assert_equal 0022, File.umask + ensure + File.umask(old_umask) + end + end end -end +end \ No newline at end of file diff --git a/test/test_root_agent.rb b/test/test_root_agent.rb index 8b44b68699..3fd0390c0c 100644 --- a/test/test_root_agent.rb +++ b/test/test_root_agent.rb @@ -1167,4 +1167,50 @@ def setup @root_agent.shutdown end end + + def test_configure_with_umask + conf = <<-EOC + + umask 0022 + +EOC + conf = Fluent::Config.parse(conf, "(test)", "(test_dir)", true) + ra = Fluent::RootAgent.new(log: $log) + old_umask = File.umask + begin + ra.configure(conf) + assert_equal 0022, File.umask + ensure + File.umask(old_umask) + end + end + + def test_configure_with_invalid_umask + conf = <<-EOC + + umask 0999 # invalid octal + +EOC + conf = Fluent::Config.parse(conf, "(test)", "(test_dir)", true) + ra = Fluent::RootAgent.new(log: $log) + assert_raise(Fluent::ConfigError) do + ra.configure(conf) + end + end + + def test_configure_without_umask + conf = <<-EOC + + +EOC + conf = Fluent::Config.parse(conf, "(test)", "(test_dir)", true) + ra = Fluent::RootAgent.new(log: $log) + old_umask = File.umask + begin + ra.configure(conf) + assert_equal 0022, File.umask + ensure + File.umask(old_umask) + end + end end From 86367ed61d4d4491649d40000548424eee6eb09a Mon Sep 17 00:00:00 2001 From: kushynoda Date: Mon, 10 Feb 2025 19:25:39 +0300 Subject: [PATCH 2/3] feat(system): Add umask option to Fluentd system configuration Previously, Fluentd users had to use the --umask command-line argument to set the umask, which was inconvenient for service deployments and containerized environments. This commit introduces a umask option in the Fluentd system configuration, allowing users to define umask directly within fluent.conf. Additionally, this commit fixes test cases related to umask handling: - The previous tests incorrectly asserted the expected umask values. - Adjusted tests to ensure that new file permissions are correctly applied according to the configured umask. - Added a test to verify that an invalid umask value raises the expected Fluent::ConfigError. Fixes #4816 Release Note: - Added umask option to Fluentd system configuration. Signed-off-by: kushynoda --- test/test_root_agent.rb | 90 ++++++++++++++++++++++++++++------------- 1 file changed, 63 insertions(+), 27 deletions(-) diff --git a/test/test_root_agent.rb b/test/test_root_agent.rb index 3fd0390c0c..cc69ce70b4 100644 --- a/test/test_root_agent.rb +++ b/test/test_root_agent.rb @@ -1168,49 +1168,85 @@ def setup end end - def test_configure_with_umask + test 'configure with umask should set proper value' do conf = <<-EOC - - umask 0022 - -EOC - conf = Fluent::Config.parse(conf, "(test)", "(test_dir)", true) + + umask 0027 + + EOC + + config = Fluent::Config.parse(conf, "(test)", "(test_dir)", true) ra = Fluent::RootAgent.new(log: $log) - old_umask = File.umask + + original_umask = File.umask + + filename = "test_umask_file" + begin - ra.configure(conf) - assert_equal 0022, File.umask + ra.configure(config) + + File.open(filename, "w") do |f| + f.write("Test data") + end + + file_mode = File.stat(filename).mode & 0777 + + # 0666 & ~0027 => 0640 (octal) + expected_mode = 0640 + + assert_equal(expected_mode, file_mode, + "Expected file mode to be #{sprintf('%o', expected_mode)}, but got #{sprintf('%o', file_mode)}") ensure - File.umask(old_umask) + File.umask(original_umask) + File.delete(filename) if File.exist?(filename) end end + - def test_configure_with_invalid_umask + test 'configure with invalid umask should raise error' do conf = <<-EOC - - umask 0999 # invalid octal - -EOC - conf = Fluent::Config.parse(conf, "(test)", "(test_dir)", true) + + umask 0999 # invalid octal + + EOC + + config = Fluent::Config.parse(conf, "(test)", "(test_dir)", true) ra = Fluent::RootAgent.new(log: $log) - assert_raise(Fluent::ConfigError) do - ra.configure(conf) + original_umask = File.umask + begin + assert_raise(Fluent::ConfigError, "Expected configuration with invalid umask to raise Fluent::ConfigError") do + ra.configure(config) + end + ensure + File.umask(original_umask) end end - def test_configure_without_umask + test 'configure without umask should use default umask and affect file permissions' do conf = <<-EOC - - -EOC - conf = Fluent::Config.parse(conf, "(test)", "(test_dir)", true) + + + EOC + + config = Fluent::Config.parse(conf, "(test)", "(test_dir)", true) ra = Fluent::RootAgent.new(log: $log) - old_umask = File.umask + original_umask = File.umask + filename = "test_umask_default_file" + begin - ra.configure(conf) - assert_equal 0022, File.umask + ra.configure(config) + assert_equal 0022, File.umask, "Expected umask to be 0022 after configuration" + # 0666 & ~0022 => 0644 (rw-r--r--) + expected_mode = 0644 + + File.open(filename, "w") { |f| f.write("Test content") } + file_mode = File.stat(filename).mode & 0777 + + assert_equal expected_mode, file_mode, + "Expected file mode to be #{sprintf('%o', expected_mode)}, but got #{sprintf('%o', file_mode)}" ensure - File.umask(old_umask) + File.umask(original_umask) + File.delete(filename) if File.exist?(filename) end end end From c5119b29eeb458abe3649d4d9870936003ed7e8e Mon Sep 17 00:00:00 2001 From: kushynoda Date: Mon, 10 Feb 2025 19:53:11 +0300 Subject: [PATCH 3/3] refactor(tests): Remove redundant umask test cases Signed-off-by: kushynoda --- test/test_config.rb | 109 ++++---------------------------------------- 1 file changed, 8 insertions(+), 101 deletions(-) diff --git a/test/test_config.rb b/test/test_config.rb index 7922afb734..cc367c4479 100644 --- a/test/test_config.rb +++ b/test/test_config.rb @@ -161,8 +161,6 @@ def test_check_not_fetchd sub_test_case "yaml config" do def test_included write_config "#{TMP_DIR}/config_test_not_fetched.yaml", <<-EOS - system: - umask: "0022" config: - source: $type: dummy @@ -197,13 +195,12 @@ def test_included flush_mode: interval flush_interval: 1s EOS - root_conf = read_config("#{TMP_DIR}/config_test_not_fetched.yaml", use_yaml: true) - system_conf = root_conf.elements.find { |e| e.name == "system" } - dummy_source_conf = root_conf.elements.find { |e| e.name == "source" && e['@type'] == "dummy" } - tcp_source_conf = root_conf.elements.find { |e| e.name == "source" && e['@type'] == "tcp" } + root_conf = read_config("#{TMP_DIR}/config_test_not_fetched.yaml", use_yaml: true) + dummy_source_conf = root_conf.elements.first + tcp_source_conf = root_conf.elements[1] parse_tcp_conf = tcp_source_conf.elements.first - match_conf = root_conf.elements.find { |e| e.name == "match" && e.arg == "tag.*" } - label_conf = root_conf.elements.find { |e| e.name == "label" } + match_conf = root_conf.elements[2] + label_conf = root_conf.elements[3] fluent_log_conf = label_conf.elements.first fluent_log_buffer_conf = fluent_log_conf.elements.first @@ -225,7 +222,6 @@ def test_included 'memory', 'interval', '1s', - '0022', ], [ dummy_source_conf['@type'], @@ -244,14 +240,11 @@ def test_included fluent_log_buffer_conf['@type'], fluent_log_buffer_conf['flush_mode'], fluent_log_buffer_conf['flush_interval'], - system_conf['umask'], ]) end def test_included_glob write_config "#{TMP_DIR}/config.yaml", <<-EOS - system: - umask: "0022" config: - !include "include/*.yaml" EOS @@ -279,11 +272,10 @@ def test_included_glob flush_interval: 1s EOS root_conf = read_config("#{TMP_DIR}/config.yaml", use_yaml: true) - system_conf = root_conf.elements.find { |e| e.name == "system" } - tcp_source_conf = root_conf.elements.find { |e| e.name == "source" && e['@type'] == "tcp" } - dummy_source_conf = root_conf.elements.find { |e| e.name == "source" && e['@type'] == "dummy" } + tcp_source_conf = root_conf.elements.first + dummy_source_conf = root_conf.elements[1] parse_tcp_conf = tcp_source_conf.elements.first - match_conf = root_conf.elements.find { |e| e.name == "match" && e.arg == "tag.*" } + match_conf = root_conf.elements[2] assert_equal( [ @@ -295,7 +287,6 @@ def test_included_glob 'tag.dummy', 'stdout', 'tag.*', - '0022', ], [ tcp_source_conf['@type'], @@ -306,7 +297,6 @@ def test_included_glob dummy_source_conf['tag'], match_conf['@type'], match_conf.arg, - system_conf['umask'], ]) end @@ -353,42 +343,6 @@ def test_check_not_fetchd 10.times { match_conf['type'] } assert_equal before_size, match_conf.unused.size end - - test 'yaml system config with umask' do - write_config "#{TMP_DIR}/config_test_umask.yaml", <<-EOS - system: - umask: "0022" - config: - - source: - $type: dummy - tag: tag.dummy - - match: - $tag: tag.* - $type: stdout - EOS - conf = read_config("#{TMP_DIR}/config_test_umask.yaml", use_yaml: true) - assert_equal "0022", conf.elements.find { |e| e.name == "system" }["umask"] - end - - test 'system config with invalid umask' do - conf = <<-EOC - - umask 0999 # invalid octal - - EOC - conf = Fluent::Config.parse(conf, "(test)", "(test_dir)", true) - assert_equal "0999", conf.elements.find { |e| e.name == "system" }["umask"] - end - - test 'system config without umask' do - conf = <<-EOC - - # no umask setting - - EOC - conf = Fluent::Config.parse(conf, "(test)", "(test_dir)", true) - assert_nil conf.elements.find { |e| e.name == "system" }["umask"] - end end def write_config(path, data, encoding: 'utf-8') @@ -417,52 +371,5 @@ def write_config(path, data, encoding: 'utf-8') assert_equal('value', c['key']) assert_equal('value2', c['key2']) end - - test 'system config with umask' do - conf = <<-EOC - - umask 0022 - - EOC - conf = Fluent::Config.parse(conf, "(test)", "(test_dir)", true) - ra = Fluent::RootAgent.new(log: $log) - old_umask = File.umask - begin - ra.configure(conf) - assert_equal 0022, File.umask - ensure - File.umask(old_umask) - end - end - - test 'system config with invalid umask' do - conf = <<-EOC - - umask 0999 # invalid octal - - EOC - conf = Fluent::Config.parse(conf, "(test)", "(test_dir)", true) - ra = Fluent::RootAgent.new(log: $log) - assert_raise(Fluent::ConfigError) do - ra.configure(conf) - end - end - - test 'system config without umask' do - conf = <<-EOC - - # no umask setting - - EOC - conf = Fluent::Config.parse(conf, "(test)", "(test_dir)", true) - ra = Fluent::RootAgent.new(log: $log) - old_umask = File.umask - begin - ra.configure(conf) - assert_equal 0022, File.umask - ensure - File.umask(old_umask) - end - end end end \ No newline at end of file