From baa053d2500b95768c196e48163da1dcfed07d9a Mon Sep 17 00:00:00 2001 From: Mix Date: Sat, 10 Oct 2026 23:39:39 +0300 Subject: [PATCH] panel-redesign: three things the first look at the deployed panel showed Seen by the owner on the test server, on real data (2026-10-10). - Overview's Processes card said "7 of 7 running" with six programs running and postfix-reload idle. The card counts the programs that are meant to stay up; a one-shot helper (health.Process.OneShot) is left out of both numbers and named only when it is in trouble. - The Milter sockets card clipped its second line ("OpenDKIM, send-log,...") once the spam filter was the third socket. The line names the sockets that do not answer when some do not; with all answering it is their names, or "incl. the spam filter" - the mockup's words - when the filter is among them. - A real 2048-bit DKIM value did not fit the three rows of its field and scrolled inside it. The field is as tall as the value needs (3 to 12 rows). The fixtures had a shortened key, two sockets and no one-shot program, so none of this showed in the evidence screenshots. go test -count=1 ./internal/health/... ./internal/web/...: ok Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 5 +++ internal/health/processes.go | 13 ++++--- .../handlers/handlers_filter_socket_test.go | 2 +- internal/web/handlers/handlers_status.go | 36 +++++++++++++++---- internal/web/handlers/handlers_status_test.go | 27 +++++++++++++- internal/web/view/outbound.go | 17 ++++++++- internal/web/view/outbound_test.go | 11 ++++++ 7 files changed, 97 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ede10bd..fd04b32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -216,6 +216,11 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version showq service: Permission denied"), the Queue card said "Could not be read" and the queue page had no listing. Present since the persistent queue of 1.3.0; mail flow was not affected. The e2e suite opens the queue page now. +- Overview's Processes card counts the programs that are meant to stay up + ("6 of 6 running"); the idle one-shot `postfix-reload` was counted as + running. The Milter sockets card no longer clips its second line with three + sockets, and a DKIM value is shown whole instead of scrolling inside its + field. - The confirmation before deleting a user names the domains that follow that user's default report address and will be left without one; it used to say the domains were not affected (replaced before release — see the diff --git a/internal/health/processes.go b/internal/health/processes.go index 3bb4e74..ff68e84 100644 --- a/internal/health/processes.go +++ b/internal/health/processes.go @@ -23,6 +23,10 @@ type Process struct { State string // supervisord's own state word, e.g. RUNNING Detail string // the rest of the line: pid/uptime, or exit information Status Status + // OneShot marks a program that is started on demand and exits at once + // (oneShotPrograms): idle is its healthy state, and it is not one of the + // programs that are expected to be running. + OneShot bool } // Processes returns the state of every supervised program (architecture.md § @@ -71,10 +75,11 @@ func parseProcesses(out string) []Process { } name, state := fields[0], fields[1] procs = append(procs, Process{ - Name: name, - State: state, - Detail: strings.Join(fields[2:], " "), - Status: processStatus(name, state), + Name: name, + State: state, + Detail: strings.Join(fields[2:], " "), + Status: processStatus(name, state), + OneShot: oneShotPrograms[name], }) } return procs diff --git a/internal/web/handlers/handlers_filter_socket_test.go b/internal/web/handlers/handlers_filter_socket_test.go index b25cd1a..2c1ecd7 100644 --- a/internal/web/handlers/handlers_filter_socket_test.go +++ b/internal/web/handlers/handlers_filter_socket_test.go @@ -92,7 +92,7 @@ func TestFilterOnAnsweringIsTheThirdSocket(t *testing.T) { } } card := socketCard(t, h) - if card.Value != "3 of 3 answering" || card.Sub != "OpenDKIM, send-log, Spam filter" || card.Level != "" { + if card.Value != "3 of 3 answering" || card.Sub != "incl. the spam filter" || card.Level != "" { t.Errorf("card = %+v", card) } diff --git a/internal/web/handlers/handlers_status.go b/internal/web/handlers/handlers_status.go index 65a0b10..c5ea020 100644 --- a/internal/web/handlers/handlers_status.go +++ b/internal/web/handlers/handlers_status.go @@ -245,15 +245,22 @@ func healthCards(c map[string]any) []view.HealthCard { ram = "RAM " + machine.Memory.PctText() } - running, notRunning := 0, []string{} + // The count is of the programs that are meant to stay up. A one-shot helper + // (postfix-reload) idles between runs: it is not "running" and not missing, + // so it is left out of both numbers and named only when it is in trouble. + running, expected, notRunning := 0, 0, []string{} for _, p := range procs { - if p.Status == health.StatusOK { - running++ - } else { + if !p.OneShot { + expected++ + if p.Status == health.StatusOK { + running++ + } + } + if p.Status != health.StatusOK { notRunning = append(notRunning, p.Name) } } - procValue, procSub := fmt.Sprintf("%d of %d running", running, len(procs)), "all programs" + procValue, procSub := fmt.Sprintf("%d of %d running", running, expected), "all programs" if len(notRunning) > 0 { procSub = strings.Join(notRunning, ", ") } @@ -271,13 +278,28 @@ func healthCards(c map[string]any) []view.HealthCard { queueValue = "Could not be read" } - answering, socketNames := 0, make([]string, 0, len(sockets)) + // The line under the count has to fit a card: the sockets that do not + // answer when some do not; otherwise their names, or - with the inbound spam + // filter among them, when three names no longer fit - that it is included. + answering, socketNames, silent, withFilter := 0, make([]string, 0, len(sockets)), []string{}, false for _, s := range sockets { if s.Present { answering++ + } else { + silent = append(silent, s.Name) + } + if s.Note != "" { + withFilter = true } socketNames = append(socketNames, s.Name) } + socketSub := strings.Join(socketNames, ", ") + switch { + case len(silent) > 0: + socketSub = strings.Join(silent, ", ") + case withFilter: + socketSub = "incl. the spam filter" + } ptrValue := "Forward = reverse" if ptr.Status != health.StatusOK { @@ -293,7 +315,7 @@ func healthCards(c map[string]any) []view.HealthCard { card("TLS certificate", "ti-certificate", "/server/health#certificate", cert.Status, certValue, cert.Subject), card("Queue", "ti-stack-2", "/outbound/queue", c["QueueStatus"].(health.Status), queueValue, "outbound"), card("Milter sockets", "ti-plug-connected", "/server/health#sockets", c["SocketStatus"].(health.Status), - fmt.Sprintf("%d of %d answering", answering, len(sockets)), strings.Join(socketNames, ", ")), + fmt.Sprintf("%d of %d answering", answering, len(sockets)), socketSub), card("Reverse DNS", "ti-arrows-exchange", "/server/health#hostname", ptr.Status, ptrValue, c["Hostname"].(string)), } } diff --git a/internal/web/handlers/handlers_status_test.go b/internal/web/handlers/handlers_status_test.go index e5a2366..c0adf63 100644 --- a/internal/web/handlers/handlers_status_test.go +++ b/internal/web/handlers/handlers_status_test.go @@ -41,7 +41,7 @@ func TestHealthCards(t *testing.T) { {"Processes", "fail", "1 of 2 running", "postfix", "/server/health#processes"}, {"TLS certificate", "warn", "Expires in 12 days", "mail.example.org", "/server/health#certificate"}, {"Queue", "", "Empty", "outbound", "/outbound/queue"}, - {"Milter sockets", "warn", "1 of 2 answering", "OpenDKIM, send-log", "/server/health#sockets"}, + {"Milter sockets", "warn", "1 of 2 answering", "send-log", "/server/health#sockets"}, {"Reverse DNS", "", "Forward = reverse", "mail.example.org", "/server/health#hostname"}, } if len(cards) != len(want) { @@ -80,3 +80,28 @@ func TestQueueCardValue(t *testing.T) { } } } + +// The Processes card counts the programs that are meant to stay up. The one-shot +// reload helper idles between runs: it is neither running nor missing. +func TestProcessesCardLeavesOutTheIdleOneShot(t *testing.T) { + c := checksFixture() + c["Processes"] = []health.Process{ + {Name: "opendkim", State: "RUNNING", Status: health.StatusOK}, + {Name: "postfix", State: "RUNNING", Status: health.StatusOK}, + {Name: "postfix-reload", State: "STOPPED", Status: health.StatusOK, OneShot: true}, + } + c["ProcessStatus"] = health.StatusOK + card := healthCards(c)[1] + if card.Value != "2 of 2 running" || card.Sub != "all programs" { + t.Errorf("card = %q / %q, want 2 of 2 running / all programs", card.Value, card.Sub) + } + + c["Processes"] = []health.Process{ + {Name: "opendkim", State: "RUNNING", Status: health.StatusOK}, + {Name: "postfix-reload", State: "FATAL", Status: health.StatusError, OneShot: true}, + } + card = healthCards(c)[1] + if card.Value != "1 of 1 running" || card.Sub != "postfix-reload" { + t.Errorf("card = %q / %q, want 1 of 1 running / postfix-reload", card.Value, card.Sub) + } +} diff --git a/internal/web/view/outbound.go b/internal/web/view/outbound.go index f23a237..fe11279 100644 --- a/internal/web/view/outbound.go +++ b/internal/web/view/outbound.go @@ -145,11 +145,26 @@ func applyCheck(r *Record, c DNSCheck, standing Text, mismatch Text) { } } +// dkimRows is the height of the field that shows a DKIM value: enough lines for +// the whole key, so the field does not scroll inside itself. A 2048-bit key is +// about 410 characters; a line of the field holds some 80 of them at the +// narrowest width the page is drawn for. +func dkimRows(value string) int { + rows := (len(value)+79)/80 + 1 + if rows < 3 { + return 3 + } + if rows > 12 { + return 12 + } + return rows +} + // DKIMRecord is the DKIM record to publish: the key is not a secret, and the // selector says which one it is. func DKIMRecord(selector, host, value string, c DNSCheck) Record { r := Record{Name: "DKIM", Note: Rich("Not a secret · selector ", Code(selector)), - Host: host, Type: "TXT", Value: value, ValueRows: 3} + Host: host, Type: "TXT", Value: value, ValueRows: dkimRows(value)} applyCheck(&r, c, nil, nil) return r } diff --git a/internal/web/view/outbound_test.go b/internal/web/view/outbound_test.go index aa10ea5..b8d682f 100644 --- a/internal/web/view/outbound_test.go +++ b/internal/web/view/outbound_test.go @@ -508,3 +508,14 @@ func TestActionConfirmIsOnlyAnAttribute(t *testing.T) { t.Errorf("Confirm changes more than the attribute:\n%s\n%s", plain, asked) } } + +// The DKIM value is shown whole: the field grows with the key, so it does not +// scroll inside itself. +func TestDKIMFieldIsAsTallAsItsValue(t *testing.T) { + short := DKIMRecord("sp", "sp._domainkey.example.org", "v=DKIM1; p=AAA", DNSCheck{}) + long := DKIMRecord("sp", "sp._domainkey.example.org", "v=DKIM1; h=sha256; k=rsa; p="+strings.Repeat("A", 392), DNSCheck{}) + huge := DKIMRecord("sp", "sp._domainkey.example.org", strings.Repeat("A", 5000), DNSCheck{}) + if short.ValueRows != 3 || long.ValueRows != 7 || huge.ValueRows != 12 { + t.Errorf("rows = %d, %d, %d; want 3, 7, 12", short.ValueRows, long.ValueRows, huge.ValueRows) + } +}