Merge branch 'main' into add_drpc_metrics

Author: suj-krishnan

.github/workflows/ci.yaml

@@ -33,7 +33,7 @@ jobs:
           go-version: '1.25.x'
       - name: Install staticcheck
         run: go install honnef.co/go/tools/cmd/staticcheck@latest
-      - uses: golangci/golangci-lint-action@v4
+      - uses: golangci/golangci-lint-action@v7
         with:
           version: latest
       - name: Lint

.golangci.yml

@@ -1,17 +1,17 @@
+version: "2"
+
 run:
-  deadline: 10m
+  timeout: 10m
   issues-exit-code: 1
   tests: true
 
-
 linters:
   enable:
     - bodyclose               # find unclosed http response bodies
     - dogsled                 # checks for too many ignored arguments
     - durationcheck           # verifies whether durations are multiplied, usually a mistake
     - gocritic                # checks for style, performance issues, and common programming errors
-    - gofmt                   # sanity check formatting
-    - goprintffuncname        # checks that printf-like functions are named with `f` at the end [fast: true, auto-fix: false]
+    - goprintffuncname        # checks that printf-like functions are named with `f` at the end
     - govet                   # check standard vet rules
     - importas                # verify that imports are consistent
     - ineffassign             # find ineffective assignments
@@ -44,114 +44,64 @@ linters:
     - exhaustive       # doesn't handle default case
     - forbidigo        # not useful
     - funlen           # no limit on func length
-    - gci              # we have custom import checking
     - gocognit         # this complexity is not a good metric
     - goconst          # check for things that could be replaced by constants
     - gocyclo          # this complexity is not a good metric
     - godox            # too many false positivies
     - goheader         # separate tool
-    - goimports        # disabled, because it's slow, using scripts/check-imports.go instead.
     - gomoddirectives  # not useful
     - gomodguard       # not useful
     - gosec            # needs tweaking
-    - gosimple         # part of staticcheck
     - lll              # don't need this check
     - nlreturn         # non-important code style
     - paralleltest     # too many false positives
     - predeclared      # kind of useful, but not critical
     - promlinter       # not relevant
-    - rowserrcheck     # checks if sql.Rows.Err is checked correctly - Disabled  because it reports false positive with defer statements after Query call
     - sqlclosecheck    # we have tagsql, which checks this better
     - staticcheck      # we use the separate staticcheck binary already
-    - stylecheck       # has false positives
     - tagliatelle      # not our style
     - testpackage      # sometimes it's useful to have tests on private funcs
     - thelper          # too many false positives
     - tparallel        # false positivies
     - unused           # part of staticcheck
     - wrapcheck        # too much noise and false positives
     - wsl              # too much noise
-  fast: false
+  settings:
+    errcheck:
+      check-type-assertions: false
+      check-blank: false
+    govet: {}
+    gocritic:
+      disabled-checks:
+        - ifElseChain
+    gocyclo:
+      min-complexity: 10
+    dupl:
+      threshold: 150
+    goconst:
+      min-len: 3
+      min-occurrences: 3
+    lll:
+      line-length: 140
+      tab-width: 1
+    unparam:
+      check-exported: false
+    nakedret:
+      max-func-lines: 30
+    prealloc:
+      simple: true
+      range-loops: true
+      for-loops: false
+
+formatters:
+  enable:
+    - gofmt                   # sanity check formatting
 
 output:
   formats:
