Merge pull request #58 from volcano-sh/copilot/add-golangci-lint-check

Add golangci-lint CI check

Author: volcano-sh-bot

.github/workflows/lint.yml

@@ -0,0 +1,34 @@
+name: Lint
+
+on:
+  pull_request:
+    branches: 
+      - main
+      - 'release-*'
+    paths:
+      - '**.go'
+      - 'go.mod'
+      - 'go.sum'
+      - '.golangci.yml'
+      - '.github/workflows/lint.yml'
+
+permissions:
+  contents: read
+  # Optional: allow read access to pull request. Use with `only-new-issues` option.
+  pull-requests: read
+
+jobs:
+  golangci:
+    name: golangci-lint
+    runs-on: ubuntu-24.04
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+
+      - name: Set up Go
+        uses: actions/setup-go@v5
+        with:
+          go-version: '1.24'
+
+      - name: Run golangci-lint
+        run: make lint

.golangci.yml

@@ -0,0 +1,55 @@
+# golangci-lint configuration file
+# Documentation: https://golangci-lint.run/usage/configuration/
+
+version: 2
+
+run:
+  # Timeout for analysis
+  timeout: 5m
+
+  # Exit code when at least one issue was found
+  issues-exit-code: 1
+
+  # Include test files
+  tests: true
+
+# output configuration options
+output:
+  formats:
+    - format: colored-line-number
+  print-issued-lines: true
+  print-linter-name: true
+  sort-results: true
+
+# all available settings of specific linters
+linters-settings:
+  errcheck:
+    check-type-assertions: true
+    check-blank: false
+  gocyclo:
+    min-complexity: 15
+  misspell:
+    locale: US
+  unused:
+    check-exported: false
+  unparam:
+    check-exported: false
+  staticcheck:
+    checks: ["all"]
+
+linters:
+  disable:
+    - gosimple
+  enable:
+    - errcheck
+    - govet
+    - ineffassign
+    - staticcheck
+    - unused
+    - misspell
+    - revive
+    - gosec
+    - unconvert
+    - unparam
+    - goconst
+    - gocyclo

Makefile

@@ -132,9 +132,9 @@ vet: ## Run go vet against code.
 	go vet ./...
 
 # Run linter
-lint:
-	@echo "Running linter..."
-	golangci-lint run ./...
+.PHONY: lint
+lint: golangci-lint ## Run golangci-lint
+	$(GOLANGCI_LINT) run ./...
 
 # Install to system
 install: build
@@ -273,15 +273,22 @@ $(LOCALBIN):
 
 ## Tool Binaries
 CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen
+GOLANGCI_LINT ?= $(LOCALBIN)/golangci-lint
 
 ## Tool Versions
 CONTROLLER_TOOLS_VERSION ?= v0.17.2
+GOLANGCI_LINT_VERSION ?= v1.64.1
 
 .PHONY: controller-gen
 controller-gen: $(CONTROLLER_GEN) ## Download controller-gen locally if necessary.
 $(CONTROLLER_GEN): $(LOCALBIN)
 	$(call go-install-tool,$(CONTROLLER_GEN),sigs.k8s.io/controller-tools/cmd/controller-gen,$(CONTROLLER_TOOLS_VERSION))
 
+.PHONY: golangci-lint
+golangci-lint: $(GOLANGCI_LINT) ## Download golangci-lint locally if necessary.
+$(GOLANGCI_LINT): $(LOCALBIN)
+	$(call go-install-tool,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/cmd/golangci-lint,$(GOLANGCI_LINT_VERSION))
+
 # go-install-tool will 'go install' any package with custom target and name of binary, if it doesn't exist
 # $1 - target path with name of binary
 # $2 - package url which can be installed

cmd/agentd/main.go

