From 81d22c0f44471d7166940cc718117ab550f18fff Mon Sep 17 00:00:00 2001 From: Ashley Dumaine Date: Tue, 5 Mar 2024 16:34:32 -0500 Subject: [PATCH] refactoring --- api/v1alpha1/linodecluster_types.go | 31 +++- api/v1alpha1/linodefirewall_types.go | 4 + api/v1alpha1/zz_generated.deepcopy.go | 43 +++-- cloud/scope/cluster.go | 30 ++-- cloud/scope/firewall.go | 7 - cloud/services/firewalls.go | 41 +++-- ...cture.cluster.x-k8s.io_linodeclusters.yaml | 155 +++--------------- ...uster.x-k8s.io_linodeclustertemplates.yaml | 155 +++--------------- ...ture.cluster.x-k8s.io_linodefirewalls.yaml | 4 + controller/linodecluster_controller.go | 112 ++++++++++++- controller/linodefirewall_controller.go | 7 +- .../linodemachine_controller_helpers.go | 5 + 12 files changed, 253 insertions(+), 341 deletions(-) diff --git a/api/v1alpha1/linodecluster_types.go b/api/v1alpha1/linodecluster_types.go index a476aa4c5..6381f8ce3 100644 --- a/api/v1alpha1/linodecluster_types.go +++ b/api/v1alpha1/linodecluster_types.go @@ -46,15 +46,10 @@ type LinodeClusterSpec struct { // +optional CredentialsRef *corev1.SecretReference `json:"credentialsRef,omitempty"` + // ControlPlaneFirewall encapsulates all things related to the Firewall for the + // control plane nodes. // +optional - // ControlPlaneFirewallRefs contains a list of LinodeFirewall references to restrict traffic - // to/from the control plane nodes - ControlPlaneFirewallRefs []*corev1.ObjectReference `json:"controlPlaneFirewallRefs,omitempty"` - - // +optional - // WorkerFirewallRefs contains a list of LinodeFirewall references to restrict traffic - // to/from the worker nodes - WorkerFirewallRefs []*corev1.ObjectReference `json:"workerFirewallRefs,omitempty"` + ControlPlaneFirewall FirewallSpec `json:"controlPlaneFirewallRefs,omitempty"` } // LinodeClusterStatus defines the observed state of LinodeCluster @@ -123,6 +118,26 @@ type NetworkSpec struct { NodeBalancerConfigID *int `json:"nodeBalancerConfigID,omitempty"` } +type FirewallSpec struct { + // +optional + // Enabled specifies if the default api server firewall should be enabled + Enabled bool `json:"enabled,omitempty"` + // +optional + // AllowedIPV4Addresses specifies additional IPV4 addresses aside from the worker nodes + // that should be permitted to reach the K8s API server + AllowedIPV4Addresses string `json:"allowedIPV4Addresses,omitempty"` + // +optional + // AllowedIPV6Addresses specifies additional IPV6 addresses aside from the worker nodes + // that should be permitted to reach the K8s API server + AllowedIPV6Addresses string `json:"allowedIPV6Addresses,omitempty"` + // +optional + // AllowSSH specifies if SSH should be permitted for the firewall + AllowSSH bool `json:"allowSSH,omitempty"` + // FirewallID is the ID of api server Firewall. + // +optional + FirewallID *int `json:"firewallID,omitempty"` +} + // +kubebuilder:object:root=true // LinodeClusterList contains a list of LinodeCluster diff --git a/api/v1alpha1/linodefirewall_types.go b/api/v1alpha1/linodefirewall_types.go index edd9c5b27..da590b032 100644 --- a/api/v1alpha1/linodefirewall_types.go +++ b/api/v1alpha1/linodefirewall_types.go @@ -30,6 +30,10 @@ type LinodeFirewallSpec struct { // +optional FirewallID *int `json:"firewallID,omitempty"` + // +optional + // ClusterUID is used by the LinodeCluster controller to associate a Cloud Firewall to a LinodeCluster + ClusterUID string `json:"clusterUID,omitempty"` + // +optional // +kubebuilder:default=false Enabled bool `json:"enabled,omitempty"` diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 8e1bbb9bc..93c96af1c 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -48,6 +48,26 @@ func (in *FirewallRule) DeepCopy() *FirewallRule { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *FirewallSpec) DeepCopyInto(out *FirewallSpec) { + *out = *in + if in.FirewallID != nil { + in, out := &in.FirewallID, &out.FirewallID + *out = new(int) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FirewallSpec. +func (in *FirewallSpec) DeepCopy() *FirewallSpec { + if in == nil { + return nil + } + out := new(FirewallSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *InstanceConfigInterfaceCreateOptions) DeepCopyInto(out *InstanceConfigInterfaceCreateOptions) { *out = *in @@ -167,28 +187,7 @@ func (in *LinodeClusterSpec) DeepCopyInto(out *LinodeClusterSpec) { *out = new(v1.SecretReference) **out = **in } - if in.ControlPlaneFirewallRefs != nil { - in, out := &in.ControlPlaneFirewallRefs, &out.ControlPlaneFirewallRefs - *out = make([]*v1.ObjectReference, len(*in)) - for i := range *in { - if (*in)[i] != nil { - in, out := &(*in)[i], &(*out)[i] - *out = new(v1.ObjectReference) - **out = **in - } - } - } - if in.WorkerFirewallRefs != nil { - in, out := &in.WorkerFirewallRefs, &out.WorkerFirewallRefs - *out = make([]*v1.ObjectReference, len(*in)) - for i := range *in { - if (*in)[i] != nil { - in, out := &(*in)[i], &(*out)[i] - *out = new(v1.ObjectReference) - **out = **in - } - } - } + in.ControlPlaneFirewall.DeepCopyInto(&out.ControlPlaneFirewall) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LinodeClusterSpec. diff --git a/cloud/scope/cluster.go b/cloud/scope/cluster.go index 05a1e6b45..d6fd4011c 100644 --- a/cloud/scope/cluster.go +++ b/cloud/scope/cluster.go @@ -32,9 +32,10 @@ import ( // ClusterScopeParams defines the input parameters used to create a new Scope. type ClusterScopeParams struct { - Client client.Client - Cluster *clusterv1.Cluster - LinodeCluster *infrav1alpha1.LinodeCluster + Client client.Client + Cluster *clusterv1.Cluster + LinodeCluster *infrav1alpha1.LinodeCluster + ControlPlaneFirewall *infrav1alpha1.LinodeFirewall } func validateClusterScopeParams(params ClusterScopeParams) error { @@ -44,6 +45,9 @@ func validateClusterScopeParams(params ClusterScopeParams) error { if params.LinodeCluster == nil { return errors.New("linodeCluster is required when creating a ClusterScope") } + if params.ControlPlaneFirewall == nil { + return errors.New("controlPlaneFirewall is required when creating a ClusterScope") + } return nil } @@ -71,11 +75,12 @@ func NewClusterScope(ctx context.Context, apiKey string, params ClusterScopePara } return &ClusterScope{ - client: params.Client, - Cluster: params.Cluster, - LinodeClient: linodeClient, - LinodeCluster: params.LinodeCluster, - PatchHelper: helper, + client: params.Client, + Cluster: params.Cluster, + LinodeClient: linodeClient, + LinodeCluster: params.LinodeCluster, + ControlPlaneFirewall: params.ControlPlaneFirewall, + PatchHelper: helper, }, nil } @@ -83,10 +88,11 @@ func NewClusterScope(ctx context.Context, apiKey string, params ClusterScopePara type ClusterScope struct { client client.Client - PatchHelper *patch.Helper - LinodeClient *linodego.Client - Cluster *clusterv1.Cluster - LinodeCluster *infrav1alpha1.LinodeCluster + PatchHelper *patch.Helper + LinodeClient *linodego.Client + Cluster *clusterv1.Cluster + LinodeCluster *infrav1alpha1.LinodeCluster + ControlPlaneFirewall *infrav1alpha1.LinodeFirewall } // PatchObject persists the cluster configuration and status. diff --git a/cloud/scope/firewall.go b/cloud/scope/firewall.go index bc1fd4a76..fe390242c 100644 --- a/cloud/scope/firewall.go +++ b/cloud/scope/firewall.go @@ -35,14 +35,12 @@ type FirewallScope struct { PatchHelper *patch.Helper LinodeClient *linodego.Client - LinodeCluster *infrav1alpha1.LinodeCluster LinodeFirewall *infrav1alpha1.LinodeFirewall } // FirewallScopeParams defines the input parameters used to create a new Scope. type FirewallScopeParams struct { Client client.Client - LinodeCluster *infrav1alpha1.LinodeCluster LinodeFirewall *infrav1alpha1.LinodeFirewall } @@ -51,10 +49,6 @@ func validateFirewallScopeParams(params FirewallScopeParams) error { return errors.New("linodeFirewall is required when creating a FirewallScope") } - if params.LinodeCluster == nil { - return errors.New("linodeCluster is required when creating a FirewallScope") - } - return nil } @@ -76,7 +70,6 @@ func NewFirewallScope(apiKey string, params FirewallScopeParams) (*FirewallScope client: params.Client, LinodeClient: linodeClient, LinodeFirewall: params.LinodeFirewall, - LinodeCluster: params.LinodeCluster, PatchHelper: helper, }, nil } diff --git a/cloud/services/firewalls.go b/cloud/services/firewalls.go index 8d2043c57..d7fc5bdb4 100644 --- a/cloud/services/firewalls.go +++ b/cloud/services/firewalls.go @@ -13,7 +13,6 @@ import ( "github.com/linode/linodego" infrav1alpha1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" - "github.com/linode/cluster-api-provider-linode/cloud/scope" ) const ( @@ -27,16 +26,16 @@ var ( errDuplicateFirewalls = errors.New("duplicate firewalls found") ) +// HandleFirewall takes the CAPL firewall representation and uses it to either create or update the Cloud Firewall +// via the given linode client func HandleFirewall( ctx context.Context, - firewallScope *scope.FirewallScope, + firewall *infrav1alpha1.LinodeFirewall, + linodeClient *linodego.Client, logger logr.Logger, ) (linodeFW *linodego.Firewall, err error) { - clusterUID := string(firewallScope.LinodeCluster.UID) - tags := []string{string(firewallScope.LinodeCluster.UID)} - fwName := firewallScope.LinodeFirewall.Name - - linodeFWs, err := fetchFirewalls(ctx, firewallScope) + clusterUID := firewall.Spec.ClusterUID + linodeFWs, err := fetchFirewalls(ctx, firewall.Name, *linodeClient) if err != nil { logger.Info("Failed to list Firewalls", "error", err.Error()) @@ -51,7 +50,7 @@ func HandleFirewall( } // build out the firewall rules for create or update - fwConfig, err := processACL(firewallScope.LinodeFirewall, tags) + fwConfig, err := processACL(firewall, []string{clusterUID}) if err != nil { logger.Info("Failed to process ACL", "error", err.Error()) @@ -59,9 +58,9 @@ func HandleFirewall( } if len(linodeFWs) == 0 { - logger.Info(fmt.Sprintf("Creating firewall %s", fwName)) + logger.Info(fmt.Sprintf("Creating firewall %s", firewall.Name)) - if linodeFW, err = firewallScope.LinodeClient.CreateFirewall(ctx, *fwConfig); err != nil { + if linodeFW, err = linodeClient.CreateFirewall(ctx, *fwConfig); err != nil { logger.Info("Failed to create Linode Firewall", "error", err.Error()) // Already exists is not an error apiErr := linodego.Error{} @@ -70,19 +69,19 @@ func HandleFirewall( } if linodeFW != nil { - logger.Info(fmt.Sprintf("Linode Firewall %s already exists", fwName)) + logger.Info(fmt.Sprintf("Linode Firewall %s already exists", firewall.Name)) } } } else { - logger.Info(fmt.Sprintf("Updating firewall %s", fwName)) + logger.Info(fmt.Sprintf("Updating firewall %s", firewall.Name)) linodeFW = &linodeFWs[0] if !slices.Contains(linodeFW.Tags, clusterUID) { err := errors.New("firewall conflict") logger.Error(err, fmt.Sprintf( "Firewall %s is not associated with cluster UID %s. Owner cluster is %s", - fwName, + firewall.Name, clusterUID, linodeFW.Tags[0], )) @@ -90,7 +89,7 @@ func HandleFirewall( return nil, err } - if _, err := firewallScope.LinodeClient.UpdateFirewallRules(ctx, linodeFW.ID, fwConfig.Rules); err != nil { + if _, err := linodeClient.UpdateFirewallRules(ctx, linodeFW.ID, fwConfig.Rules); err != nil { logger.Info("Failed to update Linode Firewall", "error", err.Error()) return nil, err @@ -100,17 +99,17 @@ func HandleFirewall( // Need to make sure the firewall is appropriately enabled or disabled after // create or update and the tags are properly set var status linodego.FirewallStatus - if firewallScope.LinodeFirewall.Spec.Enabled { + if firewall.Spec.Enabled { status = linodego.FirewallEnabled } else { status = linodego.FirewallDisabled } - if _, err = firewallScope.LinodeClient.UpdateFirewall( + if _, err = linodeClient.UpdateFirewall( ctx, linodeFW.ID, linodego.FirewallUpdateOptions{ Status: status, - Tags: util.Pointer(tags), + Tags: util.Pointer([]string{clusterUID}), }, ); err != nil { logger.Info("Failed to update Linode Firewall status and tags", "error", err.Error()) @@ -122,17 +121,15 @@ func HandleFirewall( } // fetch Firewalls returns all Linode firewalls with a label matching the CAPL Firewall name -func fetchFirewalls(ctx context.Context, firewallScope *scope.FirewallScope) (firewalls []linodego.Firewall, err error) { +func fetchFirewalls(ctx context.Context, name string, linodeClient linodego.Client) (firewalls []linodego.Firewall, err error) { var linodeFWs []linodego.Firewall - filter := map[string]string{ - "label": firewallScope.LinodeFirewall.Name, - } + filter := map[string]string{"label": name} rawFilter, err := json.Marshal(filter) if err != nil { return nil, err } - if linodeFWs, err = firewallScope.LinodeClient.ListFirewalls(ctx, linodego.NewListOptions(1, string(rawFilter))); err != nil { + if linodeFWs, err = linodeClient.ListFirewalls(ctx, linodego.NewListOptions(1, string(rawFilter))); err != nil { return nil, err } return linodeFWs, nil diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclusters.yaml index 01e051294..a0227bae4 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclusters.yaml @@ -75,70 +75,31 @@ spec: type: object controlPlaneFirewallRefs: description: |- - ControlPlaneFirewallRefs contains a list of LinodeFirewall references to restrict traffic - to/from the control plane nodes - items: - description: |- - ObjectReference contains enough information to let you inspect or modify the referred object. - --- - New uses of this type are discouraged because of difficulty describing its usage when embedded in APIs. - 1. Ignored fields. It includes many fields which are not generally honored. For instance, ResourceVersion and FieldPath are both very rarely valid in actual usage. - 2. Invalid usage help. It is impossible to add specific help for individual usage. In most embedded usages, there are particular - restrictions like, "must refer only to types A and B" or "UID not honored" or "name must be restricted". - Those cannot be well described when embedded. - 3. Inconsistent validation. Because the usages are different, the validation rules are different by usage, which makes it hard for users to predict what will happen. - 4. The fields are both imprecise and overly precise. Kind is not a precise mapping to a URL. This can produce ambiguity - during interpretation and require a REST mapping. In most cases, the dependency is on the group,resource tuple - and the version of the actual struct is irrelevant. - 5. We cannot easily change it. Because this type is embedded in many locations, updates to this type - will affect numerous schemas. Don't make new APIs embed an underspecified API type they do not control. - - - Instead of using this type, create a locally provided and used type that is well-focused on your reference. - For example, ServiceReferences for admission registration: https://github.com/kubernetes/api/blob/release-1.17/admissionregistration/v1/types.go#L533 . - 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 - type: array + ControlPlaneFirewall encapsulates all things related to the Firewall for the + control plane nodes. + properties: + allowSSH: + description: AllowSSH specifies if SSH should be permitted for + the firewall + type: boolean + allowedIPV4Addresses: + description: |- + AllowedIPV4Addresses specifies additional IPV4 addresses aside from the worker nodes + that should be permitted to reach the K8s API server + type: string + allowedIPV6Addresses: + description: |- + AllowedIPV6Addresses specifies additional IPV6 addresses aside from the worker nodes + that should be permitted to reach the K8s API server + type: string + enabled: + description: Enabled specifies if the default api server firewall + should be enabled + type: boolean + firewallID: + description: FirewallID is the ID of api server Firewall. + type: integer + type: object credentialsRef: description: |- CredentialsRef is a reference to a Secret that contains the credentials to use for provisioning this cluster. If not @@ -246,72 +207,6 @@ spec: x-kubernetes-validations: - message: Value is immutable rule: self == oldSelf - workerFirewallRefs: - description: |- - WorkerFirewallRefs contains a list of LinodeFirewall references to restrict traffic - to/from the worker nodes - items: - description: |- - ObjectReference contains enough information to let you inspect or modify the referred object. - --- - New uses of this type are discouraged because of difficulty describing its usage when embedded in APIs. - 1. Ignored fields. It includes many fields which are not generally honored. For instance, ResourceVersion and FieldPath are both very rarely valid in actual usage. - 2. Invalid usage help. It is impossible to add specific help for individual usage. In most embedded usages, there are particular - restrictions like, "must refer only to types A and B" or "UID not honored" or "name must be restricted". - Those cannot be well described when embedded. - 3. Inconsistent validation. Because the usages are different, the validation rules are different by usage, which makes it hard for users to predict what will happen. - 4. The fields are both imprecise and overly precise. Kind is not a precise mapping to a URL. This can produce ambiguity - during interpretation and require a REST mapping. In most cases, the dependency is on the group,resource tuple - and the version of the actual struct is irrelevant. - 5. We cannot easily change it. Because this type is embedded in many locations, updates to this type - will affect numerous schemas. Don't make new APIs embed an underspecified API type they do not control. - - - Instead of using this type, create a locally provided and used type that is well-focused on your reference. - For example, ServiceReferences for admission registration: https://github.com/kubernetes/api/blob/release-1.17/admissionregistration/v1/types.go#L533 . - 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 - type: array required: - region type: object diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclustertemplates.yaml index 956ccad8e..67c9782a2 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclustertemplates.yaml @@ -69,70 +69,31 @@ spec: type: object controlPlaneFirewallRefs: description: |- - ControlPlaneFirewallRefs contains a list of LinodeFirewall references to restrict traffic - to/from the control plane nodes - items: - description: |- - ObjectReference contains enough information to let you inspect or modify the referred object. - --- - New uses of this type are discouraged because of difficulty describing its usage when embedded in APIs. - 1. Ignored fields. It includes many fields which are not generally honored. For instance, ResourceVersion and FieldPath are both very rarely valid in actual usage. - 2. Invalid usage help. It is impossible to add specific help for individual usage. In most embedded usages, there are particular - restrictions like, "must refer only to types A and B" or "UID not honored" or "name must be restricted". - Those cannot be well described when embedded. - 3. Inconsistent validation. Because the usages are different, the validation rules are different by usage, which makes it hard for users to predict what will happen. - 4. The fields are both imprecise and overly precise. Kind is not a precise mapping to a URL. This can produce ambiguity - during interpretation and require a REST mapping. In most cases, the dependency is on the group,resource tuple - and the version of the actual struct is irrelevant. - 5. We cannot easily change it. Because this type is embedded in many locations, updates to this type - will affect numerous schemas. Don't make new APIs embed an underspecified API type they do not control. - - - Instead of using this type, create a locally provided and used type that is well-focused on your reference. - For example, ServiceReferences for admission registration: https://github.com/kubernetes/api/blob/release-1.17/admissionregistration/v1/types.go#L533 . - 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 - type: array + ControlPlaneFirewall encapsulates all things related to the Firewall for the + control plane nodes. + properties: + allowSSH: + description: AllowSSH specifies if SSH should be permitted + for the firewall + type: boolean + allowedIPV4Addresses: + description: |- + AllowedIPV4Addresses specifies additional IPV4 addresses aside from the worker nodes + that should be permitted to reach the K8s API server + type: string + allowedIPV6Addresses: + description: |- + AllowedIPV6Addresses specifies additional IPV6 addresses aside from the worker nodes + that should be permitted to reach the K8s API server + type: string + enabled: + description: Enabled specifies if the default api server + firewall should be enabled + type: boolean + firewallID: + description: FirewallID is the ID of api server Firewall. + type: integer + type: object credentialsRef: description: |- CredentialsRef is a reference to a Secret that contains the credentials to use for provisioning this cluster. If not @@ -240,72 +201,6 @@ spec: x-kubernetes-validations: - message: Value is immutable rule: self == oldSelf - workerFirewallRefs: - description: |- - WorkerFirewallRefs contains a list of LinodeFirewall references to restrict traffic - to/from the worker nodes - items: - description: |- - ObjectReference contains enough information to let you inspect or modify the referred object. - --- - New uses of this type are discouraged because of difficulty describing its usage when embedded in APIs. - 1. Ignored fields. It includes many fields which are not generally honored. For instance, ResourceVersion and FieldPath are both very rarely valid in actual usage. - 2. Invalid usage help. It is impossible to add specific help for individual usage. In most embedded usages, there are particular - restrictions like, "must refer only to types A and B" or "UID not honored" or "name must be restricted". - Those cannot be well described when embedded. - 3. Inconsistent validation. Because the usages are different, the validation rules are different by usage, which makes it hard for users to predict what will happen. - 4. The fields are both imprecise and overly precise. Kind is not a precise mapping to a URL. This can produce ambiguity - during interpretation and require a REST mapping. In most cases, the dependency is on the group,resource tuple - and the version of the actual struct is irrelevant. - 5. We cannot easily change it. Because this type is embedded in many locations, updates to this type - will affect numerous schemas. Don't make new APIs embed an underspecified API type they do not control. - - - Instead of using this type, create a locally provided and used type that is well-focused on your reference. - For example, ServiceReferences for admission registration: https://github.com/kubernetes/api/blob/release-1.17/admissionregistration/v1/types.go#L533 . - 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 - type: array required: - region type: object diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_linodefirewalls.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodefirewalls.yaml index 6625cc7c8..c1b7fbf2c 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_linodefirewalls.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodefirewalls.yaml @@ -39,6 +39,10 @@ spec: spec: description: LinodeFirewallSpec defines the desired state of LinodeFirewall properties: + clusterUID: + description: ClusterUID is used by the LinodeCluster controller to + associate a Cloud Firewall to a LinodeCluster + type: string enabled: default: false type: boolean diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go index 998917755..711d5f7ba 100644 --- a/controller/linodecluster_controller.go +++ b/controller/linodecluster_controller.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "net/http" "time" @@ -93,14 +94,21 @@ func (r *LinodeClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques return ctrl.Result{}, nil } + + controlPlaneFW := &infrav1alpha1.LinodeFirewall{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("%s-api-server", linodeCluster.Name), + }, + } // Create the cluster scope. clusterScope, err := scope.NewClusterScope( ctx, r.LinodeApiKey, scope.ClusterScopeParams{ - Client: r.Client, - Cluster: cluster, - LinodeCluster: linodeCluster, + Client: r.Client, + Cluster: cluster, + LinodeCluster: linodeCluster, + ControlPlaneFirewall: controlPlaneFW, }) if err != nil { logger.Info("Failed to create cluster scope", "error", err.Error()) @@ -139,12 +147,21 @@ func (r *LinodeClusterReconciler) reconcile( if err := clusterScope.AddFinalizer(ctx); err != nil { return res, err } - // Create + + // build out the control plane firewall rules + clusterScope.ControlPlaneFirewall.Spec = *createControlPlaneFirewallSpec(clusterScope) + + // Create cluster if clusterScope.LinodeCluster.Spec.ControlPlaneEndpoint.Host == "" { if err := r.reconcileCreate(ctx, logger, clusterScope); err != nil { return res, err } - r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), "Load balancer is ready") + r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), "Cluster is ready") + } else { + // Update cluster + if err := r.reconcileUpdate(ctx, logger, clusterScope); err != nil { + return res, err + } } clusterScope.LinodeCluster.Status.Ready = true @@ -153,6 +170,49 @@ func (r *LinodeClusterReconciler) reconcile( return res, nil } +func createControlPlaneFirewallSpec(clusterScope *scope.ClusterScope) *infrav1alpha1.LinodeFirewallSpec { + linodeCluster := clusterScope.LinodeCluster + controlPlaneRules := []infrav1alpha1.FirewallRule{{ + Action: "ACCEPT", + Label: "api-server", + Ports: string(linodeCluster.Spec.ControlPlaneEndpoint.Port), + Addresses: &infrav1alpha1.NetworkAddresses{ + IPv4: util.Pointer([]string{ + "0.0.0.0/0", // TODO: get node IPs + linodeCluster.Spec.ControlPlaneFirewall.AllowedIPV4Addresses, + }), + IPv6: util.Pointer([]string{ + "::/0", // TODO: get node IPs + linodeCluster.Spec.ControlPlaneFirewall.AllowedIPV6Addresses, + }), + }, + }} + if clusterScope.LinodeCluster.Spec.ControlPlaneFirewall.AllowSSH { + sshRule := infrav1alpha1.FirewallRule{ + Action: "ACCEPT", + Label: "ssh", + Ports: "22", + Addresses: &infrav1alpha1.NetworkAddresses{ + IPv4: util.Pointer([]string{ + linodeCluster.Spec.ControlPlaneFirewall.AllowedIPV4Addresses, + }), + IPv6: util.Pointer([]string{ + linodeCluster.Spec.ControlPlaneFirewall.AllowedIPV6Addresses, + }), + }, + } + controlPlaneRules = append(controlPlaneRules, sshRule) + } + return &infrav1alpha1.LinodeFirewallSpec{ + ClusterUID: string(linodeCluster.UID), + FirewallID: linodeCluster.Spec.ControlPlaneFirewall.FirewallID, + Enabled: linodeCluster.Spec.ControlPlaneFirewall.Enabled, + Label: linodeCluster.Name, + InboundPolicy: "DROP", + InboundRules: controlPlaneRules, + } +} + func setFailureReason(clusterScope *scope.ClusterScope, failureReason cerrs.ClusterStatusError, err error, lcr *LinodeClusterReconciler) { clusterScope.LinodeCluster.Status.FailureReason = util.Pointer(failureReason) clusterScope.LinodeCluster.Status.FailureMessage = util.Pointer(err.Error()) @@ -186,11 +246,52 @@ func (r *LinodeClusterReconciler) reconcileCreate(ctx context.Context, logger lo Port: int32(linodeNBConfig.Port), } + // Handle firewalls + firewall, err := services.HandleFirewall(ctx, clusterScope.ControlPlaneFirewall, clusterScope.LinodeClient, logger) + if err != nil { + setFailureReason(clusterScope, cerrs.CreateClusterError, err, r) + + return err + } + + clusterScope.LinodeCluster.Spec.ControlPlaneFirewall.FirewallID = util.Pointer(firewall.ID) + + return nil +} +func (r *LinodeClusterReconciler) reconcileUpdate( + ctx context.Context, + logger logr.Logger, + clusterScope *scope.ClusterScope, +) error { + // Handle firewalls + if _, err := services.HandleFirewall(ctx, clusterScope.ControlPlaneFirewall, clusterScope.LinodeClient, logger); err != nil { + setFailureReason(clusterScope, cerrs.UpdateClusterError, err, r) + + return err + } + return nil } func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error { logger.Info("deleting cluster") + if clusterScope.LinodeCluster.Spec.ControlPlaneFirewall.FirewallID != nil { + if err := clusterScope.LinodeClient.DeleteFirewall( + ctx, + *clusterScope.LinodeCluster.Spec.ControlPlaneFirewall.FirewallID, + ); err != nil { + logger.Info("Failed to delete control plane Firewall", "error", err.Error()) + + // Not found is not an error + apiErr := linodego.Error{} + if errors.As(err, &apiErr) && apiErr.Code != http.StatusNotFound { + setFailureReason(clusterScope, cerrs.DeleteClusterError, err, r) + + return err + } + } + } + if clusterScope.LinodeCluster.Spec.Network.NodeBalancerID == 0 { logger.Info("NodeBalancer ID is missing, nothing to do") controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1alpha1.GroupVersion.String()) @@ -213,6 +314,7 @@ func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger lo conditions.MarkFalse(clusterScope.LinodeCluster, clusterv1.ReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "Load balancer deleted") clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = 0 + clusterScope.LinodeCluster.Spec.ControlPlaneFirewall.FirewallID = nil clusterScope.LinodeCluster.Spec.Network.NodeBalancerConfigID = nil controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1alpha1.GroupVersion.String()) diff --git a/controller/linodefirewall_controller.go b/controller/linodefirewall_controller.go index ba3ced026..23b0fee8a 100644 --- a/controller/linodefirewall_controller.go +++ b/controller/linodefirewall_controller.go @@ -67,15 +67,12 @@ func (r *LinodeFirewallReconciler) Reconcile(ctx context.Context, req ctrl.Reque return ctrl.Result{}, client.IgnoreNotFound(err) } - linodeCluster := &infrav1alpha1.LinodeCluster{} - // Create the firewall scope. firewallScope, err := scope.NewFirewallScope( r.LinodeApiKey, scope.FirewallScopeParams{ Client: r.Client, LinodeFirewall: linodeFirewall, - LinodeCluster: linodeCluster, }) if err != nil { logger.Info("Failed to create firewall scope", "error", err.Error()) @@ -171,7 +168,7 @@ func (r *LinodeFirewallReconciler) reconcileCreate( logger logr.Logger, firewallScope *scope.FirewallScope, ) error { - linodeFW, err := services.HandleFirewall(ctx, firewallScope, logger) + linodeFW, err := services.HandleFirewall(ctx, firewallScope.LinodeFirewall, firewallScope.LinodeClient, logger) if err != nil || linodeFW == nil { r.setFailureReason(firewallScope, infrav1alpha1.CreateFirewallError, err) @@ -187,7 +184,7 @@ func (r *LinodeFirewallReconciler) reconcileUpdate( logger logr.Logger, firewallScope *scope.FirewallScope, ) error { - linodeFW, err := services.HandleFirewall(ctx, firewallScope, logger) + linodeFW, err := services.HandleFirewall(ctx, firewallScope.LinodeFirewall, firewallScope.LinodeClient, logger) if err != nil || linodeFW == nil { r.setFailureReason(firewallScope, infrav1alpha1.UpdateFirewallError, err) diff --git a/controller/linodemachine_controller_helpers.go b/controller/linodemachine_controller_helpers.go index 460b624ce..51705428c 100644 --- a/controller/linodemachine_controller_helpers.go +++ b/controller/linodemachine_controller_helpers.go @@ -62,6 +62,11 @@ func (*LinodeMachineReconciler) newCreateConfig(ctx context.Context, machineScop createConfig.PrivateIP = true + if kutil.IsControlPlaneMachine(machineScope.Machine) && + machineScope.LinodeCluster.Spec.ControlPlaneFirewall.FirewallID != nil { + createConfig.FirewallID = *machineScope.LinodeCluster.Spec.ControlPlaneFirewall.FirewallID + } + bootstrapData, err := machineScope.GetBootstrapData(ctx) if err != nil { logger.Info("Failed to get bootstrap data", "error", err.Error())