Improve NetworkInterface FirewallRules field

- Refactor Firewallrules field - Switch to typed strings in Direction and Action - Bump k8s.io/* to 1.28.3 - Bump controller-runtime - Bump controller-utils
ironcore-dev · Nov 6, 2023 · 716d388 · 716d388
1 parent b5eaf92
commit 716d388
Show file tree

Hide file tree

Showing 10 changed files with 196 additions and 580 deletions.
diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml
@@ -24,7 +24,7 @@ jobs:
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.53.3
+          version: v1.55.1
       - name: Delete git global config
         if: always()
         run: |

diff --git a/Makefile b/Makefile
@@ -73,8 +73,8 @@ checklicense: ## Check that every file has a license header present.
 	find . -name '*.go' -exec go run github.com/google/addlicense  -check -c 'OnMetal authors' {} +
 
 .PHONY: lint
-lint: ## Run golangci-lint against code.
-	golangci-lint run ./...
+lint: golangci-lint ## Run golangci-lint against code.
+	$(GOLANGCILINT) run ./...
 
 .PHONY: check
 check: manifests generate fmt addlicense lint test ## Generate manifests, code, lint, add licenses, test
@@ -132,10 +132,12 @@ $(LOCALBIN):
 KUSTOMIZE ?= $(LOCALBIN)/kustomize
 CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen
 ENVTEST ?= $(LOCALBIN)/setup-envtest
+GOLANGCILINT ?= $(LOCALBIN)/golangci-lint
 
 ## Tool Versions
 KUSTOMIZE_VERSION ?= v3.8.7
-CONTROLLER_TOOLS_VERSION ?= v0.9.0
+CONTROLLER_TOOLS_VERSION ?= v0.13.0
+GOLANGCILINT_VERSION ?= v1.55.1
 
 KUSTOMIZE_INSTALL_SCRIPT ?= "https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/hack/install_kustomize.sh"
 .PHONY: kustomize
@@ -152,3 +154,8 @@ $(CONTROLLER_GEN): $(LOCALBIN)
 envtest: $(ENVTEST) ## Download envtest-setup locally if necessary.
 $(ENVTEST): $(LOCALBIN)
 	GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest
+
+.PHONY: golangci-lint
+golangci-lint: $(GOLANGCILINT) ## Download golangci-lint locally if necessary.
+$(GOLANGCILINT): $(LOCALBIN)
+	test -s $(LOCALBIN)/golangci-lint || GOBIN=$(LOCALBIN) go install github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANGCILINT_VERSION)
diff --git a/api/v1alpha1/networkinterface_types.go b/api/v1alpha1/networkinterface_types.go
@@ -43,7 +43,7 @@ type NetworkInterfaceSpec struct {
 	// NodeName is the name of the node on which the interface should be created.
 	NodeName *string `json:"nodeName,omitempty"`
 	// FirewallRules are the firewall rules to be applied to this interface.
-	FirewallRules []FirewallRuleSpec `json:"firewallRules,omitempty"`
+	FirewallRules []FirewallRule `json:"firewallRules,omitempty"`
 }
 
 // NetworkInterfaceStatus defines the observed state of NetworkInterface
@@ -86,16 +86,16 @@ const (
 	NetworkInterfaceStateError NetworkInterfaceState = "Error"
 )
 
-// FirewallRuleSpec defines the desired state of FirewallRule
-type FirewallRuleSpec struct {
-	FirewallRuleID    types.UID       `json:"firewallRuleID"`
-	Direction         string          `json:"direction"`
-	Action            string          `json:"action"`
-	Priority          *int32          `json:"priority,omitempty"`
-	IpFamily          corev1.IPFamily `json:"ipFamily"`
-	SourcePrefix      *IPPrefix       `json:"sourcePrefix,omitempty"`
-	DestinationPrefix *IPPrefix       `json:"destinationPrefix,omitempty"`
-	ProtocolMatch     *ProtocolMatch  `json:"protocolMatch,omitempty"`
+// FirewallRule defines the desired state of FirewallRule
+type FirewallRule struct {
+	FirewallRuleID    types.UID             `json:"firewallRuleID"`
+	Direction         FirewallRuleDirection `json:"direction"`
+	Action            FirewallRuleAction    `json:"action"`
+	Priority          *int32                `json:"priority,omitempty"`
+	IpFamily          corev1.IPFamily       `json:"ipFamily"`
+	SourcePrefix      *IPPrefix             `json:"sourcePrefix,omitempty"`
+	DestinationPrefix *IPPrefix             `json:"destinationPrefix,omitempty"`
+	ProtocolMatch     *ProtocolMatch        `json:"protocolMatch,omitempty"`
 }
 
 type ProtocolMatch struct {
@@ -133,20 +133,20 @@ type FirewallRuleAction string
 
 // Currently only Accept rules can be used.
 const (
-	// FirewallRuleAccept is used to accept traffic.
-	FirewallRuleAccept FirewallRuleAction = "ACCEPT"
-	// FirewallRuleDeny is used to deny traffic.
-	FirewallRuleDeny FirewallRuleAction = "DENY"
+	// FirewallRuleActionAccept is used to accept traffic.
+	FirewallRuleActionAccept FirewallRuleAction = "Accept"
+	// FirewallRuleActionDeny is used to deny traffic.
+	FirewallRuleActionDeny FirewallRuleAction = "Deny"
 )
 
 // FirewallRuleDirection is the direction of the rule.
 type FirewallRuleDirection string
 
 const (
-	// FirewallRuleIngress is used to define rules for incoming traffic.
-	FirewallRuleIngress FirewallRuleDirection = "INGRESS"
-	// FirewallRuleEgress is used to define rules for outgoing traffic.
-	FirewallRuleEgress FirewallRuleDirection = "EGRESS"
+	// FirewallRuleDirectionIngress is used to define rules for incoming traffic.
+	FirewallRuleDirectionIngress FirewallRuleDirection = "Ingress"
+	// FirewallRuleDirectionEgress is used to define rules for outgoing traffic.
+	FirewallRuleDirectionEgress FirewallRuleDirection = "Egress"
 )
 
 //+kubebuilder:object:root=true

diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/controllers/controller_test.go b/controllers/controller_test.go
@@ -262,10 +262,10 @@ var _ = Describe("Network Interface and LoadBalancer Controller", func() {
 				By("wrong FirewallRule data")
 				var protocolType metalnetv1alpha1.ProtocolType = "TCP"
 				var srcPort int32 = 75000
-				wfr1 := metalnetv1alpha1.FirewallRuleSpec{
+				wfr1 := metalnetv1alpha1.FirewallRule{
 					FirewallRuleID:    "wfr1",
-					Direction:         "INGRESS",
-					Action:            "ACCEPT",
+					Direction:         metalnetv1alpha1.FirewallRuleDirectionIngress,
+					Action:            metalnetv1alpha1.FirewallRuleActionAccept,
 					IpFamily:          "IPv4",
 					SourcePrefix:      metalnetv1alpha1.MustParseNewIPPrefix("0.0.0.0/0"),
 					DestinationPrefix: metalnetv1alpha1.MustParseNewIPPrefix("10.0.0.10/32"),
@@ -294,7 +294,7 @@ var _ = Describe("Network Interface and LoadBalancer Controller", func() {
 								Addr: netip.MustParseAddr("10.0.0.1"),
 							},
 						},
-						FirewallRules: []metalnetv1alpha1.FirewallRuleSpec{wfr1},
+						FirewallRules: []metalnetv1alpha1.FirewallRule{wfr1},
 					},
 				}
 
@@ -605,10 +605,10 @@ var _ = Describe("Network Interface and LoadBalancer Controller", func() {
 				By("adding the FirewallRule")
 				var protocolType metalnetv1alpha1.ProtocolType = "TCP"
 				var srcPort int32 = 80
-				fr1 := metalnetv1alpha1.FirewallRuleSpec{
+				fr1 := metalnetv1alpha1.FirewallRule{
 					FirewallRuleID:    "fr1",
-					Direction:         "INGRESS",
-					Action:            "ACCEPT",
+					Direction:         metalnetv1alpha1.FirewallRuleDirectionIngress,
+					Action:            metalnetv1alpha1.FirewallRuleActionAccept,
 					IpFamily:          "IPv4",
 					SourcePrefix:      metalnetv1alpha1.MustParseNewIPPrefix("0.0.0.0/0"),
 					DestinationPrefix: metalnetv1alpha1.MustParseNewIPPrefix("10.0.0.10/32"),
@@ -622,7 +622,7 @@ var _ = Describe("Network Interface and LoadBalancer Controller", func() {
 				}
 
 				patchIface := networkInterface.DeepCopy()
-				patchIface.Spec.FirewallRules = []metalnetv1alpha1.FirewallRuleSpec{fr1}
+				patchIface.Spec.FirewallRules = []metalnetv1alpha1.FirewallRule{fr1}
 
 				Expect(k8sClient.Patch(ctx, patchIface, client.MergeFrom(networkInterface))).To(Succeed())
 
@@ -652,7 +652,7 @@ var _ = Describe("Network Interface and LoadBalancer Controller", func() {
 				fr1.ProtocolMatch.PortRange.DstPort = &dstPort
 
 				patchIface = updatedIface.DeepCopy()
-				patchIface.Spec.FirewallRules = []metalnetv1alpha1.FirewallRuleSpec{fr1}
+				patchIface.Spec.FirewallRules = []metalnetv1alpha1.FirewallRule{fr1}
 
 				Expect(k8sClient.Patch(ctx, patchIface, client.MergeFrom(updatedIface))).To(Succeed())
 
@@ -671,7 +671,7 @@ var _ = Describe("Network Interface and LoadBalancer Controller", func() {
 
 				By("deleting the FirewallRule")
 				patchIface = updatedIface.DeepCopy()
-				patchIface.Spec.FirewallRules = []metalnetv1alpha1.FirewallRuleSpec{}
+				patchIface.Spec.FirewallRules = []metalnetv1alpha1.FirewallRule{}
 
 				Expect(k8sClient.Patch(ctx, patchIface, client.MergeFrom(updatedIface))).To(Succeed())
 

diff --git a/controllers/network_controller.go b/controllers/network_controller.go
@@ -34,6 +34,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
+	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
@@ -377,24 +378,24 @@ func (r *NetworkReconciler) unsubscribeIfSubscribed(ctx context.Context, vni uin
 }
 
 // SetupWithManager sets up the controller with the Manager.
-func (r *NetworkReconciler) SetupWithManager(mgr ctrl.Manager) error {
+func (r *NetworkReconciler) SetupWithManager(mgr ctrl.Manager, metalnetCache cache.Cache) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&metalnetv1alpha1.Network{}).
 		WithEventFilter(predicate.ResourceVersionChangedPredicate{}).
-		Watches(
-			&source.Kind{Type: &metalnetv1alpha1.NetworkInterface{}},
+		WatchesRawSource(
+			source.Kind(metalnetCache, &metalnetv1alpha1.NetworkInterface{}),
 			handler.EnqueueRequestsFromMapFunc(r.findObjectsForNetworkInterface),
 			builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}),
 		).
-		Watches(
-			&source.Kind{Type: &metalnetv1alpha1.LoadBalancer{}},
+		WatchesRawSource(
+			source.Kind(metalnetCache, &metalnetv1alpha1.LoadBalancer{}),
 			handler.EnqueueRequestsFromMapFunc(r.findObjectsForLoadBalancer),
 			builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}),
 		).
 		Complete(r)
 }
 
-func (r *NetworkReconciler) findObjectsForNetworkInterface(obj client.Object) []reconcile.Request {
+func (r *NetworkReconciler) findObjectsForNetworkInterface(ctx context.Context, obj client.Object) []reconcile.Request {
 	networkInterface, ok := obj.(*metalnetv1alpha1.NetworkInterface)
 	if !ok {
 		return []reconcile.Request{}
@@ -412,7 +413,7 @@ func (r *NetworkReconciler) networkFinalizer() string {
 	return fmt.Sprintf("%s-%s", networkFinalizer, r.NodeName)
 }
 
-func (r *NetworkReconciler) findObjectsForLoadBalancer(obj client.Object) []reconcile.Request {
+func (r *NetworkReconciler) findObjectsForLoadBalancer(ctx context.Context, obj client.Object) []reconcile.Request {
 	loadBalancer, ok := obj.(*metalnetv1alpha1.LoadBalancer)
 	if !ok {
 		return []reconcile.Request{}

diff --git a/controllers/networkinterface_controller.go b/controllers/networkinterface_controller.go
@@ -47,6 +47,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
@@ -297,7 +298,7 @@ func (r *NetworkInterfaceReconciler) removeLBTargetRouteIfExists(ctx context.Con
 	return nil
 }
 
-func (r *NetworkInterfaceReconciler) fillTCPUDPFilter(ctx context.Context, specFirewallRule *metalnetv1alpha1.FirewallRuleSpec, protocolFilter *dpdkproto.ProtocolFilter) error {
+func (r *NetworkInterfaceReconciler) fillTCPUDPFilter(ctx context.Context, specFirewallRule *metalnetv1alpha1.FirewallRule, protocolFilter *dpdkproto.ProtocolFilter) error {
 	var SrcPortLower, DstPortLower, SrcPortUpper, DstPortUpper int32
 	if specFirewallRule.ProtocolMatch.PortRange != nil {
 		if specFirewallRule.ProtocolMatch.PortRange.SrcPort != nil {
@@ -339,7 +340,7 @@ func (r *NetworkInterfaceReconciler) fillTCPUDPFilter(ctx context.Context, specF
 	return nil
 }
 
-func (r *NetworkInterfaceReconciler) createDPDKFwRule(ctx context.Context, nic *metalnetv1alpha1.NetworkInterface, specFirewallRule *metalnetv1alpha1.FirewallRuleSpec) error {
+func (r *NetworkInterfaceReconciler) createDPDKFwRule(ctx context.Context, nic *metalnetv1alpha1.NetworkInterface, specFirewallRule *metalnetv1alpha1.FirewallRule) error {
 	var (
 		protocolFilter dpdkproto.ProtocolFilter
 		priority       uint32 = defaultFirewallRulePrio
@@ -399,8 +400,8 @@ func (r *NetworkInterfaceReconciler) createDPDKFwRule(ctx context.Context, nic *
 		},
 		Spec: dpdk.FirewallRuleSpec{
 			RuleID:            string(specFirewallRule.FirewallRuleID),
-			TrafficDirection:  specFirewallRule.Direction,
-			FirewallAction:    specFirewallRule.Action,
+			TrafficDirection:  string(specFirewallRule.Direction),
+			FirewallAction:    string(specFirewallRule.Action),
 			Priority:          priority,
 			SourcePrefix:      &sourcePrefix.Prefix,
 			DestinationPrefix: &destPrefix.Prefix,
@@ -1116,7 +1117,7 @@ func (r *NetworkInterfaceReconciler) reconcileFirewallRules(ctx context.Context,
 		})
 	}
 	var errs []error
-	var specFirewallRule metalnetv1alpha1.FirewallRuleSpec
+	var specFirewallRule metalnetv1alpha1.FirewallRule
 	for _, fwRuleID := range allFirewallRules {
 		if err := func() error {
 			log := log.WithValues("FirewallRuleID", fwRuleID)
@@ -1510,25 +1511,25 @@ func (r *NetworkInterfaceReconciler) deleteInterface(
 }
 
 // SetupWithManager sets up the controller with the Manager.
-func (r *NetworkInterfaceReconciler) SetupWithManager(mgr ctrl.Manager) error {
+func (r *NetworkInterfaceReconciler) SetupWithManager(mgr ctrl.Manager, metalnetCache cache.Cache) error {
 	log := ctrl.Log.WithName("networkinterface").WithName("setup")
 	ctx := ctrl.LoggerInto(context.TODO(), log)
 
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&metalnetv1alpha1.NetworkInterface{}).
-		Watches(
-			&source.Kind{Type: &metalnetv1alpha1.Network{}},
+		WatchesRawSource(
+			source.Kind(metalnetCache, &metalnetv1alpha1.Network{}),
 			r.enqueueNetworkInterfacesReferencingNetwork(ctx, log),
 		).
-		Watches(
-			&source.Kind{Type: &metalnetv1alpha1.LoadBalancer{}},
+		WatchesRawSource(
+			source.Kind(metalnetCache, &metalnetv1alpha1.LoadBalancer{}),
 			r.enqueueNetworkInterfacesReferencingLoadBalancer(ctx, log),
 		).
 		Complete(r)
 }
 
 func (r *NetworkInterfaceReconciler) enqueueNetworkInterfacesReferencingNetwork(ctx context.Context, log logr.Logger) handler.EventHandler {
-	return handler.EnqueueRequestsFromMapFunc(func(obj client.Object) []ctrl.Request {
+	return handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, obj client.Object) []ctrl.Request {
 		network := obj.(*metalnetv1alpha1.Network)
 		nicList := &metalnetv1alpha1.NetworkInterfaceList{}
 		if err := r.List(ctx, nicList,
@@ -1548,7 +1549,7 @@ func (r *NetworkInterfaceReconciler) enqueueNetworkInterfacesReferencingNetwork(
 }
 
 func (r *NetworkInterfaceReconciler) enqueueNetworkInterfacesReferencingLoadBalancer(ctx context.Context, log logr.Logger) handler.EventHandler {
-	return handler.EnqueueRequestsFromMapFunc(func(obj client.Object) []ctrl.Request {
+	return handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, obj client.Object) []ctrl.Request {
 		loadBalancer := obj.(*metalnetv1alpha1.LoadBalancer)
 		nicList := &metalnetv1alpha1.NetworkInterfaceList{}
 		if err := r.List(ctx, nicList,