diff --git a/docs/MANUAL.md b/docs/MANUAL.md index 59f0626b..eabb67a5 100644 --- a/docs/MANUAL.md +++ b/docs/MANUAL.md @@ -181,7 +181,6 @@ However, settings that were overridden using this technique cannot be rehashed - Due to implementation details, this technique has some limitations. Here are the known issues: -1. `opers` must be overridden in its entirety with `ERGO__OPERS` (you cannot override the properties of individual operators, e.g. with `ERGO__OPERS__ADMIN__PASSWORD`) 1. `accounts.auth-script` and `server.ip-check-script` do not work as expected (see [#2275](https://github.com/ergochat/ergo/issues/2275) for workarounds). ## Productionizing with systemd diff --git a/irc/config.go b/irc/config.go index dc573436..43f7a421 100644 --- a/irc/config.go +++ b/irc/config.go @@ -1120,55 +1120,104 @@ func mungeFromEnvironment(config *Config, envPair string) (applied bool, name st pathComponents[i] = screamingSnakeToKebab(pathComponent) } + type mapInsertion struct { + m reflect.Value + k reflect.Value + v reflect.Value + } + var mapStack []mapInsertion + v := reflect.Indirect(reflect.ValueOf(config)) t := v.Type() for _, component := range pathComponents { if component == "" { return false, "", &configPathError{name, "invalid", nil} } - if v.Kind() != reflect.Struct { - return false, "", &configPathError{name, "index into non-struct", nil} - } - var nextField reflect.StructField - success := false - n := t.NumField() - // preferentially get a field with an exact yaml tag match, - // then fall back to case-insensitive comparison of field names - for i := 0; i < n; i++ { - field := t.Field(i) - if isExported(field) && field.Tag.Get("yaml") == component { - nextField = field - success = true - break - } - } - if !success { + if v.Kind() == reflect.Struct { + var nextField reflect.StructField + success := false + n := t.NumField() + // preferentially get a field with an exact yaml tag match, + // then fall back to case-insensitive comparison of field names for i := 0; i < n; i++ { field := t.Field(i) - if isExported(field) && strings.ToLower(field.Name) == component { + if isExported(field) && field.Tag.Get("yaml") == component { nextField = field success = true break } } - } - if !success { - return false, "", &configPathError{name, fmt.Sprintf("couldn't resolve path component: `%s`", component), nil} - } - v = v.FieldByName(nextField.Name) - // dereference pointer field if necessary, initialize new value if necessary - if v.Kind() == reflect.Ptr { - if v.IsNil() { - v.Set(reflect.New(v.Type().Elem())) + if !success { + for i := 0; i < n; i++ { + field := t.Field(i) + if isExported(field) && strings.ToLower(field.Name) == component { + nextField = field + success = true + break + } + } } - v = reflect.Indirect(v) + if !success { + return false, "", &configPathError{name, fmt.Sprintf("couldn't resolve path component: `%s`", component), nil} + } + v = v.FieldByName(nextField.Name) + // dereference pointer field if necessary, initialize new value if necessary + switch v.Kind() { + case reflect.Ptr: + if v.IsNil() { + v.Set(reflect.New(v.Type().Elem())) + } + v = reflect.Indirect(v) + case reflect.Map: + if v.IsNil() { + v.Set(reflect.MakeMap(v.Type())) + } + } + t = v.Type() + } else if v.Kind() == reflect.Map { + keyType := v.Type().Key() + valueType := v.Type().Elem() + if keyType.Kind() != reflect.String { + return false, "", &configPathError{name, "can't index into map unless its keys are strings", nil} + } + // index into the map, returns the zero value (invalid) if not found + key := reflect.ValueOf(component) + v2 := v.MapIndex(key) + if v2.IsValid() { + // make an addressable copy of the existing value: + v3 := reflect.New(valueType).Elem() + v3.Set(v2) + v2 = v3 + } else { + // make an addressable value of the map value type: + v2 = reflect.New(valueType).Elem() + // if the map value type is *Baz, set it to a new(Baz): + if valueType.Kind() == reflect.Pointer { + v2.Set(reflect.New(valueType.Elem())) + } + } + // we are not operating directly on the current map member, + // we need to go back later and insert v2 into the map: + mapStack = append(mapStack, mapInsertion{m: v, k: key, v: v2}) + if valueType.Kind() != reflect.Pointer { + v = v2 + } else { + v = reflect.Indirect(v2) + } + t = v.Type() + } else { + return false, "", &configPathError{name, "can't index into fields other than struct or map", nil} } - t = v.Type() } yamlErr := yaml.Unmarshal([]byte(value), v.Addr().Interface()) if yamlErr != nil { return false, "", &configPathError{name, "couldn't deserialize YAML", yamlErr} } + // go back and do all map assignments + for i := len(mapStack) - 1; i >= 0; i-- { + elem := mapStack[i] + elem.m.SetMapIndex(elem.k, elem.v) + } return true, name, nil } diff --git a/irc/config_test.go b/irc/config_test.go index 05466daa..979bb2a9 100644 --- a/irc/config_test.go +++ b/irc/config_test.go @@ -8,6 +8,15 @@ import ( "testing" ) +func mungeEnvForTesting(config *Config, env []string, t *testing.T) { + for _, envPair := range env { + _, _, err := mungeFromEnvironment(config, envPair) + if err != nil { + t.Errorf("couldn't apply override `%s`: %v", envPair, err) + } + } +} + func TestEnvironmentOverrides(t *testing.T) { var config Config config.Server.Compatibility.SendUnprefixedSasl = true @@ -16,6 +25,12 @@ func TestEnvironmentOverrides(t *testing.T) { config.Accounts.DefaultUserModes = &defaultUserModes config.Server.WebSockets.AllowedOrigins = []string{"https://www.ircv3.net"} config.Server.MOTD = "long.motd.txt" // overwrite this + config.Opers = map[string]*OperConfig{ + "admin": { + Class: "server-admin", + Password: "adminpassword", + }, + } env := []string{ `USER=shivaram`, // unrelated var `ORAGONO_USER=oragono`, // this should be ignored as well @@ -26,13 +41,11 @@ func TestEnvironmentOverrides(t *testing.T) { `ORAGONO__ACCOUNTS__NICK_RESERVATION__ENABLED=true`, `ERGO__ACCOUNTS__DEFAULT_USER_MODES="+iR"`, `ORAGONO__SERVER__IP_CLOAKING={"enabled": true, "enabled-for-always-on": true, "netname": "irc", "cidr-len-ipv4": 32, "cidr-len-ipv6": 64, "num-bits": 64}`, + `ERGO__OPERS__ADMIN__PASSWORD="newadminpassword"`, + `ERGO__OPERS__OPERUSER={"class": "server-admin", "whois-line": "is a server admin", "password": "operpassword"}`, } - for _, envPair := range env { - _, _, err := mungeFromEnvironment(&config, envPair) - if err != nil { - t.Errorf("couldn't apply override `%s`: %v", envPair, err) - } - } + + mungeEnvForTesting(&config, env, t) if config.Network.Name != "example.com" { t.Errorf("unexpected value of network.name: %s", config.Network.Name) @@ -68,6 +81,61 @@ func TestEnvironmentOverrides(t *testing.T) { if *config.Accounts.DefaultUserModes != "+iR" { t.Errorf("couldn't override pre-set ptr field") } + + if (*config.Opers["admin"]).Password != "newadminpassword" { + t.Errorf("couldn't index into map and then overwrite") + } + + if (*config.Opers["operuser"]).Password != "operpassword" { + t.Errorf("couldn't create new entry in map") + } +} + +func TestEnvironmentInitializeNilMap(t *testing.T) { + var config Config + env := []string{ + `ERGO__OPERS__OPERUSER={"class": "server-admin", "whois-line": "is a server admin", "password": "operpassword"}`, + } + + mungeEnvForTesting(&config, env, t) + + assertEqual((*config.Opers["operuser"]).Password, "operpassword") + + // try with an initialized but empty map: + config.Opers = make(map[string]*OperConfig) + mungeEnvForTesting(&config, env, t) + assertEqual((*config.Opers["operuser"]).Password, "operpassword") +} + +func TestEnvironmentCreateNewMap(t *testing.T) { + var config Config + env := []string{ + `ERGO__OPERS={"operuser": {"class": "server-admin", "whois-line": "is a server admin", "password": "operpassword"}}`, + } + + mungeEnvForTesting(&config, env, t) + + operPassword := (*config.Opers["operuser"]).Password + if operPassword != "operpassword" { + t.Errorf("unexpected value of operator password: %s", operPassword) + } + + // try with an initialized but empty map: + config.Opers = make(map[string]*OperConfig) + mungeEnvForTesting(&config, env, t) + assertEqual((*config.Opers["operuser"]).Password, "operpassword") +} + +func TestEnvironmentNonPointerMap(t *testing.T) { + // edge cases that should not panic, even though the results are unusable + // since all "field names" get lowercased: + var config Config + config.Server.AdditionalISupport = map[string]string{"extban": "a"} + env := []string{ + `ERGO__SERVER__ADDITIONAL_ISUPPORT__EXTBAN=~,a`, + `ERGO__FAKELAG__COMMAND_BUDGETS__PRIVMSG=10`, + } + mungeEnvForTesting(&config, env, t) } func TestEnvironmentOverrideErrors(t *testing.T) {