From 53e9681a39b51dcf32f6a8113ba68098e815eea7 Mon Sep 17 00:00:00 2001 From: Ville Vesilehto Date: Sat, 31 May 2025 01:29:32 +0300 Subject: [PATCH] lint: enable protogetter linter (#7336) Enable protogetter in golangci config and update all protobuf field access to use getter methods instead of direct field access. Getter methods provide safer nil pointer handling and return appropriate default values, following protobuf best practices. Signed-off-by: Ville Vesilehto --- .golangci.yml | 1 + core/dnsserver/server_grpc.go | 2 +- core/dnsserver/server_grpc_test.go | 4 +-- plugin/dnstap/handler_test.go | 50 +++++++++++++++++++----------- plugin/grpc/proxy.go | 2 +- plugin/test/scrape.go | 16 +++++----- test/grpc_test.go | 2 +- 7 files changed, 46 insertions(+), 31 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 997a2e3aa..fcd4d4217 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -8,6 +8,7 @@ linters: - ineffassign - intrange - nolintlint + - protogetter - staticcheck - unconvert - unused diff --git a/core/dnsserver/server_grpc.go b/core/dnsserver/server_grpc.go index 9d7a95ace..dbf85d77b 100644 --- a/core/dnsserver/server_grpc.go +++ b/core/dnsserver/server_grpc.go @@ -123,7 +123,7 @@ func (s *ServergRPC) Stop() (err error) { // back to the client as a protobuf. func (s *ServergRPC) Query(ctx context.Context, in *pb.DnsPacket) (*pb.DnsPacket, error) { msg := new(dns.Msg) - err := msg.Unpack(in.Msg) + err := msg.Unpack(in.GetMsg()) if err != nil { return nil, err } diff --git a/core/dnsserver/server_grpc_test.go b/core/dnsserver/server_grpc_test.go index b405cdbe5..5dc72b55b 100644 --- a/core/dnsserver/server_grpc_test.go +++ b/core/dnsserver/server_grpc_test.go @@ -209,13 +209,13 @@ func TestServergRPC_Query(t *testing.T) { t.Errorf("Query() failed: %v", err) } - if len(response.Msg) == 0 { + if len(response.GetMsg()) == 0 { t.Error("Query() returned empty message") } // Verify the response can be unpacked respMsg := new(dns.Msg) - err = respMsg.Unpack(response.Msg) + err = respMsg.Unpack(response.GetMsg()) if err != nil { t.Errorf("Failed to unpack response message: %v", err) } diff --git a/plugin/dnstap/handler_test.go b/plugin/dnstap/handler_test.go index cb492ac10..5c292a0f5 100644 --- a/plugin/dnstap/handler_test.go +++ b/plugin/dnstap/handler_test.go @@ -47,23 +47,37 @@ func (w *writer) Dnstap(e *tap.Dnstap) { w.t.Error("Message not expected") } - ex := w.queue[0].Message - got := e.Message + ex := w.queue[0].GetMessage() + got := e.GetMessage() - if string(ex.QueryAddress) != string(got.QueryAddress) { - w.t.Errorf("Expected source address %s, got %s", ex.QueryAddress, got.QueryAddress) + eaddr := string(ex.GetQueryAddress()) + gaddr := string(got.GetQueryAddress()) + if eaddr != gaddr { + w.t.Errorf("Expected source address %s, got %s", eaddr, gaddr) } - if string(ex.ResponseAddress) != string(got.ResponseAddress) { - w.t.Errorf("Expected response address %s, got %s", ex.ResponseAddress, got.ResponseAddress) + + eraddr := string(ex.GetResponseAddress()) + graddr := string(got.GetResponseAddress()) + if eraddr != graddr { + w.t.Errorf("Expected response address %s, got %s", eraddr, graddr) } - if *ex.QueryPort != *got.QueryPort { - w.t.Errorf("Expected port %d, got %d", *ex.QueryPort, *got.QueryPort) + + ep := ex.GetQueryPort() + gp := got.GetQueryPort() + if ep != gp { + w.t.Errorf("Expected port %d, got %d", ep, gp) } - if *ex.SocketFamily != *got.SocketFamily { - w.t.Errorf("Expected socket family %d, got %d", *ex.SocketFamily, *got.SocketFamily) + + ef := ex.GetSocketFamily() + sf := got.GetSocketFamily() + if ef != sf { + w.t.Errorf("Expected socket family %d, got %d", ef, sf) } - if string(w.queue[0].Extra) != string(e.Extra) { - w.t.Errorf("Expected extra %s, got %s", w.queue[0].Extra, e.Extra) + + eext := string(w.queue[0].GetExtra()) + gext := string(e.GetExtra()) + if eext != gext { + w.t.Errorf("Expected extra %s, got %s", eext, gext) } w.queue = w.queue[1:] } @@ -80,23 +94,23 @@ func TestDnstap(t *testing.T) { tapq := &tap.Dnstap{ Message: testMessage(), } - msg.SetType(tapq.Message, tap.Message_CLIENT_QUERY) + msg.SetType(tapq.GetMessage(), tap.Message_CLIENT_QUERY) tapr := &tap.Dnstap{ Message: testMessage(), } - msg.SetType(tapr.Message, tap.Message_CLIENT_RESPONSE) + msg.SetType(tapr.GetMessage(), tap.Message_CLIENT_RESPONSE) testCase(t, tapq, tapr, q, r, "") tapq_with_extra := &tap.Dnstap{ Message: testMessage(), // leave type unset for deepEqual Extra: []byte("extra_field_MetadataValue_A_example.org._IN_udp_29_10.240.0.1_40212_127.0.0.1"), } - msg.SetType(tapq_with_extra.Message, tap.Message_CLIENT_QUERY) + msg.SetType(tapq_with_extra.GetMessage(), tap.Message_CLIENT_QUERY) tapr_with_extra := &tap.Dnstap{ Message: testMessage(), Extra: []byte("extra_field_MetadataValue_A_example.org._IN_udp_29_10.240.0.1_40212_127.0.0.1"), } - msg.SetType(tapr_with_extra.Message, tap.Message_CLIENT_RESPONSE) + msg.SetType(tapr_with_extra.GetMessage(), tap.Message_CLIENT_RESPONSE) extraFormat := "extra_field_{/metadata/test}_{type}_{name}_{class}_{proto}_{size}_{remote}_{port}_{local}" testCase(t, tapq_with_extra, tapr_with_extra, q, r, extraFormat) } @@ -120,7 +134,7 @@ func TestTapMessage(t *testing.T) { // extra field would not be replaced, since TapMessage won't pass context Extra: []byte(extraFormat), } - msg.SetType(tapq.Message, tap.Message_CLIENT_QUERY) + msg.SetType(tapq.GetMessage(), tap.Message_CLIENT_QUERY) w := writer{t: t} w.queue = append(w.queue, tapq) @@ -132,5 +146,5 @@ func TestTapMessage(t *testing.T) { io: &w, ExtraFormat: extraFormat, } - h.TapMessage(tapq.Message) + h.TapMessage(tapq.GetMessage()) } diff --git a/plugin/grpc/proxy.go b/plugin/grpc/proxy.go index f8322680b..a94e76902 100644 --- a/plugin/grpc/proxy.go +++ b/plugin/grpc/proxy.go @@ -65,7 +65,7 @@ func (p *Proxy) query(ctx context.Context, req *dns.Msg) (*dns.Msg, error) { return nil, err } ret := new(dns.Msg) - if err := ret.Unpack(reply.Msg); err != nil { + if err := ret.Unpack(reply.GetMsg()); err != nil { return nil, err } diff --git a/plugin/test/scrape.go b/plugin/test/scrape.go index 7ac22d531..23c217bab 100644 --- a/plugin/test/scrape.go +++ b/plugin/test/scrape.go @@ -150,9 +150,9 @@ func newMetricFamily(dtoMF *dto.MetricFamily) *MetricFamily { Name: dtoMF.GetName(), Help: dtoMF.GetHelp(), Type: dtoMF.GetType().String(), - Metrics: make([]interface{}, len(dtoMF.Metric)), + Metrics: make([]interface{}, len(dtoMF.GetMetric())), } - for i, m := range dtoMF.Metric { + for i, m := range dtoMF.GetMetric() { if dtoMF.GetType() == dto.MetricType_SUMMARY { mf.Metrics[i] = summary{ Labels: makeLabels(m), @@ -178,13 +178,13 @@ func newMetricFamily(dtoMF *dto.MetricFamily) *MetricFamily { } func value(m *dto.Metric) float64 { - if m.Gauge != nil { + if m.GetGauge() != nil { return m.GetGauge().GetValue() } - if m.Counter != nil { + if m.GetCounter() != nil { return m.GetCounter().GetValue() } - if m.Untyped != nil { + if m.GetUntyped() != nil { return m.GetUntyped().GetValue() } return 0. @@ -192,7 +192,7 @@ func value(m *dto.Metric) float64 { func makeLabels(m *dto.Metric) map[string]string { result := map[string]string{} - for _, lp := range m.Label { + for _, lp := range m.GetLabel() { result[lp.GetName()] = lp.GetValue() } return result @@ -200,7 +200,7 @@ func makeLabels(m *dto.Metric) map[string]string { func makeQuantiles(m *dto.Metric) map[string]string { result := map[string]string{} - for _, q := range m.GetSummary().Quantile { + for _, q := range m.GetSummary().GetQuantile() { result[fmt.Sprint(q.GetQuantile())] = fmt.Sprint(q.GetValue()) } return result @@ -208,7 +208,7 @@ func makeQuantiles(m *dto.Metric) map[string]string { func makeBuckets(m *dto.Metric) map[string]string { result := map[string]string{} - for _, b := range m.GetHistogram().Bucket { + for _, b := range m.GetHistogram().GetBucket() { result[fmt.Sprint(b.GetUpperBound())] = fmt.Sprint(b.GetCumulativeCount()) } return result diff --git a/test/grpc_test.go b/test/grpc_test.go index 8c3b03268..07e7f211b 100644 --- a/test/grpc_test.go +++ b/test/grpc_test.go @@ -40,7 +40,7 @@ func TestGrpc(t *testing.T) { } d := new(dns.Msg) - err = d.Unpack(reply.Msg) + err = d.Unpack(reply.GetMsg()) if err != nil { t.Errorf("Expected no error but got: %s", err) }