fix(config): keep idle locking alive when the config watch fails
When the OS refuses to watch the config (inotify limits, permissions), Watch reports it and polls every 2s instead of returning an error that stopped the daemon. Parse reports the first problem in line order again, and every '#' starts a comment, even inside quotes, as before kvconf. Tests cover the polling fallback, line order, '#' in quotes and the kv tags against the ports constants.
- commit
- 3421f1623e72d0b6a3401f4a19c89a68e04bd1e0
- author
- brice · · 2026-10-10 08:46 UTC
- parent
- 3b25b0f
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email - refs
- refactor/kvconf
4 files · +194 -12
M internal/adapters/config/config.go +30 -10
| @@ -7,6 +7,7 @@ import ( | |||
| 7 | 7 | "io" | |
| 8 | 8 | "os" | |
| 9 | 9 | "path/filepath" | |
| 10 | + | "strings" | |
| 10 | 11 | ||
| 11 | 12 | "github.com/bnema/kvconf" | |
| 12 | 13 | "github.com/bnema/neferafk/internal/ports" | |
| @@ -45,31 +46,50 @@ func Parse(r io.Reader) (ports.Config, error) { | |||
| 45 | 46 | return parse(data) | |
| 46 | 47 | } | |
| 47 | 48 | ||
| 48 | - | // options: any problem rejects the whole file; "#" starts a comment anywhere | |
| 49 | - | // and values are never unquoted. | |
| 50 | - | var options = kvconf.Options{Strict: true, Comments: kvconf.CommentAny, RawValues: true, MaxBytes: MaxBytes, MaxLine: MaxLine} | |
| 49 | + | // options: values are never unquoted, and parse cuts each one at its first | |
| 50 | + | // "#", even inside quotes. parse rejects the whole file on any problem. | |
| 51 | + | var options = kvconf.Options{Comments: kvconf.CommentLine, RawValues: true, MaxBytes: MaxBytes, MaxLine: MaxLine} | |
| 51 | 52 | ||
| 52 | 53 | var errTooLarge = errors.New("config exceeds 64KiB") | |
| 53 | 54 | ||
| 55 | + | // parse fails on the first problem in line order. Strict mode would report | |
| 56 | + | // a line without "=" or a duplicate before an earlier bad value, so the file | |
| 57 | + | // is decoded leniently and its problems compared; a syntax problem wins a tie. | |
| 54 | 58 | func parse(data []byte) (ports.Config, error) { | |
| 55 | - | cfg := ports.Defaults() | |
| 56 | - | if _, err := kvconf.Unmarshal(data, &cfg, options); err != nil { | |
| 59 | + | entries, syntax, err := kvconf.ParseBytes(data, options) | |
| 60 | + | if err != nil { | |
| 57 | 61 | return ports.Config{}, reject(err) | |
| 58 | 62 | } | |
| 63 | + | for i := range entries { | |
| 64 | + | if v, _, cut := strings.Cut(entries[i].Value, "#"); cut { | |
| 65 | + | entries[i].Value = strings.TrimSpace(v) | |
| 66 | + | } | |
| 67 | + | } | |
| 68 | + | cfg := ports.Defaults() | |
| 69 | + | values, err := kvconf.Decode(entries, &cfg, options) | |
| 70 | + | switch { | |
| 71 | + | case len(syntax) > 0 && (len(values) == 0 || syntax[0].Line <= values[0].Line): | |
| 72 | + | return ports.Config{}, lineError(syntax[0]) | |
| 73 | + | case len(values) > 0: | |
| 74 | + | return ports.Config{}, lineError(values[0]) | |
| 75 | + | case err != nil: | |
| 76 | + | return ports.Config{}, err | |
| 77 | + | } | |
| 59 | 78 | return cfg, nil | |
| 60 | 79 | } | |
| 61 | 80 | ||
| 62 | - | // reject rewrites kvconf errors so they carry a line number and a fixed | |
| 63 | - | // message, never a key or value, which may be secret. | |
| 64 | 81 | func reject(err error) error { | |
| 65 | - | var w kvconf.Warning | |
| 66 | 82 | switch { | |
| 67 | 83 | case errors.Is(err, kvconf.ErrTooLarge): | |
| 68 | 84 | return errTooLarge | |
| 69 | 85 | case errors.Is(err, kvconf.ErrNotUTF8): | |
| 70 | 86 | return errors.New("config is not valid UTF-8") | |
| 71 | - | case errors.As(err, &w): | |
| 72 | - | return fmt.Errorf("line %d: %s", w.Line, w.Msg) | |
| 73 | 87 | } | |
| 74 | 88 | return err | |
| 75 | 89 | } | |
| 90 | + | ||
| 91 | + | // lineError carries a line number and a fixed message, never the key or | |
| 92 | + | // value, which may be secret. | |
| 93 | + | func lineError(w kvconf.Warning) error { | |
| 94 | + | return fmt.Errorf("line %d: %s", w.Line, w.Msg) | |
| 95 | + | } | |
| @@ -7,6 +7,7 @@ import ( | |||||
| 7 | "io" | 7 | "io" | ||
| 8 | "os" | 8 | "os" | ||
| 9 | "path/filepath" | 9 | "path/filepath" | ||
| 10 | + | "strings" | |||
| 10 | 11 | ||||
| 11 | "github.com/bnema/kvconf" | 12 | "github.com/bnema/kvconf" | ||
| 12 | "github.com/bnema/neferafk/internal/ports" | 13 | "github.com/bnema/neferafk/internal/ports" | ||
| @@ -45,31 +46,50 @@ func Parse(r io.Reader) (ports.Config, error) { | |||||
| 45 | return parse(data) | 46 | return parse(data) | ||
| 46 | } | 47 | } | ||
| 47 | 48 | ||||
| 48 | - | // options: any problem rejects the whole file; "#" starts a comment anywhere | 49 | + | // options: values are never unquoted, and parse cuts each one at its first |
| 49 | - | // and values are never unquoted. | 50 | + | // "#", even inside quotes. parse rejects the whole file on any problem. |
| 50 | - | var options = kvconf.Options{Strict: true, Comments: kvconf.CommentAny, RawValues: true, MaxBytes: MaxBytes, MaxLine: MaxLine} | 51 | + | var options = kvconf.Options{Comments: kvconf.CommentLine, RawValues: true, MaxBytes: MaxBytes, MaxLine: MaxLine} |
| 51 | 52 | ||||
| 52 | var errTooLarge = errors.New("config exceeds 64KiB") | 53 | var errTooLarge = errors.New("config exceeds 64KiB") | ||
| 53 | 54 | ||||
| 55 | + | // parse fails on the first problem in line order. Strict mode would report | |||
| 56 | + | // a line without "=" or a duplicate before an earlier bad value, so the file | |||
| 57 | + | // is decoded leniently and its problems compared; a syntax problem wins a tie. | |||
| 54 | func parse(data []byte) (ports.Config, error) { | 58 | func parse(data []byte) (ports.Config, error) { | ||
| 55 | - | cfg := ports.Defaults() | 59 | + | entries, syntax, err := kvconf.ParseBytes(data, options) |
| 56 | - | if _, err := kvconf.Unmarshal(data, &cfg, options); err != nil { | 60 | + | if err != nil { |
| 57 | return ports.Config{}, reject(err) | 61 | return ports.Config{}, reject(err) | ||
| 58 | } | 62 | } | ||
| 63 | + | for i := range entries { | |||
| 64 | + | if v, _, cut := strings.Cut(entries[i].Value, "#"); cut { | |||
| 65 | + | entries[i].Value = strings.TrimSpace(v) | |||
| 66 | + | } | |||
| 67 | + | } | |||
| 68 | + | cfg := ports.Defaults() | |||
| 69 | + | values, err := kvconf.Decode(entries, &cfg, options) | |||
| 70 | + | switch { | |||
| 71 | + | case len(syntax) > 0 && (len(values) == 0 || syntax[0].Line <= values[0].Line): | |||
| 72 | + | return ports.Config{}, lineError(syntax[0]) | |||
| 73 | + | case len(values) > 0: | |||
| 74 | + | return ports.Config{}, lineError(values[0]) | |||
| 75 | + | case err != nil: | |||
| 76 | + | return ports.Config{}, err | |||
| 77 | + | } | |||
| 59 | return cfg, nil | 78 | return cfg, nil | ||
| 60 | } | 79 | } | ||
| 61 | 80 | ||||
| 62 | - | // reject rewrites kvconf errors so they carry a line number and a fixed | |||
| 63 | - | // message, never a key or value, which may be secret. | |||
| 64 | func reject(err error) error { | 81 | func reject(err error) error { | ||
| 65 | - | var w kvconf.Warning | |||
| 66 | switch { | 82 | switch { | ||
| 67 | case errors.Is(err, kvconf.ErrTooLarge): | 83 | case errors.Is(err, kvconf.ErrTooLarge): | ||
| 68 | return errTooLarge | 84 | return errTooLarge | ||
| 69 | case errors.Is(err, kvconf.ErrNotUTF8): | 85 | case errors.Is(err, kvconf.ErrNotUTF8): | ||
| 70 | return errors.New("config is not valid UTF-8") | 86 | return errors.New("config is not valid UTF-8") | ||
| 71 | - | case errors.As(err, &w): | |||
| 72 | - | return fmt.Errorf("line %d: %s", w.Line, w.Msg) | |||
| 73 | } | 87 | } | ||
| 74 | return err | 88 | return err | ||
| 75 | } | 89 | } | ||
| 90 | + | ||||
| 91 | + | // lineError carries a line number and a fixed message, never the key or | |||
| 92 | + | // value, which may be secret. | |||
| 93 | + | func lineError(w kvconf.Warning) error { | |||
| 94 | + | return fmt.Errorf("line %d: %s", w.Line, w.Msg) | |||
| 95 | + | } | |||
M internal/adapters/config/config_test.go +62 -0
| @@ -72,3 +72,65 @@ func TestRejectInvalidUTF8BeforeParsingAndDirectMetadata(t *testing.T) { | |||
| 72 | 72 | t.Fatal("direct invalid UTF-8 metadata admitted") | |
| 73 | 73 | } | |
| 74 | 74 | } | |
| 75 | + | ||
| 76 | + | func TestParseReportsFirstProblemInLineOrder(t *testing.T) { | |
| 77 | + | long := strings.Repeat("#", MaxLine+1) | |
| 78 | + | for _, tc := range []struct{ name, text, want string }{ | |
| 79 | + | {"bad value before missing =", "lock.after=bad\nfoo", "line 1: "}, | |
| 80 | + | {"bad value before duplicate", "lock.after=bad\nlock.after=1s", "line 1: "}, | |
| 81 | + | {"unknown key before missing =", "unknown=x\nnoeq", "line 1: unknown key"}, | |
| 82 | + | {"bad value before long line", "lock.after=bad\n" + long, "line 1: "}, | |
| 83 | + | {"missing = before bad value", "noeq\nlock.after=bad", "line 1: expected key = value"}, | |
| 84 | + | {"duplicate before later bad value", "lock.after=1s\nlock.after=2s\nsleep.after=bad", "line 2: overrides line 1"}, | |
| 85 | + | {"duplicate wins tie with its bad value", "lock.after=1s\nlock.after=bad", "line 2: overrides line 1"}, | |
| 86 | + | } { | |
| 87 | + | t.Run(tc.name, func(t *testing.T) { | |
| 88 | + | c, err := Parse(strings.NewReader(tc.text)) | |
| 89 | + | if err == nil || !strings.HasPrefix(err.Error(), tc.want) || c != (ports.Config{}) { | |
| 90 | + | t.Fatalf("got %v, %+v; want error starting %q", err, c, tc.want) | |
| 91 | + | } | |
| 92 | + | }) | |
| 93 | + | } | |
| 94 | + | } | |
| 95 | + | ||
| 96 | + | func TestParseHashStartsCommentEvenInQuotes(t *testing.T) { | |
| 97 | + | for _, tc := range []struct{ name, text, want string }{ | |
| 98 | + | {"plain", "lock.output = DP-2 # left", "DP-2"}, | |
| 99 | + | {"no space", "lock.output = DP-2#left", "DP-2"}, | |
| 100 | + | {"inside double quotes", `lock.output = "DP#2"`, `"DP`}, | |
| 101 | + | {"after unbalanced quote", `lock.output = DP-2 "# note`, `DP-2 "`}, | |
| 102 | + | {"inside single quotes", "lock.output = 'DP#2'", "'DP"}, | |
| 103 | + | } { | |
| 104 | + | t.Run(tc.name, func(t *testing.T) { | |
| 105 | + | c, err := Parse(strings.NewReader(tc.text)) | |
| 106 | + | if err != nil || c.LockOutput != tc.want { | |
| 107 | + | t.Fatalf("got %q, %v; want %q", c.LockOutput, err, tc.want) | |
| 108 | + | } | |
| 109 | + | }) | |
| 110 | + | } | |
| 111 | + | if _, err := Parse(strings.NewReader("lock.output = # only a comment")); err == nil { | |
| 112 | + | t.Fatal("value made empty by a comment accepted") | |
| 113 | + | } | |
| 114 | + | } | |
| 115 | + | ||
| 116 | + | // The kv tags on ports.Config repeat MaxDuration and the auth constants. | |
| 117 | + | func TestTagsMatchPortsConstants(t *testing.T) { | |
| 118 | + | max := ports.MaxDuration.String() | |
| 119 | + | for _, key := range []string{"lock.after", "fade.after", "screens.off-after", "sleep.after", "fade.duration"} { | |
| 120 | + | if _, err := Parse(strings.NewReader(key + "=" + max)); err != nil { | |
| 121 | + | t.Errorf("%s=%s rejected: %v", key, max, err) | |
| 122 | + | } | |
| 123 | + | if _, err := Parse(strings.NewReader(key + "=" + (ports.MaxDuration + 1).String())); err == nil { | |
| 124 | + | t.Errorf("%s above %s accepted", key, max) | |
| 125 | + | } | |
| 126 | + | } | |
| 127 | + | for _, text := range []string{ | |
| 128 | + | "auth.mode=" + string(ports.AuthModePassword), | |
| 129 | + | "auth.mode=" + string(ports.AuthModePIN) + "\nauth.pin-source=" + string(ports.PINSourceEnv) + "\nauth.pin-env=PIN", | |
| 130 | + | "auth.pin-source=" + string(ports.PINSourcePass) + "\nauth.pin-entry=a/b", | |
| 131 | + | } { | |
| 132 | + | if _, err := Parse(strings.NewReader(text)); err != nil { | |
| 133 | + | t.Errorf("%q rejected: %v", text, err) | |
| 134 | + | } | |
| 135 | + | } | |
| 136 | + | } | |
| @@ -72,3 +72,65 @@ func TestRejectInvalidUTF8BeforeParsingAndDirectMetadata(t *testing.T) { | |||||
| 72 | t.Fatal("direct invalid UTF-8 metadata admitted") | 72 | t.Fatal("direct invalid UTF-8 metadata admitted") | ||
| 73 | } | 73 | } | ||
| 74 | } | 74 | } | ||
| 75 | + | ||||
| 76 | + | func TestParseReportsFirstProblemInLineOrder(t *testing.T) { | |||
| 77 | + | long := strings.Repeat("#", MaxLine+1) | |||
| 78 | + | for _, tc := range []struct{ name, text, want string }{ | |||
| 79 | + | {"bad value before missing =", "lock.after=bad\nfoo", "line 1: "}, | |||
| 80 | + | {"bad value before duplicate", "lock.after=bad\nlock.after=1s", "line 1: "}, | |||
| 81 | + | {"unknown key before missing =", "unknown=x\nnoeq", "line 1: unknown key"}, | |||
| 82 | + | {"bad value before long line", "lock.after=bad\n" + long, "line 1: "}, | |||
| 83 | + | {"missing = before bad value", "noeq\nlock.after=bad", "line 1: expected key = value"}, | |||
| 84 | + | {"duplicate before later bad value", "lock.after=1s\nlock.after=2s\nsleep.after=bad", "line 2: overrides line 1"}, | |||
| 85 | + | {"duplicate wins tie with its bad value", "lock.after=1s\nlock.after=bad", "line 2: overrides line 1"}, | |||
| 86 | + | } { | |||
| 87 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 88 | + | c, err := Parse(strings.NewReader(tc.text)) | |||
| 89 | + | if err == nil || !strings.HasPrefix(err.Error(), tc.want) || c != (ports.Config{}) { | |||
| 90 | + | t.Fatalf("got %v, %+v; want error starting %q", err, c, tc.want) | |||
| 91 | + | } | |||
| 92 | + | }) | |||
| 93 | + | } | |||
| 94 | + | } | |||
| 95 | + | ||||
| 96 | + | func TestParseHashStartsCommentEvenInQuotes(t *testing.T) { | |||
| 97 | + | for _, tc := range []struct{ name, text, want string }{ | |||
| 98 | + | {"plain", "lock.output = DP-2 # left", "DP-2"}, | |||
| 99 | + | {"no space", "lock.output = DP-2#left", "DP-2"}, | |||
| 100 | + | {"inside double quotes", `lock.output = "DP#2"`, `"DP`}, | |||
| 101 | + | {"after unbalanced quote", `lock.output = DP-2 "# note`, `DP-2 "`}, | |||
| 102 | + | {"inside single quotes", "lock.output = 'DP#2'", "'DP"}, | |||
| 103 | + | } { | |||
| 104 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 105 | + | c, err := Parse(strings.NewReader(tc.text)) | |||
| 106 | + | if err != nil || c.LockOutput != tc.want { | |||
| 107 | + | t.Fatalf("got %q, %v; want %q", c.LockOutput, err, tc.want) | |||
| 108 | + | } | |||
| 109 | + | }) | |||
| 110 | + | } | |||
| 111 | + | if _, err := Parse(strings.NewReader("lock.output = # only a comment")); err == nil { | |||
| 112 | + | t.Fatal("value made empty by a comment accepted") | |||
| 113 | + | } | |||
| 114 | + | } | |||
| 115 | + | ||||
| 116 | + | // The kv tags on ports.Config repeat MaxDuration and the auth constants. | |||
| 117 | + | func TestTagsMatchPortsConstants(t *testing.T) { | |||
| 118 | + | max := ports.MaxDuration.String() | |||
| 119 | + | for _, key := range []string{"lock.after", "fade.after", "screens.off-after", "sleep.after", "fade.duration"} { | |||
| 120 | + | if _, err := Parse(strings.NewReader(key + "=" + max)); err != nil { | |||
| 121 | + | t.Errorf("%s=%s rejected: %v", key, max, err) | |||
| 122 | + | } | |||
| 123 | + | if _, err := Parse(strings.NewReader(key + "=" + (ports.MaxDuration + 1).String())); err == nil { | |||
| 124 | + | t.Errorf("%s above %s accepted", key, max) | |||
| 125 | + | } | |||
| 126 | + | } | |||
| 127 | + | for _, text := range []string{ | |||
| 128 | + | "auth.mode=" + string(ports.AuthModePassword), | |||
| 129 | + | "auth.mode=" + string(ports.AuthModePIN) + "\nauth.pin-source=" + string(ports.PINSourceEnv) + "\nauth.pin-env=PIN", | |||
| 130 | + | "auth.pin-source=" + string(ports.PINSourcePass) + "\nauth.pin-entry=a/b", | |||
| 131 | + | } { | |||
| 132 | + | if _, err := Parse(strings.NewReader(text)); err != nil { | |||
| 133 | + | t.Errorf("%q rejected: %v", text, err) | |||
| 134 | + | } | |||
| 135 | + | } | |||
| 136 | + | } | |||
M internal/adapters/config/watch.go +32 -2
| @@ -4,18 +4,27 @@ import ( | |||
| 4 | 4 | "context" | |
| 5 | 5 | "errors" | |
| 6 | 6 | "io/fs" | |
| 7 | + | "time" | |
| 7 | 8 | ||
| 8 | 9 | "github.com/bnema/kvconf" | |
| 9 | 10 | "github.com/bnema/neferafk/internal/ports" | |
| 10 | 11 | ) | |
| 11 | 12 | ||
| 13 | + | // fallbackPoll is the polling interval used when native watching fails. | |
| 14 | + | var fallbackPoll = 2 * time.Second | |
| 15 | + | ||
| 16 | + | // kvWatch is kvconf.Watch; tests replace it to make native watching fail. | |
| 17 | + | var kvWatch = kvconf.Watch | |
| 18 | + | ||
| 12 | 19 | // Watch retains last-valid state across missing files/directories and follows | |
| 13 | 20 | // config symlink targets (kvconf.Watch). It always emits the first valid | |
| 14 | 21 | // candidate, including a candidate equal to defaults: this closes an app | |
| 15 | 22 | // Load -> Watch startup gap. Ready closes after initial watch installation, | |
| 16 | 23 | // before candidate delivery. No invalid/deleted candidate emits a defaults | |
| 17 | 24 | // reset. Errors never echo file content or paths, which may themselves | |
| 18 | - | // contain sensitive source metadata. | |
| 25 | + | // contain sensitive source metadata. When the OS refuses to watch (inotify | |
| 26 | + | // limits, permissions), Watch reports it on failures and polls instead: a | |
| 27 | + | // watch error must never stop the daemon and with it idle locking. | |
| 19 | 28 | func Watch(ctx context.Context, path string, out chan<- ports.ConfigChanged, failures chan<- error, ready chan<- struct{}) error { | |
| 20 | 29 | ctx, cancel := context.WithCancel(ctx) | |
| 21 | 30 | changed := make(chan struct{}, 1) | |
| @@ -24,7 +33,28 @@ func Watch(ctx context.Context, path string, out chan<- ports.ConfigChanged, fai | |||
| 24 | 33 | var watchErr error | |
| 25 | 34 | go func() { | |
| 26 | 35 | defer close(done) | |
| 27 | - | watchErr = kvconf.Watch(ctx, path, kvconf.WatchOptions{Ready: installed}, changed) | |
| 36 | + | err := kvWatch(ctx, path, kvconf.WatchOptions{Ready: installed}, changed) | |
| 37 | + | if err == nil || ctx.Err() != nil { | |
| 38 | + | return | |
| 39 | + | } | |
| 40 | + | if failures != nil { | |
| 41 | + | select { | |
| 42 | + | case failures <- errors.New("config watch failed; polling"): | |
| 43 | + | default: | |
| 44 | + | } | |
| 45 | + | } | |
| 46 | + | opts := kvconf.WatchOptions{Poll: fallbackPoll} | |
| 47 | + | select { | |
| 48 | + | case <-installed: // closed by the native watch before it failed | |
| 49 | + | default: | |
| 50 | + | opts.Ready = installed | |
| 51 | + | } | |
| 52 | + | // Changes made while nothing watched are seen by this reload. | |
| 53 | + | select { | |
| 54 | + | case changed <- struct{}{}: | |
| 55 | + | default: | |
| 56 | + | } | |
| 57 | + | watchErr = kvWatch(ctx, path, opts, changed) | |
| 28 | 58 | }() | |
| 29 | 59 | defer func() { cancel(); <-done }() | |
| 30 | 60 | ||
| @@ -4,18 +4,27 @@ import ( | |||||
| 4 | "context" | 4 | "context" | ||
| 5 | "errors" | 5 | "errors" | ||
| 6 | "io/fs" | 6 | "io/fs" | ||
| 7 | + | "time" | |||
| 7 | 8 | ||||
| 8 | "github.com/bnema/kvconf" | 9 | "github.com/bnema/kvconf" | ||
| 9 | "github.com/bnema/neferafk/internal/ports" | 10 | "github.com/bnema/neferafk/internal/ports" | ||
| 10 | ) | 11 | ) | ||
| 11 | 12 | ||||
| 13 | + | // fallbackPoll is the polling interval used when native watching fails. | |||
| 14 | + | var fallbackPoll = 2 * time.Second | |||
| 15 | + | ||||
| 16 | + | // kvWatch is kvconf.Watch; tests replace it to make native watching fail. | |||
| 17 | + | var kvWatch = kvconf.Watch | |||
| 18 | + | ||||
| 12 | // Watch retains last-valid state across missing files/directories and follows | 19 | // Watch retains last-valid state across missing files/directories and follows | ||
| 13 | // config symlink targets (kvconf.Watch). It always emits the first valid | 20 | // config symlink targets (kvconf.Watch). It always emits the first valid | ||
| 14 | // candidate, including a candidate equal to defaults: this closes an app | 21 | // candidate, including a candidate equal to defaults: this closes an app | ||
| 15 | // Load -> Watch startup gap. Ready closes after initial watch installation, | 22 | // Load -> Watch startup gap. Ready closes after initial watch installation, | ||
| 16 | // before candidate delivery. No invalid/deleted candidate emits a defaults | 23 | // before candidate delivery. No invalid/deleted candidate emits a defaults | ||
| 17 | // reset. Errors never echo file content or paths, which may themselves | 24 | // reset. Errors never echo file content or paths, which may themselves | ||
| 18 | - | // contain sensitive source metadata. | 25 | + | // contain sensitive source metadata. When the OS refuses to watch (inotify |
| 26 | + | // limits, permissions), Watch reports it on failures and polls instead: a | |||
| 27 | + | // watch error must never stop the daemon and with it idle locking. | |||
| 19 | func Watch(ctx context.Context, path string, out chan<- ports.ConfigChanged, failures chan<- error, ready chan<- struct{}) error { | 28 | func Watch(ctx context.Context, path string, out chan<- ports.ConfigChanged, failures chan<- error, ready chan<- struct{}) error { | ||
| 20 | ctx, cancel := context.WithCancel(ctx) | 29 | ctx, cancel := context.WithCancel(ctx) | ||
| 21 | changed := make(chan struct{}, 1) | 30 | changed := make(chan struct{}, 1) | ||
| @@ -24,7 +33,28 @@ func Watch(ctx context.Context, path string, out chan<- ports.ConfigChanged, fai | |||||
| 24 | var watchErr error | 33 | var watchErr error | ||
| 25 | go func() { | 34 | go func() { | ||
| 26 | defer close(done) | 35 | defer close(done) | ||
| 27 | - | watchErr = kvconf.Watch(ctx, path, kvconf.WatchOptions{Ready: installed}, changed) | 36 | + | err := kvWatch(ctx, path, kvconf.WatchOptions{Ready: installed}, changed) |
| 37 | + | if err == nil || ctx.Err() != nil { | |||
| 38 | + | return | |||
| 39 | + | } | |||
| 40 | + | if failures != nil { | |||
| 41 | + | select { | |||
| 42 | + | case failures <- errors.New("config watch failed; polling"): | |||
| 43 | + | default: | |||
| 44 | + | } | |||
| 45 | + | } | |||
| 46 | + | opts := kvconf.WatchOptions{Poll: fallbackPoll} | |||
| 47 | + | select { | |||
| 48 | + | case <-installed: // closed by the native watch before it failed | |||
| 49 | + | default: | |||
| 50 | + | opts.Ready = installed | |||
| 51 | + | } | |||
| 52 | + | // Changes made while nothing watched are seen by this reload. | |||
| 53 | + | select { | |||
| 54 | + | case changed <- struct{}{}: | |||
| 55 | + | default: | |||
| 56 | + | } | |||
| 57 | + | watchErr = kvWatch(ctx, path, opts, changed) | |||
| 28 | }() | 58 | }() | ||
| 29 | defer func() { cancel(); <-done }() | 59 | defer func() { cancel(); <-done }() | ||
| 30 | 60 | ||||
M internal/adapters/config/watch_test.go +70 -0
| @@ -2,6 +2,8 @@ package config | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "errors" | |
| 6 | + | "github.com/bnema/kvconf" | |
| 5 | 7 | "github.com/bnema/neferafk/internal/ports" | |
| 6 | 8 | "os" | |
| 7 | 9 | "path/filepath" | |
| @@ -197,3 +199,71 @@ func TestWatchFirstCandidateClosesLoadStartupGapAndMissingCancellation(t *testin | |||
| 197 | 199 | t.Fatal(err) | |
| 198 | 200 | } | |
| 199 | 201 | } | |
| 202 | + | ||
| 203 | + | // A watch the OS refuses must fall back to polling, never stop the daemon. | |
| 204 | + | func TestWatchFallsBackToPolling(t *testing.T) { | |
| 205 | + | orig, origPoll := kvWatch, fallbackPoll | |
| 206 | + | t.Cleanup(func() { kvWatch, fallbackPoll = orig, origPoll }) | |
| 207 | + | fallbackPoll = 20 * time.Millisecond | |
| 208 | + | for _, tc := range []struct { | |
| 209 | + | name string | |
| 210 | + | setup func(t *testing.T, dir string) | |
| 211 | + | }{ | |
| 212 | + | {"native watch fails at startup", func(t *testing.T, _ string) { | |
| 213 | + | kvWatch = func(ctx context.Context, path string, opts kvconf.WatchOptions, out chan<- struct{}) error { | |
| 214 | + | if opts.Poll == 0 { | |
| 215 | + | return errors.New("no space left on device") | |
| 216 | + | } | |
| 217 | + | return orig(ctx, path, opts, out) | |
| 218 | + | } | |
| 219 | + | }}, | |
| 220 | + | {"native watch fails after install", func(t *testing.T, _ string) { | |
| 221 | + | kvWatch = func(ctx context.Context, path string, opts kvconf.WatchOptions, out chan<- struct{}) error { | |
| 222 | + | if opts.Poll == 0 { | |
| 223 | + | close(opts.Ready) | |
| 224 | + | return errors.New("no space left on device") | |
| 225 | + | } | |
| 226 | + | return orig(ctx, path, opts, out) | |
| 227 | + | } | |
| 228 | + | }}, | |
| 229 | + | {"directory not readable", func(t *testing.T, dir string) { | |
| 230 | + | if os.Geteuid() == 0 { | |
| 231 | + | t.Skip("root ignores directory permissions") | |
| 232 | + | } | |
| 233 | + | kvWatch = orig | |
| 234 | + | if err := os.Chmod(dir, 0300); err != nil { | |
| 235 | + | t.Fatal(err) | |
| 236 | + | } | |
| 237 | + | t.Cleanup(func() { os.Chmod(dir, 0700) }) | |
| 238 | + | }}, | |
| 239 | + | } { | |
| 240 | + | t.Run(tc.name, func(t *testing.T) { | |
| 241 | + | dir := t.TempDir() | |
| 242 | + | path := filepath.Join(dir, "config") | |
| 243 | + | save(t, path, "lock.after=1m\n") | |
| 244 | + | tc.setup(t, dir) | |
| 245 | + | out := make(chan ports.ConfigChanged, 4) | |
| 246 | + | errs := make(chan error, 4) | |
| 247 | + | ready := make(chan struct{}) | |
| 248 | + | ctx, cancel := context.WithCancel(context.Background()) | |
| 249 | + | defer cancel() | |
| 250 | + | done := make(chan error, 1) | |
| 251 | + | go func() { done <- Watch(ctx, path, out, errs, ready) }() | |
| 252 | + | recv(t, ready) | |
| 253 | + | if err := recv(t, errs); err.Error() != "config watch failed; polling" { | |
| 254 | + | t.Fatal(err) | |
| 255 | + | } | |
| 256 | + | if got := recv(t, out); got.Config.LockAfter != time.Minute { | |
| 257 | + | t.Fatal(got) | |
| 258 | + | } | |
| 259 | + | save(t, path, "lock.after=2m\n") | |
| 260 | + | if got := recv(t, out); got.Config.LockAfter != 2*time.Minute { | |
| 261 | + | t.Fatal(got) | |
| 262 | + | } | |
| 263 | + | cancel() | |
| 264 | + | if err := recv(t, done); err != nil { | |
| 265 | + | t.Fatal(err) | |
| 266 | + | } | |
| 267 | + | }) | |
| 268 | + | } | |
| 269 | + | } | |
| @@ -2,6 +2,8 @@ package config | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "errors" | |||
| 6 | + | "github.com/bnema/kvconf" | |||
| 5 | "github.com/bnema/neferafk/internal/ports" | 7 | "github.com/bnema/neferafk/internal/ports" | ||
| 6 | "os" | 8 | "os" | ||
| 7 | "path/filepath" | 9 | "path/filepath" | ||
| @@ -197,3 +199,71 @@ func TestWatchFirstCandidateClosesLoadStartupGapAndMissingCancellation(t *testin | |||||
| 197 | t.Fatal(err) | 199 | t.Fatal(err) | ||
| 198 | } | 200 | } | ||
| 199 | } | 201 | } | ||
| 202 | + | ||||
| 203 | + | // A watch the OS refuses must fall back to polling, never stop the daemon. | |||
| 204 | + | func TestWatchFallsBackToPolling(t *testing.T) { | |||
| 205 | + | orig, origPoll := kvWatch, fallbackPoll | |||
| 206 | + | t.Cleanup(func() { kvWatch, fallbackPoll = orig, origPoll }) | |||
| 207 | + | fallbackPoll = 20 * time.Millisecond | |||
| 208 | + | for _, tc := range []struct { | |||
| 209 | + | name string | |||
| 210 | + | setup func(t *testing.T, dir string) | |||
| 211 | + | }{ | |||
| 212 | + | {"native watch fails at startup", func(t *testing.T, _ string) { | |||
| 213 | + | kvWatch = func(ctx context.Context, path string, opts kvconf.WatchOptions, out chan<- struct{}) error { | |||
| 214 | + | if opts.Poll == 0 { | |||
| 215 | + | return errors.New("no space left on device") | |||
| 216 | + | } | |||
| 217 | + | return orig(ctx, path, opts, out) | |||
| 218 | + | } | |||
| 219 | + | }}, | |||
| 220 | + | {"native watch fails after install", func(t *testing.T, _ string) { | |||
| 221 | + | kvWatch = func(ctx context.Context, path string, opts kvconf.WatchOptions, out chan<- struct{}) error { | |||
| 222 | + | if opts.Poll == 0 { | |||
| 223 | + | close(opts.Ready) | |||
| 224 | + | return errors.New("no space left on device") | |||
| 225 | + | } | |||
| 226 | + | return orig(ctx, path, opts, out) | |||
| 227 | + | } | |||
| 228 | + | }}, | |||
| 229 | + | {"directory not readable", func(t *testing.T, dir string) { | |||
| 230 | + | if os.Geteuid() == 0 { | |||
| 231 | + | t.Skip("root ignores directory permissions") | |||
| 232 | + | } | |||
| 233 | + | kvWatch = orig | |||
| 234 | + | if err := os.Chmod(dir, 0300); err != nil { | |||
| 235 | + | t.Fatal(err) | |||
| 236 | + | } | |||
| 237 | + | t.Cleanup(func() { os.Chmod(dir, 0700) }) | |||
| 238 | + | }}, | |||
| 239 | + | } { | |||
| 240 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 241 | + | dir := t.TempDir() | |||
| 242 | + | path := filepath.Join(dir, "config") | |||
| 243 | + | save(t, path, "lock.after=1m\n") | |||
| 244 | + | tc.setup(t, dir) | |||
| 245 | + | out := make(chan ports.ConfigChanged, 4) | |||
| 246 | + | errs := make(chan error, 4) | |||
| 247 | + | ready := make(chan struct{}) | |||
| 248 | + | ctx, cancel := context.WithCancel(context.Background()) | |||
| 249 | + | defer cancel() | |||
| 250 | + | done := make(chan error, 1) | |||
| 251 | + | go func() { done <- Watch(ctx, path, out, errs, ready) }() | |||
| 252 | + | recv(t, ready) | |||
| 253 | + | if err := recv(t, errs); err.Error() != "config watch failed; polling" { | |||
| 254 | + | t.Fatal(err) | |||
| 255 | + | } | |||
| 256 | + | if got := recv(t, out); got.Config.LockAfter != time.Minute { | |||
| 257 | + | t.Fatal(got) | |||
| 258 | + | } | |||
| 259 | + | save(t, path, "lock.after=2m\n") | |||
| 260 | + | if got := recv(t, out); got.Config.LockAfter != 2*time.Minute { | |||
| 261 | + | t.Fatal(got) | |||
| 262 | + | } | |||
| 263 | + | cancel() | |||
| 264 | + | if err := recv(t, done); err != nil { | |||
| 265 | + | t.Fatal(err) | |||
| 266 | + | } | |||
| 267 | + | }) | |||
| 268 | + | } | |||
| 269 | + | } | |||