@@ -39,7 +39,7 @@ func main() {
 
 	err = ctrl.NewControllerManagedBy(mgr).
 		For(&sandboxv1alpha1.Sandbox{}).
-		Complete(&agentd.AgentdReconciler{
+		Complete(&agentd.Reconciler{
 			Client: mgr.GetClient(),
 			Scheme: mgr.GetScheme(),
 		})

pkg/agentd/agentd.go

@@ -17,13 +17,13 @@ var (
 	SessionExpirationTimeout = 15 * time.Minute
 )
 
-// AgentdReconciler reconciles a Sandbox object
-type AgentdReconciler struct {
+// Reconciler reconciles a Sandbox object
+type Reconciler struct {
 	client.Client
 	Scheme *runtime.Scheme
 }
 
-func (r *AgentdReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	sandbox := &sandboxv1alpha1.Sandbox{}
 	err := r.Get(ctx, req.NamespacedName, sandbox)
 	if err != nil {
@@ -58,7 +58,7 @@ func (r *AgentdReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 }
 
 // SetupWithManager sets up the controller with the Manager.
-func (r *AgentdReconciler) SetupWithManager(mgr ctrl.Manager) error {
+func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&sandboxv1alpha1.Sandbox{}).
 		Complete(r)

pkg/agentd/agentd_test.go

@@ -17,7 +17,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
-func TestAgentdReconciler_Reconcile_WithLastActivity(t *testing.T) {
+func TestReconciler_Reconcile_WithLastActivity(t *testing.T) {
 	testScheme := runtime.NewScheme()
 	utilruntime.Must(scheme.AddToScheme(testScheme))
 	utilruntime.Must(sandboxv1alpha1.AddToScheme(testScheme))
@@ -107,7 +107,7 @@ func TestAgentdReconciler_Reconcile_WithLastActivity(t *testing.T) {
 				WithObjects(tt.sandbox).
 				Build()
 
-			reconciler := &AgentdReconciler{
+			reconciler := &Reconciler{
 				Client: fakeClient,
 				Scheme: testScheme,
 			}

pkg/redis/client.go

@@ -325,71 +325,6 @@ func (c *client) ListInactiveSandboxes(ctx context.Context, before time.Time, li
 	return c.loadSandboxesBySessionIDs(ctx, ids)
 }
 
-// loadSandboxesByIDs loads sandbox objects for the given sandbox IDs.
-func (c *client) loadSandboxesByIDs(ctx context.Context, sandboxIDs []string) ([]*types.SandboxRedis, error) {
-	if len(sandboxIDs) == 0 {
-		return nil, nil
-	}
-
-	sessionIDCmds := make([]*redisv9.StringCmd, len(sandboxIDs))
-	pipe := c.rdb.Pipeline()
-	for i, id := range sandboxIDs {
-		sessionKey := c.sandboxKey(id)
-		sessionIDCmds[i] = pipe.Get(ctx, sessionKey)
-	}
-	_, _ = pipe.Exec(ctx)
-
-	type pair struct {
-		sandboxID string
-		sessionID string
-	}
-	pairs := make([]pair, 0, len(sandboxIDs))
-
-	for i, cmd := range sessionIDCmds {
-		sessionID, err := cmd.Result()
-		if errors.Is(err, redisv9.Nil) {
-			continue
-		}
-		if err != nil {
-			return nil, fmt.Errorf("loadSandboxesByIDs: get sessionID for sandbox %s: %w", sandboxIDs[i], err)
-		}
-		pairs = append(pairs, pair{
-			sandboxID: sandboxIDs[i],
-			sessionID: sessionID,
-		})
-	}
-
-	if len(pairs) == 0 {
-		return nil, nil
-	}
-
-	sandboxCmds := make([]*redisv9.StringCmd, len(pairs))
-	pipe = c.rdb.Pipeline()
-	for i, p := range pairs {
-		sessionKey := c.sessionKey(p.sessionID)
-		sandboxCmds[i] = pipe.Get(ctx, sessionKey)
-	}
-	_, _ = pipe.Exec(ctx)
-
-	result := make([]*types.SandboxRedis, 0, len(pairs))
-	for i, cmd := range sandboxCmds {
-		data, err := cmd.Bytes()
-		if errors.Is(err, redisv9.Nil) {
-			continue
-		}
-		if err != nil {
-			return nil, fmt.Errorf("loadSandboxesByIDs: get sandbox JSON for session %s: %w", pairs[i].sessionID, err)
-		}
-		var sandboxRedis types.SandboxRedis
-		if err := json.Unmarshal(data, &sandboxRedis); err != nil {
-			return nil, fmt.Errorf("loadSandboxesByIDs: unmarshal sandbox for session %s: %w", pairs[i].sessionID, err)
-		}
-		result = append(result, &sandboxRedis)
-	}
-
-	return result, nil
-}
-
 // loadSandboxesBySessionIDs loads sandbox objects for the given session IDs.
 func (c *client) loadSandboxesBySessionIDs(ctx context.Context, sessionIDs []string) ([]*types.SandboxRedis, error) {
 	if len(sessionIDs) == 0 {

pkg/workloadmanager/auth.go

@@ -176,12 +176,6 @@ func (s *Server) validateServiceAccountToken(ctx context.Context, token string)
 	return true, username, nil
 }
 
-// checkSandboxAccess checks if the current user has access to the sandbox
-// Returns true if the user is the creator of the sandbox
-func (s *Server) checkSandboxAccess(sandbox *Sandbox, serviceAccountName string) bool {
-	return sandbox.CreatorServiceAccount == serviceAccountName
-}
-
 // extractUserInfo extracts user information from request context
 // Returns userToken, userNamespace, serviceAccount, serviceAccountName
 // If extraction fails, returns empty strings

pkg/workloadmanager/client_cache.go

@@ -81,7 +81,7 @@ func NewClientCache(maxSize int) *ClientCache {
 // Get retrieves a client from cache based on key (service account)
 // Returns the client if found and cached token is not expired, nil otherwise
 // Different tokens for the same service account can share the same client
-func (c *ClientCache) Get(key, token string) *UserK8sClient {
+func (c *ClientCache) Get(key string) *UserK8sClient {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
@@ -158,6 +158,7 @@ func (c *ClientCache) evictOldest() {
 		return
 	}
 
+	// nolint:errcheck
 	entry := back.Value.(*clientCacheEntry)
 	c.lruList.Remove(back)
 	delete(c.cache, entry.key)
@@ -280,7 +281,7 @@ func (c *TokenCache) evictOldest() {
 	if back == nil {
 		return
 	}
-
+	// nolint:errcheck
 	entry := back.Value.(*tokenCacheEntry)
 	c.lruList.Remove(back)
 	delete(c.cache, entry.token)

pkg/workloadmanager/codeinterpreter_controller.go

@@ -278,7 +278,7 @@ func (r *CodeInterpreterReconciler) deleteSandboxTemplate(ctx context.Context, c
 }
 
 // convertToPodTemplate converts CodeInterpreterSandboxTemplate to sandboxv1alpha1.PodTemplate
-func (r *CodeInterpreterReconciler) convertToPodTemplate(template *runtimev1alpha1.CodeInterpreterSandboxTemplate, ci *runtimev1alpha1.CodeInterpreter) sandboxv1alpha1.PodTemplate {
+func (r *CodeInterpreterReconciler) convertToPodTemplate(template *runtimev1alpha1.CodeInterpreterSandboxTemplate, _ *runtimev1alpha1.CodeInterpreter) sandboxv1alpha1.PodTemplate {
 	// Build pod spec
 	podSpec := corev1.PodSpec{
 		Containers: []corev1.Container{