fix: add linters and improve code quality (#480)

* chore: add golangci-lint configuration file

* fix: group imports with local prefixes

* chore: update golangci-lint configuration and streamline linting commands

* refactor: replace hardcoded file permissions with constants from utils

* chore: add additional linters to golangci-lint configuration

* refactor: lint

* refactor: lint

* refactor: use request context in WebhookHandler

* chore: add additional linters to golangci-lint configuration

* refactor: add errname linter and improve error handling in GetDeployConfigFromYAML

* refactor: update golangci-lint command to use GO_BIN variable

* refactor: update wsl linter configuration and clean up code formatting

Author: kimdre

.golangci.yaml

@@ -0,0 +1,41 @@
+version: "2"
+linters:
+  default: standard
+  enable:
+    - asciicheck
+    - bodyclose
+    - contextcheck
+    - copyloopvar
+    - dupword
+    - durationcheck
+    - errname
+    - gosec
+    - misspell
+    - nilerr
+    - unconvert
+    - godot
+    - gocritic
+    - makezero
+    - perfsprint
+    - wsl_v5
+  settings:
+    wsl_v5:
+      allow-first-in-block: true
+      allow-whole-block: false
+      branch-max-lines: 2
+formatters:
+  enable:
+    - gofmt
+    - gofumpt
+    - goimports
+  settings:
+    #    gofmt:
+    #      extra-rules: true
+    goimports:
+      local-prefixes:
+        - github.com/kimdre/doco-cd
+issues:
+  max-issues-per-linter: 0
+  max-same-issues: 0
+run:
+  timeout: 5m

Makefile

@@ -30,15 +30,8 @@ build:
 	mkdir -p $(BINARY_DIR)
 	go build -o $(BINARY_DIR) ./...
 
-lint:
-	golangci-lint run --fix ./...
-
-fmt:
-	go fmt ./...
-	-go tool gofumpt -l -w .
-	-go tool goimports -l -w .
-	-go tool wsl -test=true -fix ./...
-	-go tool perfsprint -fix ./...
+lint fmt:
+	${GO_BIN}/golangci-lint run --fix ./...
 
 update:
 	git pull origin main

cmd/doco-cd/http_handler.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/docker/cli/cli/command"
 	"github.com/google/uuid"
+
 	"github.com/kimdre/doco-cd/internal/config"
 	"github.com/kimdre/doco-cd/internal/docker"
 	"github.com/kimdre/doco-cd/internal/git"
@@ -46,7 +47,7 @@ func onError(w http.ResponseWriter, log *slog.Logger, errMsg string, details any
 		statusCode)
 }
 
-// getRepoName extracts the repository name from the clone URL
+// getRepoName extracts the repository name from the clone URL.
 func getRepoName(cloneURL string) string {
 	repoName := strings.SplitAfter(cloneURL, "://")[1]
 
@@ -57,7 +58,7 @@ func getRepoName(cloneURL string) string {
 	return strings.TrimSuffix(repoName, ".git")
 }
 
-// HandleEvent handles the incoming webhook event
+// HandleEvent handles the incoming webhook event.
 func HandleEvent(ctx context.Context, jobLog *slog.Logger, w http.ResponseWriter, appConfig *config.AppConfig, dataMountPoint container.MountPoint, payload webhook.ParsedPayload, customTarget, jobID string, dockerCli command.Cli, dockerClient *client.Client) {
 	startTime := time.Now()
 	repoName := getRepoName(payload.CloneURL)
@@ -83,6 +84,7 @@ func HandleEvent(ctx context.Context, jobLog *slog.Logger, w http.ResponseWriter
 
 		if appConfig.GitAccessToken == "" {
 			onError(w, jobLog, "missing access token for private repository", "", jobID, http.StatusInternalServerError)
+
 			return
 		}
 
@@ -95,18 +97,21 @@ func HandleEvent(ctx context.Context, jobLog *slog.Logger, w http.ResponseWriter
 	// Validate payload.FullName to prevent directory traversal
 	if strings.Contains(payload.FullName, "..") {
 		onError(w, jobLog.With(slog.String("repository", payload.FullName)), "invalid repository name", "", jobID, http.StatusBadRequest)
+
 		return
 	}
 
 	internalRepoPath, err := utils.VerifyAndSanitizePath(filepath.Join(dataMountPoint.Destination, repoName), dataMountPoint.Destination) // Path inside the container
 	if err != nil {
 		onError(w, jobLog.With(logger.ErrAttr(err)), "failed to verify and sanitize internal filesystem path", err.Error(), jobID, http.StatusBadRequest)
+
 		return
 	}
 
 	externalRepoPath, err := utils.VerifyAndSanitizePath(filepath.Join(dataMountPoint.Destination, repoName), dataMountPoint.Destination) // Path on the host
 	if err != nil {
 		onError(w, jobLog.With(logger.ErrAttr(err)), "failed to verify and sanitize external filesystem path", err.Error(), jobID, http.StatusBadRequest)
+
 		return
 	}
 
@@ -120,10 +125,12 @@ func HandleEvent(ctx context.Context, jobLog *slog.Logger, w http.ResponseWriter
 			_, err = git.UpdateRepository(internalRepoPath, payload.Ref, appConfig.SkipTLSVerification, appConfig.HttpProxy)
 			if err != nil {
 				onError(w, jobLog.With(logger.ErrAttr(err)), "failed to checkout repository", err.Error(), jobID, http.StatusInternalServerError)
+
 				return
 			}
 		} else {
 			onError(w, jobLog.With(logger.ErrAttr(err)), "failed to clone repository", err.Error(), jobID, http.StatusInternalServerError)
+
 			return
 		}
 	} else {
@@ -139,6 +146,7 @@ func HandleEvent(ctx context.Context, jobLog *slog.Logger, w http.ResponseWriter
 			jobLog.Warn(err.Error())
 		} else {
 			onError(w, jobLog.With(logger.ErrAttr(err)), "failed to get deploy configuration", err.Error(), jobID, http.StatusInternalServerError)
+
 			return
 		}
 	}
@@ -154,12 +162,14 @@ func HandleEvent(ctx context.Context, jobLog *slog.Logger, w http.ResponseWriter
 		internalRepoPath, err = utils.VerifyAndSanitizePath(filepath.Join(dataMountPoint.Destination, repoName), dataMountPoint.Destination) // Path inside the container
 		if err != nil {
 			onError(w, subJobLog.With(logger.ErrAttr(err)), "invalid repository name", err.Error(), jobID, http.StatusBadRequest)
+
 			return
 		}
 
 		externalRepoPath, err = utils.VerifyAndSanitizePath(filepath.Join(dataMountPoint.Source, repoName), dataMountPoint.Source) // Path on the host
 		if err != nil {
 			onError(w, subJobLog.With(logger.ErrAttr(err)), "invalid repository name", err.Error(), jobID, http.StatusBadRequest)
+
 			return
 		}
 
@@ -182,6 +192,7 @@ func HandleEvent(ctx context.Context, jobLog *slog.Logger, w http.ResponseWriter
 			_, err = git.CloneRepository(internalRepoPath, cloneUrl, deployConfig.Reference, appConfig.SkipTLSVerification, appConfig.HttpProxy)
 			if err != nil && !errors.Is(err, git.ErrRepositoryAlreadyExists) {
 				onError(w, subJobLog.With(logger.ErrAttr(err)), "failed to clone remote repository", err.Error(), jobID, http.StatusInternalServerError)
+
 				return
 			}
 
@@ -193,12 +204,14 @@ func HandleEvent(ctx context.Context, jobLog *slog.Logger, w http.ResponseWriter
 		repo, err := git.UpdateRepository(internalRepoPath, deployConfig.Reference, appConfig.SkipTLSVerification, appConfig.HttpProxy)
 		if err != nil {
 			onError(w, subJobLog.With(logger.ErrAttr(err)), "failed to checkout repository", err.Error(), jobID, http.StatusInternalServerError)
+
 			return
 		}
 
 		latestCommit, err := git.GetLatestCommit(repo, deployConfig.Reference)
 		if err != nil {
 			onError(w, subJobLog.With(logger.ErrAttr(err)), "failed to get latest commit", err.Error(), jobID, http.StatusInternalServerError)
+
 			return
 		}
 
@@ -209,12 +222,14 @@ func HandleEvent(ctx context.Context, jobLog *slog.Logger, w http.ResponseWriter
 			containers, err := docker.GetLabeledContainers(ctx, dockerClient, api.ProjectLabel, deployConfig.Name)
 			if err != nil {
 				onError(w, subJobLog.With(logger.ErrAttr(err)), "failed to retrieve containers", err.Error(), jobID, http.StatusInternalServerError)
+
 				return
 			}
 
 			// If no containers are found, skip the destruction step
 			if len(containers) == 0 {
 				subJobLog.Debug("no containers found for stack, skipping...")
+
 				continue
 			}
 
@@ -251,6 +266,7 @@ func HandleEvent(ctx context.Context, jobLog *slog.Logger, w http.ResponseWriter
 			err = docker.DestroyStack(subJobLog, &ctx, &dockerCli, deployConfig)
 			if err != nil {
 				onError(w, subJobLog.With(logger.ErrAttr(err)), "failed to destroy stack", err.Error(), jobID, http.StatusInternalServerError)
+
 				return
 			}
 
@@ -263,6 +279,7 @@ func HandleEvent(ctx context.Context, jobLog *slog.Logger, w http.ResponseWriter
 				subDirs, err := os.ReadDir(parentDir)
 				if err != nil {
 					onError(w, subJobLog.With(logger.ErrAttr(err)), "failed to read parent directory", err.Error(), jobID, http.StatusInternalServerError)
+
 					return
 				}
 
@@ -274,13 +291,15 @@ func HandleEvent(ctx context.Context, jobLog *slog.Logger, w http.ResponseWriter
 					err = os.RemoveAll(internalRepoPath)
 					if err != nil {
 						onError(w, subJobLog.With(logger.ErrAttr(err)), "failed to remove deployment directory", err.Error(), jobID, http.StatusInternalServerError)
+
 						return
 					}
 				} else {
 					// Remove the parent directory if it has only one subdirectory
 					err = os.RemoveAll(parentDir)
 					if err != nil {
 						onError(w, subJobLog.With(logger.ErrAttr(err)), "failed to remove deployment directory", err.Error(), jobID, http.StatusInternalServerError)
+
 						return
 					}
 
@@ -292,6 +311,7 @@ func HandleEvent(ctx context.Context, jobLog *slog.Logger, w http.ResponseWriter
 			containers, err := docker.GetLabeledContainers(ctx, dockerClient, api.ProjectLabel, deployConfig.Name)
 			if err != nil {
 				onError(w, subJobLog.With(logger.ErrAttr(err)), "failed to retrieve containers", err.Error(), jobID, http.StatusInternalServerError)
+
 				return
 			}
 
@@ -303,6 +323,7 @@ func HandleEvent(ctx context.Context, jobLog *slog.Logger, w http.ResponseWriter
 				name, ok := cont.Labels[docker.DocoCDLabels.Repository.Name]
 				if !ok || name != payload.FullName {
 					correctRepo = false
+
 					break
 				}
 
@@ -343,6 +364,7 @@ func HandleEvent(ctx context.Context, jobLog *slog.Logger, w http.ResponseWriter
 			err = docker.DeployStack(subJobLog, internalRepoPath, externalRepoPath, &ctx, &dockerCli, &payload, deployConfig, latestCommit, Version, false)
 			if err != nil {
 				onError(w, subJobLog.With(logger.ErrAttr(err)), "deployment failed", err.Error(), jobID, http.StatusInternalServerError)
+
 				return
 			}
 		}
@@ -355,7 +377,7 @@ func HandleEvent(ctx context.Context, jobLog *slog.Logger, w http.ResponseWriter
 }
 
 func (h *handlerData) WebhookHandler(w http.ResponseWriter, r *http.Request) {
-	ctx := context.Background()
+	ctx := r.Context()
 
 	customTarget := r.PathValue("customTarget")
 
@@ -405,6 +427,7 @@ func (h *handlerData) HealthCheckHandler(w http.ResponseWriter, _ *http.Request)
 	err := docker.VerifySocketConnection()
 	if err != nil {
 		onError(w, h.log.With(logger.ErrAttr(err)), docker.ErrDockerSocketConnectionFailed.Error(), err.Error(), "", http.StatusServiceUnavailable)
+
 		return
 	}
 

cmd/doco-cd/http_handler_test.go

@@ -30,7 +30,7 @@ const (
 	githubPayloadFile = "testdata/github_payload.json"
 )
 
-// Make http call to HealthCheckHandler
+// Make http call to HealthCheckHandler.
 func TestHandlerData_HealthCheckHandler(t *testing.T) {
 	expectedResponse := fmt.Sprintln(`{"details":"healthy"}`)
 	expectedStatusCode := http.StatusOK
@@ -207,7 +207,7 @@ func TestHandlerData_WebhookHandler(t *testing.T) {
 	testContainerPort := testContainer.NetworkSettings.Ports["80/tcp"][0].HostPort
 	testURL := "http://localhost:" + testContainerPort
 
-	resp, err := http.Get(testURL)
+	resp, err := http.Get(testURL) // #nosec G107
 	if err != nil {
 		t.Fatalf("Failed to make GET request to test container: %v", err)
 	}
@@ -233,7 +233,7 @@ func TestHandlerData_WebhookHandler(t *testing.T) {
 
 	indexFile := path.Join(repoDir, "test", "index.html")
 
-	fileContent, err := os.ReadFile(indexFile)
+	fileContent, err := os.ReadFile(indexFile) // #nosec G304
 	if err != nil {
 		t.Fatalf("Failed to read index.html file: %v", err)
 	}

cmd/doco-cd/main.go

@@ -11,12 +11,16 @@ import (
 	"regexp"
 	"strings"
 	"sync"
+	"time"
+
+	"github.com/kimdre/doco-cd/internal/utils"
 
 	"github.com/docker/docker/api/types/container"
 
 	"github.com/go-git/go-git/v5/plumbing/transport"
 
 	"github.com/docker/docker/client"
+
 	"github.com/kimdre/doco-cd/internal/config"
 	"github.com/kimdre/doco-cd/internal/docker"
 	"github.com/kimdre/doco-cd/internal/logger"
@@ -33,7 +37,7 @@ var (
 	errMsg  string
 )
 
-// getAppContainerID retrieves the application container ID from the cpuset file
+// getAppContainerID retrieves the application container ID from the cpuset file.
 func getAppContainerID() (string, error) {
 	const (
 		cgroupMounts = "/proc/self/mountinfo"
@@ -91,12 +95,12 @@ func GetProxyUrlRedacted(proxyUrl string) string {
 }
 
 // CreateMountpointSymlink creates the Symlink for the data mount point to reflect the data volume path in the container.
-// Required so that the docker cli client is able to read/parse certain files (like .env files) in docker.LoadCompose
+// Required so that the docker cli client is able to read/parse certain files (like .env files) in docker.LoadCompose.
 func CreateMountpointSymlink(m container.MountPoint) error {
 	// prepare the symlink parent directory
 	symlinkParentDir := path.Dir(m.Source)
 
-	err := os.MkdirAll(symlinkParentDir, 0o755)
+	err := os.MkdirAll(symlinkParentDir, utils.PermDir)
 	if err != nil {
 		return fmt.Errorf("failed to create parent directory %s: %w", symlinkParentDir, err)
 	}
@@ -170,6 +174,7 @@ func main() {
 	dockerCli, err := docker.CreateDockerCli(c.DockerQuietDeploy, !c.SkipTLSVerification)
 	if err != nil {
 		log.Critical("failed to create docker client", logger.ErrAttr(err))
+
 		return
 	}
 	defer func(client client.APIClient) {
@@ -198,6 +203,7 @@ func main() {
 	appContainerID, err := getAppContainerID()
 	if err != nil {
 		log.Critical("failed to retrieve application container id", logger.ErrAttr(err))
+
 		return
 	}
 
@@ -225,6 +231,7 @@ func main() {
 	err = CreateMountpointSymlink(dataMountPoint)
 	if err != nil {
 		log.Critical(fmt.Sprintf("failed to create symlink for %s mount point", dataMountPoint.Destination), logger.ErrAttr(err))
+
 		return
 	}
 
@@ -255,6 +262,7 @@ func main() {
 			err = StartPoll(&h, pollConfig, &wg)
 			if err != nil {
 				log.Critical("failed to scheduling polling jobs", logger.ErrAttr(err))
+
 				return
 			}
 		}
@@ -267,7 +275,7 @@ func main() {
 		log.Error("failed to retrieve doco-cd containers", logger.ErrAttr(err))
 	}
 
-	if len(containers) <= 0 {
+	if len(containers) == 0 {
 		log.Debug("no containers found that are managed by doco-cd", slog.Int("count", len(containers)))
 	} else {
 		log.Debug("retrieved containers successfully", slog.Int("count", len(containers)))
@@ -280,8 +288,9 @@ func main() {
 		))
 
 		dir := cont.Labels[docker.DocoCDLabels.Deployment.WorkingDir]
-		if len(dir) <= 0 {
+		if len(dir) == 0 {
 			log.Error(fmt.Sprintf("failed to retrieve container %v working directory", cont.ID))
+
 			continue
 		}
 
@@ -303,7 +312,12 @@ func main() {
 		}()
 	}
 
-	err = http.ListenAndServe(fmt.Sprintf(":%d", c.HttpPort), nil)
+	server := &http.Server{
+		Addr:              fmt.Sprintf(":%d", c.HttpPort),
+		ReadHeaderTimeout: 3 * time.Second,
+	}
+
+	err = server.ListenAndServe()
 	if err != nil {
 		log.Error(fmt.Sprintf("failed to listen on port: %v", c.HttpPort), logger.ErrAttr(err))
 	}