Skip to content

[CI] Golangci-lint update part 2 #344

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Mar 5, 2025
17 changes: 9 additions & 8 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ linters-settings:
- "$all"
- "!$test"
deny:
- pkg: "reflect"
desc: "Reflection is never clear."
# TODO: Remove reflect from loadbalancers.go and reinstate this requirement
#- pkg: "reflect"
# desc: "Reflection is never clear."
- pkg: "gob"
desc: "Please convert types manually"

Expand Down Expand Up @@ -112,7 +113,7 @@ linters:
- copyloopvar
#- cyclop
- decorder
#- depguard
- depguard
- dogsled
- dupl
- dupword
Expand All @@ -123,16 +124,16 @@ linters:
- exhaustive
#- forbidigo
#- forcetypeassert
#- gci
- gci
- gocheckcompilerdirectives
- gofmt
- goimports
#- gocognit
#- goconst
- goconst
#- gocritic
- gofumpt
- goprintffuncname
#- gosec
- gosec
- importas
- loggercheck
- maintidx
Expand All @@ -147,15 +148,15 @@ linters:
- nolintlint
- nosprintfhostport
#- paralleltest
#- prealloc
- prealloc
- predeclared
- reassign
#- tenv
#- thelper
- unconvert
- unparam
- usestdlibvars
#- varnamelen
- varnamelen
- wastedassign
- whitespace

Expand Down
90 changes: 48 additions & 42 deletions cloud/linode/cilium_loadbalancers_test.go

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions cloud/linode/instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,13 @@ func (nc *nodeCache) refreshInstances(ctx context.Context, client client.Client)
}

newNodes := make(map[int]linodeInstance, len(instances))
for i, instance := range instances {
for index, instance := range instances {
// if running within VPC, only store instances in cache which are part of VPC
if Options.VPCNames != "" && len(vpcNodes[instance.ID]) == 0 {
continue
}
node := linodeInstance{
instance: &instances[i],
instance: &instances[index],
ips: nc.getInstanceAddresses(instance, vpcNodes[instance.ID]),
}
newNodes[instance.ID] = node
Expand Down
38 changes: 21 additions & 17 deletions cloud/linode/instances_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ import (
cloudprovider "k8s.io/cloud-provider"

"github.com/linode/linode-cloud-controller-manager/cloud/linode/client/mocks"

)

const (
instanceName string = "mock-instance"
usEast string = "us-east"
typeG6 string = "g6-standard-1"
)

func nodeWithProviderID(providerID string) *v1.Node {
Expand Down Expand Up @@ -53,7 +60,7 @@ func TestInstanceExists(t *testing.T) {
{
ID: 123,
Label: "mock",
Region: "us-east",
Region: usEast,
Type: "g6-standard-2",
},
}, nil)
Expand Down Expand Up @@ -114,14 +121,13 @@ func TestMetadataRetrieval(t *testing.T) {
t.Run("should return data when linode is found (by name)", func(t *testing.T) {
instances := newInstances(client)
id := 123
name := "mock-instance"
node := nodeWithName(name)
node := nodeWithName(instanceName)
publicIPv4 := net.ParseIP("45.76.101.25")
privateIPv4 := net.ParseIP("192.168.133.65")
linodeType := "g6-standard-1"
region := "us-east"
linodeType := typeG6
region := usEast
client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{
{ID: id, Label: name, Type: linodeType, Region: region, IPv4: []*net.IP{&publicIPv4, &privateIPv4}},
{ID: id, Label: instanceName, Type: linodeType, Region: region, IPv4: []*net.IP{&publicIPv4, &privateIPv4}},
}, nil)

meta, err := instances.InstanceMetadata(ctx, node)
Expand All @@ -132,7 +138,7 @@ func TestMetadataRetrieval(t *testing.T) {
assert.Equal(t, []v1.NodeAddress{
{
Type: v1.NodeHostName,
Address: name,
Address: instanceName,
},
{
Type: v1.NodeExternalIP,
Expand All @@ -148,23 +154,21 @@ func TestMetadataRetrieval(t *testing.T) {
t.Run("should return data when linode is found (by name) and addresses must be in order", func(t *testing.T) {
instances := newInstances(client)
id := 123
name := "mock-instance"
node := nodeWithName(name)
node := nodeWithName(instanceName)
publicIPv4 := net.ParseIP("45.76.101.25")
privateIPv4 := net.ParseIP("192.168.133.65")
ipv6Addr := "2001::8a2e:370:7348"
linodeType := "g6-standard-1"
region := "us-east"
linodeType := typeG6

Options.VPCNames = "test"
vpcIDs["test"] = 1
Options.EnableRouteController = true

instance := linodego.Instance{
ID: id,
Label: name,
Label: instanceName,
Type: linodeType,
Region: region,
Region: usEast,
IPv4: []*net.IP{&publicIPv4, &privateIPv4},
IPv6: ipv6Addr,
}
Expand Down Expand Up @@ -201,12 +205,12 @@ func TestMetadataRetrieval(t *testing.T) {
meta, err := instances.InstanceMetadata(ctx, node)
assert.NoError(t, err)
assert.Equal(t, providerIDPrefix+strconv.Itoa(id), meta.ProviderID)
assert.Equal(t, region, meta.Region)
assert.Equal(t, usEast, meta.Region)
assert.Equal(t, linodeType, meta.InstanceType)
assert.Equal(t, []v1.NodeAddress{
{
Type: v1.NodeHostName,
Address: name,
Address: instanceName,
},
{
Type: v1.NodeInternalIP,
Expand Down Expand Up @@ -345,8 +349,8 @@ func TestMetadataRetrieval(t *testing.T) {
ips = append(ips, &parsed)
}

linodeType := "g6-standard-1"
region := "us-east"
linodeType := typeG6
region := usEast
client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{
{ID: id, Label: name, Type: linodeType, Region: region, IPv4: ips, IPv6: test.inputIPv6},
}, nil)
Expand Down
12 changes: 6 additions & 6 deletions cloud/linode/loadbalancers.go
Original file line number Diff line number Diff line change
Expand Up @@ -861,16 +861,16 @@ func (l *loadbalancers) buildLoadBalancerRequest(ctx context.Context, clusterNam
return l.createNodeBalancer(ctx, clusterName, service, configs)
}

func coerceString(s string, minLen, maxLen int, padding string) string {
func coerceString(str string, minLen, maxLen int, padding string) string {
if len(padding) == 0 {
padding = "x"
}
if len(s) > maxLen {
return s[:maxLen]
} else if len(s) < minLen {
return coerceString(fmt.Sprintf("%s%s", padding, s), minLen, maxLen, padding)
if len(str) > maxLen {
return str[:maxLen]
} else if len(str) < minLen {
return coerceString(fmt.Sprintf("%s%s", padding, str), minLen, maxLen, padding)
}
return s
return str
}

func (l *loadbalancers) buildNodeBalancerNodeConfigRebuildOptions(node *v1.Node, nodePort int32, subnetID int) linodego.NodeBalancerConfigRebuildNodeOptions {
Expand Down
16 changes: 9 additions & 7 deletions cloud/linode/loadbalancers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ o/aoxqmE0mN1lyCPOa9UP//LlsREkWVKI3+Wld/xERtzf66hjcH+ilsXDxxpMEXo
bSiPJQsGIKtQvyCaZY2szyOoeUGgOId+He7ITlezxKrjdj+1pLMESvAxKeo=
-----END RSA PRIVATE KEY-----`

const drop string = "DROP"

func TestCCMLoadBalancers(t *testing.T) {
testCases := []struct {
name string
Expand Down Expand Up @@ -1529,8 +1531,8 @@ func testUpdateLoadBalancerAddNewFirewallACL(t *testing.T, client *linodego.Clie
t.Fatalf("Firewalls attached when none specified")
}

var ipv4s []string
var ipv6s []string
ipv4s := make([]string, 0, 400)
ipv6s := make([]string, 0, 300)
i := 0
for i < 400 {
ipv4s = append(ipv4s, fmt.Sprintf("%d.%d.%d.%d", 192, rand.Int31n(255), rand.Int31n(255), rand.Int31n(255)))
Expand Down Expand Up @@ -1587,7 +1589,7 @@ func testUpdateLoadBalancerAddNewFirewallACL(t *testing.T, client *linodego.Clie
t.Fatalf("No firewalls found")
}

if firewallsNew[0].Rules.InboundPolicy != "DROP" {
if firewallsNew[0].Rules.InboundPolicy != drop {
t.Errorf("expected DROP inbound policy, got %s", firewallsNew[0].Rules.InboundPolicy)
}

Expand Down Expand Up @@ -1663,7 +1665,7 @@ func testUpdateLoadBalancerDeleteFirewallRemoveACL(t *testing.T, client *linodeg
t.Fatalf("No firewalls attached")
}

if firewalls[0].Rules.InboundPolicy != "DROP" {
if firewalls[0].Rules.InboundPolicy != drop {
t.Errorf("expected DROP inbound policy, got %s", firewalls[0].Rules.InboundPolicy)
}

Expand Down Expand Up @@ -1756,7 +1758,7 @@ func testUpdateLoadBalancerUpdateFirewallRemoveACLaddID(t *testing.T, client *li
t.Fatalf("No firewalls attached")
}

if firewalls[0].Rules.InboundPolicy != "DROP" {
if firewalls[0].Rules.InboundPolicy != drop {
t.Errorf("expected DROP inbound policy, got %s", firewalls[0].Rules.InboundPolicy)
}

Expand Down Expand Up @@ -1942,7 +1944,7 @@ func testUpdateLoadBalancerUpdateFirewallRemoveIDaddACL(t *testing.T, client *li
t.Fatalf("No attached firewalls found")
}

if firewallsNew[0].Rules.InboundPolicy != "DROP" {
if firewallsNew[0].Rules.InboundPolicy != drop {
t.Errorf("expected DROP inbound policy, got %s", firewallsNew[0].Rules.InboundPolicy)
}

Expand Down Expand Up @@ -2022,7 +2024,7 @@ func testUpdateLoadBalancerUpdateFirewallACL(t *testing.T, client *linodego.Clie
t.Fatalf("No firewalls attached")
}

if firewalls[0].Rules.InboundPolicy != "DROP" {
if firewalls[0].Rules.InboundPolicy != drop {
t.Errorf("expected DROP inbound policy, got %s", firewalls[0].Rules.InboundPolicy)
}

Expand Down
16 changes: 8 additions & 8 deletions cloud/linode/node_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,26 +185,26 @@ func (s *nodeController) handleNode(ctx context.Context, node *v1.Node) error {

if err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
// Get a fresh copy of the node so the resource version is up-to-date
n, err := s.kubeclient.CoreV1().Nodes().Get(ctx, node.Name, metav1.GetOptions{})
nodeResult, err := s.kubeclient.CoreV1().Nodes().Get(ctx, node.Name, metav1.GetOptions{})
if err != nil {
return err
}

// Try to update the node UUID if it has not been set
if n.Labels[annotations.AnnLinodeHostUUID] != linode.HostUUID {
n.Labels[annotations.AnnLinodeHostUUID] = linode.HostUUID
if nodeResult.Labels[annotations.AnnLinodeHostUUID] != linode.HostUUID {
nodeResult.Labels[annotations.AnnLinodeHostUUID] = linode.HostUUID
}

// Try to update the node ProviderID if it has not been set
if n.Spec.ProviderID == "" {
n.Spec.ProviderID = providerIDPrefix + strconv.Itoa(linode.ID)
if nodeResult.Spec.ProviderID == "" {
nodeResult.Spec.ProviderID = providerIDPrefix + strconv.Itoa(linode.ID)
}

// Try to update the expectedPrivateIP if its not set or doesn't match
if n.Annotations[annotations.AnnLinodeNodePrivateIP] != expectedPrivateIP && expectedPrivateIP != "" {
n.Annotations[annotations.AnnLinodeNodePrivateIP] = expectedPrivateIP
if nodeResult.Annotations[annotations.AnnLinodeNodePrivateIP] != expectedPrivateIP && expectedPrivateIP != "" {
nodeResult.Annotations[annotations.AnnLinodeNodePrivateIP] = expectedPrivateIP
}
_, err = s.kubeclient.CoreV1().Nodes().Update(ctx, n, metav1.UpdateOptions{})
_, err = s.kubeclient.CoreV1().Nodes().Update(ctx, nodeResult, metav1.UpdateOptions{})
return err
}); err != nil {
klog.V(1).ErrorS(err, "Node update error")
Expand Down
Loading