Merge pull request #77 from nlewo/linting

github: add linting CI step

Author: nlewo

.github/workflows/golangci-lint.yml

@@ -0,0 +1,23 @@
+name: golangci-lint
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+
+permissions:
+  contents: read
+
+jobs:
+  golangci:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
+        with:
+          go-version: stable
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v7
+        with:
+          version: v2.0

cmd/fetcher.go

@@ -20,7 +20,7 @@ var fetchCmd = &cobra.Command{
 		if err != nil {
 			return
 		}
-		client.Do(req)
+		_, _ = client.Do(req)
 	},
 }
 

cmd/run.go

@@ -88,6 +88,6 @@ var runCmd = &cobra.Command{
 
 func init() {
 	runCmd.PersistentFlags().StringVarP(&configFilepath, "config", "", "", "the configuration file path")
-	runCmd.MarkPersistentFlagRequired("config")
+	_ = runCmd.MarkPersistentFlagRequired("config")
 	rootCmd.AddCommand(runCmd)
 }

cmd/status.go

@@ -32,7 +32,7 @@ func getStatus() (status manager.State, err error) {
 		return
 	}
 	if res.Body != nil {
-		defer res.Body.Close()
+		defer res.Body.Close() // nolint
 	}
 	body, err := io.ReadAll(res.Body)
 	if err != nil {

flake.lock

@@ -1,7 +1,7 @@
 {
   description = "Comin - GitOps for NixOS Machines";
 
-  inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixos-24.11";
+  inputs.nixpkgs.url = "github:NixOS/nixpkgs";
 
   outputs = { self, nixpkgs }:
   let
@@ -33,6 +33,7 @@
     in pkgs.mkShell {
       buildInputs = [
         pkgs.go pkgs.godef pkgs.gopls
+        pkgs.golangci-lint
       ];
     };
   };

flake.nix

@@ -192,13 +192,13 @@ func (b *Builder) Build() error {
 	defer b.mu.Unlock()
 
 	if b.generation == nil || b.generation.EvalStatus != Evaluated {
-		return fmt.Errorf("The generation is not evaluated")
+		return fmt.Errorf("the generation is not evaluated")
 	}
 	if b.IsBuilding {
-		return fmt.Errorf("The builder is already building")
+		return fmt.Errorf("the builder is already building")
 	}
 	if b.generation.BuildStatus == Built {
-		return fmt.Errorf("The generation is already built")
+		return fmt.Errorf("the generation is already built")
 	}
 	b.generation.BuildStartedAt = time.Now().UTC()
 	b.generation.BuildStatus = Building

internal/builder/builder.go

@@ -46,7 +46,7 @@ func TestBuilderBuild(t *testing.T) {
 
 	b := New("", "", "my-machine", 2*time.Second, mkNixEvalMock(evalDone), 2*time.Second, mkNixBuildMock(buildDone))
 
-	assert.ErrorContains(t, b.Build(), "The generation is not evaluated")
+	assert.ErrorContains(t, b.Build(), "the generation is not evaluated")
 	// Run the evaluator
 	b.Eval(repository.RepositoryStatus{})
 	close(evalDone)
@@ -55,12 +55,12 @@ func TestBuilderBuild(t *testing.T) {
 		assert.False(c, b.IsEvaluating)
 	}, 2*time.Second, 100*time.Millisecond)
 
-	b.Build()
+	_ = b.Build()
 	assert.EventuallyWithT(t, func(c *assert.CollectT) {
 		assert.True(c, b.IsBuilding)
 	}, 2*time.Second, 100*time.Millisecond)
 	err := b.Build()
-	assert.ErrorContains(t, err, "The builder is already building")
+	assert.ErrorContains(t, err, "the builder is already building")
 
 	// Stop the evaluator and builder
 	b.Stop()
@@ -89,7 +89,7 @@ func TestBuilderBuild(t *testing.T) {
 
 	// The generation is already built
 	err = b.Build()
-	assert.ErrorContains(t, err, "The generation is already built")
+	assert.ErrorContains(t, err, "the generation is already built")
 }
 
 func TestEval(t *testing.T) {

internal/builder/builder_test.go

@@ -103,13 +103,13 @@ func GenerationShow(g Generation) {
 		fmt.Printf("%sEvaluation started %s\n", padding, humanize.Time(g.EvalStartedAt))
 		return
 	}
-	if g.EvalStatus == Evaluated {
+	switch g.EvalStatus {
+	case Evaluated:
 		fmt.Printf("%sEvaluation succedded %s\n", padding, humanize.Time(g.EvalEndedAt))
 		fmt.Printf("%s  DrvPath: %s\n", padding, g.DrvPath)
-	} else if g.EvalStatus == EvalFailed {
+	case EvalFailed:
 		fmt.Printf("%sEvaluation failed %s\n", padding, humanize.Time(g.EvalEndedAt))
 	}
-
 	if g.BuildStatus == BuildInit {
 		fmt.Printf("%sNo build started\n", padding)
 		return
@@ -118,10 +118,11 @@ func GenerationShow(g Generation) {
 		fmt.Printf("%sBuild started %s\n", padding, humanize.Time(g.BuildStartedAt))
 		return
 	}
-	if g.BuildStatus == Built {
+	switch g.BuildStatus {
+	case Built:
 		fmt.Printf("%sBuilt %s\n", padding, humanize.Time(g.BuildEndedAt))
 		fmt.Printf("%s  Outpath:  %s\n", padding, g.OutPath)
-	} else if g.BuildStatus == BuildFailed {
+	case BuildFailed:
 		fmt.Printf("%sBuild failed %s\n", padding, humanize.Time(g.BuildEndedAt))
 	}
 }

internal/builder/generation.go

@@ -15,7 +15,7 @@ func Read(path string) (config types.Configuration, err error) {
 	if err != nil {
 		return
 	}
-	defer file.Close()
+	defer file.Close() // nolint
 
 	d := yaml.NewDecoder(file)
 	if err := d.Decode(&config); err != nil {

internal/config/config.go

@@ -61,9 +61,7 @@ type Deployer struct {
 	previousDeployment *Deployment
 	IsDeploying        bool
 	// The next generation to deploy. nil when there is no new generation to deploy
-	GenerationToDeploy *builder.Generation
-	// Is there a generation which
-	generationAvailable   bool
+	GenerationToDeploy    *builder.Generation
 	generationAvailableCh chan struct{}
 }
 
@@ -190,9 +188,7 @@ func (d *Deployer) Run() {
 			}
 			d.Deployment.RestartComin = cominNeedRestart
 			d.Deployment.ProfilePath = profilePath
-			select {
-			case d.DeploymentDoneCh <- *d.Deployment:
-			}
+			d.DeploymentDoneCh <- *d.Deployment
 			d.mu.Unlock()
 		}
 	}()

internal/deployer/deployer.go

@@ -56,7 +56,7 @@ func runNixCommand(args []string, stdout, stderr io.Writer) (err error) {
 	cmd.Stderr = stderr
 	err = cmd.Run()
 	if err != nil {
-		return fmt.Errorf("Command '%s' fails with %s", cmdStr, err)
+		return fmt.Errorf("command '%s' fails with %s", cmdStr, err)
 	}
 	return nil
 }
@@ -130,7 +130,7 @@ func switchToConfiguration(operation string, outPath string, dryRun bool) error
 		logrus.Infof("nix: dry-run enabled: '%s switch' has not been executed", switchToConfigurationExe)
 	} else {
 		if err := cmd.Run(); err != nil {
-			return fmt.Errorf("Command %s switch fails with %s", switchToConfigurationExe, err)
+			return fmt.Errorf("command %s switch fails with %s", switchToConfigurationExe, err)
 		}
 		logrus.Infof("nix: switch successfully terminated")
 	}

internal/executor/utils.go

@@ -21,8 +21,7 @@ func handlerStatus(m *manager.Manager, w http.ResponseWriter, r *http.Request) {
 	if err != nil {
 		logrus.Error(err)
 	}
-	io.WriteString(w, string(rJson))
-	return
+	_, _ = io.Writer.Write(w, rJson)
 }
 
 // Serve starts http servers. We create two HTTP servers to easily be
@@ -31,13 +30,12 @@ func handlerStatus(m *manager.Manager, w http.ResponseWriter, r *http.Request) {
 func Serve(m *manager.Manager, p prometheus.Prometheus, apiAddress string, apiPort int, metricsAddress string, metricsPort int) {
 	handlerStatusFn := func(w http.ResponseWriter, r *http.Request) {
 		handlerStatus(m, w, r)
-		return
 	}
 	handlerFetcherFn := func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)
 		s := m.GetState().Fetcher
 		rJson, _ := json.MarshalIndent(s, "", "\t")
-		io.WriteString(w, string(rJson))
+		_, _ = io.Writer.Write(w, rJson)
 	}
 	handlerFetcherFetchFn := func(w http.ResponseWriter, r *http.Request) {
 		switch r.Method {

internal/http/http.go

@@ -91,7 +91,7 @@ func (m *Manager) FetchAndBuild() {
 					logrus.Infof("manager: the comin.machineId %s is not the host machine-id %s", generation.MachineId, m.machineId)
 				} else {
 					logrus.Infof("manager: a generation is building for commit %s", generation.SelectedCommitId)
-					m.builder.Build()
+					_ = m.builder.Build()
 				}
 			case generation := <-m.builder.BuildDone:
 				if generation.BuildErr == nil {
@@ -120,7 +120,7 @@ func (m *Manager) Run() {
 			m.prometheus.SetDeploymentInfo(dpl.Generation.SelectedCommitId, deployer.StatusToString(dpl.Status))
 			getsEvicted, evicted := m.storage.DeploymentInsertAndCommit(dpl)
 			if getsEvicted && evicted.ProfilePath != "" {
-				profile.RemoveProfilePath(evicted.ProfilePath)
+				_ = profile.RemoveProfilePath(evicted.ProfilePath)
 			}
 			m.needToReboot = utils.NeedToReboot()
 			m.prometheus.SetHostInfo(m.needToReboot)

internal/manager/manager.go

@@ -18,10 +18,6 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-type metricsMock struct{}
-
-func (m metricsMock) SetDeploymentInfo(commitId, status string) {}
-
 var mkNixEvalMock = func(evalOk chan bool) builder.EvalFunc {
 	return func(ctx context.Context, repositoryPath string, hostname string) (string, string, string, error) {
 		ok := <-evalOk

internal/manager/manager_test.go

@@ -12,11 +12,11 @@ import (
 func TestRemoveProfilePath(t *testing.T) {
 	dir := t.TempDir()
 	file1 := path.Join(dir, "file1")
-	os.Create(file1)
+	_, _ = os.Create(file1)
 	file2 := path.Join(dir, "file2")
-	os.Create(file2)
+	_, _ = os.Create(file2)
 
-	RemoveProfilePath(file1)
+	_ = RemoveProfilePath(file1)
 	entries, _ := os.ReadDir(dir)
 	files := make([]string, len(entries))
 	for i, e := range entries {

internal/profile/profile_test.go

@@ -38,7 +38,7 @@ func RepositoryClone(directory, url, commitId, accessToken string) error {
 		Hash: plumbing.NewHash(commitId),
 	})
 	if err != nil {
-		return fmt.Errorf("Cannot checkout the commit ID %s: '%s'", commitId, err)
+		return fmt.Errorf("cannot checkout the commit ID %s: '%s'", commitId, err)
 	}
 	return nil
 }
@@ -66,7 +66,7 @@ func hasNotBeenHardReset(r repository, branchName string, currentMainHash *plumb
 			return err
 		}
 		if !ok {
-			return fmt.Errorf("This branch has been hard reset: its head '%s' is not on top of '%s'",
+			return fmt.Errorf("this branch has been hard reset: its head '%s' is not on top of '%s'",
 				remoteMainHead.String(), currentMainHash.String())
 		}
 	}
@@ -77,7 +77,7 @@ func getHeadFromRemoteAndBranch(r repository, remoteName, branchName, currentMai
 	var currentMainHash *plumbing.Hash
 	head := getRemoteCommitHash(r, remoteName, branchName)
 	if head == nil {
-		return newHead, "", fmt.Errorf("The branch '%s/%s' doesn't exist", remoteName, branchName)
+		return newHead, "", fmt.Errorf("the branch '%s/%s' doesn't exist", remoteName, branchName)
 	}
 	if currentMainCommitId != "" {
 		c := plumbing.NewHash(currentMainCommitId)
@@ -100,7 +100,7 @@ func hardReset(r repository, newHead plumbing.Hash) error {
 	var w *git.Worktree
 	w, err := r.Repository.Worktree()
 	if err != nil {
-		return fmt.Errorf("Failed to get the worktree")
+		return fmt.Errorf("failed to get the worktree")
 	}
 	err = w.Checkout(&git.CheckoutOptions{
 		Hash:  newHead,
@@ -154,7 +154,7 @@ func isAncestor(r *git.Repository, base, top plumbing.Hash) (found bool, err err
 
 	// To skip the first commit
 	isFirst := true
-	iter.ForEach(func(commit *object.Commit) error {
+	_ = iter.ForEach(func(commit *object.Commit) error {
 		if !isFirst && commit.Hash == base {
 			found = true
 			// This error is ignored and used to terminate early the loop :/
@@ -223,9 +223,9 @@ func manageRemote(r *git.Repository, remote types.Remote) error {
 }
 
 func headSignedBy(r *git.Repository, publicKeys []string) (signedBy *openpgp.Entity, err error) {
-	head, err := r.Head()
+	head, _ := r.Head()
 	if head == nil {
-		return nil, fmt.Errorf("Repository HEAD should not be nil")
+		return nil, fmt.Errorf("repository HEAD should not be nil")
 	}
 	commit, err := r.CommitObject(head.Hash())
 	if err != nil {
@@ -238,5 +238,5 @@ func headSignedBy(r *git.Repository, publicKeys []string) (signedBy *openpgp.Ent
 			return entity, nil
 		}
 	}
-	return nil, fmt.Errorf("Commit %s is not signed", head.Hash())
+	return nil, fmt.Errorf("commit %s is not signed", head.Hash())
 }

internal/repository/git.go

@@ -22,7 +22,7 @@ func commitFileAndSign(remoteRepository *git.Repository, dir, branch, content st
 	if err != nil {
 		return
 	}
-	err = w.Checkout(&git.CheckoutOptions{
+	_ = w.Checkout(&git.CheckoutOptions{
 		Branch: plumbing.NewBranchReferenceName(branch),
 		Force:  true,
 	})
@@ -106,7 +106,7 @@ func TestIsAncestor(t *testing.T) {
 
 	commits := make([]object.Commit, 3)
 	idx := 0
-	err = iter.ForEach(func(commit *object.Commit) error {
+	_ = iter.ForEach(func(commit *object.Commit) error {
 		commits[idx] = *commit
 		idx += 1
 		return nil
@@ -131,9 +131,9 @@ func TestHeadSignedBy(t *testing.T) {
 	dir := t.TempDir()
 	remoteRepository, _ := git.PlainInit(dir, false)
 
-	r, err := os.Open("./test.private")
+	r, _ := os.Open("./test.private")
 	entityList, _ := openpgp.ReadArmoredKeyRing(r)
-	commitFileAndSign(remoteRepository, dir, "main", "file-1", entityList[0])
+	_, _ = commitFileAndSign(remoteRepository, dir, "main", "file-1", entityList[0])
 
 	failPublic, _ := os.ReadFile("./fail.public")
 	testPublic, _ := os.ReadFile("./test.public")
@@ -145,7 +145,7 @@ func TestHeadSignedBy(t *testing.T) {
 	assert.ErrorContains(t, err, "is not signed")
 	assert.Nil(t, signedBy)
 
-	commitFileAndSign(remoteRepository, dir, "main", "file-2", nil)
+	_, _ = commitFileAndSign(remoteRepository, dir, "main", "file-2", nil)
 	signedBy, err = headSignedBy(remoteRepository, []string{string(failPublic), string(testPublic)})
 	assert.ErrorContains(t, err, "is not signed")
 	assert.Nil(t, signedBy)