detached 5b299eb

refactor(config): check the owner name in the app with the domain rules

The copy of the repository name rules in config is gone: internal/app checks
ERMITE_OWNER with domain.ParseRepoName and joins the problem to the report of
the other variables.
commit
5b299eb72805cb1716d48a073d17a5522a40c6b0
author
brice · · 2026-10-12 19:17 UTC
parent
1beb9db
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
4 files · +81 -22
Unified
UnifiedSplit
M internal/app/serve.go +28 -1
@@ -4,14 +4,17 @@ package app
44
55 import (
66 "context"
7+ "errors"
78 "os"
89 "os/signal"
10+ "strings"
911 "syscall"
1012
1113 "github.com/bnema/zerowrap"
1214 "github.com/spf13/cobra"
1315
1416 "bnema.dev/ermite/internal/config"
17+ "bnema.dev/ermite/internal/domain"
1518 )
1619
1720 func newServeCmd() *cobra.Command {
@@ -32,7 +35,7 @@ func newServeCmd() *cobra.Command {
3235
3336 // serve wires the adapters and runs the server until SIGINT or SIGTERM.
3437 func serve(ctx context.Context) error {
35- cfg, err := config.Load(os.Getenv)
38+ cfg, err := loadConfig(os.Getenv)
3639 if err != nil {
3740 return err
3841 }
@@ -67,3 +70,27 @@ func serve(ctx context.Context) error {
6770 log.Info().Msg("stopped")
6871 return nil
6972 }
73+
74+// loadConfig reads the configuration and adds the checks that need the domain,
75+// which config does not import: ERMITE_OWNER follows the rules of repository
76+// names. Its problem joins the others in one report.
77+func loadConfig(get func(string) string) (config.Config, error) {
78+ cfg, err := config.Load(get)
79+ var problems []string
80+ var ce *config.Error
81+ switch {
82+ case errors.As(err, &ce):
83+ problems = ce.Problems
84+ case err != nil:
85+ return config.Config{}, err
86+ }
87+ if owner := strings.TrimSpace(get(config.EnvOwner)); owner != "" {
88+ if _, perr := domain.ParseRepoName(owner); perr != nil {
89+ problems = append(problems, config.EnvOwner+": must follow the rules of repository names ("+perr.Error()+")")
90+ }
91+ }
92+ if len(problems) > 0 {
93+ return config.Config{}, &config.Error{Problems: problems}
94+ }
95+ return cfg, nil
96+}
@@ -4,14 +4,17 @@ package app
4 4
5 import (5 import (
6 "context"6 "context"
7+ "errors"
7 "os"8 "os"
8 "os/signal"9 "os/signal"
10+ "strings"
9 "syscall"11 "syscall"
10 12
11 "github.com/bnema/zerowrap"13 "github.com/bnema/zerowrap"
12 "github.com/spf13/cobra"14 "github.com/spf13/cobra"
13 15
14 "bnema.dev/ermite/internal/config"16 "bnema.dev/ermite/internal/config"
17+ "bnema.dev/ermite/internal/domain"
15 )18 )
16 19
17 func newServeCmd() *cobra.Command {20 func newServeCmd() *cobra.Command {
@@ -32,7 +35,7 @@ func newServeCmd() *cobra.Command {
32 35
33 // serve wires the adapters and runs the server until SIGINT or SIGTERM.36 // serve wires the adapters and runs the server until SIGINT or SIGTERM.
34 func serve(ctx context.Context) error {37 func serve(ctx context.Context) error {
35- cfg, err := config.Load(os.Getenv)38+ cfg, err := loadConfig(os.Getenv)
36 if err != nil {39 if err != nil {
37 return err40 return err
38 }41 }
@@ -67,3 +70,27 @@ func serve(ctx context.Context) error {
67 log.Info().Msg("stopped")70 log.Info().Msg("stopped")
68 return nil71 return nil
69 }72 }
73+
74+// loadConfig reads the configuration and adds the checks that need the domain,
75+// which config does not import: ERMITE_OWNER follows the rules of repository
76+// names. Its problem joins the others in one report.
77+func loadConfig(get func(string) string) (config.Config, error) {
78+ cfg, err := config.Load(get)
79+ var problems []string
80+ var ce *config.Error
81+ switch {
82+ case errors.As(err, &ce):
83+ problems = ce.Problems
84+ case err != nil:
85+ return config.Config{}, err
86+ }
87+ if owner := strings.TrimSpace(get(config.EnvOwner)); owner != "" {
88+ if _, perr := domain.ParseRepoName(owner); perr != nil {
89+ problems = append(problems, config.EnvOwner+": must follow the rules of repository names ("+perr.Error()+")")
90+ }
91+ }
92+ if len(problems) > 0 {
93+ return config.Config{}, &config.Error{Problems: problems}
94+ }
95+ return cfg, nil
96+}
A internal/app/serve_test.go +51 -0
@@ -0,0 +1,51 @@
1+// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+package app
4+
5+import (
6+ "errors"
7+ "testing"
8+
9+ "github.com/stretchr/testify/assert"
10+ "github.com/stretchr/testify/require"
11+
12+ "bnema.dev/ermite/internal/config"
13+)
14+
15+func TestLoadConfigChecksTheOwnerName(t *testing.T) {
16+ t.Parallel()
17+ base := map[string]string{
18+ config.EnvDataDir: "/data", config.EnvPublicURL: "https://forge.example.org",
19+ config.EnvAdminToken: "0123456789abcdef0123456789abcdef",
20+ }
21+ load := func(extra map[string]string) (config.Config, error) {
22+ env := map[string]string{}
23+ for k, v := range base {
24+ env[k] = v
25+ }
26+ for k, v := range extra {
27+ env[k] = v
28+ }
29+ return loadConfig(func(k string) string { return env[k] })
30+ }
31+
32+ cfg, err := load(map[string]string{config.EnvOwner: " bnema "})
33+ require.NoError(t, err)
34+ assert.Equal(t, "bnema", cfg.Owner)
35+ cfg, err = load(nil)
36+ require.NoError(t, err)
37+ assert.Equal(t, config.DefaultOwner, cfg.Owner)
38+
39+ for _, owner := range []string{"Bad Name", ".dot", "a/b", "x.git"} {
40+ _, err := load(map[string]string{config.EnvOwner: owner})
41+ var ce *config.Error
42+ require.True(t, errors.As(err, &ce), owner)
43+ assert.Contains(t, ce.Problems[0], config.EnvOwner, owner)
44+ }
45+
46+ // The problem is reported with the others, in one error.
47+ _, err = load(map[string]string{config.EnvOwner: "Bad", config.EnvCILogRetention: "soon"})
48+ var ce *config.Error
49+ require.True(t, errors.As(err, &ce))
50+ assert.Len(t, ce.Problems, 2)
51+}
@@ -0,0 +1,51 @@
1+// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+package app
4+
5+import (
6+ "errors"
7+ "testing"
8+
9+ "github.com/stretchr/testify/assert"
10+ "github.com/stretchr/testify/require"
11+
12+ "bnema.dev/ermite/internal/config"
13+)
14+
15+func TestLoadConfigChecksTheOwnerName(t *testing.T) {
16+ t.Parallel()
17+ base := map[string]string{
18+ config.EnvDataDir: "/data", config.EnvPublicURL: "https://forge.example.org",
19+ config.EnvAdminToken: "0123456789abcdef0123456789abcdef",
20+ }
21+ load := func(extra map[string]string) (config.Config, error) {
22+ env := map[string]string{}
23+ for k, v := range base {
24+ env[k] = v
25+ }
26+ for k, v := range extra {
27+ env[k] = v
28+ }
29+ return loadConfig(func(k string) string { return env[k] })
30+ }
31+
32+ cfg, err := load(map[string]string{config.EnvOwner: " bnema "})
33+ require.NoError(t, err)
34+ assert.Equal(t, "bnema", cfg.Owner)
35+ cfg, err = load(nil)
36+ require.NoError(t, err)
37+ assert.Equal(t, config.DefaultOwner, cfg.Owner)
38+
39+ for _, owner := range []string{"Bad Name", ".dot", "a/b", "x.git"} {
40+ _, err := load(map[string]string{config.EnvOwner: owner})
41+ var ce *config.Error
42+ require.True(t, errors.As(err, &ce), owner)
43+ assert.Contains(t, ce.Problems[0], config.EnvOwner, owner)
44+ }
45+
46+ // The problem is reported with the others, in one error.
47+ _, err = load(map[string]string{config.EnvOwner: "Bad", config.EnvCILogRetention: "soon"})
48+ var ce *config.Error
49+ require.True(t, errors.As(err, &ce))
50+ assert.Len(t, ce.Problems, 2)
51+}
M internal/config/config.go +2 -20
@@ -232,11 +232,11 @@ func Load(get func(string) string) (Config, error) {
232232 c.CommitterEmail = "ermite@" + u.Hostname()
233233 }
234234
235+ // The name rules of the owner are those of repository names (the domain);
236+ // config imports nothing, so internal/app checks them (see loadConfig).
235237 c.Owner = strings.TrimSpace(get(EnvOwner))
236238 if c.Owner == "" {
237239 c.Owner = DefaultOwner
238- } else if err := validateOwner(c.Owner); err != nil {
239- bad(EnvOwner, "%v", err)
240240 }
241241
242242 c.CIRetention = DefaultCIRetention
@@ -255,24 +255,6 @@ func Load(get func(string) string) (Config, error) {
255255 return c, nil
256256 }
257257
258-// validateOwner applies the rules of repository names (lowercase letters,
259-// digits, '.', '_' and '-', at most 100 characters, no leading or trailing
260-// '.'), which config cannot import from the domain.
261-func validateOwner(s string) error {
262- if len(s) > 100 {
263- return fmt.Errorf("must be at most 100 characters")
264- }
265- for _, r := range s {
266- if (r < 'a' || r > 'z') && (r < '0' || r > '9') && r != '.' && r != '_' && r != '-' {
267- return fmt.Errorf("must follow the rules of repository names: a-z, 0-9, '.', '_', '-' (got %q)", s)
268- }
269- }
270- if strings.HasPrefix(s, ".") || strings.HasSuffix(s, ".") || strings.Contains(s, "..") {
271- return fmt.Errorf("must not start or end with '.' nor contain '..' (got %q)", s)
272- }
273- return nil
274-}
275-
276258 // parseRetention reads a duration in days ("90d") or in Go's syntax ("720h"),
277259 // at least one hour.
278260 func parseRetention(raw string) (time.Duration, error) {
@@ -232,11 +232,11 @@ func Load(get func(string) string) (Config, error) {
232 c.CommitterEmail = "ermite@" + u.Hostname()232 c.CommitterEmail = "ermite@" + u.Hostname()
233 }233 }
234 234
235+ // The name rules of the owner are those of repository names (the domain);
236+ // config imports nothing, so internal/app checks them (see loadConfig).
235 c.Owner = strings.TrimSpace(get(EnvOwner))237 c.Owner = strings.TrimSpace(get(EnvOwner))
236 if c.Owner == "" {238 if c.Owner == "" {
237 c.Owner = DefaultOwner239 c.Owner = DefaultOwner
238- } else if err := validateOwner(c.Owner); err != nil {
239- bad(EnvOwner, "%v", err)
240 }240 }
241 241
242 c.CIRetention = DefaultCIRetention242 c.CIRetention = DefaultCIRetention
@@ -255,24 +255,6 @@ func Load(get func(string) string) (Config, error) {
255 return c, nil255 return c, nil
256 }256 }
257 257
258-// validateOwner applies the rules of repository names (lowercase letters,
259-// digits, '.', '_' and '-', at most 100 characters, no leading or trailing
260-// '.'), which config cannot import from the domain.
261-func validateOwner(s string) error {
262- if len(s) > 100 {
263- return fmt.Errorf("must be at most 100 characters")
264- }
265- for _, r := range s {
266- if (r < 'a' || r > 'z') && (r < '0' || r > '9') && r != '.' && r != '_' && r != '-' {
267- return fmt.Errorf("must follow the rules of repository names: a-z, 0-9, '.', '_', '-' (got %q)", s)
268- }
269- }
270- if strings.HasPrefix(s, ".") || strings.HasSuffix(s, ".") || strings.Contains(s, "..") {
271- return fmt.Errorf("must not start or end with '.' nor contain '..' (got %q)", s)
272- }
273- return nil
274-}
275-
276 // parseRetention reads a duration in days ("90d") or in Go's syntax ("720h"),258 // parseRetention reads a duration in days ("90d") or in Go's syntax ("720h"),
277 // at least one hour.259 // at least one hour.
278 func parseRetention(raw string) (time.Duration, error) {260 func parseRetention(raw string) (time.Duration, error) {
M internal/config/config_test.go +0 -1
@@ -246,7 +246,6 @@ func TestOwnerAndCIRetention(t *testing.T) {
246246 }
247247
248248 for _, extra := range []map[string]string{
249- {config.EnvOwner: "Bad Name"}, {config.EnvOwner: ".dot"},
250249 {config.EnvCILogRetention: "soon"}, {config.EnvCILogRetention: "0d"}, {config.EnvCILogRetention: "10m"},
251250 } {
252251 if _, err := load(extra); err == nil {
@@ -246,7 +246,6 @@ func TestOwnerAndCIRetention(t *testing.T) {
246 }246 }
247 247
248 for _, extra := range []map[string]string{248 for _, extra := range []map[string]string{
249- {config.EnvOwner: "Bad Name"}, {config.EnvOwner: ".dot"},
250 {config.EnvCILogRetention: "soon"}, {config.EnvCILogRetention: "0d"}, {config.EnvCILogRetention: "10m"},249 {config.EnvCILogRetention: "soon"}, {config.EnvCILogRetention: "0d"}, {config.EnvCILogRetention: "10m"},
251 } {250 } {
252 if _, err := load(extra); err == nil {251 if _, err := load(extra); err == nil {

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 2

Tags 43

↑↓ moveenter openesc close