diff --git a/TODO.md b/TODO.md index 5f47fa4..72043db 100644 --- a/TODO.md +++ b/TODO.md @@ -20,6 +20,10 @@ regress. # Completed Steps +- 2026-09-23: Deployment log files are now stored under `logs//` + instead of `logs///`, so downloads keep working after the + upaas container is recreated; logs written under an old hostname directory are + still found (#214). - 2026-09-23: Fixed the flaky `t.TempDir` cleanup race in `internal/handlers` (the one fixed in `internal/service/webhook` by #198): `TestHandleWebhookProcessesValidWebhook` now waits with the webhook service's diff --git a/internal/handlers/app.go b/internal/handlers/app.go index a985c11..22d527c 100644 --- a/internal/handlers/app.go +++ b/internal/handlers/app.go @@ -6,9 +6,11 @@ import ( "encoding/json" "errors" "fmt" + "io/fs" "net/http" "net/url" "os" + "path" "path/filepath" "strconv" "strings" @@ -640,7 +642,7 @@ func (h *Handlers) HandleDeploymentLogDownload() http.HandlerFunc { } defer func() { _ = root.Close() }() - file, openErr := root.Open(relPath) + file, openErr := openDeploymentLog(root, relPath) if openErr != nil { http.NotFound(writer, request) @@ -665,6 +667,24 @@ func (h *Handlers) HandleDeploymentLogDownload() http.HandlerFunc { } } +// openDeploymentLog opens a deployment log file inside the log root. +// Logs written by older versions sit one directory deeper, under the +// hostname of the container that wrote them, so when the file is not at +// relPath it is looked for under any directory directly below the root. +func openDeploymentLog(root *os.Root, relPath string) (*os.File, error) { + file, err := root.Open(relPath) + if err == nil { + return file, nil + } + + matches, globErr := fs.Glob(root.FS(), path.Join("*", filepath.ToSlash(relPath))) + if globErr != nil || len(matches) == 0 { + return nil, err + } + + return root.Open(matches[0]) +} + // containerLogsAPITail is the default number of log lines for the container logs API. const containerLogsAPITail = "100" diff --git a/internal/handlers/log_download_test.go b/internal/handlers/log_download_test.go index cbbfd08..595fbee 100644 --- a/internal/handlers/log_download_test.go +++ b/internal/handlers/log_download_test.go @@ -69,6 +69,56 @@ func TestHandleDeploymentLogDownloadServesLegitimateFile(t *testing.T) { assert.Contains(t, recorder.Body.String(), "deploy log contents") } +// TestGetLogFilePathHasNoHostname verifies a deployment log is stored +// directly under logs//, with no hostname directory in between, +// so it is still found after the container is recreated with a new +// hostname. +func TestGetLogFilePathHasNoHostname(t *testing.T) { + t.Parallel() + + testCtx := setupTestHandlers(t) + createdApp := createTestApp(t, testCtx, "log-path-app") + + deployment := models.NewDeployment(testCtx.database) + deployment.AppID = createdApp.ID + + logPath := testCtx.deploySvc.GetLogFilePath(createdApp, deployment) + + assert.Equal(t, + filepath.Join(testCtx.deploySvc.GetLogDir(), createdApp.Name), + filepath.Dir(logPath), + ) +} + +// TestHandleDeploymentLogDownloadServesLogFromOldHostnameDir verifies a +// log written by an older version, under the hostname of a container that +// has since been recreated, can still be downloaded. +func TestHandleDeploymentLogDownloadServesLogFromOldHostnameDir(t *testing.T) { + t.Parallel() + + testCtx := setupTestHandlers(t) + createdApp := createTestApp(t, testCtx, "log-old-hostname-app") + + deployment := models.NewDeployment(testCtx.database) + deployment.AppID = createdApp.ID + deployment.Status = models.DeploymentStatusSuccess + require.NoError(t, deployment.Save(context.Background())) + + logDir := testCtx.deploySvc.GetLogDir() + newPath := testCtx.deploySvc.GetLogFilePath(createdApp, deployment) + relPath, relErr := filepath.Rel(logDir, newPath) + require.NoError(t, relErr) + + oldPath := filepath.Join(logDir, "old-container-hostname", relPath) + require.NoError(t, os.MkdirAll(filepath.Dir(oldPath), 0o750)) + require.NoError(t, os.WriteFile(oldPath, []byte("old deploy log contents"), 0o600)) + + recorder := doLogDownload(t, testCtx, createdApp.ID, deployment.ID) + + assert.Equal(t, http.StatusOK, recorder.Code) + assert.Contains(t, recorder.Body.String(), "old deploy log contents") +} + // TestHandleDeploymentLogDownloadRejectsPathTraversal verifies the // os.Root containment guard. A traversal-shaped app name drives the // resolved log path out of the deploy log directory onto a sentinel diff --git a/internal/service/deploy/deploy.go b/internal/service/deploy/deploy.go index ab1f2c8..79343e2 100644 --- a/internal/service/deploy/deploy.go +++ b/internal/service/deploy/deploy.go @@ -261,15 +261,14 @@ func (svc *Service) GetBuildDir(appName string) string { // GetLogFilePath returns the path to the log file for a deployment. // Returns empty string if the path cannot be determined. +// +// The path must not depend on the container's hostname: Docker assigns a +// new one whenever the container is recreated, and older logs would then +// no longer be found. func (svc *Service) GetLogFilePath( app *models.App, deployment *models.Deployment, ) string { - hostname, err := os.Hostname() - if err != nil { - hostname = "unknown" - } - // Get commit SHA sha := "" if deployment.CommitSHA.Valid && deployment.CommitSHA.String != "" { @@ -291,7 +290,7 @@ func (svc *Service) GetLogFilePath( filename = fmt.Sprintf("%s_%s.log.txt", app.Name, timestamp) } - return filepath.Join(svc.config.DataDir, "logs", hostname, app.Name, filename) + return filepath.Join(svc.config.DataDir, "logs", app.Name, filename) } // GetLogDir returns the root directory under which all deployment log @@ -1294,7 +1293,7 @@ func (svc *Service) failDeployment( } // writeLogsToFile writes the deployment logs to a file on disk. -// Structure: DataDir/logs///__.log.txt +// Structure: DataDir/logs//__.log.txt func (svc *Service) writeLogsToFile(app *models.App, deployment *models.Deployment) { if !deployment.Logs.Valid || deployment.Logs.String == "" { return