deprecate Firewall ID and add FirewallRef (#465)

api/v1alpha1/conversion.go

@@ -47,6 +47,10 @@ func Convert_v1alpha2_LinodeMachineSpec_To_v1alpha1_LinodeMachineSpec(in *infras
 	return autoConvert_v1alpha2_LinodeMachineSpec_To_v1alpha1_LinodeMachineSpec(in, out, s)
 }
 
+func Convert_v1alpha1_LinodeMachineSpec_To_v1alpha2_LinodeMachineSpec(in *LinodeMachineSpec, out *infrastructurev1alpha2.LinodeMachineSpec, s conversion.Scope) error {
+	return autoConvert_v1alpha1_LinodeMachineSpec_To_v1alpha2_LinodeMachineSpec(in, out, s)
+}
+
 func Convert_v1alpha1_LinodeObjectStorageBucketSpec_To_v1alpha2_LinodeObjectStorageBucketSpec(in *LinodeObjectStorageBucketSpec, out *infrastructurev1alpha2.LinodeObjectStorageBucketSpec, s conversion.Scope) error {
 	// WARNING: in.Cluster requires manual conversion: does not exist in peer-type
 	out.Region = in.Cluster

api/v1alpha1/linodemachine_conversion_test.go

@@ -53,7 +53,6 @@ func TestLinodeMachineConvertTo(t *testing.T) {
 			BackupsEnabled:  false,
 			PrivateIP:       ptr.To(true),
 			Tags:            []string{"test instance"},
-			FirewallID:      123,
 			OSDisk: ptr.To(InstanceDisk{
 				DiskID:     0,
 				Size:       *resource.NewQuantity(12, resource.DecimalSI),
@@ -96,7 +95,6 @@ func TestLinodeMachineConvertTo(t *testing.T) {
 			BackupsEnabled:  false,
 			PrivateIP:       ptr.To(true),
 			Tags:            []string{"test instance"},
-			FirewallID:      123,
 			OSDisk: ptr.To(infrav1alpha2.InstanceDisk{
 				DiskID:     0,
 				Size:       *resource.NewQuantity(12, resource.DecimalSI),
@@ -183,7 +181,6 @@ func TestLinodeMachineConvertFrom(t *testing.T) {
 			BackupsEnabled:  false,
 			PrivateIP:       ptr.To(true),
 			Tags:            []string{"test instance"},
-			FirewallID:      123,
 			OSDisk: ptr.To(infrav1alpha2.InstanceDisk{
 				DiskID:     0,
 				Size:       *resource.NewQuantity(12, resource.DecimalSI),
@@ -231,7 +228,6 @@ func TestLinodeMachineConvertFrom(t *testing.T) {
 			BackupsEnabled:  false,
 			PrivateIP:       ptr.To(true),
 			Tags:            []string{"test instance"},
-			FirewallID:      123,
 			OSDisk: ptr.To(InstanceDisk{
 				DiskID:     0,
 				Size:       *resource.NewQuantity(12, resource.DecimalSI),

api/v1alpha1/linodemachinetemplate_conversion_test.go

@@ -55,7 +55,6 @@ func TestLinodeMachineTemplateConvertTo(t *testing.T) {
 					BackupsEnabled:  false,
 					PrivateIP:       ptr.To(true),
 					Tags:            []string{"test instance"},
-					FirewallID:      123,
 					OSDisk: ptr.To(InstanceDisk{
 						DiskID:     0,
 						Size:       *resource.NewQuantity(12, resource.DecimalSI),
@@ -101,7 +100,6 @@ func TestLinodeMachineTemplateConvertTo(t *testing.T) {
 					BackupsEnabled:  false,
 					PrivateIP:       ptr.To(true),
 					Tags:            []string{"test instance"},
-					FirewallID:      123,
 					OSDisk: ptr.To(infrav1alpha2.InstanceDisk{
 						DiskID:     0,
 						Size:       *resource.NewQuantity(12, resource.DecimalSI),
@@ -191,7 +189,6 @@ func TestLinodeMachineTemplateConvertFrom(t *testing.T) {
 					BackupsEnabled:  false,
 					PrivateIP:       ptr.To(true),
 					Tags:            []string{"test instance"},
-					FirewallID:      123,
 					OSDisk: ptr.To(infrav1alpha2.InstanceDisk{
 						DiskID:     0,
 						Size:       *resource.NewQuantity(12, resource.DecimalSI),
@@ -237,7 +234,6 @@ func TestLinodeMachineTemplateConvertFrom(t *testing.T) {
 					BackupsEnabled:  false,
 					PrivateIP:       ptr.To(true),
 					Tags:            []string{"test instance"},
-					FirewallID:      123,
 					OSDisk: ptr.To(InstanceDisk{
 						DiskID:     0,
 						Size:       *resource.NewQuantity(12, resource.DecimalSI),

api/v1alpha2/linodemachine_types.go

@@ -67,6 +67,7 @@ type LinodeMachineSpec struct {
 	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	Tags []string `json:"tags,omitempty"`
 	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
+	// +kubebuilder:deprecatedversion:warning="Firewalls should be referenced via FirewallRef"
 	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
@@ -96,6 +97,10 @@ type LinodeMachineSpec struct {
 	// +optional
 	// PlacementGroupRef is a reference to a placement group object. This makes the linode to be launched in that specific group.
 	PlacementGroupRef *corev1.ObjectReference `json:"placementGroupRef,omitempty"`
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
+	// +optional
+	// FirewallRef is a reference to a firewall object. This makes the linode use the specified firewall.
+	FirewallRef *corev1.ObjectReference `json:"firewallRef,omitempty"`
 }
 
 // InstanceDisk defines a list of disks to use for an instance

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

@@ -536,6 +536,54 @@ spec:
                 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.
+                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
               group:
                 type: string
                 x-kubernetes-validations:

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

@@ -403,6 +403,54 @@ spec:
                         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.
+                        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
                       group:
                         type: string
                         x-kubernetes-validations:

controller/linodemachine_controller_helpers.go

@@ -119,6 +119,15 @@ func (r *LinodeMachineReconciler) newCreateConfig(ctx context.Context, machineSc
 		}
 	}
 
+	if machineScope.LinodeMachine.Spec.FirewallRef != nil {
+		fwID, err := r.getFirewallID(ctx, machineScope, logger)
+		if err != nil {
+			logger.Error(err, "Failed to get Firewall config")
+			return nil, err
+		}
+		createConfig.FirewallID = fwID
+	}
+
 	return createConfig, nil
 }
 
@@ -335,6 +344,31 @@ func (r *LinodeMachineReconciler) getPlacementGroupID(ctx context.Context, machi
 	return *linodePlacementGroup.Spec.PGID, nil
 }
 
+func (r *LinodeMachineReconciler) getFirewallID(ctx context.Context, machineScope *scope.MachineScope, logger logr.Logger) (int, error) {
+	name := machineScope.LinodeMachine.Spec.FirewallRef.Name
+	namespace := machineScope.LinodeMachine.Spec.FirewallRef.Namespace
+	if namespace == "" {
+		namespace = machineScope.LinodeMachine.Namespace
+	}
+
+	logger = logger.WithValues("firewallName", name, "firewallNamespace", namespace)
+
+	linodeFirewall := infrav1alpha2.LinodeFirewall{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: namespace,
+			Name:      name,
+		},
+	}
+	objectKey := client.ObjectKeyFromObject(&linodeFirewall)
+	err := r.Get(ctx, objectKey, &linodeFirewall)
+	if err != nil {
+		logger.Error(err, "Failed to fetch LinodeFirewall")
+		return -1, err
+	}
+
+	return *linodeFirewall.Spec.FirewallID, nil
+}
+
 func (r *LinodeMachineReconciler) getVPCInterfaceConfig(ctx context.Context, machineScope *scope.MachineScope, logger logr.Logger) (*linodego.InstanceConfigInterfaceCreateOptions, error) {
 	name := machineScope.LinodeCluster.Spec.VPCRef.Name
 	namespace := machineScope.LinodeCluster.Spec.VPCRef.Namespace

controller/linodemachine_controller_helpers_test.go

@@ -58,7 +58,6 @@ func TestLinodeMachineSpecToCreateInstanceConfig(t *testing.T) {
 		BackupsEnabled: true,
 		PrivateIP:      util.Pointer(true),
 		Tags:           []string{"tag"},
-		FirewallID:     1,
 	}
 
 	createConfig := linodeMachineSpecToInstanceCreateConfig(machineSpec)

controller/linodemachine_controller_test.go

@@ -1275,6 +1275,7 @@ var _ = Describe("machine in PlacementGroup", Label("machine", "placementGroup")
 	var reconciler *LinodeMachineReconciler
 	var lpgReconciler *LinodePlacementGroupReconciler
 	var linodePlacementGroup infrav1alpha2.LinodePlacementGroup
+	var linodeFirewall infrav1alpha2.LinodeFirewall
 
 	var mockCtrl *gomock.Controller
 	var testLogs *bytes.Buffer
@@ -1343,6 +1344,22 @@ var _ = Describe("machine in PlacementGroup", Label("machine", "placementGroup")
 		}
 		Expect(k8sClient.Create(ctx, &linodePlacementGroup)).To(Succeed())
 
+		linodeFirewall = infrav1alpha2.LinodeFirewall{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test-fw",
+				Namespace: defaultNamespace,
+				UID:       "5123123",
+			},
+			Spec: infrav1alpha2.LinodeFirewallSpec{
+				FirewallID: ptr.To(2),
+				Enabled:    true,
+			},
+			Status: infrav1alpha2.LinodeFirewallStatus{
+				Ready: true,
+			},
+		}
+		Expect(k8sClient.Create(ctx, &linodeFirewall)).To(Succeed())
+
 		linodeMachine = infrav1alpha2.LinodeMachine{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "mock",
@@ -1357,6 +1374,10 @@ var _ = Describe("machine in PlacementGroup", Label("machine", "placementGroup")
 					Namespace: defaultNamespace,
 					Name:      "test-pg",
 				},
+				FirewallRef: &corev1.ObjectReference{
+					Namespace: defaultNamespace,
+					Name:      "test-fw",
+				},
 			},
 		}
 
@@ -1388,7 +1409,7 @@ var _ = Describe("machine in PlacementGroup", Label("machine", "placementGroup")
 		}
 	})
 
-	It("creates a instance in a PlacementGroup", func(ctx SpecContext) {
+	It("creates a instance in a PlacementGroup with a firewall", func(ctx SpecContext) {
 		mockLinodeClient := mock.NewMockLinodeClient(mockCtrl)
 		getRegion := mockLinodeClient.EXPECT().
 			GetRegion(ctx, gomock.Any()).
@@ -1431,6 +1452,6 @@ var _ = Describe("machine in PlacementGroup", Label("machine", "placementGroup")
 		Expect(err).NotTo(HaveOccurred())
 		Expect(createOpts).NotTo(BeNil())
 		Expect(createOpts.PlacementGroup.ID).To(Equal(1))
+		Expect(createOpts.FirewallID).To(Equal(2))
 	})
-
 })