From 0e8efe104317cb9ca243f3bc609f49b2f3bf5e3c Mon Sep 17 00:00:00 2001 From: user Date: Sat, 21 Feb 2026 02:24:51 -0800 Subject: [PATCH] fix: use imageID in createAndStartContainer (closes #124) Wire the imageID parameter (returned from docker build) through createAndStartContainer and buildContainerOptions instead of reconstructing a mutable tag via fmt.Sprintf. This ensures containers reference the immutable image digest, avoiding tag-reuse races when deploys overlap. Changes: - Rename _ string to imageID string in createAndStartContainer - Change buildContainerOptions to accept imageID string instead of deploymentID int64 - Use imageID directly as the Image field in container options - Update rollback path to pass previousImageID directly - Add test verifying imageID flows through to container options - Add database.NewTestDatabase and logger.NewForTest test helpers --- internal/database/testing.go | 41 +++++++++++++++++ internal/logger/testing.go | 11 +++++ internal/service/deploy/deploy.go | 12 +++-- .../service/deploy/deploy_container_test.go | 44 +++++++++++++++++++ internal/service/deploy/export_test.go | 10 +++++ 5 files changed, 111 insertions(+), 7 deletions(-) create mode 100644 internal/database/testing.go create mode 100644 internal/logger/testing.go create mode 100644 internal/service/deploy/deploy_container_test.go diff --git a/internal/database/testing.go b/internal/database/testing.go new file mode 100644 index 0000000..df2e737 --- /dev/null +++ b/internal/database/testing.go @@ -0,0 +1,41 @@ +package database + +import ( + "log/slog" + "os" + "testing" + + "git.eeqj.de/sneak/upaas/internal/config" + "git.eeqj.de/sneak/upaas/internal/logger" +) + +// NewTestDatabase creates an in-memory Database for testing. +// It runs migrations so all tables are available. +func NewTestDatabase(t *testing.T) *Database { + t.Helper() + + tmpDir := t.TempDir() + + cfg := &config.Config{ + DataDir: tmpDir, + } + + log := slog.New(slog.NewTextHandler(os.Stderr, nil)) + logWrapper := logger.NewForTest(log) + + db, err := New(nil, Params{ + Logger: logWrapper, + Config: cfg, + }) + if err != nil { + t.Fatalf("failed to create test database: %v", err) + } + + t.Cleanup(func() { + if db.database != nil { + _ = db.database.Close() + } + }) + + return db +} diff --git a/internal/logger/testing.go b/internal/logger/testing.go new file mode 100644 index 0000000..595733b --- /dev/null +++ b/internal/logger/testing.go @@ -0,0 +1,11 @@ +package logger + +import "log/slog" + +// NewForTest creates a Logger wrapping the given slog.Logger, for use in tests. +func NewForTest(log *slog.Logger) *Logger { + return &Logger{ + log: log, + level: new(slog.LevelVar), + } +} diff --git a/internal/service/deploy/deploy.go b/internal/service/deploy/deploy.go index 0959729..6764736 100644 --- a/internal/service/deploy/deploy.go +++ b/internal/service/deploy/deploy.go @@ -417,15 +417,13 @@ func (svc *Service) executeRollback( svc.removeOldContainer(ctx, app, deployment) - rollbackOpts, err := svc.buildContainerOptions(ctx, app, deployment.ID) + rollbackOpts, err := svc.buildContainerOptions(ctx, app, previousImageID) if err != nil { svc.failDeployment(bgCtx, app, deployment, err) return fmt.Errorf("failed to build container options: %w", err) } - rollbackOpts.Image = previousImageID - containerID, err := svc.docker.CreateContainer(ctx, rollbackOpts) if err != nil { svc.failDeployment(bgCtx, app, deployment, fmt.Errorf("failed to create rollback container: %w", err)) @@ -1018,9 +1016,9 @@ func (svc *Service) createAndStartContainer( ctx context.Context, app *models.App, deployment *models.Deployment, - _ string, + imageID string, ) (string, error) { - containerOpts, err := svc.buildContainerOptions(ctx, app, deployment.ID) + containerOpts, err := svc.buildContainerOptions(ctx, app, imageID) if err != nil { svc.failDeployment(ctx, app, deployment, err) @@ -1064,7 +1062,7 @@ func (svc *Service) createAndStartContainer( func (svc *Service) buildContainerOptions( ctx context.Context, app *models.App, - deploymentID int64, + imageID string, ) (docker.CreateContainerOptions, error) { envVars, err := app.GetEnvVars(ctx) if err != nil { @@ -1098,7 +1096,7 @@ func (svc *Service) buildContainerOptions( return docker.CreateContainerOptions{ Name: "upaas-" + app.Name, - Image: fmt.Sprintf("upaas-%s:%d", app.Name, deploymentID), + Image: imageID, Env: envMap, Labels: buildLabelMap(app, labels), Volumes: buildVolumeMounts(volumes), diff --git a/internal/service/deploy/deploy_container_test.go b/internal/service/deploy/deploy_container_test.go new file mode 100644 index 0000000..29ba3ef --- /dev/null +++ b/internal/service/deploy/deploy_container_test.go @@ -0,0 +1,44 @@ +package deploy_test + +import ( + "context" + "log/slog" + "os" + "testing" + + "git.eeqj.de/sneak/upaas/internal/database" + "git.eeqj.de/sneak/upaas/internal/models" + "git.eeqj.de/sneak/upaas/internal/service/deploy" +) + +func TestBuildContainerOptionsUsesImageID(t *testing.T) { + t.Parallel() + + db := database.NewTestDatabase(t) + + app := models.NewApp(db) + app.Name = "myapp" + + err := app.Save(context.Background()) + if err != nil { + t.Fatalf("failed to save app: %v", err) + } + + log := slog.New(slog.NewTextHandler(os.Stderr, nil)) + svc := deploy.NewTestService(log) + + const expectedImageID = "sha256:abc123def456" + + opts, err := svc.BuildContainerOptionsExported(context.Background(), app, expectedImageID) + if err != nil { + t.Fatalf("buildContainerOptions returned error: %v", err) + } + + if opts.Image != expectedImageID { + t.Errorf("expected Image=%q, got %q", expectedImageID, opts.Image) + } + + if opts.Name != "upaas-myapp" { + t.Errorf("expected Name=%q, got %q", "upaas-myapp", opts.Name) + } +} diff --git a/internal/service/deploy/export_test.go b/internal/service/deploy/export_test.go index bd90daa..8d48715 100644 --- a/internal/service/deploy/export_test.go +++ b/internal/service/deploy/export_test.go @@ -10,6 +10,7 @@ import ( "git.eeqj.de/sneak/upaas/internal/config" "git.eeqj.de/sneak/upaas/internal/docker" + "git.eeqj.de/sneak/upaas/internal/models" ) // NewTestService creates a Service with minimal dependencies for testing. @@ -80,3 +81,12 @@ func (svc *Service) CleanupCancelledDeploy( func (svc *Service) GetBuildDirExported(appName string) string { return svc.GetBuildDir(appName) } + +// BuildContainerOptionsExported exposes buildContainerOptions for testing. +func (svc *Service) BuildContainerOptionsExported( + ctx context.Context, + app *models.App, + imageID string, +) (docker.CreateContainerOptions, error) { + return svc.buildContainerOptions(ctx, app, imageID) +} -- 2.45.2