From f858cf9f22a4ab3495a274aad2b2fa7663feb945 Mon Sep 17 00:00:00 2001 From: Christian Brunner Date: Wed, 4 Mar 2026 11:58:22 +0000 Subject: [PATCH 1/2] feat: migrate from uber-go/mock to vektra/mockery - Replace go.uber.org/mock with github.com/vektra/mockery/v2 for mock generation - Update Makefile: replace mockgen target with mockery - Update go:generate directive in pkg/nftables/firewall.go - Regenerate mocks using mockery (now uses github.com/stretchr/testify/mock) - Update test imports and mock usage from gomock API to testify/mock API Mock API changes: - Mock class: MockFQDNCache -> FQDNCache - Constructor: NewMockFQDNCache(ctrl) -> NewFQDNCache(t) - Matchers: gomock.Any() -> mock.Anything - Expectations: .EXPECT().Method() -> .On("Method") --- Makefile | 14 +-- go.mod | 3 +- go.sum | 2 - pkg/nftables/firewall.go | 3 +- pkg/nftables/mocks/mock_fqdncache.go | 166 +++++++++++++++------------ pkg/nftables/networkpolicy_test.go | 22 ++-- 6 files changed, 108 insertions(+), 102 deletions(-) diff --git a/Makefile b/Makefile index 92d03df8..c7cc2c3e 100644 --- a/Makefile +++ b/Makefile @@ -4,10 +4,9 @@ BUILDDATE := $(shell date -Iseconds) VERSION := $(or ${VERSION},$(shell git describe --tags --exact-match 2> /dev/null || git symbolic-ref -q --short HEAD || git rev-parse --short HEAD)) CONTROLLER_TOOLS_VERSION ?= v0.18.0 -MOCKGEN_VERSION ?= $(shell go list -m all | grep go.uber.org/mock | awk '{print $$2}') LOCALBIN ?= $(shell pwd)/bin CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen -MOCKGEN ?= $(LOCALBIN)/mockgen +MOCKERY ?= $(LOCALBIN)/mockery ENVTEST ?= $(LOCALBIN)/setup-envtest all: firewall-controller @@ -65,7 +64,7 @@ vet: go vet ./... # Generate code -generate: controller-gen mockgen manifests +generate: controller-gen mockery manifests $(CONTROLLER_GEN) object paths="./..." go generate ./... @@ -80,8 +79,7 @@ setup-envtest: $(ENVTEST) $(ENVTEST): $(LOCALBIN) test -s $(LOCALBIN)/setup-envtest || GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest -.PHONY: mockgen -mockgen: $(MOCKGEN) -$(MOCKGEN): $(LOCALBIN) - test -s $(LOCALBIN)/mockgen && $(LOCALBIN)/mockgen -version | grep -q $(MOCKGEN_VERSION) || \ - GOBIN=$(LOCALBIN) go install go.uber.org/mock/mockgen@$(MOCKGEN_VERSION) +.PHONY: mockery +mockery: $(MOCKERY) +$(MOCKERY): $(LOCALBIN) + test -s $(LOCALBIN)/mockery || GOBIN=$(LOCALBIN) go install github.com/vektra/mockery/v2@latest diff --git a/go.mod b/go.mod index d29a615b..3fa65ffb 100644 --- a/go.mod +++ b/go.mod @@ -16,9 +16,9 @@ require ( github.com/metal-stack/metal-networker v0.46.2 github.com/metal-stack/v v1.0.3 github.com/miekg/dns v1.1.72 + github.com/stretchr/testify v1.11.1 github.com/txn2/txeh v1.7.0 github.com/vishvananda/netlink v1.3.1 - go.uber.org/mock v0.6.0 go4.org/netipx v0.0.0-20231129151722-fdeea329fbba k8s.io/api v0.34.0 k8s.io/apiextensions-apiserver v0.34.0 @@ -83,6 +83,7 @@ require ( github.com/prometheus/common v0.67.5 // indirect github.com/prometheus/procfs v0.19.2 // indirect github.com/spf13/pflag v1.0.10 // indirect + github.com/stretchr/objx v0.5.2 // indirect github.com/vishvananda/netns v0.0.5 // indirect github.com/x448/float16 v0.8.4 // indirect go.mongodb.org/mongo-driver v1.17.7 // indirect diff --git a/go.sum b/go.sum index ba5832cd..216de11e 100644 --- a/go.sum +++ b/go.sum @@ -213,8 +213,6 @@ go.mongodb.org/mongo-driver v1.17.7 h1:a9w+U3Vt67eYzcfq3k/OAv284/uUUkL0uP75VE5rC go.mongodb.org/mongo-driver v1.17.7/go.mod h1:Hy04i7O2kC4RS06ZrhPRqj/u4DTYkFDAAccj+rVKqgQ= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= -go.uber.org/mock v0.6.0 h1:hyF9dfmbgIX5EfOdasqLsWD6xqpNZlXblLB/Dbnwv3Y= -go.uber.org/mock v0.6.0/go.mod h1:KiVJ4BqZJaMj4svdfmHM0AUx4NJYO8ZNpPnZn1Z+BBU= go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8= diff --git a/pkg/nftables/firewall.go b/pkg/nftables/firewall.go index 463a616f..8549f06a 100644 --- a/pkg/nftables/firewall.go +++ b/pkg/nftables/firewall.go @@ -25,7 +25,6 @@ import ( firewallv2 "github.com/metal-stack/firewall-controller-manager/api/v2" firewallv1 "github.com/metal-stack/firewall-controller/v2/api/v1" - _ "go.uber.org/mock/mockgen/model" // required for go:generate to work ) const ( @@ -38,7 +37,7 @@ const ( //go:embed *.tpl var templates embed.FS -//go:generate ../../bin/mockgen -destination=./mocks/mock_fqdncache.go -package=mocks . FQDNCache +//go:generate ../../bin/mockery --name=FQDNCache --outpkg=mocks --output=./mocks --filename=mock_fqdncache.go type FQDNCache interface { GetSetsForRendering(fqdns []firewallv1.FQDNSelector) (result []dns.RenderIPSet) GetSetsForFQDN(fqdn firewallv1.FQDNSelector) (result []firewallv1.IPSet) diff --git a/pkg/nftables/mocks/mock_fqdncache.go b/pkg/nftables/mocks/mock_fqdncache.go index d1f26694..08c6c2d8 100644 --- a/pkg/nftables/mocks/mock_fqdncache.go +++ b/pkg/nftables/mocks/mock_fqdncache.go @@ -1,99 +1,115 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: github.com/metal-stack/firewall-controller/v2/pkg/nftables (interfaces: FQDNCache) -// -// Generated by this command: -// -// mockgen -destination=./mocks/mock_fqdncache.go -package=mocks . FQDNCache -// - -// Package mocks is a generated GoMock package. +// Code generated by mockery v2.53.6. DO NOT EDIT. + package mocks import ( - reflect "reflect" + dns "github.com/metal-stack/firewall-controller/v2/pkg/dns" + mock "github.com/stretchr/testify/mock" v1 "github.com/metal-stack/firewall-controller/v2/api/v1" - dns "github.com/metal-stack/firewall-controller/v2/pkg/dns" - gomock "go.uber.org/mock/gomock" ) -// MockFQDNCache is a mock of FQDNCache interface. -type MockFQDNCache struct { - ctrl *gomock.Controller - recorder *MockFQDNCacheMockRecorder - isgomock struct{} +// FQDNCache is an autogenerated mock type for the FQDNCache type +type FQDNCache struct { + mock.Mock } -// MockFQDNCacheMockRecorder is the mock recorder for MockFQDNCache. -type MockFQDNCacheMockRecorder struct { - mock *MockFQDNCache -} +// CacheAddr provides a mock function with no fields +func (_m *FQDNCache) CacheAddr() (string, error) { + ret := _m.Called() -// NewMockFQDNCache creates a new mock instance. -func NewMockFQDNCache(ctrl *gomock.Controller) *MockFQDNCache { - mock := &MockFQDNCache{ctrl: ctrl} - mock.recorder = &MockFQDNCacheMockRecorder{mock} - return mock -} + if len(ret) == 0 { + panic("no return value specified for CacheAddr") + } -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockFQDNCache) EXPECT() *MockFQDNCacheMockRecorder { - return m.recorder -} + var r0 string + var r1 error + if rf, ok := ret.Get(0).(func() (string, error)); ok { + return rf() + } + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } -// CacheAddr mocks base method. -func (m *MockFQDNCache) CacheAddr() (string, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CacheAddr") - ret0, _ := ret[0].(string) - ret1, _ := ret[1].(error) - return ret0, ret1 -} + if rf, ok := ret.Get(1).(func() error); ok { + r1 = rf() + } else { + r1 = ret.Error(1) + } -// CacheAddr indicates an expected call of CacheAddr. -func (mr *MockFQDNCacheMockRecorder) CacheAddr() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CacheAddr", reflect.TypeOf((*MockFQDNCache)(nil).CacheAddr)) + return r0, r1 } -// GetSetsForFQDN mocks base method. -func (m *MockFQDNCache) GetSetsForFQDN(fqdn v1.FQDNSelector) []v1.IPSet { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetSetsForFQDN", fqdn) - ret0, _ := ret[0].([]v1.IPSet) - return ret0 -} +// GetSetsForFQDN provides a mock function with given fields: fqdn +func (_m *FQDNCache) GetSetsForFQDN(fqdn v1.FQDNSelector) []v1.IPSet { + ret := _m.Called(fqdn) -// GetSetsForFQDN indicates an expected call of GetSetsForFQDN. -func (mr *MockFQDNCacheMockRecorder) GetSetsForFQDN(fqdn any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSetsForFQDN", reflect.TypeOf((*MockFQDNCache)(nil).GetSetsForFQDN), fqdn) -} + if len(ret) == 0 { + panic("no return value specified for GetSetsForFQDN") + } -// GetSetsForRendering mocks base method. -func (m *MockFQDNCache) GetSetsForRendering(fqdns []v1.FQDNSelector) []dns.RenderIPSet { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetSetsForRendering", fqdns) - ret0, _ := ret[0].([]dns.RenderIPSet) - return ret0 + var r0 []v1.IPSet + if rf, ok := ret.Get(0).(func(v1.FQDNSelector) []v1.IPSet); ok { + r0 = rf(fqdn) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]v1.IPSet) + } + } + + return r0 } -// GetSetsForRendering indicates an expected call of GetSetsForRendering. -func (mr *MockFQDNCacheMockRecorder) GetSetsForRendering(fqdns any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSetsForRendering", reflect.TypeOf((*MockFQDNCache)(nil).GetSetsForRendering), fqdns) +// GetSetsForRendering provides a mock function with given fields: fqdns +func (_m *FQDNCache) GetSetsForRendering(fqdns []v1.FQDNSelector) []dns.RenderIPSet { + ret := _m.Called(fqdns) + + if len(ret) == 0 { + panic("no return value specified for GetSetsForRendering") + } + + var r0 []dns.RenderIPSet + if rf, ok := ret.Get(0).(func([]v1.FQDNSelector) []dns.RenderIPSet); ok { + r0 = rf(fqdns) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]dns.RenderIPSet) + } + } + + return r0 } -// IsInitialized mocks base method. -func (m *MockFQDNCache) IsInitialized() bool { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsInitialized") - ret0, _ := ret[0].(bool) - return ret0 +// IsInitialized provides a mock function with no fields +func (_m *FQDNCache) IsInitialized() bool { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for IsInitialized") + } + + var r0 bool + if rf, ok := ret.Get(0).(func() bool); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(bool) + } + + return r0 } -// IsInitialized indicates an expected call of IsInitialized. -func (mr *MockFQDNCacheMockRecorder) IsInitialized() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsInitialized", reflect.TypeOf((*MockFQDNCache)(nil).IsInitialized)) +// NewFQDNCache creates a new instance of FQDNCache. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewFQDNCache(t interface { + mock.TestingT + Cleanup(func()) +}) *FQDNCache { + mock := &FQDNCache{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock } diff --git a/pkg/nftables/networkpolicy_test.go b/pkg/nftables/networkpolicy_test.go index 6580d7b5..73e3024f 100644 --- a/pkg/nftables/networkpolicy_test.go +++ b/pkg/nftables/networkpolicy_test.go @@ -4,7 +4,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" - "go.uber.org/mock/gomock" + mock "github.com/stretchr/testify/mock" corev1 "k8s.io/api/core/v1" networking "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/util/intstr" @@ -142,7 +142,7 @@ func TestClusterwideNetworkPolicyEgressRules(t *testing.T) { tests := []struct { name string input firewallv1.ClusterwideNetworkPolicy - record func(*mocks.MockFQDNCache) + record func(*mocks.FQDNCache) want want }{ { @@ -174,7 +174,7 @@ func TestClusterwideNetworkPolicyEgressRules(t *testing.T) { }, }, }, - record: func(cache *mocks.MockFQDNCache) {}, + record: func(cache *mocks.FQDNCache) {}, want: want{ egress: nftablesRules{ `ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } tcp dport { 53 } counter accept comment "accept traffic for np tcp"`, @@ -214,18 +214,15 @@ func TestClusterwideNetworkPolicyEgressRules(t *testing.T) { }, }, }, - record: func(cache *mocks.MockFQDNCache) { + record: func(cache *mocks.FQDNCache) { cache. - EXPECT(). - IsInitialized(). + On("IsInitialized"). Return(true) cache. - EXPECT(). - GetSetsForFQDN(gomock.Any()). + On("GetSetsForFQDN", mock.Anything). Return([]firewallv1.IPSet{{SetName: "test", Version: firewallv1.IPv4}}) cache. - EXPECT(). - GetSetsForFQDN(gomock.Any()). + On("GetSetsForFQDN", mock.Anything). Return([]firewallv1.IPSet{{SetName: "test2", Version: firewallv1.IPv6}}) }, want: want{ @@ -239,13 +236,10 @@ func TestClusterwideNetworkPolicyEgressRules(t *testing.T) { }, } - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - fqdnCache := mocks.NewMockFQDNCache(ctrl) for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { + fqdnCache := mocks.NewFQDNCache(t) tt.record(fqdnCache) if len(tt.want.egress) > 0 { egress, _ := clusterwideNetworkPolicyEgressRules(fqdnCache, tt.input, false) From 545bc665ff98f43157b1bc7c2c8c2094584fecfb Mon Sep 17 00:00:00 2001 From: Christian Brunner Date: Wed, 4 Mar 2026 12:16:30 +0000 Subject: [PATCH 2/2] fix: match mock expectations on specific FQDNSelector values The test was using mock.Anything for both GetSetsForFQDN calls, causing both expectations to match every call and return the same values. Now matches on the actual FQDNSelector values (MatchName and MatchPattern) to properly test the different return values for IPv4 and IPv6 sets. --- pkg/nftables/networkpolicy_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/nftables/networkpolicy_test.go b/pkg/nftables/networkpolicy_test.go index 73e3024f..9d1c7dff 100644 --- a/pkg/nftables/networkpolicy_test.go +++ b/pkg/nftables/networkpolicy_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/google/go-cmp/cmp" - mock "github.com/stretchr/testify/mock" corev1 "k8s.io/api/core/v1" networking "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/util/intstr" @@ -219,10 +218,10 @@ func TestClusterwideNetworkPolicyEgressRules(t *testing.T) { On("IsInitialized"). Return(true) cache. - On("GetSetsForFQDN", mock.Anything). + On("GetSetsForFQDN", firewallv1.FQDNSelector{MatchName: "test.com"}). Return([]firewallv1.IPSet{{SetName: "test", Version: firewallv1.IPv4}}) cache. - On("GetSetsForFQDN", mock.Anything). + On("GetSetsForFQDN", firewallv1.FQDNSelector{MatchPattern: "*.test.com"}). Return([]firewallv1.IPSet{{SetName: "test2", Version: firewallv1.IPv6}}) }, want: want{