ci: golangci-lint workflow and lint fixes

.github/workflows/go-checks.yml

@@ -0,0 +1,21 @@
+name: Go Checks
+
+on:
+  pull_request: {}
+  push:
+    branches:
+      - main
+
+jobs:
+  golangci:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v5
+      - uses: actions/setup-go@v5
+        with:
+          go-version-file: 'go.mod'
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v8
+        with:
+          version: v2.4

.gitignore

@@ -25,3 +25,4 @@ dist/
 clab-*
 tests/robot/*-yang-models/
 .idea/
+.golangci-lint-cache/

.golangci.yml

@@ -0,0 +1,8 @@
+# https://golangci-lint.run/docs/configuration/file/
+version: "2"
+
+linters:
+  exclusions:
+    paths:
+      # ygot-generated schema (see file header); do not lint.
+      - tests/sdcioygot/sdcio_schema.go

client/cmd/data_blameConfig.go

@@ -3,7 +3,6 @@ package cmd
 import (
 	"context"
 	"fmt"
-	"os"
 
 	sdcpb "github.com/sdcio/sdc-protos/sdcpb"
 	"github.com/spf13/cobra"
@@ -43,7 +42,7 @@ var dataBlameConfig = &cobra.Command{
 			}
 			b, err := opts.Marshal(rsp.ConfigTree)
 			if err != nil {
-				fmt.Fprintf(os.Stdout, "%v", err)
+				return err
 			}
 			fmt.Println(string(b))
 		}

client/cmd/datastore_get.go

@@ -73,8 +73,8 @@ func printDataStoreTable(rsp *sdcpb.GetDataStoreResponse) {
 	table := tablewriter.NewWriter(os.Stdout)
 	table.Header([]string{"Name", "Schema", "Protocol", "Address", "State"})
 	table.Options(tablewriter.WithAlignment(tw.Alignment{tw.AlignLeft}))
-	table.Bulk(toTableData(rsp))
-	table.Render()
+	_ = table.Bulk(toTableData(rsp))
+	_ = table.Render()
 }
 
 func toTableData(rsp *sdcpb.GetDataStoreResponse) [][]string {

client/cmd/datastore_list.go

@@ -78,6 +78,6 @@ func printDataStoresTable(rsp *sdcpb.ListDataStoreResponse) {
 	table := tablewriter.NewWriter(os.Stdout)
 	table.Header([]string{"Name", "Schema", "Protocol", "Address", "State", "Candidate (C/O/P)"})
 	table.Options(tablewriter.WithAlignment(tw.Alignment{tw.AlignLeft}))
-	table.Bulk(tableData)
-	table.Render()
+	_ = table.Bulk(tableData)
+	_ = table.Render()
 }

pkg/datastore/datastore_rpc.go

@@ -21,7 +21,6 @@ import (
 	"sync"
 	"time"
 
-	"github.com/sdcio/logger"
 	logf "github.com/sdcio/logger"
 	sdcpb "github.com/sdcio/sdc-protos/sdcpb"
 	"google.golang.org/grpc"
@@ -229,8 +228,8 @@ func (d *Datastore) Stop(ctx context.Context) error {
 }
 
 func (d *Datastore) BlameConfig(ctx context.Context, includeDefaults bool) (*sdcpb.BlameTreeElement, error) {
-	log := logger.FromContext(ctx).WithName("BlameConfig")
-	ctx = logger.IntoContext(ctx, log)
+	log := logf.FromContext(ctx).WithName("BlameConfig")
+	ctx = logf.IntoContext(ctx, log)
 
 	// create a new TreeRoot by copying the syncTree
 	d.syncTreeMutex.RLock()
@@ -247,6 +246,9 @@ func (d *Datastore) BlameConfig(ctx context.Context, includeDefaults bool) (*sdc
 
 	bcp := processors.NewBlameConfigProcessor(&processors.BlameConfigProcessorParams{IncludeDefaults: includeDefaults})
 	bte, err := bcp.Run(ctx, root.Entry, d.taskPool)
+	if err != nil {
+		return nil, err
+	}
 
 	// set the root level elements name to the target name
 	bte.Name = d.config.Name

pkg/datastore/deviations.go

@@ -7,7 +7,6 @@ import (
 	"github.com/sdcio/data-server/pkg/config"
 	"github.com/sdcio/data-server/pkg/tree/ops"
 	treetypes "github.com/sdcio/data-server/pkg/tree/types"
-	"github.com/sdcio/logger"
 	logf "github.com/sdcio/logger"
 	sdcpb "github.com/sdcio/sdc-protos/sdcpb"
 	"google.golang.org/grpc/codes"
@@ -180,7 +179,7 @@ type DeviationEntry interface {
 
 func (d *Datastore) calculateDeviations(ctx context.Context) (<-chan *treetypes.DeviationEntry, error) {
 
-	log := logger.FromContext(ctx)
+	log := logf.FromContext(ctx)
 
 	d.syncTreeMutex.RLock()
 	deviationTree, err := d.syncTree.DeepCopy(ctx)
@@ -199,7 +198,7 @@ func (d *Datastore) calculateDeviations(ctx context.Context) (<-chan *treetypes.
 		return nil, err
 	}
 
-	if log := log.V(logger.VTrace); log.Enabled() {
+	if log := log.V(logf.VTrace); log.Enabled() {
 		log.Info("deviation tree", "content", deviationTree.String())
 		log.Info("nonrevertive infos", "data", deviationTree.GetTreeContext().NonRevertiveInfo().String())
 	}

pkg/datastore/sync_test.go

@@ -80,7 +80,9 @@ func TestApplyToRunning(t *testing.T) {
 				}
 
 				var v any
-				json.Unmarshal([]byte(confStr), &v)
+				if err := json.Unmarshal([]byte(confStr), &v); err != nil {
+					t.Fatalf("unmarshal test config: %v", err)
+				}
 
 				vpf := pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0))
 				_, err = root.ImportConfig(ctx, &sdcpb.Path{}, jsonImporter.NewJsonTreeImporter(v, consts.RunningIntentName, consts.RunningValuesPrio, false), types.NewUpdateInsertFlags(), vpf)
@@ -109,7 +111,9 @@ func TestApplyToRunning(t *testing.T) {
 				}
 
 				var v any
-				json.Unmarshal([]byte(confStr), &v)
+				if err := json.Unmarshal([]byte(confStr), &v); err != nil {
+					t.Fatalf("unmarshal test config: %v", err)
+				}
 
 				return jsonImporter.NewJsonTreeImporter(v, consts.RunningIntentName, consts.RunningValuesPrio, false)
 			},
@@ -131,7 +135,9 @@ func TestApplyToRunning(t *testing.T) {
 				}
 
 				var v any
-				json.Unmarshal([]byte(confStr), &v)
+				if err := json.Unmarshal([]byte(confStr), &v); err != nil {
+					t.Fatalf("unmarshal test config: %v", err)
+				}
 				return v
 			},
 			wantErr: false,
@@ -186,7 +192,9 @@ func TestApplyToRunning(t *testing.T) {
 				}
 
 				var v any
-				json.Unmarshal([]byte(confStr), &v)
+				if err := json.Unmarshal([]byte(confStr), &v); err != nil {
+					t.Fatalf("unmarshal test config: %v", err)
+				}
 
 				vpf := pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0))
 				_, err = root.ImportConfig(ctx, &sdcpb.Path{}, jsonImporter.NewJsonTreeImporter(v, consts.RunningIntentName, consts.RunningValuesPrio, false), types.NewUpdateInsertFlags(), vpf)
@@ -214,7 +222,9 @@ func TestApplyToRunning(t *testing.T) {
 				}
 
 				var v any
-				json.Unmarshal([]byte(confStr), &v)
+				if err := json.Unmarshal([]byte(confStr), &v); err != nil {
+					t.Fatalf("unmarshal test config: %v", err)
+				}
 
 				return jsonImporter.NewJsonTreeImporter(v, consts.RunningIntentName, consts.RunningValuesPrio, false)
 			},
@@ -235,7 +245,9 @@ func TestApplyToRunning(t *testing.T) {
 				}
 
 				var v any
-				json.Unmarshal([]byte(confStr), &v)
+				if err := json.Unmarshal([]byte(confStr), &v); err != nil {
+					t.Fatalf("unmarshal test config: %v", err)
+				}
 				return v
 			},
 			wantErr: false,
@@ -290,7 +302,9 @@ func TestApplyToRunning(t *testing.T) {
 				}
 
 				var v any
-				json.Unmarshal([]byte(confStr), &v)
+				if err := json.Unmarshal([]byte(confStr), &v); err != nil {
+					t.Fatalf("unmarshal test config: %v", err)
+				}
 
 				vpf := pool.NewSharedTaskPool(ctx, runtime.GOMAXPROCS(0))
 				_, err = root.ImportConfig(ctx, &sdcpb.Path{}, jsonImporter.NewJsonTreeImporter(v, consts.RunningIntentName, consts.RunningValuesPrio, false), types.NewUpdateInsertFlags(), vpf)
@@ -319,7 +333,9 @@ func TestApplyToRunning(t *testing.T) {
 				}
 
 				var v any
-				json.Unmarshal([]byte(confStr), &v)
+				if err := json.Unmarshal([]byte(confStr), &v); err != nil {
+					t.Fatalf("unmarshal test config: %v", err)
+				}
 
 				return jsonImporter.NewJsonTreeImporter(v, consts.RunningIntentName, consts.RunningValuesPrio, false)
 			},
@@ -341,7 +357,9 @@ func TestApplyToRunning(t *testing.T) {
 				}
 
 				var v any
-				json.Unmarshal([]byte(confStr), &v)
+				if err := json.Unmarshal([]byte(confStr), &v); err != nil {
+					t.Fatalf("unmarshal test config: %v", err)
+				}
 				return v
 			},
 			wantErr: false,

pkg/datastore/target/gnmi/gnmi.go

@@ -30,7 +30,6 @@ import (
 	targetTypes "github.com/sdcio/data-server/pkg/datastore/target/types"
 	"github.com/sdcio/data-server/pkg/pool"
 	"github.com/sdcio/data-server/pkg/utils"
-	dsutils "github.com/sdcio/data-server/pkg/utils"
 	logf "github.com/sdcio/logger"
 	sdcpb "github.com/sdcio/sdc-protos/sdcpb"
 
@@ -47,11 +46,11 @@ type gnmiTarget struct {
 	cfg             *config.SBI
 	syncs           map[string]GnmiSync
 	runningStore    targetTypes.RunningStore
-	schemaClient    dsutils.SchemaClientBound
+	schemaClient    utils.SchemaClientBound
 	taskpoolFactory pool.VirtualPoolFactory
 }
 
-func NewTarget(ctx context.Context, name string, cfg *config.SBI, runningStore targetTypes.RunningStore, schemaClient dsutils.SchemaClientBound, taskpoolFactory pool.VirtualPoolFactory, opts ...grpc.DialOption) (*gnmiTarget, error) {
+func NewTarget(ctx context.Context, name string, cfg *config.SBI, runningStore targetTypes.RunningStore, schemaClient utils.SchemaClientBound, taskpoolFactory pool.VirtualPoolFactory, opts ...grpc.DialOption) (*gnmiTarget, error) {
 	tc := &types.TargetConfig{
 		Name:       name,
 		Address:    fmt.Sprintf("%s:%d", cfg.Address, cfg.Port),

pkg/datastore/target/gnmi/stream.go

@@ -222,7 +222,12 @@ func (s *StreamSync) gnmiSubscribe(subReq *gnmi.SubscribeRequest, updChan chan<-
 				}
 
 			case *gnmi.SubscribeResponse_Error:
-				log.Error(nil, "gnmi subscription error", "error", r.Error.Message)
+				subscribeErr := r.Error //nolint:staticcheck // gnmi SubscribeResponse_Error deprecated upstream without replacement.
+				msg := ""
+				if subscribeErr != nil {
+					msg = subscribeErr.GetMessage()
+				}
+				log.Error(nil, "gnmi subscription error", "error", msg)
 			}
 		}
 	}

pkg/datastore/target/netconf/nc.go

@@ -131,7 +131,7 @@ func (t *ncTarget) internalGet(ctx context.Context, req *sdcpb.GetDataRequest) (
 	ncResponse, err := t.driver.GetConfig(source, filterDoc)
 	if err != nil {
 		if strings.Contains(err.Error(), "EOF") {
-			t.Close(ctx)
+			_ = t.Close(ctx)
 			go t.reconnect(ctx)
 		}
 		return nil, err
@@ -262,7 +262,7 @@ func (t *ncTarget) setToDevice(ctx context.Context, commitDatastore string, sour
 	if err != nil {
 		log.Error(err, "failed during edit-config")
 		if strings.Contains(err.Error(), "EOF") {
-			t.Close(ctx)
+			_ = t.Close(ctx)
 			go t.reconnect(ctx)
 			return nil, err
 		}
@@ -289,7 +289,7 @@ func (t *ncTarget) setToDevice(ctx context.Context, commitDatastore string, sour
 		err = t.driver.Commit()
 		if err != nil {
 			if strings.Contains(err.Error(), "EOF") {
-				t.Close(ctx)
+				_ = t.Close(ctx)
 				go t.reconnect(ctx)
 			}
 			return nil, err

pkg/datastore/target/netconf/nc_test.go

@@ -290,7 +290,7 @@ func TestLeafList(t *testing.T) {
 		HonorNamespace:     true,
 	})
 
-	xmlBuilder.AddValue(ctx, &sdcpb.Path{
+	if err := xmlBuilder.AddValue(ctx, &sdcpb.Path{
 		Elem: []*sdcpb.PathElem{
 			{
 				Name: "leaflist",
@@ -299,7 +299,9 @@ func TestLeafList(t *testing.T) {
 				Name: "entry",
 			},
 		},
-	}, leaflistValue)
+	}, leaflistValue); err != nil {
+		t.Fatal(err)
+	}
 
 	expectedResult := `<leaflist xmlns="urn:sdcio/model" operation="replace">
   <entry>entry-one</entry>
@@ -367,7 +369,9 @@ func Test_filterRPCErrors(t *testing.T) {
 	`
 
 	doc := etree.NewDocument()
-	doc.ReadFromString(xml)
+	if err := doc.ReadFromString(xml); err != nil {
+		t.Fatal(err)
+	}
 
 	type args struct {
 		xml      *etree.Document

pkg/datastore/target/netconf/sync.go

@@ -70,7 +70,7 @@ func (s *NetconfSyncImpl) Start() error {
 		return nil
 	}
 
-	go s.internalSync(req)
+	go func() { _ = s.internalSync(req) }()
 
 	go func() {
 		ticker := time.NewTicker(s.config.Interval)

pkg/datastore/target/noop/noop.go

@@ -50,9 +50,6 @@ func (t *noopTarget) AddSyncs(ctx context.Context, sps ...*config.SyncProtocol)
 }
 
 func (t *noopTarget) Get(ctx context.Context, req *sdcpb.GetDataRequest) (*sdcpb.GetDataResponse, error) {
-	log := logf.FromContext(ctx).WithName("Get")
-	ctx = logf.IntoContext(ctx, log)
-
 	result := &sdcpb.GetDataResponse{
 		Notification: make([]*sdcpb.Notification, 0, len(req.GetPath())),
 	}

pkg/datastore/transaction_rpc_test.go

@@ -54,7 +54,9 @@ func TestTransactionSet_PreviouslyApplied(t *testing.T) {
 		t.Fatalf("failed to marshal running config: %v", err)
 	}
 	var runningAny any
-	json.Unmarshal([]byte(runningJson), &runningAny)
+	if err := json.Unmarshal([]byte(runningJson), &runningAny); err != nil {
+		t.Fatalf("unmarshal running config: %v", err)
+	}
 
 	// Setup Intent Data (Same as Running)
 	intentStrSame := runningJson // Same content

pkg/datastore/types/transaction.go

@@ -63,7 +63,7 @@ func (t *Transaction) Confirm() error {
 
 func (t *Transaction) rollback(ctx context.Context) func() {
 	return func() {
-		t.transactionManager.Rollback(ctx, t.GetRollbackTransaction())
+		_ = t.transactionManager.Rollback(ctx, t.GetRollbackTransaction())
 	}
 }
 
@@ -94,7 +94,7 @@ func (t *Transaction) GetRollbackTransaction() *Transaction {
 	t.timer.Stop()
 	tr := NewTransaction(t.GetTransactionId()+" - Rollback", t.transactionManager)
 	for _, v := range t.oldIntents {
-		tr.AddTransactionIntent(v, TransactionIntentNew)
+		_ = tr.AddTransactionIntent(v, TransactionIntentNew)
 	}
 	tr.isRollback = true
 	return tr

pkg/pool/virtual_pool.go

@@ -88,8 +88,6 @@ const (
 // creation of VirtualPools that submit into the shared pool.
 type SharedTaskPool struct {
 	inner *Pool[Task]
-
-	mu sync.RWMutex
 }
 
 // NewSharedTaskPool constructs a shared pool; caller should call Start() to begin workers.

pkg/pool/virtual_pool_test.go

@@ -183,14 +183,14 @@ func TestVirtualPool_CancellationHang(t *testing.T) {
 	wg.Add(2)
 
 	// Task 1: blocks until cancelled
-	vp.SubmitFunc(func(ctx context.Context, submit func(Task) error) error {
+	_ = vp.SubmitFunc(func(ctx context.Context, submit func(Task) error) error {
 		defer wg.Done()
 		<-ctx.Done()
 		return nil
 	})
 
 	// Task 2: should run even if cancelled (to decrement inflight)
-	vp.SubmitFunc(func(ctx context.Context, submit func(Task) error) error {
+	_ = vp.SubmitFunc(func(ctx context.Context, submit func(Task) error) error {
 		defer wg.Done()
 		return nil
 	})