-    - format: colored-line-number
-  print-issued-lines: true
-  print-linter-name: true
-
-linters-settings:
-  errcheck:
-    # report about not checking of errors in type assetions: `a := b.(MyStruct)`;
-    # default is false: such cases aren't reported by default.
-    check-type-assertions: false
-
-    # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`;
-    # default is false: such cases aren't reported by default.
-    check-blank: false
-  govet:
-    # report about shadowed variables
-    #TODO# check-shadowing: true
-
-    # Obtain type information from installed (to $GOPATH/pkg) package files:
-    # golangci-lint will execute `go install -i` and `go test -i` for analyzed packages
-    # before analyzing them.
-    # Enable this option only if all conditions are met:
-    #  1. you use only "fast" linters (--fast e.g.): no program loading occurs
-    #  2. you use go >= 1.10
-    #  3. you do repeated runs (false for CI) or cache $GOPATH/pkg or `go env GOCACHE` dir in CI.
-    use-installed-packages: false
-  gocritic:
-    disabled-checks:
-      - ifElseChain
-  goimports:
-    local: "storj.io"
-  golint:
-    min-confidence: 0.8
-  gofmt:
-    simplify: true
-  gocyclo:
-    min-complexity: 10
-  dupl:
-    threshold: 150
-  goconst:
-    min-len: 3
-    min-occurrences: 3
-  misspell:
-  lll:
-    line-length: 140
-    tab-width: 1
-  unused:
-    # treat code as a program (not a library) and report unused exported identifiers; default is false.
-    # XXX: if you enable this setting, unused will report a lot of false-positives in text editors:
-    # if it's called for subdir of a project it can't find funcs usages. All text editor integrations
-    # with golangci-lint call it on a directory with the changed file.
-    check-exported: false
-  unparam:
-    # call graph construction algorithm (cha, rta). In general, use cha for libraries,
-    # and rta for programs with main packages. Default is cha.
-    algo: cha
-
-    # Inspect exported functions, default is false. Set to true if no external program/library imports your code.
-    # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors:
-    # if it's called for subdir of a project it can't find external interfaces. All text editor integrations
-    # with golangci-lint call it on a directory with the changed file.
-    check-exported: false
-  nakedret:
-    # make an issue if func has more lines of code than this setting and it has naked returns; default is 30
-    max-func-lines: 30
-  prealloc:
-    # Report preallocation suggestions only on simple loops that have no returns/breaks/continues/gotos in them.
-    # True by default.
-    simple: true
-    range-loops: true # Report preallocation suggestions on range loops, true by default
-    for-loops: false # Report preallocation suggestions on for loops, false by default
+    text: {}
 
 issues:
   max-issues-per-linter: 0
   max-same-issues: 0
   new: false
-  exclude-use-default: false
-  exclude-files:
-    - ".*\\.pb\\.go$"
-    - ".*\\.dbx\\.go$"
-    - "cmd/protoc-gen-go-drpc/.*"

drpcserver/server.go

@@ -34,6 +34,17 @@ type Options struct {
 	// CollectStats controls whether the server should collect stats on the
 	// rpcs it serves.
 	CollectStats bool
+  
+	// TLSConfig, if non-nil, is used to wrap the listener with tls.NewListener
+	// in Serve(). The TLS handshake is performed explicitly in ServeOne before
+	// processing requests.
+	TLSConfig *tls.Config
+
+	// TLSCipherRestrict, if non-nil, is called in ServeOne immediately after
+	// a successful TLS handshake. It receives the net.Conn (which is a
+	// *tls.Conn) and may inspect ConnectionState to enforce cipher suite
+	// restrictions. If it returns a non-nil error the connection is rejected.
+	TLSCipherRestrict func(conn net.Conn) error
 
 	// Metrics holds optional metrics the server will populate. If nil, no
 	// metrics are recorded.
@@ -151,6 +162,12 @@ func New(handler drpc.Handler) *Server {
 // NewWithOptions constructs a new Server using the provided options to tune
 // how the drpc connections are handled.
 func NewWithOptions(handler drpc.Handler, opts Options) *Server {
+	// Clone the TLS config so the server owns its copy and the caller cannot
+	// mutate it after construction.
+	if opts.TLSConfig != nil {
+		opts.TLSConfig = opts.TLSConfig.Clone()
+	}
+
 	s := &Server{
 		opts:    opts,
 		handler: handler,
@@ -206,11 +223,17 @@ func (s *Server) ServeOne(ctx context.Context, tr drpc.Transport) (err error) {
 		// interrupting any ongoing communication. Even if we didn't call it
 		// explicitly, the first read/write operation would call it internally
 		// anyway.
-		err := tlsConn.Handshake()
+		err := tlsConn.HandshakeContext(ctx)
 		if err != nil {
 			s.opts.Metrics.addTLSHandshakeError()
 			return drpc.ConnectionError.New("server handshake [%q] failed: %w", tlsConn.RemoteAddr(), err)
 		}
+		if s.opts.TLSCipherRestrict != nil {
+			if err := s.opts.TLSCipherRestrict(tlsConn); err != nil {
+        s.opts.Metrics.addTLSHandshakeError()
+				return drpc.ConnectionError.New("server handshake [%q] failed: %w", tlsConn.RemoteAddr(), err)
+			}
+		}
 		state := tlsConn.ConnectionState()
 		if len(state.PeerCertificates) > 0 {
 			ctx = drpcctx.WithPeerConnectionInfo(
@@ -247,6 +270,10 @@ var temporarySleep = 500 * time.Millisecond
 // Serve listens for connections on the listener and serves the drpc request
 // on new connections.
 func (s *Server) Serve(ctx context.Context, lis net.Listener) (err error) {
+	if s.opts.TLSConfig != nil {
+		lis = tls.NewListener(lis, s.opts.TLSConfig)
+	}
+
 	tracker := drpcctx.NewTracker(ctx)
 	defer tracker.Wait()
 	defer tracker.Cancel()