From 5c43d5b6f8a65de116111e988852ed07c0c01147 Mon Sep 17 00:00:00 2001 From: user Date: Mon, 23 Feb 2026 11:46:24 -0800 Subject: [PATCH] refactor: remove domain types package, define types alongside implementations - Move ImageID and ContainerID to internal/docker package - Move UnparsedURL to internal/service/webhook package - Delete internal/domain package entirely - No more alias imports needed Addresses review comments on PR #126. --- internal/docker/client.go | 41 ++++++++++++++++------------- internal/domain/types.go | 16 ----------- internal/service/deploy/deploy.go | 21 +++++++-------- internal/service/webhook/webhook.go | 33 +++++++++++++---------- 4 files changed, 52 insertions(+), 59 deletions(-) delete mode 100644 internal/domain/types.go diff --git a/internal/docker/client.go b/internal/docker/client.go index 6e56b98..4dac9a3 100644 --- a/internal/docker/client.go +++ b/internal/docker/client.go @@ -26,7 +26,6 @@ import ( "go.uber.org/fx" "git.eeqj.de/sneak/upaas/internal/config" - "git.eeqj.de/sneak/upaas/internal/domain" "git.eeqj.de/sneak/upaas/internal/logger" ) @@ -43,6 +42,12 @@ const stopTimeoutSeconds = 10 // alpine/git v2.47.2 - pulled 2025-12-30 const gitImage = "alpine/git@sha256:d86f367afb53d022acc4377741e7334bc20add161bb10234272b91b459b4b7d8" +// ImageID is a Docker image identifier (ID or tag). +type ImageID string + +// ContainerID is a Docker container identifier. +type ContainerID string + // ErrNotConnected is returned when Docker client is not connected. var ErrNotConnected = errors.New("docker client not connected") @@ -117,7 +122,7 @@ type BuildImageOptions struct { func (c *Client) BuildImage( ctx context.Context, opts BuildImageOptions, -) (domain.ImageID, error) { +) (ImageID, error) { if c.docker == nil { return "", ErrNotConnected } @@ -189,7 +194,7 @@ func buildPortConfig(ports []PortMapping) (nat.PortSet, nat.PortMap) { func (c *Client) CreateContainer( ctx context.Context, opts CreateContainerOptions, -) (domain.ContainerID, error) { +) (ContainerID, error) { if c.docker == nil { return "", ErrNotConnected } @@ -242,11 +247,11 @@ func (c *Client) CreateContainer( return "", fmt.Errorf("failed to create container: %w", err) } - return domain.ContainerID(resp.ID), nil + return ContainerID(resp.ID), nil } // StartContainer starts a container. -func (c *Client) StartContainer(ctx context.Context, containerID domain.ContainerID) error { +func (c *Client) StartContainer(ctx context.Context, containerID ContainerID) error { if c.docker == nil { return ErrNotConnected } @@ -262,7 +267,7 @@ func (c *Client) StartContainer(ctx context.Context, containerID domain.Containe } // StopContainer stops a container. -func (c *Client) StopContainer(ctx context.Context, containerID domain.ContainerID) error { +func (c *Client) StopContainer(ctx context.Context, containerID ContainerID) error { if c.docker == nil { return ErrNotConnected } @@ -282,7 +287,7 @@ func (c *Client) StopContainer(ctx context.Context, containerID domain.Container // RemoveContainer removes a container. func (c *Client) RemoveContainer( ctx context.Context, - containerID domain.ContainerID, + containerID ContainerID, force bool, ) error { if c.docker == nil { @@ -302,7 +307,7 @@ func (c *Client) RemoveContainer( // ContainerLogs returns the logs for a container. func (c *Client) ContainerLogs( ctx context.Context, - containerID domain.ContainerID, + containerID ContainerID, tail string, ) (string, error) { if c.docker == nil { @@ -338,7 +343,7 @@ func (c *Client) ContainerLogs( // IsContainerRunning checks if a container is running. func (c *Client) IsContainerRunning( ctx context.Context, - containerID domain.ContainerID, + containerID ContainerID, ) (bool, error) { if c.docker == nil { return false, ErrNotConnected @@ -355,7 +360,7 @@ func (c *Client) IsContainerRunning( // IsContainerHealthy checks if a container is healthy. func (c *Client) IsContainerHealthy( ctx context.Context, - containerID domain.ContainerID, + containerID ContainerID, ) (bool, error) { if c.docker == nil { return false, ErrNotConnected @@ -379,7 +384,7 @@ const LabelUpaasID = "upaas.id" // ContainerInfo contains basic information about a container. type ContainerInfo struct { - ID domain.ContainerID + ID ContainerID Running bool } @@ -414,7 +419,7 @@ func (c *Client) FindContainerByAppID( ctr := containers[0] return &ContainerInfo{ - ID: domain.ContainerID(ctr.ID), + ID: ContainerID(ctr.ID), Running: ctr.State == "running", }, nil } @@ -483,7 +488,7 @@ func (c *Client) CloneRepo( // RemoveImage removes a Docker image by ID or tag. // It returns nil if the image was successfully removed or does not exist. -func (c *Client) RemoveImage(ctx context.Context, imageID domain.ImageID) error { +func (c *Client) RemoveImage(ctx context.Context, imageID ImageID) error { _, err := c.docker.ImageRemove(ctx, string(imageID), image.RemoveOptions{ Force: true, PruneChildren: true, @@ -498,7 +503,7 @@ func (c *Client) RemoveImage(ctx context.Context, imageID domain.ImageID) error func (c *Client) performBuild( ctx context.Context, opts BuildImageOptions, -) (domain.ImageID, error) { +) (ImageID, error) { // Create tar archive of build context tarArchive, err := archive.TarWithOptions(opts.ContextDir, &archive.TarOptions{}) if err != nil { @@ -543,7 +548,7 @@ func (c *Client) performBuild( return "", fmt.Errorf("failed to inspect image: %w", inspectErr) } - return domain.ImageID(inspect.ID), nil + return ImageID(inspect.ID), nil } return "", nil @@ -619,7 +624,7 @@ func (c *Client) performClone(ctx context.Context, cfg *cloneConfig) (*CloneResu func (c *Client) createGitContainer( ctx context.Context, cfg *cloneConfig, -) (domain.ContainerID, error) { +) (ContainerID, error) { gitSSHCmd := "ssh -i /keys/deploy_key -o StrictHostKeyChecking=no" // Build the git command using environment variables to avoid shell injection. @@ -676,10 +681,10 @@ func (c *Client) createGitContainer( return "", fmt.Errorf("failed to create git container: %w", err) } - return domain.ContainerID(resp.ID), nil + return ContainerID(resp.ID), nil } -func (c *Client) runGitClone(ctx context.Context, containerID domain.ContainerID) (*CloneResult, error) { +func (c *Client) runGitClone(ctx context.Context, containerID ContainerID) (*CloneResult, error) { err := c.docker.ContainerStart(ctx, string(containerID), container.StartOptions{}) if err != nil { return nil, fmt.Errorf("failed to start git container: %w", err) diff --git a/internal/domain/types.go b/internal/domain/types.go deleted file mode 100644 index 32c3ae8..0000000 --- a/internal/domain/types.go +++ /dev/null @@ -1,16 +0,0 @@ -// Package domain defines domain-specific string types for compile-time safety. -// Using named types prevents accidentally passing the wrong string argument -// (e.g. a container ID where an image ID is expected). -package domain - -// ImageID is a Docker image identifier (ID or tag). -type ImageID string - -// ContainerID is a Docker container identifier. -type ContainerID string - -// UnparsedURL is a URL stored as a plain string without parsing. -// Use this instead of string when the value is known to be a URL -// but should not be parsed into a net/url.URL (e.g. webhook URLs, -// compare URLs from external payloads). -type UnparsedURL string diff --git a/internal/service/deploy/deploy.go b/internal/service/deploy/deploy.go index 8ab981c..1608fd8 100644 --- a/internal/service/deploy/deploy.go +++ b/internal/service/deploy/deploy.go @@ -20,7 +20,6 @@ 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" @@ -418,7 +417,7 @@ func (svc *Service) executeRollback( svc.removeOldContainer(ctx, app, deployment) - rollbackOpts, err := svc.buildContainerOptions(ctx, app, domain.ImageID(previousImageID)) + rollbackOpts, err := svc.buildContainerOptions(ctx, app, docker.ImageID(previousImageID)) if err != nil { svc.failDeployment(bgCtx, app, deployment, err) @@ -515,7 +514,7 @@ func (svc *Service) buildImageWithTimeout( ctx context.Context, app *models.App, deployment *models.Deployment, -) (domain.ImageID, error) { +) (docker.ImageID, error) { buildCtx, cancel := context.WithTimeout(ctx, buildTimeout) defer cancel() @@ -540,7 +539,7 @@ func (svc *Service) deployContainerWithTimeout( ctx context.Context, app *models.App, deployment *models.Deployment, - imageID domain.ImageID, + imageID docker.ImageID, ) error { deployCtx, cancel := context.WithTimeout(ctx, deployTimeout) defer cancel() @@ -668,7 +667,7 @@ func (svc *Service) checkCancelled( bgCtx context.Context, app *models.App, deployment *models.Deployment, - imageID domain.ImageID, + imageID docker.ImageID, ) error { if !errors.Is(deployCtx.Err(), context.Canceled) { return nil @@ -688,7 +687,7 @@ func (svc *Service) cleanupCancelledDeploy( ctx context.Context, app *models.App, deployment *models.Deployment, - imageID domain.ImageID, + imageID docker.ImageID, ) { // Clean up the intermediate Docker image if one was built if imageID != "" { @@ -817,7 +816,7 @@ func (svc *Service) buildImage( ctx context.Context, app *models.App, deployment *models.Deployment, -) (domain.ImageID, error) { +) (docker.ImageID, error) { workDir, cleanup, err := svc.cloneRepository(ctx, app, deployment) if err != nil { return "", err @@ -1017,8 +1016,8 @@ func (svc *Service) createAndStartContainer( ctx context.Context, app *models.App, deployment *models.Deployment, - imageID domain.ImageID, -) (domain.ContainerID, error) { + imageID docker.ImageID, +) (docker.ContainerID, error) { containerOpts, err := svc.buildContainerOptions(ctx, app, imageID) if err != nil { svc.failDeployment(ctx, app, deployment, err) @@ -1063,7 +1062,7 @@ func (svc *Service) createAndStartContainer( func (svc *Service) buildContainerOptions( ctx context.Context, app *models.App, - imageID domain.ImageID, + imageID docker.ImageID, ) (docker.CreateContainerOptions, error) { envVars, err := app.GetEnvVars(ctx) if err != nil { @@ -1147,7 +1146,7 @@ func buildPortMappings(ports []*models.Port) []docker.PortMapping { func (svc *Service) updateAppRunning( ctx context.Context, app *models.App, - imageID domain.ImageID, + imageID docker.ImageID, ) error { app.ImageID = sql.NullString{String: string(imageID), Valid: true} app.Status = models.AppStatusRunning diff --git a/internal/service/webhook/webhook.go b/internal/service/webhook/webhook.go index ed7fb33..fed6864 100644 --- a/internal/service/webhook/webhook.go +++ b/internal/service/webhook/webhook.go @@ -11,12 +11,17 @@ 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" ) +// UnparsedURL is a URL stored as a plain string without parsing. +// Use this instead of string when the value is known to be a URL +// but should not be parsed into a net/url.URL (e.g. webhook URLs, +// compare URLs from external payloads). +type UnparsedURL string + // ServiceParams contains dependencies for Service. type ServiceParams struct { fx.In @@ -48,24 +53,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 domain.UnparsedURL `json:"compare_url"` + Ref string `json:"ref"` + Before string `json:"before"` + After string `json:"after"` + CompareURL UnparsedURL `json:"compare_url"` Repository struct { - FullName string `json:"full_name"` - CloneURL domain.UnparsedURL `json:"clone_url"` - SSHURL string `json:"ssh_url"` - HTMLURL domain.UnparsedURL `json:"html_url"` + FullName string `json:"full_name"` + CloneURL UnparsedURL `json:"clone_url"` + SSHURL string `json:"ssh_url"` + HTMLURL 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 domain.UnparsedURL `json:"url"` - Message string `json:"message"` + ID string `json:"id"` + URL UnparsedURL `json:"url"` + Message string `json:"message"` Author struct { Name string `json:"name"` Email string `json:"email"` @@ -169,7 +174,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) domain.UnparsedURL { +func extractCommitURL(payload GiteaPushPayload) 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 != "" { @@ -179,7 +184,7 @@ func extractCommitURL(payload GiteaPushPayload) domain.UnparsedURL { // Fall back to constructing URL from repo HTML URL if payload.Repository.HTMLURL != "" && payload.After != "" { - return domain.UnparsedURL(string(payload.Repository.HTMLURL) + "/commit/" + payload.After) + return UnparsedURL(string(payload.Repository.HTMLURL) + "/commit/" + payload.After) } return ""