Skip to content

Commit 2139b4f

Browse files
michaelmcneesMichael McNeesclaude
authored
fix: chores and reliability fixes (#123-#125, #128, #132, #137-#139) (#141)
* fix: chores and reliability fixes (#123-#125, #128, #132, #137-#139) - #123: rename release-please.yml → changeset-version.yml; update RELEASING.md - #124: pin alpine base image to digest in Dockerfile.goreleaser - #125: add go-licenses NOTICE generation to release pipeline and goreleaser archives - #128: audit events for secret.Store Create and Delete (Actor field, transactional EmitTx) - #132: retry smtp.SendMail up to 3× in TestEmailReceive_FetchesUnseenMessages - #137: numeric UID (10001) in Dockerfile and Helm podSecurityContext so runAsNonRoot resolves - #138: remove worker/reaper liveness checkers from /readyz; probe checks DB only - #139: revert unauthenticated /metrics; add comment explaining bearer-token Prometheus path; annotate serviceMonitor.bearerTokenSecret in values.yaml Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address CodeRabbit review feedback on PR #141 - Pin alpine:3.21 to correct digest (1f390db) in Dockerfile.goreleaser - Pin go-licenses to @v2.0.1 instead of @latest in release-engine.yml - Update stale comment in server.go: /readyz no longer uses worker/reaper liveness checkers; explain the intentional design decision Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Michael McNees <mmcnees@Michaels-MacBook-Pro.local> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 1bd8cbf commit 2139b4f

10 files changed

Lines changed: 92 additions & 55 deletions

File tree

.github/workflows/release-engine.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ jobs:
4444
# what triggered this workflow; goreleaser needs the plain vX.Y.Z form.
4545
git tag "${TAG}"
4646
47+
- name: Generate NOTICE
48+
run: |
49+
go install github.com/google/go-licenses@v2.0.1
50+
go-licenses report ./... > NOTICE
51+
working-directory: packages/engine
52+
4753
- name: Run goreleaser
4854
uses: goreleaser/goreleaser-action@v6
4955
with:

RELEASING.md

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ The automated CI path (Version PR → merge) requires no local tooling beyond gi
1818
```
1919
1. changeset file added during dev
2020
21-
2. push to main → release-please.yml creates/updates "Version PR"
21+
2. push to main → changeset-version.yml creates/updates "Version PR"
2222
2323
3. merge Version PR
2424
@@ -28,8 +28,6 @@ The automated CI path (Version PR → merge) requires no local tooling beyond gi
2828
5b. release-helm.yml → helm push OCI + GitHub Release
2929
```
3030

31-
> **Note on workflow naming:** `release-please.yml` is named after a tool we evaluated but didn't adopt — it actually runs the Changesets CLI. A rename to `changeset-version.yml` is tracked in [dvflw/mantle#123](https://github.com/dvflw/mantle/issues/123).
32-
3331
## Step 1: Add a Changeset (During Development)
3432

3533
Every PR that changes user-visible behavior needs a changeset. Run this from the repo root:
@@ -56,7 +54,7 @@ If a PR touches only CI, tests, or tooling with no user impact, skip the changes
5654

5755
## Step 2: The Version PR
5856

59-
After any push to `main` that contains pending changesets, the `release-please.yml` workflow automatically creates (or updates) a PR titled **"ci: version packages"**.
57+
After any push to `main` that contains pending changesets, the `changeset-version.yml` workflow automatically creates (or updates) a PR titled **"ci: version packages"**.
6058

6159
This PR:
6260
- Bumps `version` in each affected `package.json` (taking the highest bump across all pending changesets)
@@ -65,7 +63,7 @@ This PR:
6563

6664
Review the PR to confirm the version bumps and changelog entries are correct. If more changesets land on `main` before you merge, CI updates the PR automatically.
6765

68-
> **Loop guard:** the workflow has an `if` condition that skips runs where the triggering commit message starts with `"ci: version packages"`. This prevents the workflow from retriggering on its own Version PR merge commit. If you ever rename the commit message in the workflow, update the guard condition in `release-please.yml` to match.
66+
> **Loop guard:** the workflow has an `if` condition that skips runs where the triggering commit message starts with `"ci: version packages"`. This prevents the workflow from retriggering on its own Version PR merge commit. If you ever rename the commit message in the workflow, update the guard condition in `changeset-version.yml` to match.
6967
7068
## Step 3: Merge the Version PR
7169

@@ -107,7 +105,7 @@ The tag-triggered workflows are safe to retrigger — goreleaser will fail clean
107105

108106
This applies when `package.json` is already at the target version but `.changeset/` contains no pending changeset files — for example, the first-ever release of a newly bootstrapped repo, or after manually editing `package.json` outside the changeset flow.
109107

110-
In this state the `release-please.yml` workflow has nothing to process and will not create a Version PR. Push the tags directly:
108+
In this state the `changeset-version.yml` workflow has nothing to process and will not create a Version PR. Push the tags directly:
111109

112110
```bash
113111
git tag engine/v<version>

packages/engine/.goreleaser.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ archives:
3030
# (packages/engine); LICENSE lives at the repo root.
3131
- src: "../../LICENSE"
3232
dst: "LICENSE"
33+
# Third-party license notices generated by go-licenses during release CI.
34+
- src: "NOTICE"
35+
dst: "NOTICE"
3336

3437
checksum:
3538
name_template: checksums.txt

packages/engine/Dockerfile.goreleaser

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
FROM alpine:3.21
1+
FROM alpine:3.21@sha256:1f390db9d9746cc317e16a98a8be2854c599abcbf04d45b6320b040e72034e33
22

33
RUN apk add --no-cache ca-certificates tzdata \
44
&& addgroup -S mantle \
5-
&& adduser -S -G mantle -h /home/mantle -s /bin/sh mantle
5+
&& adduser -S -u 10001 -G mantle -h /home/mantle -s /bin/sh mantle
66

77
ARG TARGETPLATFORM
88
COPY $TARGETPLATFORM/mantle /usr/local/bin/mantle
@@ -15,7 +15,7 @@ EXPOSE 8080
1515
HEALTHCHECK --interval=10s --timeout=3s --start-period=5s --retries=3 \
1616
CMD wget -qO- http://localhost:8080/healthz || exit 1
1717

18-
USER mantle
18+
USER 10001:10001
1919
WORKDIR /home/mantle
2020

2121
ENTRYPOINT ["mantle"]

packages/engine/internal/auth/middleware.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,9 @@ func AuthMiddleware(store *Store, oidcValidator TokenValidator, next http.Handle
7676
}
7777

7878
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
79-
// Skip auth for health checks only. /metrics requires authentication.
79+
// Skip auth for health checks only. /metrics is intentionally protected —
80+
// path labels expose workflow names. Configure Prometheus with a bearer
81+
// token via serviceMonitor.bearerTokenSecret in the Helm values.
8082
if r.URL.Path == "/healthz" || r.URL.Path == "/readyz" {
8183
next.ServeHTTP(w, r)
8284
return

packages/engine/internal/connector/email_receive_test.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,17 @@ func TestEmailReceive_FetchesUnseenMessages(t *testing.T) {
137137
"\r\n" +
138138
"This is the body of the test email.\r\n")
139139

140-
if err := smtp.SendMail(smtpAddr, nil, "sender@example.com", []string{username}, msg); err != nil {
141-
t.Fatalf("failed to send test email: %v", err)
140+
var smtpErr error
141+
for attempt := 1; attempt <= 3; attempt++ {
142+
smtpErr = smtp.SendMail(smtpAddr, nil, "sender@example.com", []string{username}, msg)
143+
if smtpErr == nil {
144+
break
145+
}
146+
t.Logf("smtp attempt %d: %v (retrying in 500ms)", attempt, smtpErr)
147+
time.Sleep(500 * time.Millisecond)
148+
}
149+
if smtpErr != nil {
150+
t.Fatalf("failed to send test email after retries: %v", smtpErr)
142151
}
143152

144153
// Fetch via the connector — retry to allow GreenMail time to propagate

packages/engine/internal/secret/store.go

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,27 @@ import (
44
"context"
55
"database/sql"
66
"encoding/json"
7+
"errors"
78
"fmt"
89
"time"
910

11+
"github.com/dvflw/mantle/internal/audit"
1012
"github.com/dvflw/mantle/internal/auth"
1113
)
1214

1315
// Store handles CRUD operations for encrypted credentials in Postgres.
16+
// Every state-changing method emits an audit event in the same transaction.
1417
type Store struct {
1518
DB *sql.DB
1619
Encryptor *Encryptor
20+
Actor string // defaults to "cli" when empty
21+
}
22+
23+
func (s *Store) actor() string {
24+
if s.Actor == "" {
25+
return "cli"
26+
}
27+
return s.Actor
1728
}
1829

1930
// Create validates, encrypts, and stores a new credential.
@@ -62,6 +73,17 @@ func (s *Store) Create(ctx context.Context, name, typeName string, data map[stri
6273
return nil, fmt.Errorf("storing credential: %w", err)
6374
}
6475

76+
if err := audit.EmitTx(ctx, tx, audit.Event{
77+
Timestamp: time.Now(),
78+
Actor: s.actor(),
79+
Action: audit.ActionCredentialCreated,
80+
Resource: audit.Resource{Type: "credential", ID: cred.ID},
81+
TeamID: teamID,
82+
Metadata: map[string]string{"name": name, "type": typeName},
83+
}); err != nil {
84+
return nil, fmt.Errorf("emitting audit event: %w", err)
85+
}
86+
6587
if err := tx.Commit(); err != nil {
6688
return nil, fmt.Errorf("committing credential: %w", err)
6789
}
@@ -119,21 +141,42 @@ func (s *Store) List(ctx context.Context) ([]Credential, error) {
119141
return creds, rows.Err()
120142
}
121143

122-
// Delete removes a credential by name.
144+
// Delete removes a credential by name and emits a credential.deleted audit
145+
// event in the same transaction. Returns an error when no row matches.
123146
func (s *Store) Delete(ctx context.Context, name string) error {
124147
teamID := auth.TeamIDFromContext(ctx)
125-
result, err := s.DB.ExecContext(ctx,
126-
`DELETE FROM credentials WHERE name = $1 AND team_id = $2`, name, teamID,
127-
)
148+
149+
tx, err := s.DB.BeginTx(ctx, nil)
128150
if err != nil {
129-
return fmt.Errorf("deleting credential: %w", err)
151+
return fmt.Errorf("starting transaction: %w", err)
152+
}
153+
defer tx.Rollback()
154+
155+
var deletedID string
156+
err = tx.QueryRowContext(ctx,
157+
`DELETE FROM credentials WHERE name = $1 AND team_id = $2 RETURNING id`,
158+
name, teamID,
159+
).Scan(&deletedID)
160+
if errors.Is(err, sql.ErrNoRows) {
161+
return fmt.Errorf("credential %q not found", name)
130162
}
131-
rows, err := result.RowsAffected()
132163
if err != nil {
133-
return err
164+
return fmt.Errorf("deleting credential: %w", err)
134165
}
135-
if rows == 0 {
136-
return fmt.Errorf("credential %q not found", name)
166+
167+
if err := audit.EmitTx(ctx, tx, audit.Event{
168+
Timestamp: time.Now(),
169+
Actor: s.actor(),
170+
Action: audit.ActionCredentialDeleted,
171+
Resource: audit.Resource{Type: "credential", ID: deletedID},
172+
TeamID: teamID,
173+
Metadata: map[string]string{"name": name},
174+
}); err != nil {
175+
return fmt.Errorf("emitting audit event: %w", err)
176+
}
177+
178+
if err := tx.Commit(); err != nil {
179+
return fmt.Errorf("committing credential delete: %w", err)
137180
}
138181
return nil
139182
}

packages/engine/internal/server/server.go

Lines changed: 3 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -56,24 +56,6 @@ type Server struct {
5656
running map[string]context.CancelFunc // execution ID → cancel
5757
}
5858

59-
// workerLivenessChecker adapts the Worker to the health.LivenessChecker interface.
60-
type workerLivenessChecker struct {
61-
w *engine.Worker
62-
threshold time.Duration
63-
}
64-
65-
func (c *workerLivenessChecker) IsAlive() bool { return c.w.IsAlive(c.threshold) }
66-
func (c *workerLivenessChecker) Name() string { return "worker" }
67-
68-
// reaperLivenessChecker adapts the Reaper to the health.LivenessChecker interface.
69-
type reaperLivenessChecker struct {
70-
r *engine.Reaper
71-
threshold time.Duration
72-
}
73-
74-
func (c *reaperLivenessChecker) IsAlive() bool { return c.r.IsAlive(c.threshold) }
75-
func (c *reaperLivenessChecker) Name() string { return "reaper" }
76-
7759
// New creates a Server with the given configuration.
7860
func New(db *sql.DB, eng *engine.Engine, address string) *Server {
7961
logger := slog.Default()
@@ -128,9 +110,6 @@ func metricsMiddleware(next http.Handler) http.Handler {
128110
func (s *Server) Start(ctx context.Context) error {
129111
mux := http.NewServeMux()
130112

131-
// Collect liveness checkers for readyz.
132-
var livenessCheckers []health.LivenessChecker
133-
134113
// Prometheus metrics endpoint.
135114
mux.Handle("/metrics", promhttp.Handler())
136115

@@ -193,17 +172,6 @@ func (s *Server) Start(ctx context.Context) error {
193172
go s.reaper.Run(ctx)
194173
s.Logger.Info("reaper started", "interval", cfg.Engine.ReaperInterval)
195174

196-
// Register liveness checkers for readyz.
197-
workerThreshold := 3 * cfg.Engine.WorkerPollInterval
198-
if workerThreshold < time.Second {
199-
workerThreshold = 3 * time.Second
200-
}
201-
reaperThreshold := 3 * cfg.Engine.ReaperInterval
202-
livenessCheckers = append(livenessCheckers,
203-
&workerLivenessChecker{w: s.worker, threshold: workerThreshold},
204-
&reaperLivenessChecker{r: s.reaper, threshold: reaperThreshold},
205-
)
206-
207175
// Start retention cleanup if configured.
208176
if cfg.Retention.ExecutionDays > 0 || cfg.Retention.AuditDays > 0 {
209177
go func() {
@@ -270,9 +238,10 @@ func (s *Server) Start(ctx context.Context) error {
270238
go s.pollQueueDepth(ctx, cfg.Engine.ReaperInterval)
271239
}
272240

273-
// Health endpoints (registered after engine components so checkers are populated).
241+
// Health endpoints. /readyz checks DB connectivity only; worker/reaper
242+
// liveness is no longer gated here to avoid flapping between poll cycles.
274243
mux.Handle("/healthz", health.HealthzHandler())
275-
mux.Handle("/readyz", health.ReadyzHandler(s.DB, livenessCheckers...))
244+
mux.Handle("/readyz", health.ReadyzHandler(s.DB))
276245

277246
// Wrap with metrics middleware (innermost, runs for every request).
278247
handler := metricsMiddleware(mux)

packages/helm-chart/values.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ resources:
3737

3838
podSecurityContext:
3939
runAsNonRoot: true
40+
runAsUser: 10001
41+
runAsGroup: 10001
4042

4143
securityContext:
4244
readOnlyRootFilesystem: true
@@ -85,4 +87,9 @@ serviceMonitor:
8587
enabled: false
8688
interval: 30s
8789
labels: {}
90+
# bearerTokenSecret: populate with a Kubernetes Secret that holds a Mantle
91+
# API key so Prometheus can authenticate against /metrics. Example:
92+
# bearerTokenSecret:
93+
# name: mantle-prometheus-token
94+
# key: token
8895
bearerTokenSecret: {}

0 commit comments

Comments
 (0)