all: move rlimit_nofile, imp docs

CHANGELOG.md

@@ -32,6 +32,8 @@ and this project adheres to
 
 ### Changed
 
+- The setting `rlimit_nofile` is now in the `os` block of the configuration
+  file, together with the new `group` and `user` settings ([#2763]).
 - Permissions on filter files are now `0o644` instead of `0o600` ([#3198]).
 
 ### Deprecated

HACKING.md

@@ -126,6 +126,9 @@ on GitHub and most other Markdown renderers. -->
     )
     ```
 
+ *  Don't rely only on file names for build tags to work.  Always add build tags
+    as well.
+
  *  Don't use `fmt.Sprintf` where a more structured approach to string
     conversion could be used.  For example, `net.JoinHostPort` or
     `url.(*URL).String`.

internal/home/config.go

@@ -37,8 +37,15 @@ type logSettings struct {
 
 // osConfig contains OS-related configuration.
 type osConfig struct {
+	// Group is the name of the group which AdGuard Home must switch to on
+	// startup.  Empty string means no switching.
 	Group string `yaml:"group"`
-	User  string `yaml:"user"`
+	// User is the name of the user which AdGuard Home must switch to on
+	// startup.  Empty string means no switching.
+	User string `yaml:"user"`
+	// RlimitNoFile is the maximum number of opened fd's per process.  Zero
+	// means use the default value.
+	RlimitNoFile uint64 `yaml:"rlimit_nofile"`
 }
 
 // configuration is loaded from YAML
@@ -58,16 +65,9 @@ type configuration struct {
 	// AuthBlockMin is the duration, in minutes, of the block of new login
 	// attempts after AuthAttempts unsuccessful login attempts.
 	AuthBlockMin uint   `yaml:"block_auth_min"`
-	ProxyURL     string `yaml:"http_proxy"` // Proxy address for our HTTP client
-	Language     string `yaml:"language"`   // two-letter ISO 639-1 language code
-
-	// RlimitNoFile is the maximum number of opened fd's per process.  Zero
-	// means use the default value.
-	//
-	// TODO(a.garipov): Make a migration and move this to osConfig.
-	RlimitNoFile uint64 `yaml:"rlimit_nofile"`
-
-	DebugPProf bool `yaml:"debug_pprof"` // Enable pprof HTTP server on port 6060
+	ProxyURL     string `yaml:"http_proxy"`  // Proxy address for our HTTP client
+	Language     string `yaml:"language"`    // two-letter ISO 639-1 language code
+	DebugPProf   bool   `yaml:"debug_pprof"` // Enable pprof HTTP server on port 6060
 
 	// TTL for a web session (in hours)
 	// An active session is automatically refreshed once a day.

internal/home/home.go

@@ -198,37 +198,39 @@ func logIfUnsupported(msg string, err error) (outErr error) {
 
 // configureOS sets the OS-related configuration.
 func configureOS(conf *configuration) (err error) {
-	if osConf := config.OSConfig; osConf != nil {
-		log.Info("%+v", osConf)
-		if osConf.Group != "" {
-			err = aghos.SetGroup(osConf.Group)
-			err = logIfUnsupported("warning: setting group", err)
-			if err != nil {
-				return fmt.Errorf("setting group: %w", err)
-			}
+	osConf := conf.OSConfig
+	if osConf == nil {
+		return nil
+	}
 
-			log.Info("group set to %s", osConf.Group)
+	if osConf.Group != "" {
+		err = aghos.SetGroup(osConf.Group)
+		err = logIfUnsupported("warning: setting group", err)
+		if err != nil {
+			return fmt.Errorf("setting group: %w", err)
 		}
 
-		if osConf.User != "" {
-			err = aghos.SetUser(osConf.User)
-			err = logIfUnsupported("warning: setting user", err)
-			if err != nil {
-				return fmt.Errorf("setting user: %w", err)
-			}
+		log.Info("group set to %s", osConf.Group)
+	}
 
-			log.Info("user set to %s", osConf.User)
+	if osConf.User != "" {
+		err = aghos.SetUser(osConf.User)
+		err = logIfUnsupported("warning: setting user", err)
+		if err != nil {
+			return fmt.Errorf("setting user: %w", err)
 		}
+
+		log.Info("user set to %s", osConf.User)
 	}
 
-	if config.RlimitNoFile != 0 {
-		err = aghos.SetRlimit(conf.RlimitNoFile)
+	if osConf.RlimitNoFile != 0 {
+		err = aghos.SetRlimit(osConf.RlimitNoFile)
 		err = logIfUnsupported("warning: setting rlimit", err)
 		if err != nil {
 			return fmt.Errorf("setting rlimit: %w", err)
 		}
 
-		log.Info("rlimit_nofile set to %d", config.RlimitNoFile)
+		log.Info("rlimit_nofile set to %d", osConf.RlimitNoFile)
 	}
 
 	return nil

internal/home/upgrade.go

@@ -19,7 +19,7 @@ import (
 )
 
 // currentSchemaVersion is the current schema version.
-const currentSchemaVersion = 10
+const currentSchemaVersion = 11
 
 // These aliases are provided for convenience.
 type (
@@ -81,6 +81,7 @@ func upgradeConfigSchema(oldVersion int, diskConf yobj) (err error) {
 		upgradeSchema7to8,
 		upgradeSchema8to9,
 		upgradeSchema9to10,
+		upgradeSchema10to11,
 	}
 
 	n := 0
@@ -611,6 +612,41 @@ func upgradeSchema9to10(diskConf yobj) (err error) {
 	return nil
 }
 
+// upgradeSchema10to11 performs the following changes:
+//
+//   # BEFORE:
+//   'rlimit_nofile': 42
+//
+//   # AFTER:
+//   'os':
+//     'group': ''
+//     'rlimit_nofile': 42
+//     'user': ''
+//
+func upgradeSchema10to11(diskConf yobj) (err error) {
+	log.Printf("Upgrade yaml: 10 to 11")
+
+	diskConf["schema_version"] = 11
+
+	rlimit := 0
+	rlimitVal, ok := diskConf["rlimit_nofile"]
+	if ok {
+		rlimit, ok = rlimitVal.(int)
+		if !ok {
+			return fmt.Errorf("unexpected type of rlimit_nofile: %T", rlimitVal)
+		}
+	}
+
+	delete(diskConf, "rlimit_nofile")
+	diskConf["os"] = yobj{
+		"group":         "",
+		"rlimit_nofile": rlimit,
+		"user":          "",
+	}
+
+	return nil
+}
+
 // TODO(a.garipov): Replace with log.Output when we port it to our logging
 // package.
 func funcName() string {

internal/home/upgrade_test.go

@@ -368,3 +368,50 @@ func TestUpgradeSchema9to10(t *testing.T) {
 		assert.Equal(t, "unexpected type of dns: int", err.Error())
 	})
 }
+
+func TestUpgradeSchema10to11(t *testing.T) {
+	check := func(t *testing.T, conf yobj) {
+		rlimit, _ := conf["rlimit_nofile"].(int)
+
+		err := upgradeSchema10to11(conf)
+		require.NoError(t, err)
+
+		require.Equal(t, conf["schema_version"], 11)
+
+		_, ok := conf["rlimit_nofile"]
+		assert.False(t, ok)
+
+		osVal, ok := conf["os"]
+		require.True(t, ok)
+
+		newOSConf, ok := osVal.(yobj)
+		require.True(t, ok)
+
+		_, ok = newOSConf["group"]
+		assert.True(t, ok)
+
+		_, ok = newOSConf["user"]
+		assert.True(t, ok)
+
+		rlimitVal, ok := newOSConf["rlimit_nofile"].(int)
+		require.True(t, ok)
+
+		assert.Equal(t, rlimit, rlimitVal)
+	}
+
+	const rlimit = 42
+	t.Run("with_rlimit", func(t *testing.T) {
+		conf := yobj{
+			"rlimit_nofile":  rlimit,
+			"schema_version": 10,
+		}
+		check(t, conf)
+	})
+
+	t.Run("without_rlimit", func(t *testing.T) {
+		conf := yobj{
+			"schema_version": 10,
+		}
+		check(t, conf)
+	})
+}