rework: address review feedback on PR #126

Changes per sneak's review: - Delete docker-compose.yml, add example stanza to README - Define custom domain types: ImageID, ContainerID, UnparsedURL - Use custom types in all function signatures throughout codebase - Restore imageID parameter (as domain.ImageID) in deploy pipeline - buildContainerOptions now takes ImageID directly instead of constructing image tag from deploymentID - Fix pre-existing JS formatting (prettier) make check passes with zero failures.
2026-02-22 03:40:57 -08:00
parent 92fbf686bd
commit c9fe4f4bf1
8 changed files with 3702 additions and 596 deletions
--- a/internal/service/deploy/deploy.go
+++ b/internal/service/deploy/deploy.go
@@ -20,6 +20,7 @@ import (
 	"git.eeqj.de/sneak/upaas/internal/config"
 	"git.eeqj.de/sneak/upaas/internal/database"
 	"git.eeqj.de/sneak/upaas/internal/docker"
+	"git.eeqj.de/sneak/upaas/internal/domain"
 	"git.eeqj.de/sneak/upaas/internal/logger"
 	"git.eeqj.de/sneak/upaas/internal/models"
 	"git.eeqj.de/sneak/upaas/internal/service/notify"
@@ -417,15 +418,13 @@ func (svc *Service) executeRollback(

 	svc.removeOldContainer(ctx, app, deployment)

-	rollbackOpts, err := svc.buildContainerOptions(ctx, app, deployment.ID)
+	rollbackOpts, err := svc.buildContainerOptions(ctx, app, domain.ImageID(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))
@@ -433,8 +432,8 @@ func (svc *Service) executeRollback(
 		return fmt.Errorf("failed to create rollback container: %w", err)
 	}

-	deployment.ContainerID = sql.NullString{String: containerID, Valid: true}
-	_ = deployment.AppendLog(bgCtx, "Rollback container created: "+containerID)
+	deployment.ContainerID = sql.NullString{String: string(containerID), Valid: true}
+	_ = deployment.AppendLog(bgCtx, "Rollback container created: "+string(containerID))

 	startErr := svc.docker.StartContainer(ctx, containerID)
 	if startErr != nil {
@@ -484,7 +483,7 @@ func (svc *Service) runBuildAndDeploy(
 	svc.notify.NotifyBuildSuccess(bgCtx, app, deployment)

 	// Deploy phase with timeout
-	err = svc.deployContainerWithTimeout(deployCtx, app, deployment)
+	err = svc.deployContainerWithTimeout(deployCtx, app, deployment, imageID)
 	if err != nil {
 		cancelErr := svc.checkCancelled(deployCtx, bgCtx, app, deployment, imageID)
 		if cancelErr != nil {
@@ -516,7 +515,7 @@ func (svc *Service) buildImageWithTimeout(
 	ctx context.Context,
 	app *models.App,
 	deployment *models.Deployment,
-) (string, error) {
+) (domain.ImageID, error) {
 	buildCtx, cancel := context.WithTimeout(ctx, buildTimeout)
 	defer cancel()

@@ -541,6 +540,7 @@ func (svc *Service) deployContainerWithTimeout(
 	ctx context.Context,
 	app *models.App,
 	deployment *models.Deployment,
+	imageID domain.ImageID,
 ) error {
 	deployCtx, cancel := context.WithTimeout(ctx, deployTimeout)
 	defer cancel()
@@ -554,7 +554,7 @@ func (svc *Service) deployContainerWithTimeout(
 	svc.removeOldContainer(deployCtx, app, deployment)

 	// Create and start the new container
-	_, err = svc.createAndStartContainer(deployCtx, app, deployment)
+	_, err = svc.createAndStartContainer(deployCtx, app, deployment, imageID)
 	if err != nil {
 		if errors.Is(deployCtx.Err(), context.DeadlineExceeded) {
 			timeoutErr := fmt.Errorf("%w after %v", ErrDeployTimeout, deployTimeout)
@@ -668,7 +668,7 @@ func (svc *Service) checkCancelled(
 	bgCtx context.Context,
 	app *models.App,
 	deployment *models.Deployment,
-	imageID string,
+	imageID domain.ImageID,
 ) error {
 	if !errors.Is(deployCtx.Err(), context.Canceled) {
 		return nil
@@ -688,7 +688,7 @@ func (svc *Service) cleanupCancelledDeploy(
 	ctx context.Context,
 	app *models.App,
 	deployment *models.Deployment,
-	imageID string,
+	imageID domain.ImageID,
 ) {
 	// Clean up the intermediate Docker image if one was built
 	if imageID != "" {
@@ -696,11 +696,11 @@ func (svc *Service) cleanupCancelledDeploy(
 		if removeErr != nil {
 			svc.log.Error("failed to remove image from cancelled deploy",
 				"error", removeErr, "app", app.Name, "image", imageID)
-			_ = deployment.AppendLog(ctx, "WARNING: failed to clean up image "+imageID+": "+removeErr.Error())
+			_ = deployment.AppendLog(ctx, "WARNING: failed to clean up image "+string(imageID)+": "+removeErr.Error())
 		} else {
 			svc.log.Info("cleaned up image from cancelled deploy",
 				"app", app.Name, "image", imageID)
-			_ = deployment.AppendLog(ctx, "Cleaned up intermediate image: "+imageID)
+			_ = deployment.AppendLog(ctx, "Cleaned up intermediate image: "+string(imageID))
 		}
 	}

@@ -817,7 +817,7 @@ func (svc *Service) buildImage(
 	ctx context.Context,
 	app *models.App,
 	deployment *models.Deployment,
-) (string, error) {
+) (domain.ImageID, error) {
 	workDir, cleanup, err := svc.cloneRepository(ctx, app, deployment)
 	if err != nil {
 		return "", err
@@ -851,8 +851,8 @@ func (svc *Service) buildImage(
 		return "", fmt.Errorf("failed to build image: %w", err)
 	}

-	deployment.ImageID = sql.NullString{String: imageID, Valid: true}
-	_ = deployment.AppendLog(ctx, "Image built: "+imageID)
+	deployment.ImageID = sql.NullString{String: string(imageID), Valid: true}
+	_ = deployment.AppendLog(ctx, "Image built: "+string(imageID))

 	return imageID, nil
 }
@@ -1010,15 +1010,16 @@ func (svc *Service) removeOldContainer(
 		svc.log.Warn("failed to remove old container", "error", removeErr)
 	}

-	_ = deployment.AppendLog(ctx, "Old container removed: "+containerInfo.ID[:12])
+	_ = deployment.AppendLog(ctx, "Old container removed: "+string(containerInfo.ID[:12]))
 }

 func (svc *Service) createAndStartContainer(
 	ctx context.Context,
 	app *models.App,
 	deployment *models.Deployment,
-) (string, error) {
-	containerOpts, err := svc.buildContainerOptions(ctx, app, deployment.ID)
+	imageID domain.ImageID,
+) (domain.ContainerID, error) {
+	containerOpts, err := svc.buildContainerOptions(ctx, app, imageID)
 	if err != nil {
 		svc.failDeployment(ctx, app, deployment, err)

@@ -1038,8 +1039,8 @@ func (svc *Service) createAndStartContainer(
 		return "", fmt.Errorf("failed to create container: %w", err)
 	}

-	deployment.ContainerID = sql.NullString{String: containerID, Valid: true}
-	_ = deployment.AppendLog(ctx, "Container created: "+containerID)
+	deployment.ContainerID = sql.NullString{String: string(containerID), Valid: true}
+	_ = deployment.AppendLog(ctx, "Container created: "+string(containerID))

 	startErr := svc.docker.StartContainer(ctx, containerID)
 	if startErr != nil {
@@ -1062,7 +1063,7 @@ func (svc *Service) createAndStartContainer(
 func (svc *Service) buildContainerOptions(
 	ctx context.Context,
 	app *models.App,
-	deploymentID int64,
+	imageID domain.ImageID,
 ) (docker.CreateContainerOptions, error) {
 	envVars, err := app.GetEnvVars(ctx)
 	if err != nil {
@@ -1096,7 +1097,7 @@ func (svc *Service) buildContainerOptions(

 	return docker.CreateContainerOptions{
 		Name:    "upaas-" + app.Name,
-		Image:   fmt.Sprintf("upaas-%s:%d", app.Name, deploymentID),
+		Image:   string(imageID),
 		Env:     envMap,
 		Labels:  buildLabelMap(app, labels),
 		Volumes: buildVolumeMounts(volumes),
@@ -1146,9 +1147,9 @@ func buildPortMappings(ports []*models.Port) []docker.PortMapping {
 func (svc *Service) updateAppRunning(
 	ctx context.Context,
 	app *models.App,
-	imageID string,
+	imageID domain.ImageID,
 ) error {
-	app.ImageID = sql.NullString{String: imageID, Valid: true}
+	app.ImageID = sql.NullString{String: string(imageID), Valid: true}
 	app.Status = models.AppStatusRunning

 	saveErr := app.Save(ctx)
--- a/internal/service/webhook/webhook.go
+++ b/internal/service/webhook/webhook.go
@@ -11,6 +11,7 @@ import (
 	"go.uber.org/fx"

 	"git.eeqj.de/sneak/upaas/internal/database"
+	"git.eeqj.de/sneak/upaas/internal/domain"
 	"git.eeqj.de/sneak/upaas/internal/logger"
 	"git.eeqj.de/sneak/upaas/internal/models"
 	"git.eeqj.de/sneak/upaas/internal/service/deploy"
@@ -47,24 +48,24 @@ func New(_ fx.Lifecycle, params ServiceParams) (*Service, error) {
 //
 //nolint:tagliatelle // Field names match Gitea API (snake_case)
 type GiteaPushPayload struct {
-	Ref        string `json:"ref"`
-	Before     string `json:"before"`
-	After      string `json:"after"`
-	CompareURL string `json:"compare_url"`
+	Ref        string             `json:"ref"`
+	Before     string             `json:"before"`
+	After      string             `json:"after"`
+	CompareURL domain.UnparsedURL `json:"compare_url"`
 	Repository struct {
-		FullName string `json:"full_name"`
-		CloneURL string `json:"clone_url"`
-		SSHURL   string `json:"ssh_url"`
-		HTMLURL  string `json:"html_url"`
+		FullName string             `json:"full_name"`
+		CloneURL domain.UnparsedURL `json:"clone_url"`
+		SSHURL   string             `json:"ssh_url"`
+		HTMLURL  domain.UnparsedURL `json:"html_url"`
 	} `json:"repository"`
 	Pusher struct {
 		Username string `json:"username"`
 		Email    string `json:"email"`
 	} `json:"pusher"`
 	Commits []struct {
-		ID      string `json:"id"`
-		URL     string `json:"url"`
-		Message string `json:"message"`
+		ID      string             `json:"id"`
+		URL     domain.UnparsedURL `json:"url"`
+		Message string             `json:"message"`
 		Author  struct {
 			Name  string `json:"name"`
 			Email string `json:"email"`
@@ -104,7 +105,7 @@ func (svc *Service) HandleWebhook(
 	event.EventType = eventType
 	event.Branch = branch
 	event.CommitSHA = sql.NullString{String: commitSHA, Valid: commitSHA != ""}
-	event.CommitURL = sql.NullString{String: commitURL, Valid: commitURL != ""}
+	event.CommitURL = sql.NullString{String: string(commitURL), Valid: commitURL != ""}
 	event.Payload = sql.NullString{String: string(payload), Valid: true}
 	event.Matched = matched
 	event.Processed = false
@@ -168,7 +169,7 @@ func extractBranch(ref string) string {

 // extractCommitURL extracts the commit URL from the webhook payload.
 // Prefers the URL from the head commit, falls back to constructing from repo URL.
-func extractCommitURL(payload GiteaPushPayload) string {
+func extractCommitURL(payload GiteaPushPayload) domain.UnparsedURL {
 	// Try to find the URL from the head commit (matching After SHA)
 	for _, commit := range payload.Commits {
 		if commit.ID == payload.After && commit.URL != "" {
@@ -178,7 +179,7 @@ func extractCommitURL(payload GiteaPushPayload) string {

 	// Fall back to constructing URL from repo HTML URL
 	if payload.Repository.HTMLURL != "" && payload.After != "" {
-		return payload.Repository.HTMLURL + "/commit/" + payload.After
+		return domain.UnparsedURL(string(payload.Repository.HTMLURL) + "/commit/" + payload.After)
 	}

 	return ""