Skip to content

Conversation

ivanvc
Copy link
Member

@ivanvc ivanvc commented Sep 30, 2025

Alternative to depguard (#20721). Follow up on #20721 (comment).

Uses gomodguard stand-alone, as we can't hook it with golangci-lint, as it doesn't support receiving a list of modules or files to check.

I verified the linter fails, by doing #19423 (comment):

diff --git a/api/go.mod b/api/go.mod
index 2a825b489..d2d5f0d62 100644
--- a/api/go.mod
+++ b/api/go.mod
@@ -10,6 +10,7 @@ require (
        github.com/golang/protobuf v1.5.4
        github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.2
        github.com/stretchr/testify v1.11.1
+       go.etcd.io/etcd/pkg/v3 v3.6.5
        google.golang.org/genproto/googleapis/api v0.0.0-20250825161204-c5933d9347a5
        google.golang.org/grpc v1.75.1
        google.golang.org/protobuf v1.36.8
diff --git a/api/go.sum b/api/go.sum
index f14bec486..0c03985b4 100644
--- a/api/go.sum
+++ b/api/go.sum
@@ -31,6 +31,8 @@ github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu
 github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
+go.etcd.io/etcd/pkg/v3 v3.6.5 h1:byxWB4AqIKI4SBmquZUG1WGtvMfMaorXFoCcFbVeoxM=
+go.etcd.io/etcd/pkg/v3 v3.6.5/go.mod h1:uqrXrzmMIJDEy5j00bCqhVLzR5jEJIwDp5wTlLwPGOU=
 go.opentelemetry.io/auto/sdk v1.1.0 h1:cH53jehLUN6UFLY71z+NDOiNJqDdPRaXzTel0sJySYA=
 go.opentelemetry.io/auto/sdk v1.1.0/go.mod h1:3wSPjt5PWp2RhlCcmmOial7AvC4DQqZb7a7wCow3W8A=
 go.opentelemetry.io/otel v1.37.0 h1:9zhNfelUvx0KBfu/gb+ZgeAfAgtWrfHJZcAqFC228wQ=
diff --git a/api/version/version.go b/api/version/version.go
index 31ca2f14d..f469a01fa 100644
--- a/api/version/version.go
+++ b/api/version/version.go
@@ -21,6 +21,8 @@ import (
        "strings"
 
        "github.com/coreos/go-semver/semver"
+
+       "go.etcd.io/etcd/pkg/v3/httputil"
 )
 
 var (
@@ -66,6 +68,7 @@ type Versions struct {
 
 // Cluster only keeps the major.minor.
 func Cluster(v string) string {
+       httputil.GracefulClose(nil)
        vs := strings.Split(v, ".")
        if len(vs) <= 2 {
                return v

Result:

PASSES="gomodguard" ./scripts/test.sh
Running with --race
Starting at: Tue Sep 30 11:45:54 AM PDT 2025

'gomodguard' started at Tue Sep 30 11:45:54 AM PDT 2025
% (cd tools/mod && 'go' 'install' 'github.com/ryancurrah/gomodguard/cmd/gomodguard')
% (cd api && '/home/ivan/.local/share/asdf/installs/golang/1.25.0/packages/bin/gomodguard')
stderr: info: allowed modules, []
stderr: info: allowed module domains, []
stderr: info: blocked modules, [go.etcd.io/etcd/api/v3 go.etcd.io/etcd/pkg/v3 go.etcd.io/etcd/tests/v3 go.etcd.io/etcd/v3]
stderr: info: blocked modules with version constraints, []
version/version.go:25:1 import of package `go.etcd.io/etcd/pkg/v3/httputil` is blocked because the module is in the blocked modules list. Forbidden dependency.
FAIL: (code:2):
  % (cd api && '/home/ivan/.local/share/asdf/installs/golang/1.25.0/packages/bin/gomodguard')
There was a Failure in module api, aborting...
FAIL: 'gomodguard' FAILED at Tue Sep 30 11:45:54 AM PDT 2025
make: *** [Makefile:238: verify-gomodguard] Error 255

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

@ivanvc
Copy link
Member Author

ivanvc commented Sep 30, 2025

/cc @ahrtr @serathius

/hold
Please don't merge. If we decide to go this route, I'd like to update the commit messages before merging.

Copy link

codecov bot commented Sep 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.17%. Comparing base (bc6a17c) to head (3062d2b).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files

see 29 files with indirect coverage changes

@@            Coverage Diff             @@
##             main   #20748      +/-   ##
==========================================
+ Coverage   69.08%   69.17%   +0.08%     
==========================================
  Files         422      422              
  Lines       34817    34819       +2     
==========================================
+ Hits        24054    24086      +32     
+ Misses       9360     9336      -24     
+ Partials     1403     1397       -6     

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bc6a17c...3062d2b. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ivanvc ivanvc force-pushed the use-gomodguard-to-forbid-dependencies branch from 5fb4af4 to 5c2cef3 Compare September 30, 2025 19:28
@ahrtr
Copy link
Member

ahrtr commented Oct 1, 2025

I like this PR, and it is much better than #20721.

One question: I see that https://github.com/ryancurrah/gomodguard has already been integrated into golangci-lint. Is it possible to define all the rules in .golangci.yaml? Note: I am not asking you to make the change. It's just a pure question for curiosity.

@ivanvc
Copy link
Member Author

ivanvc commented Oct 1, 2025

One question: I see that https://github.com/ryancurrah/gomodguard has already been integrated into golangci-lint. Is it possible to define all the rules in .golangci.yaml?

Please refer to #20721 (comment)

  • There's another linter, gomodguard. It's available through golangci-lint. But it has the same issue as revive.

    • It works at a module level per its documentation

      The linter looks for blocked modules in go.mod and searches for imported packages where the imported packages module is blocked. Indirect modules are not considered.

    • As it doesn't have a way to configure paths we would need to use it stand-alone, we can add a .gomodguard.yaml in each module.

      • Pro: This is very similar to how we currently do it

      • Cons:

        1. Adds a new linter to execute, as it needs to be called outside of golangci-lint (maybe not a con, but a change we need)
        2. This linter wouldn't be possible to call from the top-level module once we integrate the Go workspace.

@ivanvc ivanvc force-pushed the use-gomodguard-to-forbid-dependencies branch from 5c2cef3 to 3062d2b Compare October 2, 2025 22:12
@fuweid
Copy link
Member

fuweid commented Oct 2, 2025

+1 for this change. that yaml configuration looks clean and straightforward.

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thanks for keeping driving this. Great work!

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahrtr, fuweid, ivanvc

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [ahrtr,fuweid,ivanvc]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ahrtr
Copy link
Member

ahrtr commented Oct 3, 2025

As discussed in yesterday community, merging this PR, thx

@ahrtr ahrtr merged commit da267f1 into etcd-io:main Oct 3, 2025
31 checks passed
@ivanvc ivanvc deleted the use-gomodguard-to-forbid-dependencies branch October 3, 2025 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants