Merge pull request #108 from telekom/update/crds

Descriptions get a new format and [email protected] + Lint fixes

.github/workflows/pullrequests.yaml

@@ -23,7 +23,7 @@ jobs:
       - name: golangci-lint
         uses: golangci/[email protected]
         with:
-          version: v1.53.2
+          version: v1.56.2
           args: --timeout=10m
 
   test:

.golangci.yml

@@ -66,12 +66,11 @@ linters-settings:
         alias: ctrl
   nolintlint:
     allow-unused: false
-    allow-leading-space: false
     require-specific: true
   staticcheck:
-    go: "1.19"
+    go: "1.22"
   stylecheck:
-    go: "1.19"
+    go: "1.22"
   gocritic:
     enabled-tags:
       - diagnostic
@@ -82,8 +81,6 @@ linters-settings:
     disabled-checks:
       - regexpSimplify
       - whyNoLint
-  unused:
-    go: "1.19"
   cyclop:
     max-complexity: 13
   revive:
@@ -154,6 +151,15 @@ issues:
       text: "ST1003: should not use ALL_CAPS in Go names; use CamelCase instead"
     - path: '(.+)/unix/(coil|frr|nl)\.go'
       text: "var-naming: don't use ALL_CAPS in Go names; use CamelCase"
+    - path: '(.+)/nl/(route|layer2)\.go'
+      text: 'import-alias-naming:.+\(schiff_unix\).*$'
+    # Test file exclusions
+    - path: '(.+)_test\.go'
+      text: 'unchecked-type-assertion:'
+    - path: '(.+)_test\.go'
+      text: 'import-alias-naming:'
+    - path: '(.+)_test\.go'
+      text: 'dot-imports:'
     # fix issues i currently can't fix.
     - path: '(.+)\.go'
       text: "string `vrf` has (\\d+) occurrences, make it a constant"

cmd/manager/main.go

@@ -151,7 +151,7 @@ func main() {
 		os.Exit(1)
 	}
 
-	if len(interfacePrefix) > 0 {
+	if interfacePrefix != "" {
 		setupLog.Info("start macvlan sync")
 		macvlan.RunMACSync(interfacePrefix)
 	}

config/crd/bases/network.schiff.telekom.de_layer2networkconfigurations.yaml

@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.12.0
+    controller-gen.kubebuilder.io/version: v0.14.0
   name: layer2networkconfigurations.network.schiff.telekom.de
 spec:
   group: network.schiff.telekom.de
@@ -37,14 +37,19 @@ spec:
           API.
         properties:
           apiVersion:
-            description: 'APIVersion defines the versioned schema of this representation
-              of an object. Servers should convert recognized schemas to the latest
-              internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
+            description: |-
+              APIVersion defines the versioned schema of this representation of an object.
+              Servers should convert recognized schemas to the latest internal value, and
+              may reject unrecognized values.
+              More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources
             type: string
           kind:
-            description: 'Kind is a string value representing the REST resource this
-              object represents. Servers may infer this from the endpoint the client
-              submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
+            description: |-
+              Kind is a string value representing the REST resource this object represents.
+              Servers may infer this from the endpoint the client submits requests to.
+              Cannot be updated.
+              In CamelCase.
+              More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
             type: string
           metadata:
             type: object
@@ -86,24 +91,24 @@ spec:
                     description: matchExpressions is a list of label selector requirements.
                       The requirements are ANDed.
                     items:
-                      description: A label selector requirement is a selector that
-                        contains values, a key, and an operator that relates the key
-                        and values.
+                      description: |-
+                        A label selector requirement is a selector that contains values, a key, and an operator that
+                        relates the key and values.
                       properties:
                         key:
                           description: key is the label key that the selector applies
                             to.
                           type: string
                         operator:
-                          description: operator represents a key's relationship to
-                            a set of values. Valid operators are In, NotIn, Exists
-                            and DoesNotExist.
+                          description: |-
+                            operator represents a key's relationship to a set of values.
+                            Valid operators are In, NotIn, Exists and DoesNotExist.
                           type: string
                         values:
-                          description: values is an array of string values. If the
-                            operator is In or NotIn, the values array must be non-empty.
-                            If the operator is Exists or DoesNotExist, the values
-                            array must be empty. This array is replaced during a strategic
+                          description: |-
+                            values is an array of string values. If the operator is In or NotIn,
+                            the values array must be non-empty. If the operator is Exists or DoesNotExist,
+                            the values array must be empty. This array is replaced during a strategic
                             merge patch.
                           items:
                             type: string
@@ -116,11 +121,10 @@ spec:
                   matchLabels:
                     additionalProperties:
                       type: string
-                    description: matchLabels is a map of {key,value} pairs. A single
-                      {key,value} in the matchLabels map is equivalent to an element
-                      of matchExpressions, whose key field is "key", the operator
-                      is "In", and the values array contains only "value". The requirements
-                      are ANDed.
+                    description: |-
+                      matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels
+                      map is equivalent to an element of matchExpressions, whose key field is "key", the
+                      operator is "In", and the values array contains only "value". The requirements are ANDed.
                     type: object
                 type: object
                 x-kubernetes-map-type: atomic

config/crd/bases/network.schiff.telekom.de_routingtables.yaml

@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.12.0
+    controller-gen.kubebuilder.io/version: v0.14.0
   name: routingtables.network.schiff.telekom.de
 spec:
   group: network.schiff.telekom.de
@@ -26,14 +26,19 @@ spec:
         description: RoutingTable is the Schema for the routingtables API.
         properties:
           apiVersion:
-            description: 'APIVersion defines the versioned schema of this representation
-              of an object. Servers should convert recognized schemas to the latest
-              internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
+            description: |-
+              APIVersion defines the versioned schema of this representation of an object.
+              Servers should convert recognized schemas to the latest internal value, and
+              may reject unrecognized values.
+              More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources
             type: string
           kind:
-            description: 'Kind is a string value representing the REST resource this
-              object represents. Servers may infer this from the endpoint the client
-              submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
+            description: |-
+              Kind is a string value representing the REST resource this object represents.
+              Servers may infer this from the endpoint the client submits requests to.
+              Cannot be updated.
+              In CamelCase.
+              More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
             type: string
           metadata:
             type: object

config/crd/bases/network.schiff.telekom.de_vrfrouteconfigurations.yaml

@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.12.0
+    controller-gen.kubebuilder.io/version: v0.14.0
   name: vrfrouteconfigurations.network.schiff.telekom.de
 spec:
   group: network.schiff.telekom.de
@@ -33,14 +33,19 @@ spec:
           API.
         properties:
           apiVersion:
-            description: 'APIVersion defines the versioned schema of this representation
-              of an object. Servers should convert recognized schemas to the latest
-              internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
+            description: |-
+              APIVersion defines the versioned schema of this representation of an object.
+              Servers should convert recognized schemas to the latest internal value, and
+              may reject unrecognized values.
+              More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources
             type: string
           kind:
-            description: 'Kind is a string value representing the REST resource this
-              object represents. Servers may infer this from the endpoint the client
-              submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
+            description: |-
+              Kind is a string value representing the REST resource this object represents.
+              Servers may infer this from the endpoint the client submits requests to.
+              Cannot be updated.
+              In CamelCase.
+              More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
             type: string
           metadata:
             type: object

controllers/layer2networkconfiguration_controller.go

@@ -71,12 +71,12 @@ func (r *Layer2NetworkConfigurationReconciler) SetupWithManager(mgr ctrl.Manager
 	// Create empty request for changes to node
 	nodesMapFn := handler.EnqueueRequestsFromMapFunc(func(_ context.Context, _ client.Object) []reconcile.Request { return []reconcile.Request{{}} })
 	nodePredicates := predicate.Funcs{
-		CreateFunc: func(e event.CreateEvent) bool { return false },
+		CreateFunc: func(_ event.CreateEvent) bool { return false },
 		UpdateFunc: func(e event.UpdateEvent) bool {
 			return os.Getenv(healthcheck.NodenameEnv) == e.ObjectNew.GetName() && !cmp.Equal(e.ObjectNew.GetLabels(), e.ObjectOld.GetLabels())
 		},
-		DeleteFunc:  func(e event.DeleteEvent) bool { return false },
-		GenericFunc: func(e event.GenericEvent) bool { return false },
+		DeleteFunc:  func(_ event.DeleteEvent) bool { return false },
+		GenericFunc: func(_ event.GenericEvent) bool { return false },
 	}
 
 	err := ctrl.NewControllerManagedBy(mgr).

pkg/frr/manager.go

@@ -134,8 +134,16 @@ func (*Manager) GetStatusFRR() (activeState, subState string, err error) {
 	if err != nil {
 		return "", "", fmt.Errorf("error getting unit %s properties: %w", frrUnit, err)
 	}
-
-	return prop["ActiveState"].(string), prop["SubState"].(string), nil
+	var ok bool
+	activeState, ok = prop["ActiveState"].(string)
+	if !ok {
+		return "", "", fmt.Errorf("error casting property %v [\"ActiveState\"] as string", prop)
+	}
+	subState, ok = prop["SubState"].(string)
+	if !ok {
+		return activeState, "", fmt.Errorf("error casting property %v [\"SubState\"] as string", prop)
+	}
+	return activeState, subState, nil
 }
 
 func (v *VRFConfiguration) ShouldTemplateVRF() bool {

pkg/healthcheck/healthcheck.go

@@ -157,9 +157,7 @@ func (hc *HealthChecker) CheckReachability() error {
 				// just actively refuses connections (e.g. port is blocked)
 				continue
 			}
-			if err != nil {
-				return err
-			}
+			return err
 		}
 	}
 	return nil

pkg/healthcheck/healthcheck_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"net"
 	"os"
 	"testing"
@@ -346,10 +347,13 @@ func (*updateErrorClient) Update(
 }
 
 func (*updateErrorClient) Get(_ context.Context, _ types.NamespacedName, o client.Object, _ ...client.GetOption) error {
-	a := o.(*corev1.Node)
-	a.Name = testHostname
+	node, ok := o.(*corev1.Node)
+	if !ok {
+		return fmt.Errorf("error casting object %v as corev1.Node", o)
+	}
+	node.Name = testHostname
 	for _, t := range InitTaints {
-		a.Spec.Taints = append(a.Spec.Taints, corev1.Taint{
+		node.Spec.Taints = append(node.Spec.Taints, corev1.Taint{
 			Key:    t,
 			Effect: corev1.TaintEffectNoSchedule,
 		})

pkg/nl/layer2.go

@@ -108,7 +108,7 @@ func (*NetlinkManager) ParseIPAddresses(addresses []string) ([]*netlink.Addr, er
 
 func (n *NetlinkManager) CreateL2(info *Layer2Information) error {
 	masterIdx := -1
-	if len(info.VRF) > 0 {
+	if info.VRF != "" {
 		l3Info, err := n.GetL3ByName(info.VRF)
 		if err != nil {
 			return err
@@ -376,7 +376,7 @@ func (n *NetlinkManager) isL2VNIreattachRequired(current, desired *Layer2Informa
 	// Reconcile VRF
 	if current.VRF != desired.VRF {
 		shouldReattachL2VNI = true
-		if len(desired.VRF) > 0 {
+		if desired.VRF != "" {
 			l3Info, err := n.GetL3ByName(desired.VRF)
 			if err != nil {
 				return shouldReattachL2VNI, fmt.Errorf("error while getting L3 by name: %w", err)

pkg/nl/list.go

@@ -40,15 +40,18 @@ func (*NetlinkManager) ListVRFInterfaces() ([]VRFInformation, error) {
 
 	links, err := netlink.LinkList()
 	if err != nil {
-		return nil, fmt.Errorf("cannot get links from netlink: %w", err)
+		return nil, fmt.Errorf("error listing links: %w", err)
 	}
 
 	for _, link := range links {
 		if link.Type() != "vrf" {
 			continue
 		}
-		vrf := link.(*netlink.Vrf)
 
+		vrf, ok := link.(*netlink.Vrf)
+		if !ok {
+			return nil, fmt.Errorf("error casting link %v as netlink.Vrf", link)
+		}
 		info := VRFInformation{}
 		info.table = int(vrf.Table)
 		info.Name = link.Attrs().Name
@@ -71,8 +74,10 @@ func (n *NetlinkManager) ListL3() ([]VRFInformation, error) {
 		if !(link.Type() == "vrf" && strings.HasPrefix(link.Attrs().Name, vrfPrefix)) {
 			continue
 		}
-		vrf := link.(*netlink.Vrf)
-
+		vrf, ok := link.(*netlink.Vrf)
+		if !ok {
+			return nil, fmt.Errorf("error casting link %v as netlink.Vrf", link)
+		}
 		info := VRFInformation{}
 		info.table = int(vrf.Table)
 		info.Name = link.Attrs().Name[3:]
@@ -146,13 +151,21 @@ func (*NetlinkManager) updateL2Indices(info *Layer2Information, links []netlink.
 func updateLink(info *Layer2Information, link netlink.Link) error {
 	// If subinterface is VXLAN
 	if link.Type() == "vxlan" && strings.HasPrefix(link.Attrs().Name, vxlanPrefix) {
-		info.vxlan = link.(*netlink.Vxlan)
+		vxlan, ok := link.(*netlink.Vxlan)
+		if !ok {
+			return fmt.Errorf("error casting link %v as netlink.Vxlan", link)
+		}
+		info.vxlan = vxlan
 		info.VNI = info.vxlan.VxlanId
 	}
 
 	// If subinterface is VETH
 	if link.Type() == "veth" && strings.HasPrefix(link.Attrs().Name, vethL2Prefix) {
-		info.macvlanBridge = link.(*netlink.Veth)
+		macvlanBridge, ok := link.(*netlink.Veth)
+		if !ok {
+			return fmt.Errorf("error casting link %v as netlink.Veth", link)
+		}
+		info.macvlanBridge = macvlanBridge
 		peerIdx, err := netlink.VethPeerIndex(info.macvlanBridge)
 		if err != nil {
 			return fmt.Errorf("error getting veth perr by index: %w", err)
@@ -161,7 +174,11 @@ func updateLink(info *Layer2Information, link netlink.Link) error {
 		if err != nil {
 			return fmt.Errorf("error getting link by index: %w", err)
 		}
-		info.macvlanHost = peerInterface.(*netlink.Veth)
+		macvlanHost, ok := peerInterface.(*netlink.Veth)
+		if !ok {
+			return fmt.Errorf("error casting link %v as netlink.Veth", link)
+		}
+		info.macvlanHost = macvlanHost
 		info.CreateMACVLANInterface = true
 	}
 
@@ -181,7 +198,12 @@ func (n *NetlinkManager) ListL2() ([]Layer2Information, error) {
 			continue
 		}
 		info := Layer2Information{}
-		info.bridge = link.(*netlink.Bridge)
+
+		bridge, ok := link.(*netlink.Bridge)
+		if !ok {
+			return nil, fmt.Errorf("cannot cast link %v as netlink.Bridge", link)
+		}
+		info.bridge = bridge
 		info.AnycastMAC = &info.bridge.HardwareAddr
 		info.MTU = info.bridge.MTU
 		vlanID, err := strconv.Atoi(info.bridge.Name[3:])