From a836bc5f80e9223bcdb8b91aabfcc301fade9770 Mon Sep 17 00:00:00 2001 From: clawbot Date: Tue, 29 Sep 2026 12:02:23 +0200 Subject: [PATCH 1/5] Show the 10 most recent deployments on the deployments page (closes #238) An app's deployments page listed up to 50 deployments; it now lists the 10 most recent, newest first. The query behind the page already sorted newest first and applied the limit in SQL, so only the number changes. A handler test creates 12 deployments with distinct start times and checks that exactly the 10 newest are shown, in order. Model: opus-5-5 Co-authored-by: clawbot --- TODO.md | 3 ++ internal/handlers/app.go | 2 +- internal/handlers/handlers_test.go | 68 ++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 1 deletion(-) diff --git a/TODO.md b/TODO.md index ff43640..54cdfbd 100644 --- a/TODO.md +++ b/TODO.md @@ -20,6 +20,9 @@ regress. # Completed Steps +- 2026-09-29: An app's deployments page now lists only its 10 most recent + deployments, newest first, instead of 50 (#238). + - 2026-09-29: `docker-compose.yml` now sets `UPAAS_PORT` to 8080 as well as `PORT`, since upaas reads `UPAAS_PORT` first and a `UPAAS_PORT` in `.env` made it listen away from the port mapping and healthcheck; the README's Compose diff --git a/internal/handlers/app.go b/internal/handlers/app.go index 22d527c..e03faf8 100644 --- a/internal/handlers/app.go +++ b/internal/handlers/app.go @@ -28,7 +28,7 @@ const ( // recentDeploymentsLimit is the number of recent deployments to show. recentDeploymentsLimit = 5 // deploymentsHistoryLimit is the number of deployments to show in history. - deploymentsHistoryLimit = 50 + deploymentsHistoryLimit = 10 ) // redirectToApp issues a SeeOther redirect to the page for the given diff --git a/internal/handlers/handlers_test.go b/internal/handlers/handlers_test.go index 6aa2037..211d3c6 100644 --- a/internal/handlers/handlers_test.go +++ b/internal/handlers/handlers_test.go @@ -8,6 +8,7 @@ import ( "strconv" "strings" "testing" + "time" "github.com/go-chi/chi/v5" "github.com/stretchr/testify/assert" @@ -1148,6 +1149,73 @@ func TestHandleCancelDeployReturns404ForUnknownApp(t *testing.T) { assert.Equal(t, http.StatusNotFound, recorder.Code) } +// TestHandleAppDeploymentsShowsTenNewest verifies the deployments page +// lists only the 10 most recent deployments, newest first. +func TestHandleAppDeploymentsShowsTenNewest(t *testing.T) { + t.Parallel() + + testCtx := setupTestHandlers(t) + createdApp := createTestApp(t, testCtx, "deployments-page-app") + + // Create 12 deployments, each started one minute after the one before. + firstStart := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + + ids := make([]int64, 0, 12) + + for idx := range 12 { + deployment := models.NewDeployment(testCtx.database) + deployment.AppID = createdApp.ID + deployment.Status = models.DeploymentStatusSuccess + require.NoError(t, deployment.Save(context.Background())) + + _, err := testCtx.database.Exec( + context.Background(), + "UPDATE deployments SET started_at = ? WHERE id = ?", + firstStart.Add(time.Duration(idx)*time.Minute), + deployment.ID, + ) + require.NoError(t, err) + + ids = append(ids, deployment.ID) + } + + request := httptest.NewRequestWithContext( + t.Context(), + http.MethodGet, + "/apps/"+createdApp.ID+"/deployments", + nil, + ) + request = addChiURLParams(request, map[string]string{"id": createdApp.ID}) + recorder := httptest.NewRecorder() + + handler := testCtx.handlers.HandleAppDeployments() + handler.ServeHTTP(recorder, request) + + require.Equal(t, http.StatusOK, recorder.Code) + + body := recorder.Body.String() + card := func(id int64) string { + return `data-deployment-id="` + strconv.FormatInt(id, 10) + `"` + } + + assert.Equal(t, 10, strings.Count(body, `data-deployment-id="`)) + + // The two oldest are left out. + assert.NotContains(t, body, card(ids[0])) + assert.NotContains(t, body, card(ids[1])) + + // The ten newest are shown, newest first. + previous := -1 + + for idx := len(ids) - 1; idx >= 2; idx-- { + position := strings.Index(body, card(ids[idx])) + require.Greater(t, position, previous, + "deployment %d missing or out of order", ids[idx]) + + previous = position + } +} + func TestHandleWebhookReturns404ForUnknownSecret(t *testing.T) { t.Parallel() -- 2.54.0 From 211e2a4a5a4398d040a59a9d0bf94c68f5dcd547 Mon Sep 17 00:00:00 2001 From: clawbot Date: Tue, 29 Sep 2026 12:21:35 +0200 Subject: [PATCH 2/5] Show the deploy branch in the app page title (closes #240) The app page shows the app's configured branch as a neutral label next to the status badge, so it can be read without opening the edit page. The line under the title now shows only the repository. A new test renders the app page for an app on a non-main branch and checks the branch is in the title row. Model: opus-5-5 Co-authored-by: clawbot --- TODO.md | 3 ++ internal/handlers/app_branch_test.go | 46 ++++++++++++++++++++++++++++ templates/app_detail.html | 5 +-- 3 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 internal/handlers/app_branch_test.go diff --git a/TODO.md b/TODO.md index 54cdfbd..15b6e9e 100644 --- a/TODO.md +++ b/TODO.md @@ -20,6 +20,9 @@ regress. # Completed Steps +- 2026-09-29: The app page shows the app's branch as a label in its title, next + to the status badge, instead of after the repository under it (#240). + - 2026-09-29: An app's deployments page now lists only its 10 most recent deployments, newest first, instead of 50 (#238). diff --git a/internal/handlers/app_branch_test.go b/internal/handlers/app_branch_test.go new file mode 100644 index 0000000..c529006 --- /dev/null +++ b/internal/handlers/app_branch_test.go @@ -0,0 +1,46 @@ +package handlers_test + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "sneak.berlin/go/upaas/internal/service/app" +) + +// TestAppPageTitleShowsBranch checks that an app's branch can be read from +// the app page title, next to the status badge, without opening the edit page. +func TestAppPageTitleShowsBranch(t *testing.T) { + t.Parallel() + + testCtx := setupTestHandlers(t) + + createdApp, err := testCtx.appSvc.CreateApp(t.Context(), app.CreateAppInput{ + Name: "branch-shown-app", + RepoURL: "git@example.com:user/branch-shown-app.git", + Branch: "staging", + }) + require.NoError(t, err) + + request := httptest.NewRequestWithContext( + t.Context(), http.MethodGet, "/apps/"+createdApp.ID, nil, + ) + request = addChiURLParams(request, map[string]string{"id": createdApp.ID}) + recorder := httptest.NewRecorder() + + testCtx.handlers.HandleAppDetail().ServeHTTP(recorder, request) + + require.Equal(t, http.StatusOK, recorder.Code) + + // The title row runs from the app name heading to the end of its div. + _, afterHeading, found := strings.Cut(recorder.Body.String(), "") + assert.Contains(t, titleRow, `x-text="statusLabel"`) + assert.Contains(t, titleRow, ">staging") +} diff --git a/templates/app_detail.html b/templates/app_detail.html index ecb085e..02d65c5 100644 --- a/templates/app_detail.html +++ b/templates/app_detail.html @@ -26,11 +26,12 @@
-
+

{{.App.Name}}

+ {{.App.Branch}}
-

{{.App.RepoURL}}@{{.App.Branch}}

+

{{.App.RepoURL}}

Edit -- 2.54.0 From a48d90f5eabe0c86367baf0584ca717b69afd466 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 12:35:45 +0200 Subject: [PATCH 3/5] Stamp the git commit into images built from the Dockerfile (closes #236) .dockerignore left out .git, so `make build` in the image found no git metadata and stamped `dev`. It now sends .git, and no longer leaves out tracked files (LICENSE, README.md, ...), which git would see as deleted and mark the version -dirty. The footer and /health already read the same version. The startup log line that reports it, the logger's Identify(), was never called; main now calls it. The README says to build from a git clone. Side effect: image layers after `COPY . .` now rebuild whenever `.git` changes. Disclosure: merged after a rebase that changed only TODO.md; the second review gated this same tree on this base. Model: opus-5-5 --- .dockerignore | 9 +++------ Dockerfile | 1 + README.md | 4 ++++ TODO.md | 6 ++++++ cmd/upaasd/main.go | 4 +++- 5 files changed, 17 insertions(+), 7 deletions(-) diff --git a/.dockerignore b/.dockerignore index de5ff91..5b2701b 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,11 +1,8 @@ -.git +# .git is sent so that `make build` in the Dockerfile can stamp the commit into +# upaas. List no tracked file here: git would see it as deleted in the build and +# the version would end in -dirty. .env bin/ -.editorconfig .vscode/ .idea/ *.test -LICENSE -CONVENTIONS.md -REPO_POLICIES.md -README.md diff --git a/Dockerfile b/Dockerfile index 1a996e8..572ef99 100644 --- a/Dockerfile +++ b/Dockerfile @@ -31,6 +31,7 @@ RUN go mod download COPY . . RUN make test +# Takes the version from `git describe` on the .git copied in above. RUN make build # Runtime stage diff --git a/README.md b/README.md index c7bce0f..785aab2 100644 --- a/README.md +++ b/README.md @@ -226,6 +226,10 @@ This recipe serves plain HTTP, so `UPAAS_PLAINTEXT_HTTP=true` is required for setup and every other form to pass the CSRF origin check. Behind a TLS-terminating reverse proxy, drop that line. +The image shows the commit it was built from (the `git describe` output) in the +page footer, the startup log and `/health`. Build it from a git clone: without +the `.git` directory it shows `dev`. + ### Deploying with Docker Compose [`docker-compose.yml`](docker-compose.yml) builds the image from this repo and diff --git a/TODO.md b/TODO.md index 15b6e9e..2dd0af0 100644 --- a/TODO.md +++ b/TODO.md @@ -20,6 +20,12 @@ regress. # Completed Steps +- 2026-09-29: An image built from the `Dockerfile` now shows the commit it was + built from (the `git describe` output) in the footer and `/health` instead of + `dev`: `.dockerignore` no longer leaves out `.git`, nor any tracked file, + which git would count as deleted and mark `-dirty`. upaas now also logs its + version at startup; the logger's `Identify()` was never called (#236). + - 2026-09-29: The app page shows the app's branch as a label in its title, next to the status badge, instead of after the repository under it (#240). diff --git a/cmd/upaasd/main.go b/cmd/upaasd/main.go index 1d37fd3..29a4c29 100644 --- a/cmd/upaasd/main.go +++ b/cmd/upaasd/main.go @@ -52,6 +52,8 @@ func main() { handlers.New, server.New, ), - fx.Invoke(func(*server.Server) {}), + fx.Invoke(func(log *logger.Logger, _ *server.Server) { + log.Identify() + }), ).Run() } -- 2.54.0 From 679c80700fdf1cb88ab723d934210ded0aeaa342 Mon Sep 17 00:00:00 2001 From: clawbot Date: Tue, 29 Sep 2026 12:42:55 +0200 Subject: [PATCH 4/5] Report the build's own error and refuse daemons too old for BuildKit (closes #234) A build whose output ends in Docker's error line now fails with that error, instead of going on to inspect a tag that was never created. The build output is written to the deployment log before the failure is recorded, so the log ends in order. Before building, upaas compares the daemon's API version with 1.39 (Docker Engine 18.09), the first that builds with BuildKit without experimental mode, and fails the deploy on an older daemon instead of letting it use the legacy builder. The README's Compose section gives the update command and the Docker Engine versions builds need. Disclosure: merged after a rebase that changed only TODO.md; the review gated this tree on the current next. Model: opus-5-5 Co-authored-by: clawbot --- README.md | 9 +- TODO.md | 7 ++ internal/docker/client.go | 35 +++++++- internal/docker/validation_test.go | 81 +++++++++++++++++ internal/service/deploy/deploy.go | 5 +- internal/service/deploy/deploy_build_test.go | 91 ++++++++++++++++++++ internal/service/deploy/export_test.go | 9 ++ 7 files changed, 233 insertions(+), 4 deletions(-) create mode 100644 internal/service/deploy/deploy_build_test.go diff --git a/README.md b/README.md index 785aab2..3cc777e 100644 --- a/README.md +++ b/README.md @@ -245,7 +245,9 @@ Other settings from [Configuration](#configuration) go in the same file, except settings to 8080 and `UPAAS_DATA_DIR` to `/var/lib/upaas`, overriding `.env`, to match its port mapping, healthcheck and data directory mount. Then run `docker compose up -d` from the repo root; `docker compose ps` shows the -container as healthy once `/health` answers. +container as healthy once `/health` answers. To update, run `git pull` and then +`docker compose up -d --build`: without `--build`, Compose keeps running the +image built from the old checkout. **Important**: `HOST_DATA_DIR` **must** be an **absolute path** on the host. It is bind-mounted into the container and passed as `UPAAS_HOST_DATA_DIR` so that @@ -261,6 +263,11 @@ Docker's build cache rather than as untagged images. Docker Engine 28.2 and later keeps that cache under a size limit by default; on older engines, set `"builder": {"gc": {"enabled": true}}` in the host's `daemon.json`. +Building with BuildKit needs Docker Engine 18.09 or later; on an older engine, +upaas fails the deploy instead of building. A Dockerfile that uses +`RUN --network` needs Docker Engine 23.0 or later unless its `# syntax=` line +names Dockerfile frontend 1.3 or later, such as `docker/dockerfile:1`. + Session secrets are automatically generated on first startup and persisted to `$UPAAS_DATA_DIR/session.key`. diff --git a/TODO.md b/TODO.md index 2dd0af0..4b9ed0d 100644 --- a/TODO.md +++ b/TODO.md @@ -20,6 +20,13 @@ regress. # Completed Steps +- 2026-09-29: A failed build now fails the deploy with the build's own error + instead of a later "failed to inspect image", and the deployment log shows the + end of the build output before that error. upaas refuses to build on a Docker + Engine older than 18.09, which cannot build with BuildKit. The README's + Compose section says to update with `docker compose up -d --build` and names + the Docker Engine versions builds need (#234). + - 2026-09-29: An image built from the `Dockerfile` now shows the commit it was built from (the `git describe` output) in the footer and `/health` instead of `dev`: `.dockerignore` no longer leaves out `.git`, nor any tracked file, diff --git a/internal/docker/client.go b/internal/docker/client.go index 46d2d53..2af6f52 100644 --- a/internal/docker/client.go +++ b/internal/docker/client.go @@ -21,6 +21,7 @@ import ( "github.com/docker/docker/api/types/image" "github.com/docker/docker/api/types/mount" "github.com/docker/docker/api/types/network" + "github.com/docker/docker/api/types/versions" "github.com/docker/docker/client" "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/jsonmessage" @@ -61,6 +62,15 @@ var ErrInvalidBranch = errors.New("invalid branch name") // ErrInvalidCommitSHA is returned when a commit SHA is not a valid hex string. var ErrInvalidCommitSHA = errors.New("invalid commit SHA") +// ErrBuildKitUnavailable is returned when the Docker daemon is too old to +// build with BuildKit. +var ErrBuildKitUnavailable = errors.New("BuildKit is unavailable on the Docker daemon") + +// minBuildKitAPIVersion is the API version of Docker Engine 18.09, the first +// that builds with BuildKit when asked to without experimental mode. Older +// daemons refuse the request or silently use the legacy builder. +const minBuildKitAPIVersion = "1.39" + // validBranchRe matches safe git branch names. var validBranchRe = regexp.MustCompile(`^[a-zA-Z0-9._/\-]+$`) @@ -595,6 +605,20 @@ func (c *Client) performBuild( ctx context.Context, opts BuildImageOptions, ) (ImageID, error) { + server, err := c.docker.ServerVersion(ctx) + if err != nil { + return "", fmt.Errorf("failed to get Docker version: %w", err) + } + + if versions.LessThan(server.APIVersion, minBuildKitAPIVersion) { + return "", fmt.Errorf( + "%w: Docker Engine %s (API %s) is older than 18.09 (API %s); "+ + "upgrade Docker Engine", + ErrBuildKitUnavailable, server.Version, server.APIVersion, + minBuildKitAPIVersion, + ) + } + // Create tar archive of build context tarArchive, err := archive.TarWithOptions(opts.ContextDir, &archive.TarOptions{}) if err != nil { @@ -660,7 +684,8 @@ const scannerMaxBufferSize = 1024 * 1024 // 1MB // newline-delimited JSON. BuildKit's progress arrives encoded in // "moby.buildkit.trace" messages; these are decoded and written as plain // text, as "docker build --progress=plain" shows it. Other lines, such as -// build errors, are written unchanged. +// build errors, are written unchanged. Docker ends a failed build with a line +// carrying the error; it is returned once the output is written. func (c *Client) streamBuildOutput( ctx context.Context, body io.Reader, @@ -690,6 +715,8 @@ func (c *Client) streamBuildOutput( buf := make([]byte, 0, scannerInitialBufferSize) scanner.Buffer(buf, scannerMaxBufferSize) + var buildErr error + for scanner.Scan() { line := scanner.Bytes() @@ -708,6 +735,10 @@ func (c *Client) streamBuildOutput( continue } + if err == nil && msg.Error != nil { + buildErr = msg.Error + } + // One write per line, so it is not split by the display's output. _, _ = fmt.Fprintf(out, "%s\n", line) } @@ -720,7 +751,7 @@ func (c *Client) streamBuildOutput( return fmt.Errorf("failed to read build output: %w", scanErr) } - return nil + return buildErr } func (c *Client) performClone( diff --git a/internal/docker/validation_test.go b/internal/docker/validation_test.go index 7257344..f496655 100644 --- a/internal/docker/validation_test.go +++ b/internal/docker/validation_test.go @@ -271,6 +271,8 @@ func TestPerformBuildUsesBuildKit(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc( func(w http.ResponseWriter, r *http.Request) { switch { + case strings.HasSuffix(r.URL.Path, "/version"): + _, _ = w.Write([]byte(`{"Version":"27.3.1","ApiVersion":"1.47"}`)) case strings.HasSuffix(r.URL.Path, "/build"): if r.URL.Query().Get("version") != "2" { http.Error(w, "not a BuildKit build", http.StatusBadRequest) @@ -314,3 +316,82 @@ func TestPerformBuildUsesBuildKit(t *testing.T) { t.Errorf("build log is missing the build step:\n%s", buildLog.String()) } } + +// TestPerformBuildFails runs builds that fail against a fake Docker API and +// checks that each returns its own error and that no image is inspected +// afterwards. +func TestPerformBuildFails(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + engine string // Docker Engine version the fake daemon reports + apiVersion string // API version the fake daemon reports + buildOutput string + wantErr string + }{ + { + name: "build step fails", + engine: "27.3.1", + apiVersion: "1.47", + buildOutput: `{"stream":"Step 1/1 : RUN false\n"}` + "\n" + + `{"errorDetail":{"message":"exit code: 1"},"error":"exit code: 1"}`, + wantErr: "exit code: 1", + }, + { + name: "daemon too old for BuildKit", + engine: "18.06.3-ce", + apiVersion: "1.38", + wantErr: "BuildKit is unavailable on the Docker daemon: " + + "Docker Engine 18.06.3-ce (API 1.38) is older than 18.09 (API 1.39); " + + "upgrade Docker Engine", + }, + { + // The build step's own error shows the build went ahead. + name: "daemon at API 1.39 builds", + engine: "18.09.9", + apiVersion: "1.39", + buildOutput: `{"stream":"Step 1/1 : RUN false\n"}` + "\n" + + `{"errorDetail":{"message":"exit code: 1"},"error":"exit code: 1"}`, + wantErr: "exit code: 1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasSuffix(r.URL.Path, "/version"): + _, _ = fmt.Fprintf(w, `{"Version":%q,"ApiVersion":%q}`, + tt.engine, tt.apiVersion) + case strings.HasSuffix(r.URL.Path, "/build"): + _, _ = w.Write([]byte(tt.buildOutput)) + default: + t.Errorf("unexpected request to %s", r.URL.Path) + } + }, + )) + t.Cleanup(srv.Close) + + dockerAPI, err := client.NewClientWithOpts( + client.WithHost("tcp://" + srv.Listener.Addr().String()), + ) + if err != nil { + t.Fatal(err) + } + + c := &Client{docker: dockerAPI, log: slog.Default()} + + _, err = c.performBuild(t.Context(), BuildImageOptions{ + ContextDir: t.TempDir(), + Tags: []string{"upaas-test:1"}, + }) + if err == nil || err.Error() != tt.wantErr { + t.Errorf("got error %v, want %q", err, tt.wantErr) + } + }) + } +} diff --git a/internal/service/deploy/deploy.go b/internal/service/deploy/deploy.go index e2e1d30..4245be2 100644 --- a/internal/service/deploy/deploy.go +++ b/internal/service/deploy/deploy.go @@ -923,7 +923,6 @@ func (svc *Service) buildImage( // Create log writer that flushes build output to deployment logs every second logWriter := newDeploymentLogWriter(ctx, deployment) - defer logWriter.Close() // BuildImage creates a tar archive from the local filesystem, // so it needs the container path where files exist, not the host path. @@ -933,6 +932,10 @@ func (svc *Service) buildImage( Tags: []string{imageTag}, LogWriter: logWriter, }) + + // Write the rest of the build output to the log before the result. + logWriter.Close() + if err != nil { svc.notify.NotifyBuildFailed(ctx, app, deployment, err) svc.failDeployment( diff --git a/internal/service/deploy/deploy_build_test.go b/internal/service/deploy/deploy_build_test.go new file mode 100644 index 0000000..a2cf9f4 --- /dev/null +++ b/internal/service/deploy/deploy_build_test.go @@ -0,0 +1,91 @@ +package deploy_test + +import ( + "context" + "log/slog" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/fx/fxtest" + + "sneak.berlin/go/upaas/internal/config" + "sneak.berlin/go/upaas/internal/database" + "sneak.berlin/go/upaas/internal/docker" + "sneak.berlin/go/upaas/internal/logger" + "sneak.berlin/go/upaas/internal/models" + "sneak.berlin/go/upaas/internal/service/deploy" +) + +// TestBuildImageLogsBuildErrorBeforeDeployError runs a build that fails +// against a fake Docker API and checks that the deploy fails with the +// build's own error, which the deployment log shows before the deploy's. +func TestBuildImageLogsBuildErrorBeforeDeployError(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + + switch { + case strings.HasSuffix(r.URL.Path, "/containers/create"): + _, _ = w.Write([]byte(`{"Id":"gitcontainer"}`)) + case strings.HasSuffix(r.URL.Path, "/version"): + _, _ = w.Write([]byte(`{"Version":"27.3.1","ApiVersion":"1.47"}`)) + case strings.HasSuffix(r.URL.Path, "/build"): + _, _ = w.Write([]byte(`{"stream":"Step 1/1 : RUN false\n"}` + "\n" + + `{"errorDetail":{"message":"exit code: 1"},"error":"exit code: 1"}`)) + default: + // The steps of the git clone, which succeeds. + _, _ = w.Write([]byte(`{}`)) + } + }, + )) + t.Cleanup(srv.Close) + + log := slog.New(slog.NewTextHandler(os.Stderr, nil)) + lifecycle := fxtest.NewLifecycle(t) + + dockerClient, err := docker.New(lifecycle, docker.Params{ + Logger: logger.NewForTest(log), + Config: &config.Config{DockerHost: "tcp://" + srv.Listener.Addr().String()}, + }) + require.NoError(t, err) + + lifecycle.RequireStart() + t.Cleanup(lifecycle.RequireStop) + + db := database.NewTestDatabase(t) + ctx := context.Background() + + app := models.NewApp(db) + app.ID = "buildapp-id" + app.Name = "buildapp" + app.Branch = "main" + require.NoError(t, app.Save(ctx)) + + deployment := models.NewDeployment(db) + deployment.AppID = app.ID + require.NoError(t, deployment.Save(ctx)) + + dataDir := t.TempDir() + cfg := &config.Config{DataDir: dataDir, HostDataDir: dataDir} + + // The service has no notify service: the app has no ntfy topic and no + // Slack webhook, so the build failure notification sends nothing. + svc := deploy.NewTestServiceWithConfig(log, cfg, dockerClient) + + _, err = svc.BuildImage(ctx, app, deployment) + require.EqualError(t, err, "failed to build image: exit code: 1") + + logs := deployment.Logs.String + buildError := strings.Index(logs, "ERROR: exit code: 1") + deployError := strings.Index(logs, "ERROR: failed to build image: exit code: 1") + + require.NotEqual(t, -1, buildError, logs) + assert.Less(t, buildError, deployError, logs) +} diff --git a/internal/service/deploy/export_test.go b/internal/service/deploy/export_test.go index 5b68733..95ce82e 100644 --- a/internal/service/deploy/export_test.go +++ b/internal/service/deploy/export_test.go @@ -100,6 +100,15 @@ func (svc *Service) RecordDeployedImage( return svc.recordDeployedImage(ctx, app, deployment, imageID) } +// BuildImage exposes buildImage for testing. +func (svc *Service) BuildImage( + ctx context.Context, + app *models.App, + deployment *models.Deployment, +) (docker.ImageID, error) { + return svc.buildImage(ctx, app, deployment) +} + // BuildContainerOptionsExported exposes buildContainerOptions for testing. func (svc *Service) BuildContainerOptionsExported( ctx context.Context, -- 2.54.0 From 9754b73f27d75a4db2f3d3dd4c604c584e2e8641 Mon Sep 17 00:00:00 2001 From: clawbot Date: Tue, 29 Sep 2026 12:46:47 +0200 Subject: [PATCH 5/5] Widen the app page, double its log heights and move the logs (closes #246) The app page's content column is now at most 84rem wide instead of 56rem (max-w-4xl), set inline because the committed Tailwind CSS has no class for that width. The build log and container log boxes are 800px tall instead of 400px. The build log section moves to between the webhook URL and the environment variables, and the container log section moves to directly above the deploy key; nothing else moves. A new handler test renders the app page and checks the width, the section order and both log heights. Disclosure: merged after a rebase that changed only TODO.md; the review gated this change on the next before https://git.eeqj.de/sneak/upaas/issues/234, which touches no template. Model: opus-5-5 Co-authored-by: clawbot --- TODO.md | 5 ++ internal/handlers/app_page_layout_test.go | 76 +++++++++++++++++++++ templates/app_detail.html | 82 +++++++++++------------ 3 files changed, 122 insertions(+), 41 deletions(-) create mode 100644 internal/handlers/app_page_layout_test.go diff --git a/TODO.md b/TODO.md index 4b9ed0d..7684cdc 100644 --- a/TODO.md +++ b/TODO.md @@ -20,6 +20,11 @@ regress. # Completed Steps +- 2026-09-29: The app page is 50% wider on large screens (84rem instead of + 56rem), its build log and container log boxes are twice as tall, the build log + sits between the webhook URL and the environment variables, and the container + log sits above the deploy key (#246). + - 2026-09-29: A failed build now fails the deploy with the build's own error instead of a later "failed to inspect image", and the deployment log shows the end of the build output before that error. upaas refuses to build on a Docker diff --git a/internal/handlers/app_page_layout_test.go b/internal/handlers/app_page_layout_test.go new file mode 100644 index 0000000..002ff3c --- /dev/null +++ b/internal/handlers/app_page_layout_test.go @@ -0,0 +1,76 @@ +package handlers_test + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "sneak.berlin/go/upaas/internal/service/app" +) + +// TestAppPageLayout checks the app page's width, the order of its sections, +// and the height of its two log boxes. +func TestAppPageLayout(t *testing.T) { + t.Parallel() + + testCtx := setupTestHandlers(t) + + createdApp, err := testCtx.appSvc.CreateApp(t.Context(), app.CreateAppInput{ + Name: "layout-app", + RepoURL: "git@example.com:user/layout-app.git", + }) + require.NoError(t, err) + + request := httptest.NewRequestWithContext( + t.Context(), http.MethodGet, "/apps/"+createdApp.ID, nil, + ) + request = addChiURLParams(request, map[string]string{"id": createdApp.ID}) + recorder := httptest.NewRecorder() + + testCtx.handlers.HandleAppDetail().ServeHTTP(recorder, request) + + require.Equal(t, http.StatusOK, recorder.Code) + + body := recorder.Body.String() + + _, afterMain, found := strings.Cut(body, "") + assert.Contains(t, mainTag, "max-width: 84rem;") + + sectionTitles := []string{ + "Container Logs", + "Deploy Key", + "Webhook URL", + "Last Deployment Build Logs", + "Environment Variables", + "Docker Labels", + "Volume Mounts", + "Port Mappings", + "Recent Deployments", + "Danger Zone", + } + + previousIndex := -1 + + for _, title := range sectionTitles { + index := strings.Index(body, ">"+title+"") + require.NotEqual(t, -1, index, "app page has no %q section", title) + assert.Greater(t, index, previousIndex, "%q section is out of order", title) + + previousIndex = index + } + + for _, logBox := range []string{"containerLogsWrapper", "buildLogsWrapper"} { + _, afterRef, found := strings.Cut(body, `x-ref="`+logBox+`"`) + require.True(t, found, "app page has no %s", logBox) + + logBoxTag, _, _ := strings.Cut(afterRef, ">") + assert.Contains(t, logBoxTag, "max-height: 800px;", logBox) + } +} diff --git a/templates/app_detail.html b/templates/app_detail.html index 02d65c5..eb6f146 100644 --- a/templates/app_detail.html +++ b/templates/app_detail.html @@ -5,7 +5,7 @@ {{define "content"}} {{template "nav" .}} -
+
+

Container Logs

+ +
+
+
+ 

+            

+            
+        

+    

+
     
     

         
Deploy Key

@@ -101,6 +121,26 @@
         

     

 
+    
+    

+        

+            
Last Deployment Build Logs

+            
+        

+        

+            

+                

+            

+            
+        

+    

+
     
     

         
Environment Variables

@@ -354,26 +394,6 @@
         
     

 
-    
-    

-        

-            
Container Logs

-            
-        

-        

-            

-                

-            

-            
-        

-    

-
     
     

         

@@ -413,26 +433,6 @@
         
     

 
-    
-    

-        

-            
Last Deployment Build Logs

-            
-        

-        

-            

-                

-            

-            
-        

-    

-
     
     

         
Danger Zone

-- 
2.54.0