[Feat] Enable adding NodeBalancer to Linode Firewall

api/v1alpha2/linodecluster_types.go

@@ -47,6 +47,11 @@ type LinodeClusterSpec struct {
 	// +optional
 	VPCRef *corev1.ObjectReference `json:"vpcRef,omitempty"`
 
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
+	// +optional
+	// NodeBalancerFirewallRef is a reference to a NodeBalancer Firewall object. This makes the linode use the specified NodeBalancer Firewall.
+	NodeBalancerFirewallRef *corev1.ObjectReference `json:"nodeBalancerFirewallRef,omitempty"`
+
 	// CredentialsRef is a reference to a Secret that contains the credentials to use for provisioning this cluster. If not
 	// supplied then the credentials of the controller will be used.
 	// +optional
@@ -142,6 +147,9 @@ type NetworkSpec struct {
 	// NodeBalancerID is the id of NodeBalancer.
 	// +optional
 	NodeBalancerID *int `json:"nodeBalancerID,omitempty"`
+	// NodeBalancerFirewallID is the id of NodeBalancer Firewall.
+	// +optional
+	NodeBalancerFirewallID *int `json:"nodeBalancerFirewallID,omitempty"`
 	// apiserverNodeBalancerConfigID is the config ID of api server NodeBalancer config.
 	// +optional
 	ApiserverNodeBalancerConfigID *int `json:"apiserverNodeBalancerConfigID,omitempty"`

cloud/scope/common.go

@@ -148,7 +148,7 @@ func addCredentialsFinalizer(ctx context.Context, crClient K8sClient, credential
 func removeCredentialsFinalizer(ctx context.Context, crClient K8sClient, credentialsRef corev1.SecretReference, defaultNamespace, finalizer string) error {
 	secret, err := getCredentials(ctx, crClient, credentialsRef, defaultNamespace)
 	if err != nil {
-		return err
+		return client.IgnoreNotFound(err)
 	}
 
 	controllerutil.RemoveFinalizer(secret, finalizer)

cloud/scope/firewall.go

@@ -18,6 +18,7 @@ import (
 	"errors"
 	"fmt"
 
+	"k8s.io/client-go/util/retry"
 	"sigs.k8s.io/cluster-api/util/patch"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
@@ -102,19 +103,25 @@ func (s *FirewallScope) AddCredentialsRefFinalizer(ctx context.Context) error {
 		return nil
 	}
 
-	return addCredentialsFinalizer(ctx, s.Client,
-		*s.LinodeFirewall.Spec.CredentialsRef, s.LinodeFirewall.GetNamespace(),
-		toFinalizer(s.LinodeFirewall))
+	// Retry on conflict to handle the case where the secret is being updated concurrently.
+	return retry.RetryOnConflict(retry.DefaultRetry, func() error {
+		return addCredentialsFinalizer(ctx, s.Client,
+			*s.LinodeFirewall.Spec.CredentialsRef, s.LinodeFirewall.GetNamespace(),
+			toFinalizer(s.LinodeFirewall))
+	})
 }
 
 func (s *FirewallScope) RemoveCredentialsRefFinalizer(ctx context.Context) error {
 	if s.LinodeFirewall.Spec.CredentialsRef == nil {
 		return nil
 	}
 
-	return removeCredentialsFinalizer(ctx, s.Client,
-		*s.LinodeFirewall.Spec.CredentialsRef, s.LinodeFirewall.GetNamespace(),
-		toFinalizer(s.LinodeFirewall))
+	// Retry on conflict to handle the case where the secret is being updated concurrently.
+	return retry.RetryOnConflict(retry.DefaultRetry, func() error {
+		return removeCredentialsFinalizer(ctx, s.Client,
+			*s.LinodeFirewall.Spec.CredentialsRef, s.LinodeFirewall.GetNamespace(),
+			toFinalizer(s.LinodeFirewall))
+	})
 }
 
 func (s *FirewallScope) SetCredentialRefTokenForLinodeClients(ctx context.Context) error {

cloud/services/loadbalancers.go

@@ -9,7 +9,9 @@
 
 	"github.com/go-logr/logr"
 	"github.com/linode/linodego"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/cluster-api/api/v1beta1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/linode/cluster-api-provider-linode/api/v1alpha2"
 	"github.com/linode/cluster-api-provider-linode/cloud/scope"
@@ -35,15 +37,69 @@
 	}
 
 	logger.Info(fmt.Sprintf("Creating NodeBalancer %s", clusterScope.LinodeCluster.Name))
+
 	createConfig := linodego.NodeBalancerCreateOptions{
 		Label:  util.Pointer(clusterScope.LinodeCluster.Name),
 		Region: clusterScope.LinodeCluster.Spec.Region,
 		Tags:   []string{string(clusterScope.LinodeCluster.UID)},
 	}
 
+	// get firewall ID from firewallRef if it exists
+	if clusterScope.LinodeCluster.Spec.NodeBalancerFirewallRef != nil {
+		firewallID, err := getFirewallID(ctx, clusterScope, logger)
+		if err != nil {
+			logger.Error(err, "Failed to fetch LinodeFirewall ID")
+			return nil, err
+		}
+		createConfig.FirewallID = firewallID
+		clusterScope.LinodeCluster.Spec.Network.NodeBalancerFirewallID = &firewallID
+	}
+
+	// Use a firewall created outside of the CAPL ecosystem
+	// get & validate firewall ID from .Spec.Network.FirewallID if it exists
+	if clusterScope.LinodeCluster.Spec.Network.NodeBalancerFirewallID != nil {
+		firewallID := *clusterScope.LinodeCluster.Spec.Network.NodeBalancerFirewallID
+		firewall, err := clusterScope.LinodeClient.GetFirewall(ctx, firewallID)
+		if err != nil {
+			logger.Error(err, "Failed to fetch Linode Firewall from the Linode API")
+			return nil, err
+		}
+		createConfig.FirewallID = firewall.ID
+	}
+
 	return clusterScope.LinodeClient.CreateNodeBalancer(ctx, createConfig)
 }
 
+func getFirewallID(ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger) (int, error) {
+	name := clusterScope.LinodeCluster.Spec.NodeBalancerFirewallRef.Name
+	namespace := clusterScope.LinodeCluster.Spec.NodeBalancerFirewallRef.Namespace
+	if namespace == "" {
+		namespace = clusterScope.LinodeCluster.Namespace
+	}
+
+	logger = logger.WithValues("firewallName", name, "firewallNamespace", namespace)
+
+	linodeFirewall := &v1alpha2.LinodeFirewall{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: namespace,
+			Name:      name,
+		},
+	}
+	objectKey := client.ObjectKeyFromObject(linodeFirewall)
+	err := clusterScope.Client.Get(ctx, objectKey, linodeFirewall)
+	if err != nil {
+		logger.Error(err, "Failed to fetch LinodeFirewall")
+		return -1, err
+	}
+	if linodeFirewall.Spec.FirewallID == nil {
+		err = errors.New("nil firewallID")
+		logger.Error(err, "Failed to fetch LinodeFirewall")
+		return -1, err
+	}
+
+	return *linodeFirewall.Spec.FirewallID, nil
+}
+
 // EnsureNodeBalancerConfigs creates NodeBalancer configs if it does not exist or returns the existing NodeBalancerConfig
 func EnsureNodeBalancerConfigs(
 	ctx context.Context,

config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclusters.yaml

@@ -400,6 +400,10 @@ spec:
                     - dns
                     - external
                     type: string
+                  nodeBalancerFirewallID:
+                    description: NodeBalancerFirewallID is the id of NodeBalancer
+                      Firewall.
+                    type: integer
                   nodeBalancerID:
                     description: NodeBalancerID is the id of NodeBalancer.
                     type: integer
@@ -411,6 +415,55 @@ spec:
                     - message: Value is immutable
                       rule: self == oldSelf
                 type: object
+              nodeBalancerFirewallRef:
+                description: NodeBalancerFirewallRef is a reference to a NodeBalancer
+                  Firewall object. This makes the linode use the specified NodeBalancer
+                  Firewall.
+                properties:
+                  apiVersion:
+                    description: API version of the referent.
+                    type: string
+                  fieldPath:
+                    description: |-
+                      If referring to a piece of an object instead of an entire object, this string
+                      should contain a valid JSON/Go field access statement, such as desiredState.manifest.containers[2].
+                      For example, if the object reference is to a container within a pod, this would take on a value like:
+                      "spec.containers{name}" (where "name" refers to the name of the container that triggered
+                      the event) or if no container name is specified "spec.containers[2]" (container with
+                      index 2 in this pod). This syntax is chosen only to have some well-defined way of
+                      referencing a part of an object.
+                      TODO: this design is not final and this field is subject to change in the future.
+                    type: string
+                  kind:
+                    description: |-
+                      Kind of the referent.
+                      More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
+                    type: string
+                  name:
+                    description: |-
+                      Name of the referent.
+                      More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                    type: string
+                  namespace:
+                    description: |-
+                      Namespace of the referent.
+                      More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/
+                    type: string
+                  resourceVersion:
+                    description: |-
+                      Specific resourceVersion to which this reference is made, if any.
+                      More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency
+                    type: string
+                  uid:
+                    description: |-
+                      UID of the referent.
+                      More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids
+                    type: string
+                type: object
+                x-kubernetes-map-type: atomic
+                x-kubernetes-validations:
+                - message: Value is immutable
+                  rule: self == oldSelf
               region:
                 description: The Linode Region the LinodeCluster lives in.
                 type: string

config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclustertemplates.yaml

@@ -328,6 +328,10 @@ spec:
                             - dns
                             - external
                             type: string
+                          nodeBalancerFirewallID:
+                            description: NodeBalancerFirewallID is the id of NodeBalancer
+                              Firewall.
+                            type: integer
                           nodeBalancerID:
                             description: NodeBalancerID is the id of NodeBalancer.
                             type: integer
@@ -339,6 +343,55 @@ spec:
                             - message: Value is immutable
                               rule: self == oldSelf
                         type: object
+                      nodeBalancerFirewallRef:
+                        description: NodeBalancerFirewallRef is a reference to a NodeBalancer
+                          Firewall object. This makes the linode use the specified
+                          NodeBalancer Firewall.
+                        properties:
+                          apiVersion:
+                            description: API version of the referent.
+                            type: string
+                          fieldPath:
+                            description: |-
+                              If referring to a piece of an object instead of an entire object, this string
+                              should contain a valid JSON/Go field access statement, such as desiredState.manifest.containers[2].
+                              For example, if the object reference is to a container within a pod, this would take on a value like:
+                              "spec.containers{name}" (where "name" refers to the name of the container that triggered
+                              the event) or if no container name is specified "spec.containers[2]" (container with
+                              index 2 in this pod). This syntax is chosen only to have some well-defined way of
+                              referencing a part of an object.
+                              TODO: this design is not final and this field is subject to change in the future.
+                            type: string
+                          kind:
+                            description: |-
+                              Kind of the referent.
+                              More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
+                            type: string
+                          name:
+                            description: |-
+                              Name of the referent.
+                              More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                            type: string
+                          namespace:
+                            description: |-
+                              Namespace of the referent.
+                              More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/
+                            type: string
+                          resourceVersion:
+                            description: |-
+                              Specific resourceVersion to which this reference is made, if any.
+                              More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency
+                            type: string
+                          uid:
+                            description: |-
+                              UID of the referent.
+                              More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids
+                            type: string
+                        type: object
+                        x-kubernetes-map-type: atomic
+                        x-kubernetes-validations:
+                        - message: Value is immutable
+                          rule: self == oldSelf
                       region:
                         description: The Linode Region the LinodeCluster lives in.
                         type: string

templates/flavors/k3s/dns-loadbalancing/kustomization.yaml

@@ -4,6 +4,7 @@ resources:
   - ../default
 
 patches:
+  - path: remove-nb-firewall.yaml
   - target:
       group: infrastructure.cluster.x-k8s.io
       version: v1alpha2
@@ -14,6 +15,7 @@ patches:
       metadata:
         name: ${CLUSTER_NAME}
       spec:
+        nodeBalancerFirewallRef: null
         network:
           loadBalancerType: dns
           dnsRootDomain: ${DNS_ROOT_DOMAIN}

templates/flavors/k3s/dns-loadbalancing/remove-nb-firewall.yaml

@@ -0,0 +1,5 @@
+apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
+kind: LinodeFirewall
+metadata:
+  name: ${CLUSTER_NAME}-nb-firewall
+$patch: delete

templates/flavors/k3s/dual-stack/kustomization.yaml

@@ -4,6 +4,17 @@ resources:
   - ../vpcless
 
 patches:
+  - target:
+      group: infrastructure.cluster.x-k8s.io
+      version: v1alpha2
+      kind: LinodeCluster
+    patch: |-
+      apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
+      kind: LinodeCluster
+      metadata:
+        name: ${CLUSTER_NAME}
+      spec:
+        nodeBalancerFirewallRef: null
   - target:
       group: cluster.x-k8s.io
       version: v1beta1

templates/flavors/kubeadm/cilium-bgp-lb/kustomization.yaml

@@ -7,6 +7,17 @@ resources:
   - kubeadmConfigTemplate.yaml
 
 patches:
+  - target:
+      group: infrastructure.cluster.x-k8s.io
+      version: v1alpha2
+      kind: LinodeCluster
+    patch: |-
+      apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
+      kind: LinodeCluster
+      metadata:
+        name: ${CLUSTER_NAME}
+      spec:
+        nodeBalancerFirewallRef: null
   - target:
       kind: HelmChartProxy
       name: .*-linode-cloud-controller-manager