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) + } +}