chumask: fix issue where it only works with standalone worker

Fix fluent#3671 (88a7260)

After fluent#3671, the value of `chumask` passed to ServerEngine was
always `nil`.
The `chumask` value was being used only when standalone.
It appears to be unintentional.

This fixes it.

* By default, `0` as `chumask` is passed to ServerEngine.
* If specifying `--umask` option, that value is passed.
* This `chumask` value is applied when using `daemonize`.
  * When not using `daemonize`, the value is not used.
  * This is the specification of ServerEngine.

Specification change:
(Just undo the specification that fluent#3671 unintentionally changed.)

* This changes the default `umask` when using `daemonize`.
  * fluent-package (RPM/DEB), for example.
  * Before: system default (`nil` for ServerEngine)
  * After: `0`

Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>

Author: daipom

lib/fluent/supervisor.rb

@@ -587,7 +587,7 @@ def self.serverengine_config(params = {})
         log_level: params['log_level'],
         chuser: params['chuser'],
         chgroup: params['chgroup'],
-        chumask: params['chumask'],
+        chumask: params['chumask'].is_a?(Integer) ? params['chumask'] : params['chumask']&.to_i(8),
         daemonize: daemonize,
         rpc_endpoint: params['rpc_endpoint'],
         counter_server: params['counter_server'],
@@ -924,6 +924,7 @@ def supervise
         'inline_config' => @inline_config,
         'chuser' => @chuser,
         'chgroup' => @chgroup,
+        'chumask' => @chumask,
         'fluentd_conf_path' => @config_path,
         'fluentd_conf' => @conf.to_s,
         'use_v1_config' => @use_v1_config,

test/command/test_fluentd.rb

@@ -1336,6 +1336,14 @@ def multi_workers_ready?; true; end
     end
   end
 
+  test "--umask should be recognized as command line" do
+    conf_path = create_conf_file("empty.conf", "")
+    assert File.exist?(conf_path)
+    assert_log_matches(create_cmdline(conf_path, "--umask", "222"),
+                       "spawn command to main:", '"--umask", "222"',
+                       patterns_not_match: ["[error]"])
+  end
+
   sub_test_case "--with-source-only" do
     setup do
       omit "Not supported on Windows" if Fluent.windows?

test/test_supervisor.rb

@@ -610,51 +610,84 @@ def server.config
     assert_equal('{"ok":true}', response)
   end
 
-  def test_serverengine_config
-    params = {}
-    params['workers'] = 1
-    params['fluentd_conf_path'] = "fluentd.conf"
-    params['use_v1_config'] = true
-    params['conf_encoding'] = 'utf-8'
-    params['log_level'] = Fluent::Log::LEVEL_INFO
-    load_config_proc =  Proc.new { Fluent::Supervisor.serverengine_config(params) }
-
-    se_config = load_config_proc.call
-    assert_equal Fluent::Log::LEVEL_INFO, se_config[:log_level]
-    assert_equal 'spawn', se_config[:worker_type]
-    assert_equal 1, se_config[:workers]
-    assert_equal false, se_config[:log_stdin]
-    assert_equal false, se_config[:log_stdout]
-    assert_equal false, se_config[:log_stderr]
-    assert_equal true, se_config[:enable_heartbeat]
-    assert_equal false, se_config[:auto_heartbeat]
-    assert_equal "fluentd.conf", se_config[:config_path]
-    assert_equal false, se_config[:daemonize]
-    assert_nil se_config[:pid_path]
+  sub_test_case "serverengine_config" do
+    def test_normal
+      params = {}
+      params['workers'] = 1
+      params['fluentd_conf_path'] = "fluentd.conf"
+      params['use_v1_config'] = true
+      params['conf_encoding'] = 'utf-8'
+      params['log_level'] = Fluent::Log::LEVEL_INFO
+      load_config_proc =  Proc.new { Fluent::Supervisor.serverengine_config(params) }
+
+      se_config = load_config_proc.call
+      assert_equal Fluent::Log::LEVEL_INFO, se_config[:log_level]
+      assert_equal 'spawn', se_config[:worker_type]
+      assert_equal 1, se_config[:workers]
+      assert_equal false, se_config[:log_stdin]
+      assert_equal false, se_config[:log_stdout]
+      assert_equal false, se_config[:log_stderr]
+      assert_equal true, se_config[:enable_heartbeat]
+      assert_equal false, se_config[:auto_heartbeat]
+      assert_equal "fluentd.conf", se_config[:config_path]
+      assert_equal false, se_config[:daemonize]
+      assert_nil se_config[:pid_path]
+    end
+
+    def test_daemonize
+      params = {}
+      params['workers'] = 1
+      params['fluentd_conf_path'] = "fluentd.conf"
+      params['use_v1_config'] = true
+      params['conf_encoding'] = 'utf-8'
+      params['log_level'] = Fluent::Log::LEVEL_INFO
+      params['daemonize'] = './fluentd.pid'
+      load_config_proc = Proc.new { Fluent::Supervisor.serverengine_config(params) }
+
+      se_config = load_config_proc.call
+      assert_equal Fluent::Log::LEVEL_INFO, se_config[:log_level]
+      assert_equal 'spawn', se_config[:worker_type]
+      assert_equal 1, se_config[:workers]
+      assert_equal false, se_config[:log_stdin]
+      assert_equal false, se_config[:log_stdout]
+      assert_equal false, se_config[:log_stderr]
+      assert_equal true, se_config[:enable_heartbeat]
+      assert_equal false, se_config[:auto_heartbeat]
+      assert_equal "fluentd.conf", se_config[:config_path]
+      assert_equal true, se_config[:daemonize]
+      assert_equal './fluentd.pid', se_config[:pid_path]
+    end
+
+    data("nil", [nil, nil])
+    data("default", ["0", 0])
+    data("000", ["000", 0])
+    data("0000", ["0000", 0])
+    data("2", ["2", 2])
+    data("222", ["222", 146])
+    data("0222", ["0222", 146])
+    data("0 as integer", [0, 0])
+    def test_chumask((chumask, expected))
+      params = { "chumask" => chumask }
+      load_config_proc = Proc.new { Fluent::Supervisor.serverengine_config(params) }
+
+      se_config = load_config_proc.call
+
+      assert_equal expected, se_config[:chumask]
+    end
   end
 
-  def test_serverengine_config_for_daemonize
-    params = {}
-    params['workers'] = 1
-    params['fluentd_conf_path'] = "fluentd.conf"
-    params['use_v1_config'] = true
-    params['conf_encoding'] = 'utf-8'
-    params['log_level'] = Fluent::Log::LEVEL_INFO
-    params['daemonize'] = './fluentd.pid'
-    load_config_proc = Proc.new { Fluent::Supervisor.serverengine_config(params) }
-
-    se_config = load_config_proc.call
-    assert_equal Fluent::Log::LEVEL_INFO, se_config[:log_level]
-    assert_equal 'spawn', se_config[:worker_type]
-    assert_equal 1, se_config[:workers]
-    assert_equal false, se_config[:log_stdin]
-    assert_equal false, se_config[:log_stdout]
-    assert_equal false, se_config[:log_stderr]
-    assert_equal true, se_config[:enable_heartbeat]
-    assert_equal false, se_config[:auto_heartbeat]
-    assert_equal "fluentd.conf", se_config[:config_path]
-    assert_equal true, se_config[:daemonize]
-    assert_equal './fluentd.pid', se_config[:pid_path]
+  data("default", [{}, "0"])
+  data("222", [{chumask: "222"}, "222"])
+  def test_chumask_should_be_passed_to_ServerEngine((cl_opt, expected_chumask_value))
+    proxy.mock(Fluent::Supervisor).serverengine_config(hash_including("chumask" => expected_chumask_value))
+    any_instance_of(ServerEngine::Daemon) { |daemon| mock(daemon).run.once }
+
+    supervisor = Fluent::Supervisor.new(cl_opt)
+    stub(Fluent::Config).build { config_element('ROOT') }
+    stub(supervisor).build_spawn_command { "dummy command line" }
+    supervisor.configure(supervisor: true)
+
+    supervisor.run_supervisor
   end
 
   sub_test_case "init logger" do