feat: make firewallID mutable, propagate it from lmt->lm

Author: tchinmai7

api/v1alpha2/linodemachine_types.go

@@ -67,7 +67,7 @@ type LinodeMachineSpec struct {
 	PrivateIP *bool `json:"privateIP,omitempty"`
 	// Tags is a list of tags to apply to the Linode instance.
 	Tags []string `json:"tags,omitempty"`
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
+	// FirewallID is the id of the cloud firewall to apply to the Linode Instance
 	FirewallID int `json:"firewallID,omitempty"`
 	// OSDisk is configuration for the root disk that includes the OS,
 	// if not specified this defaults to whatever space is not taken up by the DataDisks

api/v1alpha2/linodemachinetemplate_types.go

@@ -33,6 +33,10 @@ type LinodeMachineTemplateStatus struct {
 	// +optional
 	Tags []string `json:"tags,omitempty"`
 
+	// Firewall ID that is currently applied to the LinodeMachineTemplate.
+	// +optional
+	FirewallID int `json:"firewallID,omitempty"`
+
 	// Conditions represent the latest available observations of a LinodeMachineTemplate's current state.
 	// +optional
 	Conditions []metav1.Condition `json:"conditions,omitempty"`

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

@@ -164,10 +164,9 @@ spec:
                 - message: Value is immutable
                   rule: self == oldSelf
               firewallID:
+                description: FirewallID is the id of the cloud firewall to apply to
+                  the Linode Instance
                 type: integer
-                x-kubernetes-validations:
-                - message: Value is immutable
-                  rule: self == oldSelf
               firewallRef:
                 description: FirewallRef is a reference to a firewall object. This
                   makes the linode use the specified firewall.

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

@@ -154,10 +154,9 @@ spec:
                         - message: Value is immutable
                           rule: self == oldSelf
                       firewallID:
+                        description: FirewallID is the id of the cloud firewall to
+                          apply to the Linode Instance
                         type: integer
-                        x-kubernetes-validations:
-                        - message: Value is immutable
-                          rule: self == oldSelf
                       firewallRef:
                         description: FirewallRef is a reference to a firewall object.
                           This makes the linode use the specified firewall.
@@ -541,6 +540,9 @@ spec:
                   - type
                   type: object
                 type: array
+              firewallID:
+                description: Firewall ID that is currently applied to the LinodeMachineTemplate.
+                type: integer
               tags:
                 description: tags that are currently applied to the LinodeMachineTemplate.
                 items:

docs/src/reference/out.md

@@ -629,7 +629,7 @@ _Appears in:_
 | `backupsEnabled` _boolean_ |  |  |  |
 | `privateIP` _boolean_ |  |  |  |
 | `tags` _string array_ | Tags is a list of tags to apply to the Linode instance. |  |  |
-| `firewallID` _integer_ |  |  |  |
+| `firewallID` _integer_ | FirewallID is the id of the cloud firewall to apply to the Linode Instance |  |  |
 | `osDisk` _[InstanceDisk](#instancedisk)_ | OSDisk is configuration for the root disk that includes the OS,<br />if not specified this defaults to whatever space is not taken up by the DataDisks |  |  |
 | `dataDisks` _object (keys:string, values:[InstanceDisk](#instancedisk))_ | DataDisks is a map of any additional disks to add to an instance,<br />The sum of these disks + the OSDisk must not be more than allowed on a linodes plan |  |  |
 | `diskEncryption` _string_ | DiskEncryption determines if the disks of the instance should be encrypted. The default is disabled. |  | Enum: [enabled disabled] <br /> |
@@ -755,6 +755,7 @@ _Appears in:_
 | Field | Description | Default | Validation |
 | --- | --- | --- | --- |
 | `tags` _string array_ | tags that are currently applied to the LinodeMachineTemplate. |  |  |
+| `firewallID` _integer_ | Firewall ID that is currently applied to the LinodeMachineTemplate. |  |  |
 | `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.32/#condition-v1-meta) array_ | Conditions represent the latest available observations of a LinodeMachineTemplate's current state. |  |  |
 
 

internal/controller/linodemachinetemplate_controller.go

@@ -126,13 +126,36 @@ func (lmtr *LinodeMachineTemplateReconciler) reconcile(ctx context.Context, lmtS
 		}
 
 		machinesFoundForTemplate = true
-		if !slices.Equal(lmtScope.LinodeMachineTemplate.Spec.Template.Spec.Tags, lmtScope.LinodeMachineTemplate.Status.Tags) {
-			err := lmtr.reconcileTags(ctx, lmtScope.LinodeMachineTemplate, &machine)
-			if err != nil {
-				lmtr.Logger.Error(err, "Failed to add tags to LinodeMachine", "template", lmtScope.LinodeMachineTemplate.Name, "machine", machine.Name)
-				outErr = errors.Join(outErr, err)
 
-				failureReason = "FailedToPatchLinodeMachine"
+		// Define reconciliation tasks
+		reconciliationTasks := []struct {
+			shouldReconcile bool
+			fieldType       string
+			failureReason   string
+			logMessage      string
+		}{
+			{
+				shouldReconcile: !slices.Equal(lmtScope.LinodeMachineTemplate.Spec.Template.Spec.Tags, lmtScope.LinodeMachineTemplate.Status.Tags),
+				fieldType:       "tags",
+				failureReason:   "FailedToPatchLinodeMachineWithTags",
+				logMessage:      "Failed to update tags on LinodeMachine",
+			},
+			{
+				shouldReconcile: lmtScope.LinodeMachineTemplate.Spec.Template.Spec.FirewallID != lmtScope.LinodeMachineTemplate.Status.FirewallID,
+				fieldType:       "firewallID",
+				failureReason:   "FailedToPatchLinodeMachineWithFirewallID",
+				logMessage:      "Failed to update firewall ID on LinodeMachine",
+			},
+		}
+
+		// Execute reconciliation tasks
+		for _, task := range reconciliationTasks {
+			if task.shouldReconcile {
+				if err := lmtr.reconcileField(ctx, lmtScope.LinodeMachineTemplate, &machine, task.fieldType); err != nil {
+					lmtr.Logger.Error(err, task.logMessage, "template", lmtScope.LinodeMachineTemplate.Name, "machine", machine.Name)
+					outErr = errors.Join(outErr, err)
+					failureReason = task.failureReason
+				}
 			}
 		}
 	}
@@ -142,28 +165,41 @@ func (lmtr *LinodeMachineTemplateReconciler) reconcile(ctx context.Context, lmtS
 		return ctrl.Result{}, nil
 	}
 
-	// update the LMT status.tags if all the linodeMachines spec.tags is successfully updated.
+	// update the LMT status if all the linodeMachines are successfully updated.
 	if outErr == nil {
 		lmtScope.LinodeMachineTemplate.Status.Tags = slices.Clone(lmtScope.LinodeMachineTemplate.Spec.Template.Spec.Tags)
+		lmtScope.LinodeMachineTemplate.Status.FirewallID = lmtScope.LinodeMachineTemplate.Spec.Template.Spec.FirewallID
 		lmtr.Logger.Info("Successfully reconciled LinodeMachineTemplate", "name", lmtScope.LinodeMachineTemplate.Name)
 	} else {
 		lmtr.Logger.Error(outErr, "Error in reconciling LinodeMachineTemplate, retrying..", "name", lmtScope.LinodeMachineTemplate.Name)
 	}
 	return ctrl.Result{}, outErr
 }
 
-func (lmtr *LinodeMachineTemplateReconciler) reconcileTags(ctx context.Context, lmt *infrav1alpha2.LinodeMachineTemplate, machine *infrav1alpha2.LinodeMachine) error {
+// reconcileField updates a specific field on a LinodeMachine based on the field type
+func (lmtr *LinodeMachineTemplateReconciler) reconcileField(ctx context.Context, lmt *infrav1alpha2.LinodeMachineTemplate, machine *infrav1alpha2.LinodeMachine, fieldType string) error {
 	helper, err := patch.NewHelper(machine, lmtr.Client)
 	if err != nil {
 		return fmt.Errorf("failed to init patch helper: %w", err)
 	}
 
-	machine.Spec.Tags = lmt.Spec.Template.Spec.Tags
-
-	if err := helper.Patch(ctx, machine); err != nil {
-		return fmt.Errorf("failed to patch LinodeMachine %s with new tags: %w", machine.Name, err)
+	switch fieldType {
+	case "tags":
+		machine.Spec.Tags = lmt.Spec.Template.Spec.Tags
+		if err := helper.Patch(ctx, machine); err != nil {
+			return fmt.Errorf("failed to patch LinodeMachine %s with new %s: %w", machine.Name, fieldType, err)
+		}
+		lmtr.Logger.Info("Patched LinodeMachine with new tags", "machine", machine.Name, "tags", lmt.Spec.Template.Spec.Tags)
+	case "firewallID":
+		machine.Spec.FirewallID = lmt.Spec.Template.Spec.FirewallID
+		if err := helper.Patch(ctx, machine); err != nil {
+			return fmt.Errorf("failed to patch LinodeMachine %s with new %s: %w", machine.Name, fieldType, err)
+		}
+		lmtr.Logger.Info("Patched LinodeMachine with new firewall ID", "machine", machine.Name, "firewallID", lmt.Spec.Template.Spec.FirewallID)
+	default:
+		return fmt.Errorf("unsupported field type: %s", fieldType)
 	}
-	lmtr.Logger.Info("Patched LinodeMachine with new tags", "machine", machine.Name, "tags", lmt.Spec.Template.Spec.Tags)
+
 	return nil
 }